Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix callback names for aliased DSL methods #8

Merged

Conversation

stokarenko
Copy link
Contributor

@stokarenko stokarenko commented Mar 22, 2020

It uses __method__ lookup as callback name instead of __callee__.

As described on stackoverflow, there is a difference:

method looks up the name statically, it refers to the name of the nearest lexically enclosing method definition. callee looks up the name dynamically, it refers to the name under which the method was called. Neither of the two necessarily needs to correspond to the message that was originally sent:

class << (foo = Object.new)
  def bar(*) return __method__, __callee__ end
  alias_method :baz, :bar
  alias_method :method_missing, :baz
end

foo.bar # => [:bar, :bar]
foo.baz # => [:bar, :baz]
foo.qux # => [:bar, :method_missing]

Thus, under __callee__ lookup the aliased DSL method like

class MyClass
  include ::AfterCommitEverywhere
  alias_method :aliased_after_commit, :after_commit
end

MyClass.new.aliased_after_commit do
   # some useful stuff
end

will land as :aliased_after_commit handler to the Wrap instance, so will never be executed within Wrap#committed! method.

@Envek
Copy link
Owner

Envek commented Mar 22, 2020

Can you please tell more about your use case? Why do you need to alias methods?

@stokarenko
Copy link
Contributor Author

stokarenko commented Mar 22, 2020

@Envek

We are going to use after_commit_everywhere within aasm gem. That will be pretty nice to engage it via alias method as follows:

module AASM
  module Persistence
    module ActiveRecordPersistence
      def self.included(base)
        begin
          require 'after_commit_everywhere'
          base.send(:include, ::AfterCommitEverywhere) unless base.include?(::AfterCommitEverywhere)
          base.send(:alias_method, :aasm_execute_after_commit, :after_commit)

# ...

However, the __callee__ lookup is just a defect in fact, since the kind of AR callback registered by after_commit method should not be dynamic in any possible sense )

@Envek Envek merged commit 0d46662 into Envek:master Mar 22, 2020
@Envek
Copy link
Owner

Envek commented Mar 22, 2020

Released as 0.1.5.

Thank you very much for the contribution and for the detailed explanation!

@stokarenko
Copy link
Contributor Author

@Envek

Thank you too! after_commit_everywhere is a great stuff )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants