volta-cli / volta

Volta: JS Toolchains as Code. ⚡
https://volta.sh
Other
10.86k stars 227 forks source link

[Idea] Use external sources to resolve version for `volta pin node` #959

Open charlespierce opened 3 years ago

charlespierce commented 3 years ago

From a discussion on Discord, along with the multiple questions we've had around using engines instead of the custom volta key in package.json: One possible way to improve interoperability and convenience would be to change how the version is resolved when a user runs volta pin node.

Currently, it will find the latest LTS Node version and use that. However, we could update that logic to read the engines key and find the latest version that fits into that range (possibly the latest LTS that fits, to maintain the existing behavior). We could also look for .nvmrc or .node-version files and use those to resolve. We likely wouldn't be able to support everything that is supported by those files, but we could get close and show an informational message about why we couldn't read a version.

This would allow users to conveniently pull the value when engines (or other sources) were updated, while still maintaining Volta's model that requires a concrete version. It also would still be easy to be more explicit, using the volta pin node@<version> syntax, so there wouldn't be any loss of control over the installed version.

We'll need to define the precedence order for when there are multiple sources, as well as document well how it works. We'd also need to decide if this counts as a breaking change or not (I would lean towards it being an enhancement, bumping the minor version, not a breaking change, but I'm open to being convinced otherwise).

cprecioso commented 3 years ago

Hi, I'm interested in helping this feature move forward — I don't know any Rust though 😞. Is there a way I can help? Maybe research, specification, or a pseudo code implementation?

charlespierce commented 3 years ago

Hi @cprecioso, thanks for the offer! This would actually be a great issue to tackle without needing to know Rust. The current roadblock is that we need a rough idea of the proposed change in an RFC that covers the above topics (and any I missed, of which I'm sure there are many). So some research into the existing state of .nvmrc, .node-version, and engines along with a proposed specification for the new behavior of Volta would be exactly the thing to get this moving!

The RFC process happens in the rfcs repo here: https://github.com/volta-cli/rfcs and is described in the README for that repo. I will also note that it doesn't have to be a completely finished suggestion, rather the best way (in my experience) is for the RFC to give a starting point to discussions, then we can iterate from there based on feedback.

If that's something you're interested in doing, we'd be delighted to have the help! And if you have any other questions feel free to reach out here or on our Discord.

cprecioso commented 3 years ago

Understood! I'll find some time this weekend and do a first pass then 😄

darcyparker commented 3 years ago

I would like to see .node-version support too. For me the main reason for why is interchangeability of node version managers that contributors may be using.

A project could create both a volta property in package.json and .node-version file... but this isn't very DRY. People would need to remember to keep both up to date.

.nvmrc is deprecated... but its easy to support it for older versions of nvm by creating a symbolic link: ln -s .node-version .nvmrc

engines property of package.json is not as helpful as .node-version file and volta property because engines can specify a range of versions. Whereas .node-version file and volta property pin the version to a single/specific version.

The only thing missing from .node-version file that volta property has, is the ability to specify the npm version. I am not aware of a working standard for pinning the npm version.

charlespierce commented 3 years ago

@darcyparker I'm not up-to-date with the work on .node-version, is the intent that it only supports complete, specific versions? If so, adding support definitely seems like a value-add for all the reasons you mention. My only concern is if it supports partial versions (e.g. setting 14 to mean any Node on major versions 14), which are essentially equivalent to ranges, which then run into the technical problems discussed in this comment around why we chose not to use engines.

darcyparker commented 3 years ago

@charlespierce - Yes that's right. It is for one specific version. I have never seen anyone specify multiple versions with it. It would be great if you could support it. (I understand why engines is not supported and agree that supporting it is not feasible. nvm doesn't use it either.)

This is a popular way to create it: node -v > .node-version. Then, optionally, ln -s .node-version .nvmrc if you want to support the deprecated .nvmrc file. (The file contents are identical.)

This repo provides a kind of spec for it. I don't think there is a formal/official specification. But it feels correct to me based on my experience/understanding of the .node-version file. As well, it has good references to discussion about the format.

charlespierce commented 3 years ago

@darcyparker Oh that's very good to know, thanks! We've actually had a long-standing issue around using a separate file from package.json to specify the version. While .node-version is specific to only Node (vs the package managers that Volta also supports in package.json), it's definitely a big step in that direction. I created #983 to track that, it will take an RFC but given that it seems the format of .node-version aligns with what Volta needs, supporting it makes sense.

darcyparker commented 3 years ago

I may be wrong about .nvmrc being deprecated...

The spec for .nvmrc is very similar though: https://github.com/nvm-sh/nvm#nvmrc "The contents of a .nvmrc file must be the (as described by nvm --help) followed by a newline. No trailing spaces are allowed, and the trailing newline is required.". nvm list specifies versions the same as .node-version, but also supports lts etc... So by my interpretation, .nvmrc is a superset of .node-version

charlespierce commented 3 years ago

Yeah, I know that .nvmrc is a superset that supports ranges / imprecise versions, which was ultimately why we didn't go too deep into investigating that support (there was some discussion in #282).

cprecioso commented 3 years ago

Hi, just an update that I said I would try to take a look at it, but life is getting in the way, so if anyone is not taking a crack at this waiting for me (I doubt that, but just in case), please do get at it!

charlespierce commented 3 years ago

@cprecioso No worries! This actually may also dovetail with the investigation into #983, or at least they both may be solved by the same RFC.

jondcallahan commented 3 years ago

It would be helpful to even allow for a volta.json file at project root with the same contents as package.json. As a contributor to other repos I would like to keep this file uncommitted and not have to think about re-pinning the node version when I check out a branch that lacks the volta entry in package.json. This happens to me a lot when I need to pull down branches to run locally.

delucca commented 2 years ago

No updates on this? 😢

Crafoord commented 2 years ago

Yeah when working in a team where people might use different tools (volta/nvm/fnm etc) it's kind of a must to be able to use the .node-version file.

matthew-dean commented 1 year ago

Please support using .nvmrc and .node-version

camflan commented 1 year ago

I think adding support for .tool-versions would be amazing too

Ragnoroct commented 1 year ago

Here's a git workaround to temporarily ignore volta's changes to package.json git update-index --assume-unchanged package.json

daliusd commented 11 months ago

Here is something you can add to your config if you are using fish shell and are forced to use .nvmrc:

function __check_nvmrc --on-variable PWD --description 'check .nvmrc on pwd change and run volta install'
  status --is-command-substitution; and return

  set -l dir (pwd)

  while not test "$dir" = ''
    set nvmrc_file "$dir/.nvmrc"

    if test -e "$nvmrc_file"
      set nodeversion (cat $nvmrc_file)
      volta install node@$nodeversion
      break
    end

    set dir (string split -r -m1 / $dir)[1]
  end
end