Closed samstickland closed 2 years ago
By the sounds of it this is a Ruby 3.0 issue where keyword arguments are not marked as kwargs using **
somewhere here?
Pretty sure it's this: https://github.com/apotonick/disposable/commit/aec0870bfe28f993da3129fa25a3b5d612e2e463 When calling property
or super
you have to **
the option part. We can think about reverting this for now.
Hmm but it works when delegate
call is removed ?
@samstickland Could you please confirm this by reverting changes from https://github.com/apotonick/disposable/commit/aec0870bfe28f993da3129fa25a3b5d612e2e463# ?
I'm very sure @samstickland overwrites #property
somewhere and doesn't call super
with **
... :grimacing:
@apotonick Thanks for the reply, but it's not that.. This is only part of the module that calls property
:
def money_property(name, currency: nil, **options)
property(name, **options)
Please upgrade to disposable-0.6.1
, I reverted the kws.
@apotonick Thanks for the change - unfortunately I still get the same issue on 0.6.1.
Shall I try and put together a demonstration project?
@samstickland Oh, how sad! I thought it fixed it. You must be doing something "naughty" then somewhere since we did not get any complaints so far :crossed_fingers: - that example project would be great! Let me know if you need help.
I'm seeing the exact same delegation issue in 2.6.1, though only with EAGER_LOAD=1
; otherwise no issue.
Replacing delegate :method, to: :instance
with instance_delegate method: :instance
"fixes" the issue, but doesn't seem ideal.
ArgumentError: wrong number of arguments (given 2, expected 1)
app/forms/my_form.rb:4:in `<class:MyForm>'
app/forms/my_form.rb:1:in `<top (required)>'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/kernel.rb:27:in `require'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/kernel.rb:27:in `require'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader/helpers.rb:95:in `const_get'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader/helpers.rb:95:in `cget'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader.rb:232:in `block (2 levels) in eager_load'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader/helpers.rb:26:in `block in ls'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader/helpers.rb:18:in `each_child'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader/helpers.rb:18:in `ls'
.rvm/gems/ruby-2.7.2/gems/zeitwerk-2.5.1/lib/zeitwerk/loader.rb:227:in `block in eager_load'
@pauljacksonrodgers Where do you add that line? Can I see the form class?
Experiencing the same problem, happy to provide more info if necessary.
@majksner A mini Rails app provoking the error would be great, we tried and couldn't make this bug "work"! :bomb: :joy_cat:
@apotonick
Here you go:
Code is a bit messy, but this was the quickest thing to do.
Let me know if you need anything else.
Thanks for the repo, @majksner! I was able to make it work, but it's a weird error, I suspect ActiveSupport's delegate
to be the problem source.
Here's how I fixed it.
class Factory < Reform::Form
model :project
property :state
extend Forwardable
def_delegators :model, :name
This also works: delegate :name => :model
It seems to be a change in Forwardable
? Where does this :to
option come from in the first place?
That works as well. delegate
method is defined in https://github.com/rails/rails/blob/v6.1.4/activesupport/lib/active_support/core_ext/module/delegation.rb
So ActiveSupport::delegate
is the problem :wink: No idea though why this arises with Reform 2.6.1?
delegate :name, to: :model
works fine if I use Reform 2.6.0. I see in the Gemfile.lock
that disposable dependency is updated to newer version.
@majksner And that disposable
update is the only change?
I think the ActiveSupport::delegate
method/module is included too early in 2.6.1 for whatever reasons, which makes you use the same-named Forwardable
delegation :bomb: I hate Ruby's include
magic... I think I know where to "fix" that.
Here is the diff from my Gemfile.lock
Thanks @majksner :green_heart: Here's the problem: https://github.com/apotonick/disposable/commit/979dc5f4950881c913263340a12628ec1ea52566#diff-cbfbccff8b533b3c195c7058cf7245b5559ecf5f0a16b4e7b83390a2ade42be6R1 The underlying Twin
(which is the form) now uses Forwardable
which overrides the same-named method haha. I hate Ruby.
Whoops, that wasn't supposed to close this ticket. :laughing:
@majksner @samstickland Could you kindly test this? It fixes the example repo.
In your Gemfile
, add
gem "disposable", github: "apotonick/disposable"
@apotonick It works!
Pushing gem to https://rubygems.org...
Successfully registered gem: disposable (0.6.2)
On Wed, Dec 1, 2021 at 1:13 PM Nikola Majksner @.***> wrote:
@apotonick https://github.com/apotonick It works!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/trailblazer/reform/issues/534#issuecomment-983582656, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACRJQWQAEYG5WJDSSPBPDUOYGNHANCNFSM5IMELFCQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
I blogged about this https://dev.to/trailblazer/dont-mix-forwardable-and-activesupportdelegate-5hmm
Complete Description of Issue
I have this in a contract:
It works fine with Reform 2.6.0, but with 2.6.1 I get the following error:
Line 56 is the delegation line, line 2 is the class definition, which is weird.
If I remove the delegation and do this instead then it works:
However, those methods can no longer be private, which suggests to me that execution scope has changed?
money_property
is defined like this - not pasting the whole thing here - but you can see how the proc passed to currency is called.Has the
self
that is passed tocall
changed?Steps to reproduce
Expected behavior
Should function as on 2.6.0
Actual behavior
Getting
wrong number of arguments (given 2, expected 1)
on use of delegateSystem configuration
Reform version: 2.6.1 Ruby version: 3.0.2
Full Backtrace of Exception (if any)