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

is JSON.parseImmutable significantly different enough that it deserves its own API? #1

Open
michaelficarra opened this issue Aug 5, 2022 · 5 comments

Comments

@michaelficarra
Copy link
Member

(copied from tc39/proposal-record-tuple#330)

JSON.parseImmutable seems like it can be expressed as an option given to JSON.parse since it's otherwise identical as far as I'm aware. I see that in tc39/proposal-record-tuple#74 it was originally imagined this way but quickly rejected. But couldn't we take an options bag in place of the reviver and branch on its type?

@Andrew-Cottrell
Copy link

Andrew-Cottrell commented Aug 5, 2022

My initial thought is the rules for authoring and using a reviver could vary depending on the immutable flag and these may be more easily explained by authors and understood by readers when given two distinct parse functions.

For example, stating that the following reviver should be used only with JSON.parse (i.e. not with JSON.parseImmutable) might be easier than describing how to use it & not use it with an overloaded JSON.parse.

let dateFromISOString = (value) => /* if `value` is a valid ISO string return a `Date` else return `null` */;
let reviver = (key, value) => typeof value === "string" && dateFromISOString(value) || value;
let text = '{"title": "Example", "author": "Andrew", "published": "2011-11-18T14:54:39.929Z"}';
console.log(JSON.parse(text, reviver).published.toUTCString()); // "Fri, 18 Nov 2011 14:54:39 GMT"

@acutmore
Copy link
Collaborator

acutmore commented Aug 5, 2022

An upside to the full dedicated JSON.parseImmutable method is that engines have the option to optimize out the intermediary arrays and objects, and create the records and tuples directly. While engines could also do this if they detect that the reviver is the built-in one this would lead to a subtle performance cliff.

const reviver = JSON.revivers.immutable; // or wherever it is.
JSON.parse(deepObject, reviver); // engine takes fast path
JSON.parse(deepObject, (key, value) => reviver (key, value)); // no fast-path

@michaelficarra
Copy link
Member Author

@acutmore I'm not suggesting that a built-in reviver would be the solution here. A simple flag would do.

JSON.parse(str, { immutable: true });

@acutmore
Copy link
Collaborator

acutmore commented Aug 5, 2022

@acutmore I'm not suggesting that a built-in reviver would be the solution here. A simple flag would do.

JSON.parse(str, { immutable: true });

Ah yes, I miss read. Any reason why it would replace the reviver argument, rather than be a new 3rd argument? So someone can still add their own reviver logic into the mix.

@michaelficarra
Copy link
Member Author

@acutmore To not force a user to provide a reviver when all they want is the immutability behaviour. Making it an options bag still allows a reviver to be provided if desired.

JSON.parse(str, { immutable: true, reviver: ... });

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

No branches or pull requests

3 participants