-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Support for JSON5 #40714
Comments
What do you mean exactly with "Adding native support for JSON5 in Node" ? That Node.js provides a function to parse JSON5 strings? |
The specific problem I want to solve is the ability to use JSON5 for the I suspect (although not confident of) that to do this right, it means adding first-level support for JSON5 for anything you can do with JSON files and strings in Node. In which case you would be able to:
|
Presumably to parse, and to stringify, just like JSON. Stringification is a bit trickier tho because of things like comments, optional quotes and trailing commas, etc - there'd need to be a pretty complex API to support all that. |
Yeah I think if this is ever going to make any headway, it needs to start as a language spec for |
I think that to achieve the package.json5 goal, this is the order of operations:
|
The great thing about JSON is that it simply works everywhere, and most of the time, no extra libraries or tools are needed. For example, parsing a I agree that JSON5 looks like a nice format, but also that serialization won't be simple. For example, tools such as npm that modify JSON files need to preserve comments etc. |
Hi, I created the RRFC for npm to allow for a more permissive manifest format. The key points of it is:
I do not see a new manifest format as requiring any changes on Node's part, this can just be an additional dependency shipped with npm to handle this feature. It seems like the most economical and simplest solution that would be easiest for everyone to agree upon. However, there is some pushback from npm, preferring Node support this feature directly. So if that is the easiest path forward to achieve this goal, I won't argue. |
@TheJaredWilcurt npm never exists in a vacuum; tons of tooling and libraries need to read from, and write to, the source of truth ( |
This comment has been minimized.
This comment has been minimized.
It's more likely that an additional file is added for scripts and other config, like I'd campaign for that instead of this as I very much doubt |
Again, in my proposal, it is 100% BACKWARDS COMPATIBLE. Anything doing All you need to do is import an extra library and add 1 additional if statement at the top and bottom of your code (for reading and writing). It is not a high burden, it is a very small, and optional, change. If libraries do not wish to support the new system, they don't have to. Not everyone will adopt the new manifest format, and those that do will either update the tools they use via PR's, or switch to alternative tooling that works for their new workflow.
That's what we are asking for. The |
No it’s not. You’re asking for people to check 2 identical files into git, connected by a “build” command ( What I suggest is literally just an additional file, not a “copy with comments” file. Your RFC even mentions a “transition period” as if package.json would ever stop being supported or generated. |
@TheJaredWilcurt most tools don’t just read package.json, they write to it also - the ones that just read are a minority. As a maintainer of some of these tools, i can assure you this is a large burden, especially since these tools are often transitively depended on, many layers deep. @fregante making a separate files just for dev-time scripts seems much more feasible, and would require only ~2-3 of the steps mentioned in #40714 (comment), including that only npm, not node, would need (ever) to look at this new file, so it seems like a much more feasible option. |
Most tools (that are used seriously) feature any of these 4 things, if not all of them:
FUD can be managed |
Thank you for the feature request, @EvHaus. In my view, adding JSON5 support for package.json in Node.js isn't going to happen any time in the foreseeable future, and probably never. I'm going to close this as leaving it open provides the misleading perception that it is being seriously considered. @nodejs/tsc If anyone disagrees and thinks the original feature request here is likely to come to pass, please feel free to say so and/or re-open. |
How are we supposed to document what packages are for, why a version is pinned, and so on? Doing so is a regular part of the maintenance of our rails backend, with comments explaining why we've pinned a version and linking a ticket to fix that problem. For example, sometimes this is an issue we've filed in the package's repo, and so anyone on the team can periodically check that link to see if the version pin can be updated or removed. Using "//" to comment doesn't work: it's a single line, with not enough space to put everything, and neither npm or dependabot support it. I'm disappointed that this aspect of "comments in package.json" has been completely trumped by backward compatibility, with seemingly no consideration to the problems of actually maintaining a real-life application that makes active use of package.json and version pinning, where it's important that knowledge is persisted in situ. Workarounds like putting package notes in the README or elsewhere make maintaining knowledge connected to packages and versions are far more likely to not be updated. It's also near-impossible to enforce, and so simply doesn't scale. I don't understand why the package.json5 proposal wasn't even considered? Yes, it's an additional file, and it's not pretty, but it would unblock us from actually being able to connect knowledge with code. Commenting your code is something that is widely considered to be best practice in the industry -- why does that best practice stop when documenting dependencies? Heck, with all the attention that supply chain management is getting these days, one would think that documenting dependencies would be even more important. Instead, npm is ignoring this because node is ignoring it. And dependabot isn't even supporting yarn's solution because npm doesn't support it. Maybe I'll just make a package that does the |
Checking all the package managers that dependabot support, here's a list of package managers that do support comments in their dependency declarations: bundler, cargo, docker, hex, go, gradle, maven, nuget, pip, terraform, pub Ones that don't: composer, elm... and npm (because node doesn't) & yarn (because node doesn't). |
I'd like to remind everyone that Node.js is a community-driven project, it's not a corporation or a self-aware entity that takes arbitrary decisions. It's also up to you to take ownership of this problem and find a solution. See #41927 (comment) for a roadmap if you feel like that's something you want to take up.
That's probably the most reasonable move atm, and hopefully if |
I see your point, and I don't want to disagree or argue, I just want to clarify that my saying "node is ignoring it" was in reference to @Trott's final comment and closing of the issue. |
Using a package.json5 for an app - something not published or consumed by anyone else - is fine, and the downsides of that are minimal. Uncountable problems would stem from using it in a published package. Due to node and npm not separating those use cases into different files, anything supported with package.json for one, affects the other. I think that’s one of the bigger constraints, personally (to not affect the package ecosystem). |
|
Here's my MVP (obviously for yarn, please post if you adapt for npm):
"scripts": {
"compile-package-json": "json5 -o package.json --space 2 package.json5",
"prepack": "yarn compile-package-json"
}
One issue is it needs to be bootstrapped, hence the dedicated |
@targos that would only be viable if npm (like package-lock.json) never published |
As I wrote in that comment:
That comment was directed at the TSC to discourage drive-by requests to re-open, but let me scope it more properly: I don't mind this being re-opened if someone is (publicly, transparently) actually working on it or investigating it seriously. I understand the frustration. It's wrong, though, to conclude that Node.js maintainers don't appreciate the seriousness of the problem. (OMG, I would love to be able to put comments in |
I agree that I am probably the least informed person in this thread about all of the Node.js codebase, either package manager, and the impacts choices here make. I'm sorry that my comments came across as disrespectful to the maintainers. Not at all how I want to be communicating; I appreciate your understanding of my frustration and the ensuing poor wording choices. I also appreciate your clarity that you (and presumably other maintainers) want this feature, too. I really did have the opposite impression from previous discussions on the topic. That may well by my own fault in interpreting what I read :). From what I hear, achieving this isn't totally impossible, it's just threading a needle of broad tool support and other impact. In particular (forgive the terminology, it's probably not quite right):
Please let me know if there's anything I've missed. While there's some very tricky parts here, I wonder if there's some basic solution that can meet these constraints. Ideally, this would be an ecosystem-wide approach. IMHO, if node maintainers could agree on and indicate a workable direction, that may give yarn and npm the "permission" they need to make something. (Maybe that's an uninformed outsider's perspective.) It thus would need to be very simple, and achieve some level of general consensus or at least consent. From what I understand supporting this for applications ("end users" of node) is likely highest impact, lowest risk. With all that said, one idea off the top of my head: Indicate that Node endorses the approach of using a third package file (maybe just one, maybe any of a set such as json5, toml). Scope that only to end applications (i.e. not supported for published packages), and constrain it to dev machines only (so no tooling needs to be updated). Get some level of agreement from yarn and npm (I'm happy to help, though I imagine the maintainers of these projects have direct relationships and don't need me.) Perhaps all that's needed from yarn and npm is a single, consistent lifecycle hook that an outside package can use to run a compile step -- this makes it an optional, power user feature that can be used to measure interest and adoption. (Fundamentally, I suppose this would also be a bet that e.g. JSON5 gets broader stdlib support. YAML could be an option that has nearly the same level of language support already. Though Node supporting JSON5 could certainly help solve the chicken-and-egg problem of "why bundle JSON5, no one uses it".) I imagine there could also be valid concern about "what happens next." This post is already long enough, but I think if the initial approach is chosen with enough constraints, there would be a roll-out option that is simply removing constraints on a published timeline. This would require some project management, but not as much community management. Or maybe this won't even be needed... Feel free to tell me why this wouldn't work, ideally paired with a "and here's what could" :). |
I think it's not possible for Node to support two formats as it will break the ecosystem. Your best option is to ask npm and yarn to give you enough hooks to implement a transpilation step. |
Hey, look, this is really frustrating and discouraging. I have made statements about how the Node maintainers are blocking anything to do with this, and I've rightfully gotten pushback that that is not the case. But then, based on the openness that suggests, I take the time to make a constructive proposal on how to move forward -- incorporating all issues raised to date -- and your response is saying outright no. What's especially frustrating is that it is a different person saying no, in a different way, with each response I've received. When that happens, I lose all motivation to try to help out. Is that the sort of impression you want to give potential contributors to Node.js? To directly address your comments, I have already linked to examples where Yarn and NPM have explicitly said no to something like this because Node does not support it. That's why my suggestion was for a form of publish endorsement, without any code changes or ecosystem impact to Node.js itself. Anyway, I would appreciate if you Node.js Members could talk between yourself and decide whether you're open to the idea or closed to it, because this inconsistency is driving me crazy. |
I don't really understand this point, I don't think you need blessing from this project to be honest, if you make a Yarn plugin that converts your
Well that's what we are doing in this thread, where else would you want us to talk about it? x) |
When I said "While Yarn and NPM could support something directly, they choose not to because Node.js doesn't support it." I was referring to these responses: |
fwiw the people who answered those questions are also node maintainers. i think they were saying less that node as some entity unto itself refuses and more that they can not see a path forward to the node ecosystem migrating beyond normal json.
I'm sorry for the confusion. We generally don't make decisions like this until concrete implementation is brought to discussion, which is why you're getting what sounds like conflicting answers. In terms of our official process, this would not actually be blocked until someone opened a PR and collaborators stated active objections to it (and even then, only that specific PR would be blocked). The openness of the issue is generally an indicator of whether someone would be tracking/implementing such a feature, or whether we think it is likely for that to happen in the "near" future. |
I guess it depends what you mean by "this" and "it". You wrote:
To me, that says that any version of this that makes a JSON5 version of package.json a first-class citizen is a non-starter (impossible). But I'm not sure we're understanding things exactly the same way here.
Yes, and making a change that causes some users to have a package.json that can't even be used or parsed by the previous version of Node.js, npm, yarn, and pnpm is worse than that. It's a never-ending boiling-the-ocean task. I won't say a breaking change like that is impossible, but the bar is fantastically high. I don't think I've ever seen anything that rises to that level in my 7 years of involvement in Node.js, and I don't think I ever will. Not impossible, but extremely unlikely.
I think this hints at a key obstacle. To introduce a new and backwards-incompatible format, you'd need to basically get four projects on board to do it together: Node.js, npm, yarn, and pnpm. Getting Node.js to do ANYTHING in a "hot stove" area like package.json format is a big lift. Adding on the others makes it a colossal lift. Again, I won't say it's impossible, but it needs to be something extremely compelling. If npm, yarn, and pnpm all came to Node.js with "We want this to happen", that might move the needle. (And... it also might not.) I know it's frustrating because you feel like they've basically said "We need Node.js to say this is going to happen". I don't know what the answer is other than spending a lot of time to talking to a lot of people and making the case (and that has a high probability of failing anyway, but you never know until you try).
I don't understand this. What does Node endorsing an approach involve? Just a statement? Or...?
Node.js is not a monolith and no one individual speaks for the project. Matteo is certainly at or near the top of any list of our most influential maintainers, but there are over 20 people on the Node.js Technical Steering Committee and nearly 100 people who meet the conventional definition of maintainer (meaning they have the ability to commit to the repository and can approve or reject pull requests). This organizational approach has lots of advantages, but also considerable disadvantages. One is that people can feel like they are getting a lot of mixed messages from "the project". So try to bear in mind that no one is speaking on behalf of Node.js, only themselves. In the uncommon cases where conversation runs its course but agreement has still not been reached, things get escalated to the Technical Steering Committee. |
@bmulholland if it is any consolation pnpm actually supports both json5 + yaml out of the box. great work @zkochan! This could practically unlock the flows you want today. I could not find a ton of documentation but I did find doing a simple The major road block I hit was that when running the application node.js had no local package.json to reference so I lost the ability to use package.exports + package.imports in my own project. This could likely be dealt with by transpiling the json5 -> json when running This shows that this is indeed a feature that various CLIs could implement without requiring the support from node, but it is kinda edge case'y and I'd argue probably not the best solution for the average person today. Play around with it a bit and if you still want to propose json5 or yaml or whatever configuration language you can make an earnest proposal at https://github.com/npm/rfcs. The bar will be much higher to ship in npm, which is a reason why clients like pnpm tend to really shine for these use cases. |
There was some backlash when I added this feature to pnpm. I was accused of introducing division in the ecosystem and incompatibilities with Node.js, npm, and other tools. So, I decided not to talk about this feature. I keep it because a few times, users have asked for it. But I don't think this feature is mentioned anywhere in the docs. |
@zkochan I'd appreciate if you could take some time to review the proposal I've written up here: For adopting an optional |
JSON is a great (de-)serialization format for apps' configs and data. As a modern application environment, it would be great to be able to use |
Node needs a simple global switch to have it read all .json files as .json5. Strict json is a plague on humanity, especially those trailing commas. |
please guide me is there a way to create under nodejs app a server able to parse json5 POST bodies, like
because i use typescript and the exising library does not work with |
As a developer, I sometimes have trouble with the regular JSON format when dealing with my project dependencies. JSON5 is like an upgraded version of JSON. It has extra features such as comments and trailing commas. But, unfortunately, NPM, a popular tool for managing packages, doesn't accept JSON5 for its package.json file. If Node.js could accept JSON5, it would help developers handle dependencies and settings in a clearer way. And don't worry, it would still work with the usual JSON format. |
@cwtuan, fortunately, you have some options here.
Since npm has always been (...), and yarn went into a weird direction since v2, pnpm is the best package manager right now anyway (on both usability and speed). Downside of that approach is that 3rd party tools would still expect package.json. You can bother maintainers of those packages to support json5, or write your own tools.
If you use deno, you specify your dependencies in the individual source files (people use It installs dependencies much faster (native executable would outperform package managers written in js every time), it has security model that node.js doesn't have, so a choice worth considering anyway. Since last year, deno has been pushing for backward compatibility with node.js, so everything should run fine by now, but support from 3rd party tools will be lacking. Again, if a tool doesn't do what you need, consider writing your own. Personally, I've completely switched to deno a few months ago (because of this issue + dependency install time). But it'd be interesting to see more wide adoption of package.json5 managed by pnpm as well in node.js packages. |
Is your feature request related to a problem? Please describe.
The lack of comments in
package.json
is clearly a big problem that the community faces (examples 1, 2, 3, 4). Currently the community is forced to use all sorts of ugly and hacky workarounds (examples 1, 2, 3, 4). Discussions to add it tonpm
have failed.Describe the solution you'd like
Adding native support for JSON5 in Node would help address this pain point and move the entire ecosystem forward.
Describe alternatives you've considered
Other alternatives would be exploring non-JSON formats like TOML, but that seems like it would be a bigger project. JSON5 is backwards compatible so it should be a simpler integration.
The text was updated successfully, but these errors were encountered: