JSON: more general type of JSON.parse()

[dev-itsheng]

lib Update Request

Configuration Check

My compilation target is ES2015 and my lib is the default.

Missing / Incorrect Definition

JSON.parse。

Sample Code

In Node.js:

import fs from 'fs';

const packageJson = JSON.parse(fs.readFileSync('./package.json'));

TypeScript throw a new Error TS2345: Argument of type 'Buffer' is not assignable to parameter of type 'string'.

But it works.

Because according to the ECMAScript 5 version of the documentation, JSON.parse(), when executed, will first convert the incoming parameters Call the ToString internal method.

And according to Node.js related code, the Buffer object will be converted to utf8 when calling toString string.

That is, this parameter can be of any type, for example:

const obj = {
    toString() {
        return '[1, 2, 3]'
    }
};

const arr = JSON.parse(obj);    // [1, 2, 3]

TypeScript still throw error:

You can also try it on TypeScript PlayGround.

I had to disable an ESLint rule for the same reason.

Documentation Link

[MartinJohns]

Duplicate of #44944. Used search terms: JSON.parse buffer

[dev-itsheng]

Duplicate of #44944. Used search terms: JSON.parse buffer

yes.

In fact, I also searched for this before raising this issue, but I don't agree with it.

First of all, Buffer is a type that exists only in Node.js, it should not be put into the generic lib.es5.d.ts.

Second, JSON.parse can handle the Buffer type because Buffer.prototype has a legal toString method, which can convert a Buffer into a correct string. In this case, other non- String values should be treated the same, not just this one specific type.

but:

The first time I didn't notice, there was a very old issue linked under that issue #17203 , and although it focused on a different point than mine (parseFloat), the point of view there is controversial and worth discussing.

Let's pay attention to the last two comments:

I would argue that for built-ins like parseFloat, isNaN, etc. that the input parameters should be any. The main reason for type checking input parameters for a function is to prevent runtime errors, but in these cases we know that no matter what input you pass to these functions you will not get a runtime error.

and:

@jcready I think a primary reason for type checking is to prevent bugs. Not all bugs manifest as runtime errors.

On the second floor, the leader of TS also has a sentence:

at worst it means the entirely wrong thing is happening.

This may be the reason why the previous problem was not solved, so let's rethink:

Should this be wrong?

See MDN:

The JSON.parse() method parses a JSON string, constructing the JavaScript value or object described by the string. An optional reviver function can be provided to perform a transformation on the resulting object before it is returned.

It seems that almost everyone agrees that JSON.parse() should be passed in a string.

This may be more of a conceptual issue than a technical one.

Maybe we should ask the authors of sindresorhus/eslint-plugin-unicorn#1273 to discuss this, especially the author @fisker.

[MartinJohns]

but I don't agree with it.

Such an issue has been raised again and again, and the answer was always the same by the TypeScript team.

This may be more of a conceptual issue than a technical one.

What do you believe will cause more issues and trouble for most developers:

• Being able to accidentally pass completely incompatible types, or
• having to call toString() on your object?

I'm confident to say the first option is by far worse than the second.

---

What you actually want is something like #35945. The ability to pass types that explicitly coerce to string in place of string.

[dev-itsheng]

but I don't agree with it.

Such an issue has been raised again and again, and the answer was always the same by the TypeScript team.

This may be more of a conceptual issue than a technical one.

What do you believe will cause more issues and trouble for most developers:

• Being able to accidentally pass completely incompatible types, or
• having to call toString() on your object?

I'm confident to say the first option is by far worse than the second.

What you actually want is something like #35945. The ability to pass types that explicitly coerce to string in place of string.

You're right, I think it touches on a deeper issue:

TypeScript (or ESLint, or some other best practice) wants to make the code clearer and more intuitive, for example, we recommend using strict operators (like ===) instead of operations that may cause implicit type conversions character (such as ==).

That said, the above principle encourages us to do more explicit work than implicit work.

Continuing in this line of thought, our code might look like this:

const obj = {
     toString() {
         return '[1, 2, 3]'
     }
};

