aboutsummaryrefslogtreecommitdiffstats
path: root/activesupport
diff options
context:
space:
mode:
authorEdouard CHIN <edouard.chin@shopify.com>2018-07-03 23:15:30 -0400
committerEdouard CHIN <edouard.chin@shopify.com>2018-07-03 23:29:43 -0400
commit6cf7a0b0e9eaaa57fba0b0cea0f3015664baa0d8 (patch)
treed9db13530d8e292229e29e5be7280392b13764e9 /activesupport
parent0a6b866ff2f45cea50598fc054763b89ea7affbe (diff)
downloadrails-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.rb6
-rw-r--r--activesupport/test/lazy_load_hooks_test.rb49
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