1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

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
This commit is contained in:
Edouard CHIN 2018-07-03 23:15:30 -04:00
parent 0a6b866ff2
commit 6cf7a0b0e9
2 changed files with 54 additions and 1 deletions

View file

@ -67,12 +67,16 @@ module ActiveSupport
with_execution_control(name, block, options[:run_once]) do with_execution_control(name, block, options[:run_once]) do
if options[:yield] if options[:yield]
block.call(base) block.call(base)
else
if base.is_a?(Class) || base.is_a?(Module)
base.class_eval(&block)
else else
base.instance_eval(&block) base.instance_eval(&block)
end end
end end
end end
end end
end
extend LazyLoadHooks extend LazyLoadHooks
end end

View file

@ -1,6 +1,7 @@
# frozen_string_literal: true # frozen_string_literal: true
require "abstract_unit" require "abstract_unit"
require "active_support/core_ext/module/remove_method"
class LazyLoadHooksTest < ActiveSupport::TestCase class LazyLoadHooksTest < ActiveSupport::TestCase
def test_basic_hook def test_basic_hook
@ -125,6 +126,54 @@ class LazyLoadHooksTest < ActiveSupport::TestCase
assert_equal 7, i assert_equal 7, i
end 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 private
def incr_amt def incr_amt