corepack icon indicating copy to clipboard operation
corepack copied to clipboard

Allow packageManager to be specified by name alone

Open rotu opened this issue 2 years ago • 15 comments

  • Treat name as name@* when specifying package manager for corepack use et al. Fixes #298.
  • Resolve an inexact package specifier from package.json#packageManager. corepack use makes it easy to specify an exact version, but disallowing version specifiers is more confusing than helpful. Fixes #95.

rotu avatar Sep 04 '23 05:09 rotu

And for corepack install -g (and this command only), yarn should alias to yarn@^1, not yarn@*.

arcanis avatar Sep 04 '23 07:09 arcanis

This allows omitting the version from the package.json packageManager field as well which it shouldn't do.

Hmm... I thought that was enforced in resolveDescriptor, which has the allowTags option. Under what circumstances (if any) would we allow a version range but not a tag?

rotu avatar Sep 04 '23 08:09 rotu

And for corepack install -g (and this command only), yarn should alias to yarn@^1, not yarn@*.

@arcanis I left that out of scope on purpose. I can't figure out the intended behavior so I punted.

At the very least it seems like corepack use yarn should be synonymous with corepack use yarn@* and either yarn@stable or yarn@latest.

But here's what it is right now:

  • corepack install -g yarn@stable -> yarn 3.6.3

  • corepack install -g 'yarn@*' -> yarn 4.0.0-rc.50

  • corepack install -g 'yarn@latest' -> Usage Error: Tag not found (latest)

  • npm exec --package yarn -c 'yarn --version' -> 1.22.19

  • npm exec --package 'yarn@*' -c 'yarn --version' -> 1.22.19

  • yarn dlx --package 'yarn' yarn --version -> 1.22.19

  • yarn dlx --package 'yarn@*' yarn --version -> 2.4.3


Heck, even https://www.npmjs.com/package/yarn and https://yarnpkg.com/package?q=yarn&name=yarn don't acknowledge any version of yarn exists past 2.4.3, so how should a user know what a valid version specifier is?

Yarn is a wonderful project but the critical user story of "how should I install it, and what version should I use?" is a dumpster fire. I am of the opinion that the yarn cli needs to provide its own backwards compatibility affordances, not require other tools to implement special rules.

rotu avatar Sep 04 '23 08:09 rotu

Yeah, I think it's fine to keep it as a follow-up. I have another idea to make this process a little less magic, I'll handle that.

Heck, even https://www.npmjs.com/package/yarn and https://yarnpkg.com/package?q=yarn&name=yarn don't acknowledge any version of yarn exists past 2.4.3, so how should a user know what a valid version specifier is?

Because you're looking at the npm package, and Yarn hasn't been distributed on npm for the past couple of years. So yeah, the version you're seeing there is outdated. The proper list of versions is here: https://repo.yarnpkg.com/tags

arcanis avatar Sep 04 '23 09:09 arcanis

Because you're looking at the npm package, and Yarn hasn't been distributed on npm for the past couple of years. So yeah, the version you're seeing there is outdated. The proper list of versions is here: https://repo.yarnpkg.com/tags

That's awesome documentation and makes a lot of sense!

The npm page and yarn page are the de facto storefront for a javascript package, and even if they aren't the place releases happen, they need guidance in the readmes (even if those instructions start with "Don't install this version and don't install with npm" and a link to the recommendations for how to get started with yarn).

The npm page has also been confusing because the "berry" dist-tag implies that "berry" is the name for major version 2, but the active development repo is also called "berry" and has releases for major versions 2, 3, and 4.

I have definitely been turned off (perhaps unfairly) from trying yarn in the past by these sorts of issues!

rotu avatar Sep 04 '23 09:09 rotu

This allows omitting the version from the package.json packageManager field as well which it shouldn't do.

@merceyz Why not?

Please see my latest commit, which is how I feel it should work, somewhat like npm install --save-exact.

rotu avatar Sep 04 '23 19:09 rotu

@merceyz @arcanis Can we move forward with this?

aduh95 avatar Oct 08 '23 07:10 aduh95

@merceyz Why not?

Reproducibility, if a project is installable and works today it should work in a year, if a range is used that might not be the case anymore.

@merceyz @arcanis Can we move forward with this?

@aduh95 I don't think this should land in its current state and it needs some tests to ensure it works as expected.

merceyz avatar Oct 08 '23 10:10 merceyz

@merceyz Why not?

Reproducibility, if a project is installable and works today it should work in a year, if a range is used that might not be the case anymore.

Note that corepack use npm@stable is accepted and has the same issue. This locks down the version in the same way.

