Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Support for comments in package.json #4482

Closed
saschagehlich opened this issue Jan 14, 2014 · 45 comments
Closed

Support for comments in package.json #4482

saschagehlich opened this issue Jan 14, 2014 · 45 comments

Comments

@saschagehlich
Copy link

I know that there are no comments allowed in JSON, but I'm running a product that has 30+ dependencies, so package.json gets very large and unclear.

Are there any solutions for that? A package.js file that exports a hash would be a solution but it could also contain executable code.

@rlidwka
Copy link
Contributor

rlidwka commented Jan 14, 2014

It was javascript three years ago, look at #408. Sadly it was removed. Anyway, javascript doesn't really fit here because ideally npm should not be executing any code at install time.

But we can use YAML or JSON5 instead, they are both support comments and have much nicer syntax, I tried this approach with my packages for half a year now.

I'm currently seriously thinking about forking npm to add this support. Not sure how it'll go in the future, but I'll be more than happy to make a pull request against npm if somebody with nickname starting with "i" changes his mind.

But until then can you try yapm to see if it works?

@saschagehlich
Copy link
Author

I'd love to use yapm, but it's a real fork of npm and I don't know for how long you will keep it up-to-date.

I created a pre-npm hook 2 years ago but apparently it doesn't work anymore: https://github.com/saschagehlich/npm-yaml

@domenic
Copy link
Contributor

domenic commented Jan 14, 2014

Yeah, this is not happening. You can always use

{
  "//": "a comment",
  "//": "another comment"
}

@domenic domenic closed this as completed Jan 14, 2014
@saschagehlich
Copy link
Author

Can you also describe WHY it is not happening?

@domenic
Copy link
Contributor

domenic commented Jan 14, 2014

See #408 and #3336.

@rlidwka
Copy link
Contributor

rlidwka commented Jan 14, 2014

I'd love to use yapm, but it's a real fork of npm

There is a simple wrapper, versions yapm@0.7.2 and less. I moved to a "real fork" only a month ago, and not convinced it's a right path.

Also, I asked to introduce npm plugins in #4416, but got negative response straight up.

Any other suggestions? Anyone?

@saschagehlich
Copy link
Author

@domenic Your solution does not work.

npm http GET https://registry.npmjs.org//
npm http 200 https://registry.npmjs.org//
npm ERR! TypeError: Object.keys called on non-object
npm ERR!     at Function.keys (native)
npm ERR!     at installTargetsError (/usr/local/Cellar/node/0.10.24/lib/node_modules/npm/lib/cache.js:709:24)
npm ERR!     at /usr/local/Cellar/node/0.10.24/lib/node_modules/npm/lib/cache.js:639:10
npm ERR!     at saved (/usr/local/Cellar/node/0.10.24/lib/node_modules/npm/node_modules/npm-registry-client/lib/get.js:142:7)
npm ERR!     at /usr/local/Cellar/node/0.10.24/lib/node_modules/npm/node_modules/graceful-fs/polyfills.js:133:7
npm ERR!     at Object.oncomplete (fs.js:107:15)

@domenic
Copy link
Contributor

domenic commented Jan 14, 2014

Notice my comments were at top level.

@isaacs
Copy link
Contributor

isaacs commented Jan 14, 2014

Yeah, the keys in the dependencies hash are relevant. You can't put comments there.

@dandv
Copy link

dandv commented Sep 11, 2015

