feat: .node-version support

[andreassjoberg]

Support for .node-version file in accordance with volta-cli/rfcs#54.

Extended on #1974:

• Fixed naming .node_version -> .node-version
• Support for optional leading v and ignoring line endings

[andreassjoberg]

Hi @rwjblue!

I opened this PR in accordance with our discussions in the RFC.

[rwjblue]

Implementation seems generally correct, I think we are missing (from the RFC) a few things:

• implementation for error messages mentioned in RFC
• tests
  • package.json with volta + .node-version (uses package.json)
  • no volta in package.json + .node-version (uses .node-version)
  • no volta in package.json + no .node-version (uses global default)
  • no volta in package.json + .node-version with invalid semver (e.g. lts/*) (should error with nice message)
  • no volta in package.json + .node-version with extra unexpected content (should error with nice message)

[andreassjoberg]

@rwjblue I added some test cases and better error handling.

I did not differentiate between:

• no volta in package.json + .node-version with invalid semver (e.g. lts/*) (should error with nice message)
• no volta in package.json + .node-version with extra unexpected content (should error with nice message)

since that would involve parsing the contents of the file further, even after error.

Plus; how do we determine invalid semver compared to just a malformed string? Quite hard logic to specify I believe. 🙃

[rwjblue]

how do we determine invalid semver compared to just a malformed string? Quite hard logic to specify I believe

Ya good point, I totally agree with you. I think that there might be something useful RE: specifying a valid looking value that just isn't a valid Node version (e.g. .node-version with 99999.99999.9999 -- which doesn't exist).

Right now (with volta.node in package.json) we have the following error:

❯ node -v
Volta error: Could not download [email protected]
from https://nodejs.org/dist/v999.9999.999/node-v999.9999.999-darwin-arm64.tar.gz

Please verify your internet connection and ensure the correct version is specified.

I think that's probably still fine, but I think we need to update the output to indicate why we were looking for that version. In today's world it always comes from the same place (package.json), but we are introducing a new way to have specified the version so we should probably make sure the user knows where the version came from.

Probably best to update the RFC with some of this info too.

[andreassjoberg]

Thanks for the input @rwjblue.

This last bit about printing the source of the node version appears a bit more complex, and unfortunately, I don't have the time to implement this right now.

I still think the implementation is solid though.
My recommendation is that we merge this feature and create an issue for improving the error message you mention above.
Sounds good?

[rwjblue]

My recommendation is that we merge this feature and create an issue for improving the error message you mention above.

Sure, seems good to me.

[rwjblue]

One thing I'm not sure of, is if we should feature flag this (defaulting to on). I'm not really sure what our normal process there is. The actual implementation seems like something fairly easy to back out if we needed to (e.g. it isn't spanning a ton of different files and whatnot), so I'm not sure that we really need a feature flag?

Maybe @charlespierce or @chriskrycho have opinions?

[andreassjoberg]

@rwjblue - What's the status of this?
Is there anything I can do to progress this into a new release?