Repository navigation
JSON: more general type of JSON.parse() #48907
Description
Activity
Duplicate of #44944. Used search terms:
JSON.parse bufferReacted by whzx5byb and Joe CalzarettaDuplicate of #44944. Used search terms:
JSON.parse bufferyes.
In fact, I also searched for this before raising this issue, but I don't agree with it.
First of all,
Bufferis a type that exists only in Node.js, it should not be put into the genericlib.es5.d.ts.Second,
JSON.parsecan handle theBuffertype becauseBuffer.prototypehas a legaltoStringmethod, which can convert aBufferinto 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:
James Wyatt Cready-Pyle (@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 Cheung (@fisker).
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
stringin place ofstring.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
stringin place ofstring.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
Bufferobjects, and even other more complex and general types.What do you think?
FYI：I proposed
unicorn/prefer-json-parse-bufferrule for these reasons:Anyway,
- The Node.js fs performance issue already fixed. fs.promises.readFile is 40% slower than fs.readFile nodejs/node#37583 (comment)
- Node.js already supports import JSON module. module: Unflag JSON modules nodejs/node#41736
Use that rule on your own judgement.
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
stringinstead of just any type for an argument that effectively should only be astringis part of it.Reacted by Gerrit BirkelandThe 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.readFileandBuffer.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.
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.
Reacted by Toni VillenaI'm getting confused by the conversation... 我这一生 如绿豆冰 (@dev-itsheng) You opened the issue saying
JSON.parse()should allowany, but your follow-up comments seem to lean towards the originalstringparameter type?Reacted by Toni VillenaI'm getting confused by the conversation... 我这一生 如绿豆冰 (@dev-itsheng) You opened the issue saying
JSON.parse()should allowany, but your follow-up comments seem to lean towards the originalstringparameter type?Because I was convinced by them and decided to change my original idea.
In fact, I was also influenced by fisker Cheung (@fisker) when I first made this point, so I @fisker Cheung (@fisker) to join the discussion at the beginning.
Maybe I should close this issue and let them continue the discussion?
Reacted by Toni VillenaIt 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.Reacted by Toni VillenaWell, let this be the end of it.
Reacted by Toni Villena
lib Update Request
Configuration Check
My compilation target is
ES2015and my lib isthe default.Missing / Incorrect Definition
JSON.parse。Sample Code
In Node.js:
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 theToStringinternal method.And according to Node.js related code, the Buffer object will be converted to utf8 when calling
toStringstring.That is, this parameter can be of any type, for example:
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