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

Allow comments in package.json #178

Closed
sebmck opened this issue Jul 31, 2016 · 21 comments
Closed

Allow comments in package.json #178

sebmck opened this issue Jul 31, 2016 · 21 comments

Comments

@sebmck
Copy link
Contributor

sebmck commented Jul 31, 2016

No description provided.

@bestander
Copy link
Member

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

@sebmck
Copy link
Contributor Author

sebmck commented Aug 2, 2016

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
Copy link
Member

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

@kentaromiura
Copy link
Contributor

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
Copy link
Contributor

cpojer commented Sep 8, 2016

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
Copy link
Contributor Author

sebmck commented Sep 15, 2016

Going to punt this to later.

@danielbayley
Copy link

Supporting package.cson would solve this for free.

@ljharb
Copy link

ljharb commented Nov 24, 2016

@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
Copy link

tombh commented Apr 8, 2017

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
Copy link

ljharb commented Apr 8, 2017

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
Copy link

tombh commented Apr 28, 2017

Yarn is really our only morsel of hope. Npm is resolute in its intention not to include comments: npm/npm#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: #1669

@jcrben
Copy link

jcrben commented Jun 2, 2018

Duplicate request: #3959

@httpJunkie
Copy link

httpJunkie commented Dec 9, 2018

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:
webpack-contrib/extract-text-webpack-plugin#265 (comment)

@kajmagnus
Copy link

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

Recently there was this event-stream vulnerability: dominictarr/event-stream#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
Copy link

ljharb commented Jan 28, 2019

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
Copy link

kajmagnus commented Jan 28, 2019

@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
Copy link

ljharb commented Jan 28, 2019

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
Copy link

jcrben commented Jan 28, 2019

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 wiredmax/npm-json5#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
Copy link

kajmagnus commented Jan 29, 2019

@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).

@nick4fake
Copy link

nick4fake commented Jul 7, 2023

@sebmck

Going to punt this to later.

7 years have passed, so just to recheck, is it still postponed?

@VelocityPulse
Copy link

I still don't see any evolution...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests