tc39 / proposal-json-parse-with-source

Proposal for extending JSON.parse to expose input source text.
https://tc39.github.io/proposal-json-parse-with-source
MIT License
213 stars 9 forks source link

What about object/class as argument ? #3

Closed lifaon74 closed 4 years ago

lifaon74 commented 5 years ago

What if instead of providing a string as 3rd argument it was an object ? This would allow future enhancement easy and could eventually provide some useful methods:

class JSONParseContext {
  readonly source: string;
  readonly keys: (string | number)[];
  readonly startPosition: number;
  readonly endPosition: number;
  // ... more properties allowed in the future
}
gibson042 commented 5 years ago

Thank you, I intended to post a similar issue. An object has obvious future extensibility benefits, at the cost of more operations (both spec and implementation). I think I'm nonetheless inclined towards it, though, for reasons very similar to those also motivating passing transient objects to String.prototype.replace callbacks.

jlyonsmith commented 4 years ago

The main downside to this approach are the performance implications of always having to allocate context objects even when they are not needed unless you have some way to indicate that you don't want them.

ExE-Boss commented 4 years ago

Well, if the function definition doesn’t declare a third argument, and doesn’t reference arguments, then the engine can perform internal optimisation, which skips allocating the JSONParseContext object.

jlyonsmith commented 4 years ago

That's a good point @ExE-Boss. I'm thinking I like this approach better than adding an options argument to parse. However, I can see a lot of discussion about what exactly the properties are in the context. I personally would like to see line number, offset, source file, value-as-a-string and type ("null", "boolean", "number", "string", "array", "object") given the use cases that I'm interested in.

gibson042 commented 4 years ago

Suggestion adopted.