Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"json" encoding support for fs.readX functions #44150

Closed
Milk-Cool opened this issue Aug 6, 2022 · 9 comments
Closed

"json" encoding support for fs.readX functions #44150

Milk-Cool opened this issue Aug 6, 2022 · 9 comments
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. stale

Comments

@Milk-Cool
Copy link

What is the problem this feature will solve?

The JSON.parse(fs.readFileSync(path)), fs.readFile(path, "utf-8", res => { res = JSON.parse(res); /* ... */ }) look just way too long and complex.

What is the feature you are proposing to solve the problem?

The "json" encoding for the builtin fs module. Here's how it might look like:

const users = fs.readFileSync("users.json", "json"); // Or { "encoding": "json" }

As you can see, it's much shorter and more clear.

What alternatives have you considered?

As I've shown above, JSON.parse is a variant right now, but it's just way too long. There's also r-json package, but having a separate package just for reading JSON files takes much more space.

@Milk-Cool Milk-Cool added the feature request Issues that request new features to be added to Node.js. label Aug 6, 2022
@LiviaMedeiros LiviaMedeiros added the fs Issues and PRs related to the fs subsystem / file system. label Aug 6, 2022
@LiviaMedeiros
Copy link
Contributor

This can be roughly achieved via one helper function in userspace without external packages, e.g.:

const readJSON = async path => JSON.parse(await fsPromises.readFile(path));
// or
const readJSONSync = path => JSON.parse(fs.readFileSync(path));

Parsing JSON format probably would not have any performance benefits from being included in core, and there might be some discrepancy between possible/expected behaviours (e.g. should we strip BOM for convenience or keep it strict).
Are there usecases where it is required specifically as part of fs?

@benjamingr
Copy link
Member

benjamingr commented Aug 7, 2022

For what it's worth you can do require(path) instead of JSON.parse(fs.readFileSync(path)) which would work since you can require JSON files.

@aduh95
Copy link
Contributor

aduh95 commented Aug 8, 2022

For what it's worth you can do require(path) instead of JSON.parse(fs.readFileSync(path)) which would work since you can require JSON files.

Except that it would depends on the module cache, which may or may not be in sync with the actual file. You can also import(pathToFileUrl(path), { assert: { type: 'json' } }) with the same caveat.

@Milk-Cool
Copy link
Author

For what it's worth you can do require(path) instead of JSON.parse(fs.readFileSync(path)) which would work since you can require JSON files.

Yoooooo that's cool

@Milk-Cool
Copy link
Author

This can be roughly achieved via one helper function in userspace without external packages, e.g.:

const readJSON = async path => JSON.parse(await fsPromises.readFile(path));
// or
const readJSONSync = path => JSON.parse(fs.readFileSync(path));

Parsing JSON format probably would not have any performance benefits from being included in core, and there might be some discrepancy between possible/expected behaviours (e.g. should we strip BOM for convenience or keep it strict). Are there usecases where it is required specifically as part of fs?

Probably not, it's all about conviniency

@himself65
Copy link
Member

I think encoding here is absolutely not related to JSON.

Also, as shown in the document, fs is a POSIX-like API. But JSON is an RFC standard.

The node:fs module enables interacting with the file system in a way modeled on standard POSIX functions.

So, I don't think we need to add JSON support for reading files. If we really do that, what about writing a JSON file? What about other formats like XML, CSV...

@tniessen
Copy link
Member

tniessen commented Sep 5, 2022

My opinion hasn't changed from the last time this feature was suggested: if this were to be implemented in Node.js core, it should have significant benefits over readFile + JSON.parse, such as failing early when a syntax error occurs.

@devsnek
Copy link
Member

devsnek commented Sep 5, 2022

I'd be fine with fs.readJSON or similar. Using encoding would be kind of bad, the encoding of a file is sort of unrelated to whether it contains json. One explicit benefit of fs.readJSON is that it could default to utf8 which prevents the case of JSON.parse(fs.readFile(path)) with no encoding specified, which loads the file into memory as a buffer and then copies that to a string, which isn't great.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2023

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Mar 5, 2023
@Milk-Cool Milk-Cool closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2023
@targos targos moved this from Pending Triage to Stale in Node.js feature requests Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system. stale
Projects
None yet
Development

No branches or pull requests

7 participants