Symbolize names and freeze values when loading from JSON#587
Merged
radar merged 1 commit intoruby-i18n:masterfrom Jan 25, 2022
Merged
Symbolize names and freeze values when loading from JSON#587radar merged 1 commit intoruby-i18n:masterfrom
radar merged 1 commit intoruby-i18n:masterfrom
Conversation
ad41bdc to
b73ef81
Compare
radar
requested changes
Dec 13, 2021
| def load_json(filename) | ||
| begin | ||
| ::JSON.parse(File.read(filename)) | ||
| ::JSON.parse(File.read(filename), symbolize_names: true, freeze: true) |
Collaborator
There was a problem hiding this comment.
I couldn't find the freeze option mentioned in the docs. So I tried it out:
[10] pry(main)> obj = JSON.parse(%Q{{ "one": "two" }}, freeze: true)
=> {"one"=>"two"}
[11] pry(main)> obj["one"].frozen?
=> false
It seems like it might not be a supported option. Unsupported options appear to be ignored. Here's an example of me using my name as an option:
obj = JSON.parse(%Q{{ "one": "two" }}, ryan: true)
=> {"one"=>"two"}There was a problem hiding this comment.
It's because you JSON is too old:
>> JSON.parse('"foo"', freeze: true).frozen?
=> true
>> JSON::VERSION
=> "2.5.1"
Which is fine, the feature is best effort.
| def load_json(filename) | ||
| begin | ||
| ::JSON.parse(File.read(filename)) | ||
| ::JSON.parse(File.read(filename), symbolize_names: true, freeze: true) |
There was a problem hiding this comment.
@paarthmadan you could do the same than for Psych, check for JSON.respond_to?(:load_file), it's a method that was added recently, a bit after freeze, and it will also give you bootsnap caching https://github.com/Shopify/bootsnap/blob/master/CHANGELOG.md#190
Contributor
Author
There was a problem hiding this comment.
Good point, added 👍
JSON.load_file being defined implies that symbolize_names and freeze options are supported at parse time. We use this as a best effort feature test.
b73ef81 to
2ab9b37
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following a comment in #583, this PR uses new options in JSON to freeze and symbolize names at parse time.
I can append this commit to the other PR if that'd make it easier, but I opted to keep PRs as granular as possible for ease of review.