rotu avatar Oct 08 '23 15:10 rotu

That command would set the package.json#packageManager field to a specific version of npm, not a range.

merceyz avatar Oct 08 '23 15:10 merceyz

That command would set the package.json#packageManager field to a specific version of npm, not a range.

That's my understanding too - I feel like I don't understand your point. I'm picturing the user specifying a value loosely with corepack use. Then corepack install will set up the project locked to that particular package manager.

if a range is used that might not be the case anymore.

What's the use case you're envisioning here which, after this PR, would create a non-reproducible build?

rotu avatar Oct 08 '23 22:10 rotu

@merceyz updated the tests. I think this clarifies how I expect this to work, basically like npm install --save-exact, where the user can be fuzzy but the result will be specific.

EDIT: I'm having second thoughts about this. Maybe the right thing to do is to allow a specifier. corepack use will automatically reify the version, but if the user wants to put in a package specifier (a la #95), why shouldn't we trust them? It might be more important to the user to have bug fixes than to have reproducibility, especially for nascent package managers like bun.

A reproducible build will always rely on developer choices. Like committing the lockfile or pnp files, using a particular version of the node runtime, using deterministic build steps, and setting package manager options like install-strategy. To that end, I think it makes sense to respect the user's choice of how they want to express their packageManager specifier, rather than forcing a version (which may or may not result in a reproducible build anyway; e.g. #308).

rotu avatar Oct 09 '23 02:10 rotu

Why shouldn't we trust them? It might be more important to the user to have bug fixes than to have reproducibility, especially for nascent package managers like bun.

The current model is to favour good practices, in particular reproducibility. It's a major point of Corepack, otherwise you can just npm install whatever package manager you want and that's it, no need for Corepack.

Good practice is, when reproducibility is key, to track the full version with hash, which is exactly what corepack use does. If the package manager actually follows the semantic versioning spec, then good practice is to stick with compatible releases, including tracking prerelease lines as they mature.

Even without this restriction, corepack is necessary because:

  1. Typically node_modules/.bin is not in $PATH so even if you install your preferred package manager for a project, it's unwieldy to call.
  2. The yarn package manager does not make itself available in a package repository. That is, npm install yarn@latest and npm install yarn@berry install versions which are very out of date.

I agree with @merceyz on this - adding support for loose specifiers is fine on the CLI commands (because they only matter at the time the commands are run), but allowing them in the packageManager field is a different story that should be evaluated in its own PR (and I'll push back on it for the reasons described above).

That's going to wind up with those CLI commands in build scripts and commit hooks. The whole idea behind a human-editable package.json is to track the developer's intent, not enforce reproducibility (that's what the lockfile is for). #95 has 28 👍 versus 2 👎, which seems to clearly demonstrate that we should accept loose versions here.

@arcanis and @merceyz, would you be happy with instead reifying the version on corepack install as I previously had?

rotu avatar Oct 09 '23 08:10 rotu

@arcanis and @merceyz, would you be happy with instead reifying the version on corepack install as I previously had?

Not sure what you mean by that - as far as I can tell from the tests you link, this still lets you write a range in the packageManager field, which is what I don't think we should merge as part of this PR.

Essentially, I believe this PR is conflating two different things (ranges in the CLI, and ranges in the project definition), and I'm against using the merits of the first request (which has consensus) to land the second (which hasn't).

arcanis avatar Oct 09 '23 12:10 arcanis

@arcanis and @merceyz, would you be happy with instead reifying the version on corepack install as I previously had?

Not sure what you mean by that - as far as I can tell from the tests you link, this still lets you write a range in the packageManager field, which is what I don't think we should merge as part of this PR.

Yes. In that revision of the PR, this lets you write a range in the packageManager field and then, the first time you corepack install, it rewrites the packageManager field to the specific version. This is a much more comfortable dev experience than crashing out with a "Usage Error" (even on corepack use and corepack up!).

Essentially, I believe this PR is conflating two different things (ranges in the CLI, and ranges in the project definition), and I'm against using the merits of the first request (which has consensus) to land the second (which hasn't).

Yes, in fixing the first, I realized how arbitrary and unwieldy the second one is and what it would take to fix. The feedback on #18 and #95 seems clear consensus that having something package.json which looks like a package specifier but doesn't work like one is bad DX, and that users should be able to edit package.json by hand.

So @arcanis and @merceyz, what do you think the developer's EXPECTATION is if they have written an imprecise version (or none at all) into package.json?

rotu avatar Oct 09 '23 15:10 rotu