Don't print Validation Warning for "//" comments in package.json

[melissachang]

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

package.json doesn't support comments. The official, npm-sanctioned way to add comments is to use "//" key:

{ "//": "this is the first line of a comment", 
  "//": "this is the second line of the comment" } 

Running https://repl.it/repls/ThornyHatefulVirus prints a Validation Warning:

● Validation Warning:

  Unknown option "//" with value "comment" was found.
  This is probably a typing mistake. Fixing it will remove this message.

  Configuration Documentation:
  https://facebook.github.io/jest/docs/configuration.html

What is the expected behavior?

No warning should be printed for "//" keys.

[thymikee]

I don't know, is this a widely used practice? Honestly I see this for the first time, but maybe it's just me. We would probably need to add more if these special characters to the whitelist, which I'm not happy with.

[thymikee]

Is this actually documented somewhere outside of node mailing list? I'd be happy to accept a PR if this really exists

[melissachang]

This does not appear to be documented at docs.npmjs.com, but the mailing list looks pretty official to me. It was written by the creator (and current CEO) of npm.

Unfortunately github doesn't allow searching on special characters, so I can't see how common this is.

FYI the SO post is the first Google hit for comments package.json

[SimenB]

I remember seeing this as the response on the npm issue tracker, I think I actually submitted a PR at one point to npm supporting comments. Lemme see if I can dig it up!

EDIT: Yeah, see npm/npm#4482 (and my rejected PR, for posterity: npm/read-package-json#46)

[rickhanlonii]

I've seen this used before and since node people are directing folks to do it I'm also +1 for supporting 👍It's not like we're ever going to use "//" as a config for anything

Labeling good first issue! @melissachang any interesting in taking it?

[melissachang]

Sorry, not at the moment :/

[rickhanlonii]

No sweat!

[andrew-pyle]

@rickhanlonii First-time contributor here. I’d like to take a stab at implementing this feature. Can you point me at the validation logic for Jest configuration in package.json?

[SimenB]

That's awesome @andrew-pyle!

It's here: https://github.com/facebook/jest/blob/c22d9f584627a1ae6c476d44f10c3c186b480722/packages/jest-validate/src/validate.js#L59-L63

What I would do (I think, haven't tested it) is add // to the default blacklist. Feel free to ask questions if you're stuck 🙂

[andrew-pyle]

Pull request #7295 sent to implement @SimenB's suggestion of adding "//" to the default blacklist

[SimenB]

Fixed in #7295

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.