Rule proposal: prefer-json-parse-buffer

[fisker]

When parsing a json file, there is no need to read it as string, because JSON.parse can parse Buffer. Real world case

Fail

JSON.parse(await fs.readFile(file, 'utf8'));

Pass

JSON.parse(await fs.readFile(file));

JSON.parse(await fs.readFile(file, 'gbk'));

This is also faster when use fsPromises, but this is bug, should not be the main reason for this rule. nodejs/node#37583

[silverwind]

JSON.parse can parse Buffer

That seems like a non-standard extension JSON.parse of Node.js, so it may not work in other environments like Deno, but I guess fs will not be there either, so it's probably fine to match that specific case. It should work with destructured readFile and readFileSync too.

[fisker]

That seems like a non-standard extension JSON.parse

I think it's not, JSON.parse({[Symbol.toPrimitive](){return '[1]'}})

[silverwind]

It does not generally seem to work with byte arrays:

JSON.parse((new TextEncoder()).encode("{}"))
// Uncaught SyntaxError: Unexpected token , in JSON at position 3

Anyways, I think it's a good rule to have for a node-specific optimization.

[sindresorhus]

This is now accepted.

[zloirock]

@silverwind

That seems like a non-standard extension JSON.parse of Node.js
It does not generally seem to work with byte arrays

It works just because JSON.parse calls ToString inside (by the spec) and Buffer#toString converts the buffer to the string with utf8 encoding by default. TextEncoder#encode returns just a typed array with %TypedArray%#toString that does not convert it to the original string.

[crystalfp]

Sorry to disagree, but typescript JSON.parse() does not accept anything but a string. See microsoft/TypeScript #44944. The only solution for me is to disable this rule.

[fisker]

I guess you'll have to disable in TS file.

[crystalfp]

Or better, in the eslint.yaml file where I added: unicorn/prefer-json-parse-buffer: off to the rules: section.

[devinrhode2]

I don't know why this is preferable. I assume the argument is simply: shorter code is preferable.

This rule forces/strongly encourages developers to chase down this GH thread to realize JSON.parse calls Buffer#toString which uses utf8.

Why not go a step further, if we want short code, encourage using some sort of fs-extras readJson method?
(Example: await fs.readJson('./package.json'))

[fisker]

Mentioned in the description

This is also faster when use fsPromises, but this is bug, should not be the main reason for this rule. nodejs/node#37583

[fisker]

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.

[silverwind]

we can't import JSON yet

JSON modules are unflagged since nodejs/node#41736, fyi.

[fisker]

Wow~ But, without assertion? Surprised.

[silverwind]

Yeah, assertion still required, I'm not sure I like it.

[addaleax]

Reading a file as a Buffer and then converting it to a string is absolutely worse than just reading it as a string, with worse CPU and memory performance. The only sensible lint rule is the exact opposite of what is being enforced here.

[devinrhode2]

I'm content with this being the main reason for this rule:

This is also faster when use fsPromises, but this is bug, should not be the main reason for this rule. nodejs/node#37583

@addaleax do you mind providing some insight why Buffer->string is worse for CPU and memory?

A benchmark could assume json files are less than 10 pages, where a page is around 100 lines.

[privatenumber]

I came here hoping for a data-driven explanation for this rule but the best reason so far is a bug that may already be fixed and @fisker's personal preference.

If there's actually a performance benefit, can benchmarks be referenced?

If there's no justification for this beyond preference, can this be a warning instead of an error by default and/or moved out of recommended?

[fisker]

This rule is still usefully to me, but I agree the reason behind this rule is arguable. Maybe we should disable this rule in recommend?

[privatenumber]

@fisker Should I open an new issue to request prefer-json-parse-buffer to be moved out of recommended?

[fisker]

Send a PR, I'll approve.