Skip to content

Conversation

@paarthmadan
Copy link
Contributor

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.

def load_json(filename)
begin
::JSON.parse(File.read(filename))
::JSON.parse(File.read(filename), symbolize_names: true, freeze: true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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"}

Choose a reason for hiding this comment

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

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)

Choose a reason for hiding this comment

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

@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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.
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.

3 participants