JSON.parse interface should allow any type as first argument

[nicknisi]

Per the ES5 spec, JSON.parse should toString the first argument passed to parse. However, the interface in lib.d.ts states the first argument must be a string.

This causes issues as shown in the code below where a Buffer is passed to JSON.parse

TypeScript Version: 2.0.3

Code

const fileContents = fs.readFileSync('some-file.json'); // No encoding given, Buffer returned

// tsc: TS2345: Argument of type 'Buffer' is not assignable to parameter of type 'string'.
const obj = JSON.parse(fileContents);

Expected behavior:

No tsc errors should be thrown, as the first argument's toString method will be called.

Actual behavior:

tsc: TS2345: Argument of type 'Buffer' is not assignable to parameter of type 'string'.

[RyanCavanaugh]

While all objects have a toString, the vast majority of them do not return something suitable for JSON.parse. It'd be a major gap to allow JSON.parse(getConfigFileContents) (note the lack of invocation parens there) just because getConfigFileContents.toString exists and returns something.

[jason0x43]

Is that really relevant, though? I mean, this is how JSON.parse is defined to work. It takes an argument, that argument is converted to a string, and the parser tries to handle it. Just because there are invalid use cases doesn't necessarily mean TS should add a non-spec API on top of the actual JS API.

[RyanCavanaugh]

Everything in JavaScript has defined behavior. [] * { } has a defined behavior of producing NaN. Should we allow that? "hello world".subtr(1) has the defined behavior of throwing an exception -- maybe that's what you meant to do. You can write console.log([1, 2].lenth); which has the defined behavior of printing undefined. "hello world".substr([3, 4]) is "hello world"; perhaps that was what you wanted to do.

But these things are all super sketchy and much more likely to be a bug. JSON.parse(window.setTimeout) will do something... but the odds you meant to do that are very, very, very low.

[jason0x43]

Hmmm...good points. There are plenty of cases where you would want to do such a thing, like JSON.parse(getConfigFileContents()), or JSON.parse(myObjectThatSerializesToJson) (where intention is clear and the spec behavior avoids potentially redundant conversions), but there are certainly a larger number of cases where someone has probably screwed up.

[aluanhaddad]

declare global {
  interface JSON {
      parse(text: Buffer, reviver?: (key: any, value: any) => any): any;
  }
}

Solves the problem nicely but I realize that's beside the point.

Semantically JSON.parse requires a string.
At least that's what I thought. I'm conflicted here because it doesn't throw but rather attempts to coerce its input. I'm actually curious as to why it is specified the way it is but I suppose that's beside the point as well. However it's so likely to be an error...

[aluanhaddad]

It may be worth noting that the current version of Douglas Crockford's reference implementation does not call toString
https://github.com/douglascrockford/JSON-js/blob/master/json_parse.js

[jason0x43]

Well, as @nicknisi mentioned, the ES2015 spec does specify that the first argument to JSON.parse will be coerced to a string (using an abstract ToString function rather than explicitly calling value.toString()), and that is how all browsers (that I've tested) and Node behave. However, as @RyanCavanaugh points out, that's most likely part of a "make everything work" philosophy rather than an explicitly intended behavior, so requiring the input to be a string is the safer way to go.

[aluanhaddad]

@jason0x43 I was looking for the intent of the API author. That is why I referenced the reference implementation. Good point that it is in fact the abstract operation ToString as opposed to the prototype method although linked spec is the ES5.1 spec as opposed to the ES2015 spec. Regardless it doesn't imply that .toString will be called or that it exists.

[jason0x43]

D'oh, here's the ES2015 reference (pretty much the same, as is the ES2017 draft). But yes, that was my original question, too (author intent). It would have been easy enough to state that JSON.parse should throw an error for an invalid data type, just as it currently throws an error for an invalid JSON string. However, the initial sentence in the spec ("The parse function parses a JSON text (a JSON-formatted String) and produces an ECMAScript value.") does seem to be pretty clear on intent. Maybe the goal was to leave it open to handle string-like objects (or future string implementations) as well as the current string type.

[aluanhaddad]

@jason0x43 I think your interpretation is correct. If you read the "Section 4: Parsers" of rfc4627, it states that

A JSON parser transforms a JSON text into another representation. A
JSON parser MUST accept all texts that conform to the JSON grammar.
A JSON parser MAY accept non-JSON forms or extensions.

Interestingly, older versions of ecma262, such as ES5.1, explicitly call out the above, stating that such extensions would be in violation of the ECMAScript spec. ES5.1 section 15.12

This restriction is still stated in the current draft, see ecma262/#sec-json-object, but the informative link to original rfc4627 is no longer present.

[RyanCavanaugh]

Nearly all of the runtime behavior is specified to coerce inputs using the corresponding primitive coercion routine. For example, the multiplication operator x * y will perform the ToNumber operation on its operands, rather than doing the sane thing of rejecting non-numeric inputs.