volta-cli / volta

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

`volta install corepack` doesn't provide corepack's `yarn` #987

Open gamoutatsumi opened 3 years ago

gamoutatsumi commented 3 years ago

I installed https://github.com/nodejs/corepack. Normally corepack provide yarn binary, but using volta, yarn is linked to volta-shim. Volta has not compatibility for corepack?

charlespierce commented 3 years ago

Hi @gamoutatsumi, unfortunately, I think it's true that Volta isn't compatible with globally installing corepack. Ultimately, both Volta and corepack are looking to solve the same problem: Making a package manager available to you without relying on a specific version being installed.

Volta's has special-case handling for the package managers, so when you run yarn it uses Volta's logic for detecting the package manager, as opposed to delegating to Corepack.

ljharb commented 3 years ago

Given that node is planning to integrate corepack by default, you may want to consider supporting it.

charlespierce commented 3 years ago

Yeah, I think we'll need to evaluate how to do a partial passthrough for when Node starts shipping Corepack. We need to do it in a way that won't break down on older Node versions that don't include Corepack. I don't know that we can reasonably support the 3rd-party package version without a significant refactor, however, and once it's included by default the value-add is quite a bit lower.

eugene1g commented 3 years ago

corepack is now bundled with Node as of 16.9.0 (https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V16.md#corepack) but not yet enabled by default.

Today we use Volta to control versions for both Node and Yarn, but we now plan to use Volta for Node (in dev / prod), and corepack for Yarn (in dev). Does Volta plan to support this dual proxying, or is that getting too complex?

cspotcode commented 3 years ago

I was able to make this work as follows:

npm install -g corepack

# Specifying an explicit install-directory makes corepack overwrite volta's yarn shims, which is what we want
corepack enable --install-directory ~/.volta/bin

# Now yarn should be controlled by corepack
yarn set version berry
charlespierce commented 3 years ago

Looking at how we handle the passthrough behavior, I think that we can make Volta interop with Corepack nicely with a minimal change. It should work as it currently does for Node without Corepack (showing a nice error about missing Yarn), while with Corepack it will delegate to that behavior if you run yarn.

sirreal commented 3 years ago

Corepack is bundled with recent node versions. The documentation suggests it's available as an executable just like node:

… run corepack enable … run corepack prepare

It's experimental at the moment, but given Corepack is expected to be core functionality, when installing a compatible version of node should volta shim corepack to the bundled version as well?

charlespierce commented 3 years ago

@sirreal Yeah, once we have support for calling the Corepack version of yarn (Started in #1024, got blocked on showing a proper error message so will need some design work), I think we'll also need to add a shim for corepack itself. That should be relatively straightforward, and use the same logic as the npx executable, showing a helpful error message if you're on an older version of Node that doesn't include corepack.

brandonchinn178 commented 2 years ago

IIUC, the only benefit of using corepack's yarn is because corepack looks at the packageManager key in package.json and automatically uses the right version of yarn (instead of checking in the yarn binary). In lieu of getting some sort of compatibility between volta and corepack, wouldn't it be sufficient to update volta to support looking at packageManager and using the same logic?

charlespierce commented 2 years ago

@brandonchinn178 Yeah, that definitely would be an intermediate approach to help improve compatibility. There's a bit of design that would need to happen there (we'd definitely love a small RFC!) around how we handle volta pin when the version is pulled from packageManager instead of volta.

Somewhat related to #282, which tracks the potential for pulling version information from different sources.

devinrhode2 commented 2 years ago

Deprecate volta inside package.json and use the new standard packageManger

devinrhode2 commented 2 years ago

If we can avoid adding "branded" metadata into package.json, more people can have more freedom wrt their choice of node manager

devinrhode2 commented 2 years ago

Some teams don't like adding metadata for individual developers unique tooling choices :)

charlespierce commented 2 years ago

Hi @devinrhode2, that's definitely an option we could investigate. The issue with that is it doesn't support choosing the Node version, which is necessary for Volta. We're certainly sympathetic to teams / projects that don't want to accommodate a myriad of ways to support the different developer tooling. We have another issue #282 focused specifically around supporting other mechanisms for defining the tool versions. The main blocker is there are a lot of edge cases and nuances that would need to be worked out before we can move forward.

0xdevalias commented 1 year ago

Was just wondering if any more progress had been made on potentially landing this PR?:


Deprecate volta inside package.json and use the new standard packageManger

This would be my suggestion as well, at least for the aspects that corepack and that packageManager field cover.

Probably done as an opt-in release of volta at first, that lets users elect to read from the packageManager versions. If that is opted into, then it should through at least a warning (maybe an error) if versions are set in the old volta keys, and should tell the user to set/update them in the packageManager keys.

Then after some time as deemed appropriate, a new 'breaking change' release of volta could be made that switches this to the default mode.

The issue with that is it doesn't support choosing the Node version, which is necessary for Volta.

I think one simple solution to this part is to not fully deprecate the volta key within package.json, but to only use it for node/other versions that packageManager doesn't currently support, and probably for the "opt in to using packageManager key for version config" option.

Another alternative would be to use the .node-version file format that many other tools have used for quite a while, and continue deprecating the volta key in package.json.

Or if you wanted to use a slightly less common, but more versatile standard name, asdf's .tool-versions would seem a good choice.

I also quite like the suggestion made in this other issue:

+1 for .voltarc / .nvmrc / .nodeversion / whatever works best for you and doesn't have to be committed to repo, and no need to search upper folders, it's just enough to test if it exists in the same folder as package.json

Originally posted by @iki in https://github.com/volta-cli/volta/issues/282#issuecomment-1079402776

And this:

IMHO I think it's become expected behaviour to have tool-specific configuration files in the root of a repository (monorepo or otherwise). These feel like fairly well-established patterns at this point (see: eslint, prettier, nvm, etc.)

The majority of these tools all follow similar conventions:

  • config is in json format
  • a configuration file named so that it's unmistakable what tool it's for
  • a package.json key as an alternative method of configuration
  • if a config is found at the cwd, it is used. If not it searches up the tree until it finds one, ending at the user's home directory

So, given this pervasive pattern already exists, I guess I expected that it would be in place here as well. If I were a newcomer, I would not expect for Volta to read from an .nvmrc file, as nvm is a separate tool entirely. If there were a more generalized convention (looking at you .node-version) that became more established, then I'd also be delighted to see support for that as well.

I feel that package.json-only-config doesn't work exceptionally well for a tool like this, as it must be committed to the repository and that can violate team standards. Without an alternate config method I'm forced to reach for something else 😕

Originally posted by @christiannaths in https://github.com/volta-cli/volta/issues/282#issuecomment-1262590044

0xdevalias commented 1 year ago

Another alternative would be to use the .node-version file format that many other tools have used for quite a while, and continue deprecating the volta key in package.json.

Or if you wanted to use a slightly less common, but more versatile standard name, asdf's .tool-versions would seem a good choice.

Related to the above, I just stumbled upon this repo that supports loading the version from a number of locations, in a specified hierachy. It might be overkill for the needs here, but I figured I would mention it in case it's useful:

dherman commented 1 year ago

@0xdevalias I would actually advocate your approach, I like it. Well-defined precedence rules are typical for tools like Volta (see for example rustup's docs). This creates a smooth way for us to evolve in the direction of using standard solutions when they're available, but still let Volta be out ahead when there is no established standard.

Edit: I mean some well-defined precedence rules, not necessarily exactly the set you quoted. I'm not sure we should be consulting the config from every tool in existence. I might stick to just defaulting to standards like corepack and falling back to Volta's specific configs.