const arr = JSON.parse(String(obj)); // [1, 2, 3]
// or
const arr = JSON.parse(obj.toString()); // [1, 2, 3]
// even
const arr = JSON.parse(obj + ''); // [1, 2, 3]

In this way, TypeScript will not report an error.

Similar things are done for Buffer objects, and even other more complex and general types.

What do you think?

[fisker]

FYI：I proposed unicorn/prefer-json-parse-buffer rule for these reasons:

1. Rule proposal: prefer-json-parse-buffer sindresorhus/eslint-plugin-unicorn#1273 (comment)
2. Rule proposal: prefer-json-parse-buffer sindresorhus/eslint-plugin-unicorn#1273 (comment)

[fisker]

Anyway,

1. The Node.js fs performance issue already fixed. fs.promises.readFile is 40% slower than fs.readFile nodejs/node#37583 (comment)
2. Node.js already supports import JSON module. module: Unflag JSON modules nodejs/node#41736

Use that rule on your own judgement.

[MartinJohns]

TypeScript [...] wants to make the code clearer and more intuitive

I'd argue most people use TypeScript for type-safety to avoid bugs at design and compilation time. Accepting only string instead of just any type for an argument that effectively should only be a string is part of it.

[dev-itsheng]

The Node.js fs performance issue already fixed. nodejs/node#37583 (comment)

This could be the difference between these two:

// 1
import fsP from 'fs/promises';
JSON.parse(await fsP.readFile('./***.json'));

// 2
import fs from 'fs';
JSON.parse(fs.readFileSync('./***.json'));

instead of:

// 1-1
import fs from 'fs';
JSON.parse(fs.readFileSync('./***.json'));

// 1-2
import fs from 'fs';
JSON.parse(fs.readFileSync('./***.json'), 'utf8');

// 2-1
import fsP from 'fs/promises';
JSON.parse(await fs.readFile('./***.json'));

// 1-2
import fsP from 'fs/promises';
JSON.parse(await fs.readFile('./***.json'), 'utf8');

When I see

Passing in a buffer may not be performant

in https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-json-parse-buffer.md, I think that 1-1 (or 2-1) is slower than 1-2 (or 2-2), but I see the source code of fs.readFile and Buffer.prototype.toString, they seem to do the same thing, that is, there won't be a very noticeable performance difference.

Of course that's not the point.

Such an issue has been raised again and again, and the answer was always the same by the TypeScript team.

I read the previous reasons why the TypeScript team objected to a request similar to mine, and realized that they didn't expect "implicit type conversions" to happen, but to pass each parameter in the most natural and intuitive way possible.

most people use TypeScript for type-safety to avoid bugs at design and compilation time

Yes, we shouldn't break the type safety that most people expect in order to take care of such an extreme case that hardly anyone uses.

Main reason we are reading JSON files a lot is because we can't import JSON yet, compare to require('./foo.json') we used do, JSON.parse(await fs.readFile(file, 'utf8')) vs JSON.parse(await fs.readFile(file)), I prefer the shorter one.

(from sindresorhus/eslint-plugin-unicorn#1273 (comment))

If this scenario is indeed used very frequently, I think it may be a better choice to encapsulate a function:

import fsP from 'fs/promises';

const readJsonFile = async (path: string) => JSON.parse(await fsP.readFile(path, 'utf8'));

In order not to break type safety, I think it's worth it.

[Josh-Cena]

I'm getting confused by the conversation... @dev-itsheng You opened the issue saying JSON.parse() should allow any, but your follow-up comments seem to lean towards the original string parameter type?

[dev-itsheng]

I'm getting confused by the conversation... @dev-itsheng You opened the issue saying JSON.parse() should allow any, but your follow-up comments seem to lean towards the original string parameter type?

Because I was convinced by them and decided to change my original idea.

In fact, I was also influenced by @fisker when I first made this point, so I @@fisker to join the discussion at the beginning.

Maybe I should close this issue and let them continue the discussion?

[Josh-Cena]

It seems no-one's actually in favor of allowing passing anything to JSON.parse(), if TS doesn't like it. Fisker's preference is mainly personal, and this rule is already removed from unicorn's recommended set.

[dev-itsheng]

Well, let this be the end of it.