@domenic: The refusal to implement support for comments, or an alternative format like JSON5, is most unfortunate. There are official efforts to use npm for front-end packaging (http://blog.npmjs.org/post/101775448305/npm-and-front-end-packaging) and unofficial ones to show how npm is often sufficient instead of grunt or gulp (@keithamus's How to Use npm as a Build Tool).

All these efforts are annoyingly limited by a format that doesn't allow comments or multi-line strings.

By comparison the manifest.json for Chrome extensison supports comments, and my experience using both for the last months has been so much better in the case of manifest.json.

Please reconsider a mechanism to allow comments in package.json.

@keithamus
Copy link

Don't forget it isn't just npm that reads the package.json file, Node's module system also does. It's one (massive, unwieldy, big breaking) change for npm to support this, but a whole new level of pain for Node to also. Supporting comments means using a different JSON parser, which means it'll probably be slower, which means it'll never get picked up by Node, which means it should never be picked up by npm.

As much as I'd welcome comments in JSON, I can very much understand and accept why it's probably never going to happen. Ho hum, there's lots of other places to document stuff, README.md, js files, etc.

@aredridel
Copy link
Contributor

So what kind of commentary are you hoping to add?

@rlidwka
Copy link
Contributor

rlidwka commented Sep 26, 2015

Node's module system also does.

Well, it shouldn't.

But it doesn't matter here 'cause node module system doesn't fail if package.json can't be parsed. And since only thing node needs is "main" field which can be replaced with "index.js", I'm perfectly fine with node module system failing to read those.

So what kind of commentary are you hoping to add?

This kind - that's bunyan wannabe comments I usually refer to. Also, this kind (there are a lot of comments there) - that's my comments, I'm using them liberally since I switched to YAML format. Comments about which packages are deprecated and why. Comments about which packages are using semver and which don't. And so on and so forth.

@dandv
Copy link

dandv commented Sep 27, 2015

So what kind of commentary are you hoping to add?

TODO comments are another type. E.g. "TODO rm this dep once XYZ is in place."

@mike-lang
Copy link

This is a bummer for me as well. In my application I have at least one dependency pegged to a specific version because the latest version has unresolved issues. It'd be nice to communicate in the natural place to my team (and myself in the future) that this was intentional and why it was done.

@keithamus
Copy link

This is a great example of documenting your package.json outside of the JSON - comments would not be as thorough as this.

@mike-lang
Copy link

@keithamus while thorough documentation is nice, there's virtues to brevity and locality too. A short comment next to the relevant line of package.json is more likely to be noticed and heeded by a developer than a lengthy document stored elsewhere.

@mike-lang
Copy link

This seems a bit of an ugly hack, but passing package.json through a preprocessor pass that stripped all lines whose first non-whitespace characters are // before passing the contents along to JSON.parse might be a nice middleground. We could have a constrained form of comments while also getting the benefits of demanding proper JSON otherwise.

@rauschma
Copy link

rauschma commented Dec 3, 2015

For maintaining larger projects, I’ve found comments to be very important. It’s probably too big a change, but Node’s JSON.parse() could be upgraded to support JSON + comments. I’m not sure whether or not that would be in violation of the ECMAScript spec.

@insin
Copy link

insin commented Dec 3, 2015

I use blank lines and grouping to make this manageable - e.g. blank line before webpack and all of the dependencies which work with it are immediately below that.

Different solution, but same aim, I'd like it if npm would recurse dependency objects so you could group them, e.g.: https://twitter.com/jbscript/status/654653797407457280

@naholyr
Copy link

naholyr commented Dec 3, 2015

If package.json supports comments, how would those comments be moved when a dependency is moved by npm install --save? How is it supposed to keep blank lines too?
How is npm version or npm init (re-run) supposed to format file AND keep human changes?

I like to generally consider this file as a binary file: not intended to be read or modified by anything but tools (more likely npm). This vision cannot be fully applied on day-to-day basis because npm has its limitations (performance being the most important) and some sections still have to be edited manually ("engines", "scripts"…), but I think it's the way it's intended to be used.

@localjo
Copy link

localjo commented Dec 3, 2015

I agree that comments in package.json would be nice. Our team has used the .js instead of JSON solution in some situations, but I agree with @rlidwka that it's not the right solution here. Another solution that we use, that works well for us, but may not fit in with everyone's workflow; using descriptive commit messages when we make changes to package.json, and then checking the blame for comments. I tend to prefer that approach in a lot of other situations as well, and keep my actual in-line code comments brief.

@kevinSuttle
Copy link

+1 @josiahsprague.

@mistadikay
Copy link

I would absolutely thrilled to see an alternative package.json format with comments support as well. The only thing we could do right now is to divide groups of dependencies into sections by having additional line breaks. It helps a little, but the comments would be so much more helpful.

@andylima
Copy link

andylima commented Dec 5, 2015

The refusal to implement support for comments, or an alternative format like JSON5,
is most unfortunate

By comparison the manifest.json for Chrome extensison supports comments,
and my experience using both for the last months has been so much better
in the case of manifest.json.

Totally agree. I hope that the maintainers of npm will see how useful comments can be in package.json (and .json config files in general).

@kevinSuttle
Copy link

@kevinSuttle
Copy link

Actually, leave it to Crock to remind us why we shouldn't do this. The running answer is:

"Put all the comments you want in there. Just run it through a parser before you distribute it."

https://plus.google.com/+DouglasCrockfordEsq/posts/RK8qyGVaGSr

Good comments here
https://news.ycombinator.com/item?id=3912149

And I can't disagree with the comments made here:
https://news.ycombinator.com/item?id=4031699

@mike-lang
Copy link

Ok so what if npm piped package.json through jsmin and used the result?

@kevinSuttle
Copy link

Can that be handled via a prepublish or preinstall script?

https://docs.npmjs.com/misc/scripts#common-uses

@mathieumg
Copy link

How would you deal with commands like npm install mydep --save then?

@mike-lang
Copy link

@mathieumg You're absolutely right. That's the case I keep forgetting about, and I have no idea how to handle it properly. npm would have to be able to both read the file with and comments and write it back out, preserving the comments.

@mathieumg
Copy link

https://github.com/rlidwka/jju handles that scenario correctly!

@kevinSuttle
Copy link

Since NPM's package.json mostly conforms to the CommonJS 1.1 spec, it's possible to be creative here. http://stackoverflow.com/a/27232456/398574

@kevinSuttle
Copy link

And https://github.com/kof/node-xpkg#x-packagejson5 does exactly this.

That overlay property seems useful, too.

@mathieumg
Copy link

How would that fare with the scenario I described above?

@kevinSuttle
Copy link

JSON5 supports comments. You could put them in your package.json and then pipe the file through https://github.com/kof/node-xpkg#x-packagejson5

@mathieumg
Copy link

I'm referring to the npm commands, see #4482 (comment)

@kevinSuttle
Copy link

Both the JSON and YAML configuration file formats support comments (package.json files should not include them). You can use JavaScript-style comments or YAML-style comments in either type of file and ESLint will safely ignore them. This allows your configuration files to be more human-friendly.

http://eslint.org/docs/user-guide/configuring#comments-in-configuration-files

@laktak
Copy link

laktak commented Dec 16, 2015

This allows your configuration files to be more human-friendly.

package.json is one of the reasons why Human JSON exists.

@merriam
Copy link

merriam commented Jan 13, 2016

The hack of "//": "comment" will not work in a list of dependencies. How about just special casing this? That is, no package dependency named "//" will be allowed and the line will be ignored?

@Katharsas
Copy link

Please make http://json5.org/ possible. A format without comments is not feasable for human-written configuration files imho.

Or is there some automated way to convert to a comment-free package.json while running npm install (e.g. using the frontend-maven-plugin )?

@rightaway
Copy link

+1 for json5. The larger that package.json gets the more difficult it is to keep track of why specific things were added.

@mattecapu
Copy link

mattecapu commented Apr 19, 2016

I would add that a lot of tools now support configuration from within package.json, and it would be convenient to be able to // some line while debugging or add explanations for them.
Also npm scripts are getting a lot of momentum and it's frustrating to not be able to add explanations for my future self.
JSON5 seems a great solution for this.
NPM could start supporting package.json5 and convert it to package.json on publishing, so it won't break any builds on older versions of npm.
Moreover, it could be a good opportunity to change name to something more sensical like npm.json5 or .npmrc, idk

@kevinSuttle
Copy link

Wondering what flavor of JSON Sublime Text and Visual Studio Code use for their config. They both allow comments.

@othiym23
Copy link
Contributor

This is highly unlikely to ever change in npm itself. package.json is a control file, a manifest, instructions for Node.js's module loader, install metadata, registry metadata, and cache metadata all combined into a single file, and is used everywhere, by everything (including many, many older versions of npm), all of which expect it to be pure JSON. To change it within npm itself at this point would be a truly massive undertaking, especially when set against the gain.

As #408 hints, having an executable manifest leads to whole classes of security flaws (and the complexity of YAML has, in reality, caused problems for the Ruby world for this reason). As #3336 hints, and as many comments on this issue explicitly say, there's nothing preventing you from creating a different representation of the metadata in package.json and using lifecycle scripts or external tooling to sync it to package.json.

Because this is unlikely to change, further bikeshedding is unlikely to be productive. Therefore, I'm locking this issue. package.json being the control file for npm, and it being parseable by JSON.parse(), is as close to a fixed point for the npm commons as any aspect of its design.

@npm npm locked and limited conversation to collaborators Apr 19, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests