diff options
author | claudiob <claudiob@gmail.com> | 2014-12-14 22:46:23 -0800 |
---|---|---|
committer | claudiob <claudiob@gmail.com> | 2015-01-02 15:31:55 -0800 |
commit | d217daf6a740de7e4925872abe632982cfaab89b (patch) | |
tree | ab0fe69ba6f3bc4d07aaaa3530633ebd8ca14d63 /activesupport/lib | |
parent | 2386daabe7f8c979b453010dc0de3e1f6bbf859d (diff) | |
download | rails-d217daf6a740de7e4925872abe632982cfaab89b.tar.gz rails-d217daf6a740de7e4925872abe632982cfaab89b.tar.bz2 rails-d217daf6a740de7e4925872abe632982cfaab89b.zip |
Deprecate `false` as the way to halt AS callbacks
After this commit, returning `false` in a callback will display a deprecation
warning to make developers aware of the fact that they need to explicitly
`throw(:abort)` if their intention is to halt a callback chain.
This commit also patches two internal uses of AS::Callbacks (inside
ActiveRecord and ActionDispatch) which sometimes return `false` but whose
returned value is not meaningful for the purpose of execution.
In both cases, the returned value is set to `true`, which does not affect the
execution of the callbacks but prevents unrequested deprecation warnings from
showing up.
Diffstat (limited to 'activesupport/lib')
-rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 16 | ||||
-rw-r--r-- | activesupport/lib/active_support/i18n_railtie.rb | 8 |
2 files changed, 21 insertions, 3 deletions
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 3a3061e536..7dd97eb1ab 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/kernel/reporting' require 'active_support/core_ext/kernel/singleton_class' +require 'active_support/core_ext/string/filters' require 'thread' module ActiveSupport @@ -595,12 +596,23 @@ module ActiveSupport Proc.new do |target, result_lambda| terminate = true catch(:abort) do - result_lambda.call if result_lambda.is_a?(Proc) - terminate = false + result = result_lambda.call if result_lambda.is_a?(Proc) + if result == false + display_deprecation_warning_for_false_terminator + else + terminate = false + end end terminate end end + + def display_deprecation_warning_for_false_terminator + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Returning `false` in a callback will not implicitly halt a callback chain in the next release of Rails. + To explicitly halt a callback chain, please use `throw :abort` instead. + MSG + end end module ClassMethods diff --git a/activesupport/lib/active_support/i18n_railtie.rb b/activesupport/lib/active_support/i18n_railtie.rb index affcfb7398..9e742b1917 100644 --- a/activesupport/lib/active_support/i18n_railtie.rb +++ b/activesupport/lib/active_support/i18n_railtie.rb @@ -55,7 +55,13 @@ module I18n reloader = ActiveSupport::FileUpdateChecker.new(I18n.load_path.dup){ I18n.reload! } app.reloaders << reloader - ActionDispatch::Reloader.to_prepare { reloader.execute_if_updated } + ActionDispatch::Reloader.to_prepare do + reloader.execute_if_updated + # TODO: remove the following line as soon as the return value of + # callbacks is ignored, that is, returning `false` does not + # display a deprecation warning or halts the callback chain. + true + end reloader.execute @i18n_inited = true |