Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Add a warning or error in Yarn Classic if package.json's packageManager key is set, but corepack is not enabled #5912

Open
2 tasks done
blimmer opened this issue Oct 31, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@blimmer
Copy link
Contributor

blimmer commented Oct 31, 2023

  • I'd be willing to implement this feature (contributing guide)
  • This feature is important to have in this repository; a contrib plugin wouldn't do

Describe the user story

As a consultant working with many different companies and projects using yarn, it has been difficult to migrate from the "checked in Yarn version" (.yarnrc.yml's yarnPath setting) to corepack. When migrating to corepack, users who have not enabled corepack can accidentally use global Yarn Classic and they receive no warning about it.

Checked In Yarn Version referenced by yarnPath

I'm using yarn set version 3.6.4 to show how this worked in Yarn Berry v3 when checking in the release to the repo.

Source:

> docker run -it --rm --entrypoint bash node:20.9.0
root@2d0e84fe9c26:/# yarn --version
1.22.19
root@2d0e84fe9c26:/# mkdir test-project
root@2d0e84fe9c26:/# cd test-project/
root@2d0e84fe9c26:/test-project# npm init -y
root@2d0e84fe9c26:/test-project# yarn set version 3.6.4
➤ YN0000: Retrieving https://repo.yarnpkg.com/3.6.4/packages/yarnpkg-cli/bin/yarn.js
➤ YN0000: Saving the new release in .yarn/releases/yarn-3.6.4.cjs
➤ YN0000: Done in 1s 7ms
root@2d0e84fe9c26:/test-project# cat .yarnrc.yml
yarnPath: .yarn/releases/yarn-3.6.4.cjs

✅ Global Yarn Classic

root@2d0e84fe9c26:/# yarn --version
1.22.19
root@2d0e84fe9c26:/# cd test-project/
root@2d0e84fe9c26:/test-project# yarn --version
3.6.4

✅ Corepack

root@2d0e84fe9c26:/test-project# corepack enable
root@2d0e84fe9c26:/test-project# yarn --version
3.6.4

Corepack (v4 recommendation)

I'm only writing the packageManager key in package.json. Currently, yarn set version <VERSION> will only write the packageManager key if the local machine has corepack already enabled.

Source:

> docker run -it --rm --entrypoint bash node:20.9.0
root@870c6624fa62:/# yarn --version
1.22.19
root@870c6624fa62:/# mkdir test-project
root@870c6624fa62:/# cd test-project/
root@870c6624fa62:/test-project# npm init -y
# write `packageManager` in `package.json`
root@870c6624fa62:/test-project# jq '.packageManager = "yarn@4.0.1"' package.json > tmp.$$.json && mv tmp.$$.json package.json
root@870c6624fa62:/test-project# cat package.json
{
  "name": "test-project",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "packageManager": "yarn@4.0.1"
}
root@870c6624fa62:/test-project# cat .yarnrc.yml
cat: .yarnrc.yml: No such file or directory

❌ Global Yarn Classic

Even in the test-project, where the packageManager key is set to yarn@4.0.1, global yarn still wins. This is different than the behavior when the release is checked in.

root@870c6624fa62:/# yarn --version
1.22.19
root@870c6624fa62:/test-project# cat package.json
{
  "name": "test-project",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "packageManager": "yarn@4.0.1"
}
root@870c6624fa62:/test-project# yarn --version
1.22.19

✅ Corepack

However, if I enable corepack, the proper version is used.

root@870c6624fa62:/test-project# corepack enable
root@870c6624fa62:/test-project# yarn --version
4.0.1

The Issue

The main issues I see are:

  1. Official node images have global yarn classic and corepack disabled by default. It is challenging to assume that the packageManager key will do the right thing. You have to extend the official image to remove global yarn and/or enable corepack.
  2. yarn set version <VERSION> makes global assumptions based on one developer's machine. If I have corepack enabled and run yarn set version berry, yarn does not write the release to the repo. In this case, yarn is making a global assumption that my CI environment, runtime container, other developer's machines, etc. will also have corepack enabled.
  3. Yarn Classic does not warn you that you're intended to be using Yarn Berry. If I'm in a scenario where packageManager is set to a Yarn Berry version, Yarn Classic will run without warning or erroring. Most users will be unaware that there could be incompatibilities.
  4. Node version managers (asdf, nvm) require corepack to be enabled for each Node version. Since corepack enable overwrites the yarn binary, it must be run for each version of Node you have installed via a Node Version Manager. Therefore, when switching around between projects using different versions of Node, it's easy to forget that you need to run corepack enable. Since there's no warning, you can transparently/accidentally use global yarn instead of berry.

I want to be able to guarantee that all consumers running yarn commands are using the proper version. It appears that Yarn Classic can read .yarnrc.yml's yarnPath config and always run the the correct version. However it does not pay attention to the packageManager key in package.json.

Describe the solution you'd like

A new release of Yarn Classic that throws an error if package.json's packageManager key is set, but corepack is not enabled. If the release is checked in and referenced by yarnPath, the packageManager key is ignored.

Describe the drawbacks of your solution

  • I believe Yarn Classic has had its final release and is not intended to be released again.
  • It is hard to tell if corepack is enabled. There's no corepack status that easily verifies that corepack is enabled (How can I detect if corepack is already enabled? nodejs/corepack#113).
  • This could be considered a major release of Classic since it could break people's installs that rely on classic. We could make it a very visible warning instead.

Describe alternatives you've considered

  • Have new versions of Yarn Berry warn if corepack is not enabled, no matter the settings currently used. This doesn't help Classic users, though.
  • We could create a new feature in Berry that reads package.json's packageManager version and compares it against its own version. If there's a mismatch it refuses to run. Again, doesn't help Classic users.
  • We could introduce a new key in .yarnrc.yml that requires corepack to be enabled in v4 and newer. If .yarnrc.yml's requireCorepack is set to true, we somehow verify for the user (see issue above RE: no corepack status command). Again, doesn't help Classic users.
  • We could try to push for the global yarn Classic binaries to be removed from the official Node image, and corepack to be enabled by default. Since corepack is experimental, it feels like this will be a hard sell.
  • Why not make it a plugin? Because it feels like a core feature of Yarn. If we're going to require people to use an experimental/disabled-by-default feature of node (corepack), we should do our best to help people configure their environment.
@blimmer blimmer added the enhancement New feature or request label Oct 31, 2023
@RoXuS
Copy link

RoXuS commented Nov 1, 2023

+1 I searched for about 1 hour why docker not use yarn 4.x.x...

@arcanis
Copy link
Member

arcanis commented Nov 1, 2023

Yarn Classic does not warn you that you're intended to be using Yarn Berry. If I'm in a scenario where packageManager is set to a Yarn Berry version, Yarn Classic will run without warning or erroring. Most users will be unaware that there could be incompatibilities.

I agree this is a problem, we should release a patch in 1.22 that fixes that. I even had the idea of integrating the Corepack engine within the 1.22 release, so that even if Corepack isn't explicitly installed the yarn npm package would still be able to interpret the packageManager field.

[...] and corepack to be enabled by default. Since corepack is experimental, it feels like this will be a hard sell.

Perhaps not necessarily; I revived the thread on nodejs/docker-node#1768, we'll see if something comes out of that. In any case, I had little time to spend on Corepack lately as I was working on the 4.0 release, but now I think I'll focus a little more on it to bring it out of experimental.

@blimmer
Copy link
Contributor Author

blimmer commented Nov 1, 2023

I agree this is a problem, we should release a patch in 1.22 that fixes that.

Awesome!

I even had the idea of integrating the Corepack engine within the 1.22 release, so that even if Corepack isn't explicitly installed the yarn npm package would still be able to interpret the packageManager field.

This would be a big help to people using older versions of Node. Less of a problem now that new LTS'es have been minted, but still helpful.

revived the thread on nodejs/docker-node#1768

Nice - that would be great!

Thanks for being so open to hearing me out on this problem. I think you'll have a lot of happy yarn users by making this work more predictably 😄

@blimmer
Copy link
Contributor Author

blimmer commented Nov 24, 2023

For those following this thread, it looks like Yarn Classic v1.22.20 and beyond now warn if the packageManager key is set, but Corepack is not enabled:

error This project's package.json defines "packageManager": "yarn@3.6.4". However the current global version of Yarn is 1.22.21.

Presence of the "packageManager" field indicates that the project is meant to be used with Corepack, a tool included by default with all official Node.js distributions starting from 16.9 and 14.19.
Corepack must currently be enabled by running corepack enable in your terminal. For more information, check out https://yarnpkg.com/corepack.

Thanks, @arcanis, for this improvement!

@camertron
Copy link

camertron commented Feb 21, 2024

Weirdly I get this error message even if there is no "packageManager" field set in my package.json. It's quite frustrating, and I haven't been able to find a workaround.

mnvr added a commit to ente-io/ente that referenced this issue Mar 8, 2024
People often run into issues with `yarn install` because they're using a
newer yarn. The situation is generally bad - we don't want to update to
Yarn v4 yet because it is marked experimental and is not the default
yarn that gets installed by node currently. We could add a
`packageManager` field to our package.json, but this will only fail the
build with a better (hopefully) error message, and will necessitate the
user to `corepack_enable`.

I'm not sure what's the best approach right now to make the initial
setup be seamless (I think we're using the approach that works for the
maximum of all the alternatives, but I'm not sure). At least, let me add
a note about it.

Ref:
* yarnpkg/berry#5912
@skagedal
Copy link

Honestly I think it would be great if the latest version of Yarn Classic – whatever gets installed when you install Yarn with Homebrew or NPM or something like that – would always present a big "deprecated" warning whenever run, whether or not a packageManager field is present!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants