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

Export all Dash properties with their defaults, even when not set internally #535

Merged
merged 3 commits into from Nov 8, 2021

Conversation

michaelherold
Copy link
Member

In #437, we added the ability to have nilable codependent attributes. However, this change introduced a regression where those codependent attributes are no longer exported because they are not stored internally.

This change adds exporting for all Dash properties, whether they are set or not. The behavior uses the default value as the export for a key that was missing during initialization. If you set the value to nil later, it will be nil in the export.

Note that there is behavior in here that is related to an implementation detail of MRI (and currently JRuby) as noted in the last commit and in the comment above two example groups in the specs. I wanted to explicitly document it because it's not commonly known behavior.

Lastly, this includes a small refactor of how IndifferentAccess converts keys. This made it cleaner to be consistent with Dash::IndifferentAccess and gives us a singular place for us to handle converting things for indifferent access.

Closes #469

The check here doesn't make sense because it's testing the behavior of
another method. The best way to check identity is but checking the
`#object_id` of an object - thus, this check actually does what the test
says it does.
In some cases --- like writing meta-extensions of `IndifferentAccess`
--- we need access to the ability to convert a key for the purposes of
`IndifferentAccess`. Exposing this behavior as a module function makes
this possible.
When exporting a Dash via `#to_h` or `#to_hash`, we expect all
properties to be exported whether or not they are set. However, in the
change that allows codependent properties to be nilified, we regressed
the behavior of exporting all properties.

There is a gotcha here, which I note in the tests for the specs. For
posterity, MRI does not send the `#to_hash` method to anything that
subclasses `Hash` when you double-splat it. Thus, we cannot override the
behavior within MRI. For more information, see [this comment][1] where I
detail the behavior of double-splat within MRI.

Currently, JRuby also follows this behavior, but it's not guaranteed
that other Rubies will.

[1]: hashie#353 (comment)
@michaelherold
Copy link
Member Author

@hlascelles, can you verify that this restores the behavior that you expect?

@dblock
Copy link
Member

dblock commented Oct 23, 2020

LGTM pending rubocop -a

@hlascelles
Copy link

Yes, looks good, thank you 👍. And excellent spot about the double splatting behaviour.

@michaelherold michaelherold force-pushed the dash-nil-keys branch 2 times, most recently from 6974c69 to 98df995 Compare October 30, 2020 00:50
@dblock dblock merged commit 25a3ff6 into hashie:master Nov 8, 2021
@dblock
Copy link
Member

dblock commented Nov 8, 2021

Merged via #554

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.

Nil values cause keys not to be written out in 3.6.0
3 participants