-
Notifications
You must be signed in to change notification settings - Fork 99
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 for Ruby 3 keyword arg splatting change #92
base: master
Are you sure you want to change the base?
Fix for Ruby 3 keyword arg splatting change #92
Conversation
reload = Memoist.extract_reload!(method(#{unmemoized_method.inspect}), args) | ||
|
||
skip_cache = reload || !(instance_variable_defined?(#{memoized_ivar.inspect}) && #{memoized_ivar} && #{memoized_ivar}.has_key?(args)) | ||
skip_cache = reload || !(instance_variable_defined?(#{memoized_ivar.inspect}) && #{memoized_ivar} && #{memoized_ivar}.has_key?(args+kwargs.values)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignoring of kwargs.keys
is not quite accurate, this way we make no difference between account(local: true)
and account(remote: true)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. Appreciate any help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using kwargs.to_a
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we simply use cache_key = args + [kwargs]
at the moment )
Hey @sebjacobs, is there any chance to get it released? I'm ready to improve this PR a bit as per my comment above ) |
ping! I am starting to use this on a Ruby 3 project now. Until this is fixed I'll only be able to use memoist on methods without arguments |
FYI: Added this alert to the new memoist repo Important RecommendationConsider using MemoWise instead, as it is maintained, fully tested, provides thread safety guarantees, and is much, much faster. Other AlternativesIn case you need a tool with this feature set that is currently maintained, check out: Tip Seriously though, read the important note above. Warning If you must continue - be aware that this is unmaintained software. |
This is how I addressed the Ruby 3 keyword args splatting change incompatibility as mentioned here: https://bugs.ruby-lang.org/issues/14183.
Tests pass. Up to you if you want to accept this change.