Conversation

julioalucero

STJ-23: BUGFIX: deprecation_tracker breaking with unknown keywords

Description

BUGFIX: Prevent DeprecationTracker from crashing when receiving unknown keyword arguments.

In certain environments (e.g. when using sass-embedded), the warn method receives unexpected keyword arguments such as :deprecation, :span, or :stack. Previously, these unrecognized keywords would raise an ArgumentError and interrupt execution. This safely handles unknown keyword arguments while preserving known ones.

Motivation and Context

Fixes #152 — DeprecationTracker was breaking during setup in projects using sass-embedded due to unhandled keyword arguments in warn.

This update ensures that warn forwards all known and unknown keyword arguments safely to super, and gracefully falls back to known keywords if the parent method does not accept extras.

How Has This Been Tested?

  • Added a test to verify that warn can accept unknown keyword arguments (e.g. :deprecation, :span, :stack) without raising errors.
  • Ran the full test suite using Ruby 2.7 and Ruby 3.2 to ensure backward and forward compatibility.
  • Verified that the warn method continues to capture and store deprecation messages as expected.

Screenshots:

I will abide by the code of conduct

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julioalucero looks good to me 👍🏼

(spoke too soon)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you've addressed my one comment, please add an entry to the CHANGELOG

super(*messages, **keyword_args, **kwargs)
rescue ArgumentError
super(*messages, **keyword_args)
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julioalucero if you know when the signature of the method changed, it would be best to use if/else vs. begin/rescue

@julioalucerojulioalucero marked this pull request as ready for review May 29, 2025 18:11
@julioalucero

@etagwerker, adding the changelog like this is ok? 99576c2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@julioalucero You can add a line like this one to the CHANGELOG and we should be all good to go: 3d35c23#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR4

@julioalucero

You can add a line like this one to the CHANGELOG and we should be all good to go: 3d35c23#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR4

@etagwerker added! Thanks for the review!

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

Successfully merging this pull request may close these issues.

[BUG] NextRails deprecation_tracker breaking with unknown keywords: