diff options
author | Edouard CHIN <edouard.chin@shopify.com> | 2018-07-03 23:15:30 -0400 |
---|---|---|
committer | Edouard CHIN <edouard.chin@shopify.com> | 2018-07-03 23:29:43 -0400 |
commit | 6cf7a0b0e9eaaa57fba0b0cea0f3015664baa0d8 (patch) | |
tree | d9db13530d8e292229e29e5be7280392b13764e9 /activesupport | |
parent | 0a6b866ff2f45cea50598fc054763b89ea7affbe (diff) | |
download | rails-6cf7a0b0e9eaaa57fba0b0cea0f3015664baa0d8.tar.gz rails-6cf7a0b0e9eaaa57fba0b0cea0f3015664baa0d8.tar.bz2 rails-6cf7a0b0e9eaaa57fba0b0cea0f3015664baa0d8.zip |
Use class_eval or instance_eval when triggering lazy load hooks:
- When lazy load hooks were triggered we were using
`Object.instance_eval` which evaluates the block in the context of
the class being passed. Most of the time that class was a
`Class`. If one wants to define a instance method on the class then
it wasn't possible.
```ruby
class A; end;
A.instance_eval do
def foo
puts 'bar'
end
end
A.new.foo #> NoMethodError: undefined method `foo`
A.foo #> bar
```
- This PR checks what object is passed when triggering the hooks and
either call `class_eval` or `instance_eval`. My rational and assumptions being
that if an instance of a class is passed, then the blocks needs to
evaluate in the context of that instance (i.e. defining a method
should only define it on that instance).
On the other hand, if a Class or Module is passed when triggering
hooks, then defining a method should define it on the class itself
- #32776 Pushed me to introduce this change
Diffstat (limited to 'activesupport')
-rw-r--r-- | activesupport/lib/active_support/lazy_load_hooks.rb | 6 | ||||
-rw-r--r-- | activesupport/test/lazy_load_hooks_test.rb | 49 |
2 files changed, 54 insertions, 1 deletions
diff --git a/activesupport/lib/active_support/lazy_load_hooks.rb b/activesupport/lib/active_support/lazy_load_hooks.rb index dc8080c469..97fe8a06c1 100644 --- a/activesupport/lib/active_support/lazy_load_hooks.rb +++ b/activesupport/lib/active_support/lazy_load_hooks.rb @@ -68,7 +68,11 @@ module ActiveSupport if options[:yield] block.call(base) else - base.instance_eval(&block) + if base.is_a?(Class) || base.is_a?(Module) + base.class_eval(&block) + else + base.instance_eval(&block) + end end end end diff --git a/activesupport/test/lazy_load_hooks_test.rb b/activesupport/test/lazy_load_hooks_test.rb index 721d44d0c1..50a703e49f 100644 --- a/activesupport/test/lazy_load_hooks_test.rb +++ b/activesupport/test/lazy_load_hooks_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "abstract_unit" +require "active_support/core_ext/module/remove_method" class LazyLoadHooksTest < ActiveSupport::TestCase def test_basic_hook @@ -125,6 +126,54 @@ class LazyLoadHooksTest < ActiveSupport::TestCase assert_equal 7, i end + def test_hook_uses_class_eval_when_base_is_a_class + ActiveSupport.on_load(:uses_class_eval) do + def first_wrestler + "John Cena" + end + end + + ActiveSupport.run_load_hooks(:uses_class_eval, FakeContext) + assert_equal "John Cena", FakeContext.new(0).first_wrestler + ensure + FakeContext.remove_possible_method(:first_wrestler) + end + + def test_hook_uses_class_eval_when_base_is_a_module + mod = Module.new + ActiveSupport.on_load(:uses_class_eval2) do + def last_wrestler + "Dwayne Johnson" + end + end + ActiveSupport.run_load_hooks(:uses_class_eval2, mod) + + klass = Class.new do + include mod + end + + assert_equal "Dwayne Johnson", klass.new.last_wrestler + end + + def test_hook_uses_instance_eval_when_base_is_an_instance + ActiveSupport.on_load(:uses_instance_eval) do + def second_wrestler + "Hulk Hogan" + end + end + + context = FakeContext.new(1) + ActiveSupport.run_load_hooks(:uses_instance_eval, context) + + assert_raises NoMethodError do + FakeContext.new(2).second_wrestler + end + assert_raises NoMethodError do + FakeContext.second_wrestler + end + assert_equal "Hulk Hogan", context.second_wrestler + end + private def incr_amt |