[FEATURE] Allow jsonc or JSON5 for package.json for commenting #793
Comments
|
@admosity unfortunatelly, this would be backwards incompatible :/ Also, it would require all other software that uses package.json, as well as many npm packages, to understand the new syntax... |
|
See #677 |
admosity
changed the title
You could have two files to keep it backwards compatible: The user would just edit package.json5 to add the comments. Npm could create (or override) package.json and just ignore the comments from package.json5. |
|
Any new feature is by definition backwards incompatible - isn't that normal and in the nature of progressing in the evolution of any piece of software? Besides being very easily fixed in other projects that load Babel added JSON5 support a while ago and the community just moves forward and fixes these tiny issues, which takes no time at all - and the improvements of JSON5 just sort of spreads organically to other packages, documentation improvements spreads to projects, and so on. That seems like a good thing. IMO, don't add more complexity/confusion just to provide an excuse for not moving forward. |
|
with vscode we can set the parser to |
|
I find comments invaluable! Currently we have to just remember or keep a separate document for stuff like this. {
"name": "comments-pls",
"version": "0.0.0",
"private": true,
"dependencies": {
"fooPkg": "^6.6.6" // don't update until <link to issue> have been fixed
},
"devDependencies": {
"husky": "^4.3.8" // WARN do not update to 5 the license have changed
}
}And no one looks at this sparate document until the house is burning. So in reality we just have to remember. And remember to tell new team members that they have to remember. It's not ideal. JSON5 also supports trailing commas. Yet another quality of dev life feature that is very welcome. |
|
JSON already supports comments: {
"foo": "foo is an important field!",
"foo": "the real value of foo"
}I’d love to see npm ensure these aren’t dropped whenever it changed package.json. |
|
To me that looks like unintended usage to hack a workaround rather then support. VS Code doesn't like it either |
|
It's quite intended, since JSON explicitly allows duplicate keys. vscode being wrong doesn't make it a hack :-) |
Can you please proof your words? I didn't find info about in at json.org, and the question leads me here: https://stackoverflow.com/a/21833017/2630849
|
|
See https://stackoverflow.com/questions/21832701/does-json-syntax-allow-duplicate-keys-in-an-object/21833017#comment33046713_21833017 and section 6 in the standard, which says “ does not require that name strings be unique” (https://www.ecma-international.org/wp-content/uploads/ECMA-404_2nd_edition_december_2017.pdf). |
I find both of these statements to be somewhat inaccurate. Quoting section 3 of the ECMA-404 spec:
"Does not require" does not imply "explicitly allows" - and, in fact, it goes on to clarify:
In other words, the behavior is explicitly undefined. VSCode can't be "wrong" on this point, since the behavior is explicitly undefined - it's really up to anyone what they want to do. Highlighting this as an error seems completely appropriate - since, per the specification, you can't know how an implementation will handle this, the only safe option is to treat this like an error. There's simply no right way to comment an JSON file. Only hacks. Which is what we're stuck with at the moment. |
Thank you.
I agree. And, for example, it means, that some JSON parsers can count the first duplicate key, others can the last one, there is no standard, guarantee, hereby (hacky) support of comments, returning to the subject. |
|
There is https://github.com/wiredmax/npm-json5 which works for npm 3. With a bit of love, I suspect later versions could be supported also. That's a less controversial way to transition |
|
Simply allowing for In the meantime, as part of your build/install process you could use https://github.com/sindresorhus/strip-json-comments-cli to generate a |
Probably not that simple, as many other tools (and websites etc) read and parse the
Same problem here, as other tools may not be able to parse the If npm officially supported JSON5, that would be a different situation - everyone would expect to have to upgrade their packages and web-services etc. Probably the best bet in either case though (whether officially supported or via 3rd party add-in) is to use an alternative file-name, e.g. |
Where would all the big softwares be today without all these BreakChanges updates. Progressive ! Do not stagnate in mediocrity for fear of angering the conservatives. |
|
I think npm could just on publish parse JSON5 and create TLDR: |
I expect a lot of sites parse I suppose these tools would need to upgrade - just that this could be disruptive to the ecosystem during a transition period. If people start renaming the package file, a lot of tools will lose track of the fact that these repositories were packages. |
|
To be clear, I'm not saying that means we shouldn't do it! I just mean to create awareness of the kind of problems we should prepare for. |
I think they could adapt, it might be a breaking change but thats what |
Again, not to say we shouldn't do it, but semantic versioning doesn't help here, at all. I'm talking about third-party services that read and parse |
|
npm If your bug is preproducible on If your issue was a feature request, please consider opening a new RRFC or RFC. If your issue was a question or other idea that was not CLI-specific, consider opening a discussion on our feedback repo Closing: This is an automated message. |
What / Why
For better or for worse the content within package.json has been growing with greater and greater complexity. In the nascent days of npm, the file was simply just some metadata on the dependencies that a project/package depended on. Projects grew in complexity and now testing, frameworks, other libraries are all dependencies that would appear sitting together. There was some separation with
dependenciesanddevDependencies, but that is a rough categorization. We have tools now contributing and taking residence within the package.json with greater complexity... monorepo configurations, lint configurations, commit configurations, git hooks, nodemon, jest, the list goes on...Comments offer a way to explain all the complexity.
I still think that library maintainers should still be using pure JSON for backwards compatibility, but at least offering this moving forward will allow project creators (the vast majority of users) the ability to document their package.json. Granted, json allows for duplicate keys to serve as comments, but this feels too much like a hack.
How
Current Behavior
A redacted and contrived example:
{ "name": "my-project", "version": "0.0.1", "license": "MIT", "scripts": { "build": "a bunch of scripts", "ci": "a bunch of scripts", "ci:local": "a bunch of scripts", "docsite": "a bunch of scripts", "docsite:combiner": "a bunch of scripts", "docsite:sassdoc": "a bunch of scripts", "docsite:tsdoc": "a bunch of scripts", "e2e": "a bunch of scripts", "html-sketchapp-install": "a bunch of scripts", "html-sketchapp": "a bunch of scripts", "lint": "a bunch of scripts", "sassdoc": "a bunch of scripts", "sassdoc:comp": "a bunch of scripts", "sassdoc:core": "a bunch of scripts", "start": "a bunch of scripts", "start:hmr": "a bunch of scripts", "start:qa": "a bunch of scripts", "start:dev": "a bunch of scripts", "generate-examples": "a bunch of scripts", "rebuild-markdown": "a bunch of scripts", "watch-examples": "a bunch of scripts", "test:cov": "a bunch of scripts", "test": "jest", "test:watch": "jest --watch", "test:cc": "jest --coverage" }, "config": { "commitizen": { "path": "./node_modules/cz-conventional-changelog" } }, "dependencies": { "@angular-devkit/build-angular": "~0.900.1", "@angular-devkit/build-ng-packagr": "~0.900.1", "@angular-devkit/core": "~9.0.0", "@angular-devkit/schematics": "~9.0.0", "@angular/animations": "~9.0.0", "@angular/cdk": "~9.0.0", "@angular/cli": "~9.0.1", "@angular/common": "~9.0.0", "@angular/compiler": "~9.0.0", "@angular/compiler-cli": "~9.0.0", "@angular/core": "~9.0.0", "@angular/forms": "~9.0.0", "@angular/language-service": "~9.0.0", "@angular/platform-browser": "~9.0.0", "@angular/platform-browser-dynamic": "~9.0.0", "@angular/router": "~9.0.0", "@angular/service-worker": "~9.0.0", "@angularclass/hmr": "2.1.3", "@ngrx/effects": "~8.6.0", "@ngrx/entity": "~8.6.0", "@ngrx/router-store": "~8.6.0", "@ngrx/schematics": "~8.6.0", "@ngrx/store": "~8.6.0", "@ngrx/store-devtools": "~8.6.0", "@types/jasmine": "~3.5.0", "@types/jasminewd2": "~2.0.3", "@types/lodash-es": "^4.17.3", "@types/node": "^12.11.1", "classlist.js": "1.1.20150312", "codelyzer": "^5.1.2", "console-polyfill": "0.3.0", "core-js": "^2.5.4", "cz-conventional-changelog": "1.2.0", "date-fns": "1.30.1", "highcharts": "^7.2.1", "highcharts-angular": "^2.4.0", "html2canvas": "^1.0.0-rc.5", "jasmine-core": "~3.5.0", "jasmine-spec-reporter": "~4.2.1", "karma": "~4.3.0", "karma-chrome-launcher": "~3.1.0", "karma-coverage-istanbul-reporter": "~2.1.0", "karma-jasmine": "~2.0.1", "karma-jasmine-html-reporter": "^1.4.2", "lodash-es": "^4.17.11", "mime": "~2.4.2", "ngrx-store-freeze": "0.2.4", "ngx-monaco-editor": "~8.0.0", "ngx-quill": "^7.3.12", "pdfmake": "^0.1.64", "prettier": "1.19.1", "protractor": "~5.4.3", "quill": "^1.3.7", "rxjs": "~6.5.4", "rxjs-compat": "^6.0.0", "standard-changelog": "1.0.19", "svgxuse": "1.2.6", "ts-node": "~8.3.0", "tsickle": "^0.35.0", "tslib": "^1.10.0", "tslint": "~5.18.0", "typescript": "~3.7.5", "web-animations-js": "~2.3.1", "zone.js": "~0.10.2" }, "devDependencies": { "@types/jest": "^24.0.6", "jest": "^24.1.0", "jest-preset-angular": "^6.0.2", "ts-node": "~7.0.1", "typescript": "3.2.4" }, "jest": { "preset": "jest-preset-angular", "setupTestFrameworkScriptFile": "<rootDir>/setupJest.ts" }, "nodemonConfig": { "ignore": [ "**/example-module.ts" ], "watch": [ "./a/bunch/of/stuff" ], "ext": "js ts md html" }, "workspaces": { "packages": [ "packages/*" ], "nohoist": [ "**" ] } }Expected Behavior
An example of a package.json with comments.
References
https://github.com/microsoft/node-jsonc-parser
Typescript uses comments in their tsconfig.json files.
The text was updated successfully, but these errors were encountered: