Allow comments in .nvmrc config file

[JohnAlbin]

PR #2288 has a discussion about comments in .nvmrc and I'm moving this to a proper issue so it doesn't get buried in the opinions about the code in that PR.

Why support comments?

Projects should, when necessary, have comments in their configuration files like .nvmrc.

The main reason I have comments in configuration files is to document bugs that force me to use a specific configuration option, .e.g. "Bug X causes Y so we set our config to Z as a work-around. [link to issue]" That kind of information needs to be right next to the configuration or someone (maybe even me!) will "fix it" later not realizing it causes a bug, e.g. "Huh, this file is out-of-date. The value of v14.13.1 should be updated to the latest v14.19.1. Oops. I broke something."

IMO, I think all configuration files should be able to have comments in them. Don't get me started on .json files for configuration.

Current .nvmrc specification

I was looking at the code in NVM and noticed this line in the nvm.sh file:

NVM_RC_VERSION="$(command head -n 1 "${NVMRC_PATH}" | command tr -d '\r')" || command printf ''

Only the first line in .nvmrc is ever used to determine the value of NVM_RC_VERSION. That means all the other lines in .nvmrc could potentially be used for comments without any code changes. I tested this by creating a .nvmrc file with an intentionally blank first line:

v14

And I got an error: Warning: empty .nvmrc file found

And when I changed the file to:

v14
# This is a comment
// And so is this
; And so is this
And so is this

The nvm use command reported Found '.nvmrc' with version <14> and ignored all other lines except the first.

Right now, afaics, the entire .nvmrc syntax specification is:

1. The first line must contain a Node.js version number; see https://github.com/nvm-sh/nvm#nvmrc
2. Other lines are currently ignored but are reserved for future usage.

Plan for allowing comments in .nvmrc

In this comment, the nvm maintainer said:

eventually nvm will support parsing additional content from successive lines.

If we want to officially support comments, no code changes will be needed in this version of NVM. But we'll need to be mindful of how complex the code will need to be to support code comments in future versions of NVM.

What we need to discuss:

1. At a minimum, we need to support line comments. We need to decide if we optionally want to support inline comments and/or block comments.

   # a line comment means the entire line is ignored
   some-config-syntax # an inline comment means just the bit after the comment marker is ignored
   <!-- a block comment can span multiple lines and everything
        between the start and end comment marker is ignored -->
   

2. Decide which comment format(s) to use. Examples:

   # hash-prefixed comments (like Perl, Python and shell comments)
   // JavaScript inline comments
   ; ini-style comments
   -- ADA/AppleScript comments
   /* JavaScript block comments */
   <!-- HTML block comments -->
   

   Wikipedia has many other examples of code comment syntax.

