rom-rb / rom

Data mapping and persistence toolkit for Ruby
https://rom-rb.org
MIT License
2.07k stars 161 forks source link

Rails 7.1 incompatibility (Object#with) #684

Closed palkan closed 3 months ago

palkan commented 1 year ago

Describe the bug

Loading rom-sql with Rails 7.1 (main) fails due to the recently added Object#with monkey-patch (yeah, it strikes again):

/opt/homebrew/lib/ruby/gems/3.2.0/bundler/gems/rails-d954155dd8cc/activesupport/lib/active_support/core_ext/object/with.rb:34:in `public_send': undefined method `raise_on_error=' for #<ROM::SQL::Schema::Inferrer options={:attr_class=>ROM::SQL::Attribute, :enabled=>true, :attributes_inferrer=>#<Proc:0x000000010ffa2200 /opt/homebrew/lib/ruby/gems/3.2.0/gems/rom-sql-3.6.1/lib/rom/sql/schema/inferrer.rb:16 (lambda)>, :silent=>false, :raise_on_error=>true}> (NoMethodError)

        public_send("#{key}=", old_value)
        ^^^^^^^^^^^
        from /opt/homebrew/lib/ruby/gems/3.2.0/bundler/gems/rails-d954155dd8cc/activesupport/lib/active_support/core_ext/object/with.rb:34:in `block in with'
        from /opt/homebrew/lib/ruby/gems/3.2.0/bundler/gems/rails-d954155dd8cc/activesupport/lib/active_support/core_ext/object/with.rb:33:in `each'
        from /opt/homebrew/lib/ruby/gems/3.2.0/bundler/gems/rails-d954155dd8cc/activesupport/lib/active_support/core_ext/object/with.rb:33:in `ensure in with'
        from /opt/homebrew/lib/ruby/gems/3.2.0/bundler/gems/rails-d954155dd8cc/activesupport/lib/active_support/core_ext/object/with.rb:33:in `with'
        from /opt/homebrew/lib/ruby/gems/3.2.0/gems/rom-sql-3.6.1/lib/rom/sql/schema/inferrer.rb:115:in `suppress_errors'

Looks like this line is the reason: https://github.com/rom-rb/rom/blob/7fb82cf7ffa86805d9c5499a4ecc64d5d3c20f14/lib/rom/initializer.rb#L19 (that's why I open issue here, not in rom-sql).

I wonder if we use method_defined?(:with, false)?

To Reproduce

Load a Rails app, using Rails from main and add rom and rom-sql.

solnic commented 1 year ago

I wonder if we use method_defined?(:with, false)?

How would this help?

solnic commented 1 year ago

Removing bug label because this is ActiveSupport bullsh*t causing issues again.

palkan commented 1 year ago

How would this help?

This way, we will only check if the #with is defined in this class, not its ancestors:

class Object
  def with; end
end

class A
end

A.method_defined?(:with) #=> true
A.method_defined?(:with, false) #=> false

That's just a guess that this could be the solution; I don't have a full content on why this check has been added in the first place.

flash-gordon commented 1 year ago

Seems like a good enough workaround to me

palkan commented 1 year ago

Oops, the snippet in the previous message was incorrect. Fixed.