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

JSON.dump: handle unenclosed hashes regression #554

Merged
merged 1 commit into from Dec 1, 2023

Conversation

casperisfine
Copy link

Fix: #553

We can never add keyword arguments to dump otherwise existing code using unenclosed hash will break.

cc @hsbt

Fix: flori#553

We can never add keyword arguments to `dump` otherwise
existing code using unenclosed hash will break.
@hsbt
Copy link
Collaborator

hsbt commented Dec 1, 2023

👍

@hsbt hsbt merged commit 691d6b1 into flori:master Dec 1, 2023
60 checks passed
@mame
Copy link
Contributor

mame commented Dec 1, 2023

@byroot Now JSON.dump({}, strict: true) would not work well. Is it ok?

@byroot
Copy link
Contributor

byroot commented Dec 1, 2023

No it's not... but since it's a new feature it's less if a problem. I'll submit another PR soon. Thank you for letting me know

eileencodes added a commit to rails/rails that referenced this pull request Dec 1, 2023
Locks the json gem to 2.6.3 until there is a new
release that includes flori/json#554
@k0kubun
Copy link
Contributor

k0kubun commented Dec 1, 2023

If I may: #556

private

def merge_dump_options(opts, strict: NOT_SET)
opts[:strict] = strict if NOT_SET != strict
Copy link
Contributor

Choose a reason for hiding this comment

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

This can destructively modify JSON.dump_default_options. I addressed that in #556.

zzak pushed a commit to rails/rails that referenced this pull request Dec 1, 2023
Locks the json gem to 2.6.3 until there is a new
release that includes flori/json#554
casperisfine pushed a commit to Shopify/rails that referenced this pull request Dec 2, 2023
Don't use `json` 2.7.0 release that has a regression.

Ref: flori/json#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.

JSON.dump does not work in version 2.7.0 when the argument is an implicit Hash
5 participants