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

Consider JSON.parse #74

Closed
FUDCo opened this issue Sep 27, 2019 · 13 comments · Fixed by #89
Closed

Consider JSON.parse #74

FUDCo opened this issue Sep 27, 2019 · 13 comments · Fixed by #89

Comments

@FUDCo
Copy link

FUDCo commented Sep 27, 2019

I concur with the proposal that records and tuples serialize to JSON via JSON.stringify as object and array. It strikes me that there may be value in providing support for going in the other direction, to somehow parameterize JSON.parse to yield a structure consisting of records and tuples. Obviously, this could be implemented in user land with the existing JSON.parse and some tree crawling and type conversion logic, but supporting it directly would be a good deal more efficient. Since many use cases for parsing JSON (actually, I'd speculate a majority of them) neither need nor want the parsed result to be mutable this could be a win.

@rricard
Copy link
Member

rricard commented Oct 3, 2019

Agreed, the question being where would we add the option to parse into Record/Tuple instead of Objects/Arrays. Parameterizing JSON.parse might be tricky as it already takes a second reviver function argument.

@papb
Copy link

papb commented Oct 3, 2019

What if a new JSON.parseAsImmutable was provided?

@littledan
Copy link
Member

Let's add this function--everyone is asking for it, and it seems useful to me too. As a starting point, I would suggest Record.parseJSON (even though it doesn't always return a record), and then we can bikeshed on the naming from there.

@rricard
Copy link
Member

rricard commented Oct 29, 2019

Agreed, putting it on Record is actually the best option so far

@dcporter
Copy link

(Isn't [] valid JSON though?)

@rricard
Copy link
Member

rricard commented Oct 30, 2019

Well, yes, but it wouldn't be the end of the world to have this Record.parseJSON be able to return a Tuple. The main advantage of that approach is to avoid extending an existing namespace such as JSON...

@dcporter
Copy link

The main advantage of that approach is to avoid extending an existing namespace such as JSON...

Thanks Robin. What's the benefit of this? I would expect JSON parsers to live on JSON.

@rricard
Copy link
Member

rricard commented Oct 30, 2019

Of course, the thing is we have to guarantee compatibility with existing websites. However unlikely it is, there might be an instance of library that adds methods to JSON or tweak JSON.parse.

That being said, research and measurements can be done to ensure websites do not do that. Adding to a new namespace however, clears us of doing any of that work. As of now, I'll probably put that on the table for the committee to advise. Maybe it is a non-problem.

@dcporter
Copy link

As of now, I'll probably put that on the table for the committee to advise. Maybe it is a non-problem.

Sounds great. My hope and expectation is that it's a non-problem, in which case using the JSON namespace for JSON stuff seems to me like a big win. =)

@littledan
Copy link
Member

I wouldn't worry too much about the web compatibility of extending the JSON namespace (at least not any more than I would worry about the web compatibility of adding the Record global...). We just need to reach a name that we can all agree on. Do you have an idea for what the method should be called?

@papb
Copy link

papb commented Nov 1, 2019

What if a new JSON.parseAsImmutable was provided?

This was my first idea. Perhaps JSON.parseImmutable could work as well.

@Olian04
Copy link

Olian04 commented Nov 2, 2019

Wouldn't it make more sense to parse data as the most tolerant datatype of the two; then create the more strict datatype using the tolerant one.

Record.from(JSON.parse(string))

That way we wouldn't have to pollute the JSON namespace.

@rricard
Copy link
Member

rricard commented Nov 3, 2019

@Olian04 you would generate an intermediary object representation in memory, by doing this you would cancel the advantages of such parser.

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

Successfully merging a pull request may close this issue.

6 participants