Yarn: Allow comments in package.json

[sebmck]

[bestander]

Converting it to yaml? :)
Would it break npm interoperability?

[sebmck]

Only if you attempt to npm install inside a directory that has a package.json with comments. When we have a kpm publish command we can strip the comments from the JSON we put in the tarball.

[bestander]

Ok, but this will make it non parseable by native JSON.parse command, right?
It is an extension of json.

[kentaromiura]

I'm not sure this is a good idea, people might have expectation of .json files being in "proper" json format so it might break this expectation (breaking tools / interop problems with foreign languages etc).
What I can suggest it might be to support an alternative package.ejs (exported js) that will be just a standard js with an implicit module.exports = in front of it (or if you prefer export default)

[cpojer]

I think this works well if someone fully bought into kpm and everyone who contributes to a project also does. As @kentaromiura points out, it does indeed happen that package.json files are read and parsed (Jest does this and rn packager too). We just need to make sure they never make it to the registry and projects that require package.json files don't use comments. The problem is though, when people do want to use comments and also need to require it, they need to use a custom parser, which does kinda suck.

[sebmck]

Going to punt this to later.

[danielbayley]

Supporting package.cson would solve this for free.

[ljharb]

@danielbayley assuming yarn doesn't wish to break the npm ecosystem, it wouldn't be entirely free because it'd have to be converted to a proper package.json prepublish.

[tombh]

Just some inspiration on this topic:

A dependency manifest (and even more so package.json) is a remarkably comprehensive, accessibly concise, syntactically enforced and implicitly maintained piece of _humanly_ consumable documentation. It is the doorway into understanding a new project. That comments are not supported is not merely a minor omission but an existential misrepresentation.

But of course I appreciate the rabbit hole of technical hurdles. Nevertheless I don't think the above point is made clear often enough.

[ljharb]

JSON already supports comments:

{
  "foo": "this field is for the version of foo",
  "foo": "^1.2.3"
}

Be as humane as you like! Don't forget to file bugs on tools that inadvertently destroy these duplicate keys (npm install --save, for example).

[tombh]

Yarn is really our only morsel of hope. Npm is resolute in its intention not to include comments: https://github.com/npm/npm/issues/4482

I'm not going to keep rehashing the same arguments. The point here is, the lack of comments in Javascript's canonical configuration specification is indicative of the broader critiques often levelled at the language. Yarn, taking the burden of initiative, is a commendable response to such critiques such that it contributes to an overall iterative improvement to the language and ecosystem as a whole. Considering this, is it appropriate that the community look to Yarn to, at the very least, explore the ultimate ideals of dependency management and the manifest document that defines it?

@ljharb somewhat ironically, even Yarn itself will not fix key deduplication: https://github.com/yarnpkg/yarn/issues/1669

[jcrben]

Duplicate request: https://github.com/yarnpkg/yarn/issues/3959

[httpJunkie]

Why not just allow for a package.json.comments file that is constantly watched and strips out the comments and outputs to package.json?

This is obviously something that could be setup as part of a webpack configuration. Not sure if this has anything to do with yarn though...

Kind of reminds me of this comment:
https://github.com/webpack-contrib/extract-text-webpack-plugin/issues/265#issuecomment-252325142

[kajmagnus]

Not being able to easily add comments is to some degree, a security issue.

Recently there was this event-stream vulnerability: https://github.com/dominictarr/event-stream/issues/116 — and then it's imporant (I think) with a comment in package.json that tells people to not upgade past v3.3.4. When it's not obvious & simple to add comments (instead, the hacky workarounds mentioned above are required), some people won't add any comment about this security vuln, resulting in possibly accidental upgrades later on, to the compromised higher version numbers.

[ljharb]

That’s already achieved with npm depreciations and a semver range that doesn’t include the problem versions; nobody would read that comment since npm install/yarn add commands alter package.json without any human editing.

[kajmagnus]

@ljharb Thanks for the reply. Hmm, what I had in mind, is people editing the package.json file in a text editor, and changing the semver range, without knowing that that brings in a vulnerability. Not everyone updates dependencies via the command line tools — sometimes peolpe edit the files directly, well at least I do sometimes. Then to me it seems like useful with a comment, to prevent people from accidentally editing the semver range to something dangerous.

That no one reads that comment isn't totally true — I look at package.json files of libs I think about using, or packages I'd like to edit / change, and I would think some percentage of other developers, do too. In my own projcects, I'm going to prevent Yarn and Npm from removing the comments I now added to my package.json, by looking at the Git diff during code review and undoing any deletions.

Anyway, never mind, barely matters.

[ljharb]

Fair, I was being hyperbolic - I think not enough people would read the comment for that to be a useful argument in favor of package.json comments.

[jcrben]

Most projects are missing a handy "why do I have this dependency and how did we vet it" comment, which could just be a pointer to the vetting ticket. Admittedly, git blame can sometimes help for that. Also, for those who really care (myself included): https://github.com/wiredmax/npm-json5 - altho I discovered it doesn't work with npm 4 or 5 https://github.com/wiredmax/npm-json5/pull/2 much less yarn.

There are of course a variety of tools which will read package.json and provide reports, but that's not exactly lightweight...

[kajmagnus]

@jcrben ok, interesting to read about those projects. Now I had a look in my Scala project dependencies file, and there, I've added comments for most of the dependencies about why they're needed, and what they do, in case they have not-descriptive names. And why one lib was choosen, in preference to another lib that does mostly the same thing. ... Comparing with package.json with zero comments (well, now, 1 comment).