3. Ensure the chosen comment format doesn't interfere with NVM's:

   • Node.js versions
   • built-in aliases for Node.js versions
   • custom aliases for Node.js versions
   • the syntax for planned, but not-yet-implemented features in the future (I'm unsure what those requirements will be).

   For example: /* js-style */ block comments are INCOMPATIBLE with NVM's built-in alias, lts/*, so we can't use that comment syntax.

4. Document how comments work by adding it to the .nvmrc section of README.md

Both 1 and 2 will affect how complicated the code will need to be when we need to implement comment parsing. It's clear the NVM maintainer wants very low complexity and maintenance costs. totally understandable.

[JohnAlbin]

FYI, I prefer line and inline #-prefixed comment syntax because:

• it matches most of the other dot file comment syntaxes
• it should be simple to add parsing for it
• it doesn't appear to conflict with Node.js versions and alias syntax. Although, we would need to explicitly state that # is not allowed in custom Node.js aliases.

If it turns out that # conflicts with the syntax of planned features (of which I am not aware of), then I'd favor only allowing line comments where the first character in the line is a #. In other words, a line MUST start with a # to be a comment and the entire line is ignored when determining the syntax of the rest of the file.

[JohnAlbin]

As a work-around, devs can add comments anywhere after the first line of .nvmrc. It's possible those comments will break NVM in future versions and you'll have to remove them. Fortunately, my .nvmrc comments explain why the comment may break NVM. 😁

This is what one of my projects' .nvmrc file looks like:

v18.19

# Node.js v18.20 and v20.10 (and later) have a bug that causes Jest code
# coverage to randomly fail.
# TODO: Switch to "v18" or "v20" when this bug is fixed.
# https://github.com/nodejs/node/issues/51251

# Note: This is a non-standard .nvmrc comment. If these comments cause an error,
# see https://github.com/nvm-sh/nvm/issues/3336

[ljharb]

Additional lines will be used in the future for additional configuration, so if you put comments there things will break for you - don’t do that.

[JohnAlbin]

Additional lines will be used in the future for additional configuration, so if you put comments there things will break for you - don’t do that.

That's why I called it a work-around while comments are still not officially supported.

[ljharb]

Regarding the OP, it's an rc file, so the only two options that would make sense are ; or # - ie, ini or shell.

I think "shell" makes the most sense for nvm, so i'd say that "lines starting with #", at a minimum, would be skipped, and thus would support comments. Whether it's necessary to add in "inline" comments is worth discussing, but I'm not sure it's particularly useful, and it adds extra complexity in case that character is ever actually needed.

[ljharb]

Alrighty, npx nvmrc will validate the file in the current directory.

Officially, if this tool says it's valid, then your nvmrc file won't break in the future. If it says it's invalid, you're bad and you should feel bad, and you should fix it.

Notably, this does not validate the content - iow, if you put foo in there, this tool just assumes that foo is a valid alias. nvm itself will error at runtime here.

This tool does allow comments, and nvm currently does not - I'll close this issue when I've updated nvm itself to match the rules I encoded into the nvmrc package.

[JohnAlbin]

Alrighty, npx nvmrc will validate the file in the current directory. If it says it's invalid, you're bad and you should feel bad, and you should fix it.

I ran npx nvmrc against the work-around .nvmrc example in my April 18 comment above and it appears to validate the file:

$ npx nvmrc 
{ "node": "v18.19" }

So… I don't feel bad. And don't have to fix it, right? 😀

In #3265, you said:

The next version of nvm will ignore trailing lines (ie, only regard the first line) but I’d like to extend it in the future so it can contain additional information.

And above you said:

I'll close this issue when I've updated nvm itself to match the rules I encoded into the nvmrc package.

Can we close this issue after we finish the discussion on whether to and how to add comments to .nvmrc?

Because it's unclear to me whether you are okay with documenting that #-style comments can be included in .nvmrc as long as they aren't in the first line of the file. Should we update the docs to say this is allowed?

[ljharb]

@JohnAlbin The nvmrc tool already allows comments, so I've already settled "whether" :-) However, nvm itself doesn't yet allow them.

I'll close this issue when I add that feature, which will include updating the docs, and at that time I'll also document that npx nvmrc will validate the file (because that's when the feature support will match)

[JohnAlbin]

I've already settled "whether" :-)

🎉

I'll close this issue when I add that feature, which will include updating the docs

While you are working on the docs, feel free to take any of the text from my old PR #2288 and then closing it.

Thanks, Jordan!!!

[ljharb]

@JohnAlbin i didn't realize you and @Yash-Singh1 were the same account. I'll rebase that PR and use it to add the feature, thanks for the pointer.

[JohnAlbin]

@JohnAlbin i didn't realize you and @Yash-Singh1 were the same account

Oh, geez. doh. No, that PR is not my work or my "other account". I had a brain fart. I contribute to lots of different open source projects and sometimes it takes years for those PRs to get reviewed. And apparently, I thought this was one of my long-running PRs and linked to it without even reading the original issue description and author. 🙈

lol. I'm dumb.

@Yash-Singh1 deserves any credit for that PR.

I'm still thankful that this issue is getting resolved. And sorry for any confusion that I have managed to create all by myself.