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 | |
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.
-rw-r--r-- | activerecord/lib/active_record/autosave_association.rb | 8 | ||||
-rw-r--r-- | activesupport/CHANGELOG.md | 8 | ||||
-rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 16 | ||||
-rw-r--r-- | activesupport/lib/active_support/i18n_railtie.rb | 8 |
4 files changed, 36 insertions, 4 deletions
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index c39b045a5e..5045e3065e 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -206,7 +206,13 @@ module ActiveRecord if reflection.validate? && !method_defined?(validation_method) method = (collection ? :validate_collection_association : :validate_single_association) - define_non_cyclic_method(validation_method) { send(method, reflection) } + define_non_cyclic_method(validation_method) do + send(method, reflection) + # 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 validate validation_method end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 01bb816923..30e2bdd1e1 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,11 @@ +* Deprecate returning `false` as a way to halt callback chains. + + Returning `false` in a callback will display a deprecation warning + explaining that the preferred method to halt a callback chain is to + explicitly `throw(:abort)`. + + *claudiob* + * Changes arguments and default value of CallbackChain's :terminator option Chains of callbacks defined without an explicit `:terminator` option will 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 |