berry icon indicating copy to clipboard operation
berry copied to clipboard

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

Open blimmer opened this issue 2 years ago • 9 comments
trafficstars

  • [x] I'd be willing to implement this feature (contributing guide)
  • [x] 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 = "[email protected]"' 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": "[email protected]"
}
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 [email protected], 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": "[email protected]"
}
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 (https://github.com/nodejs/corepack/issues/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 avatar Oct 31 '23 22:10 blimmer

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

RoXuS avatar Nov 01 '23 18:11 RoXuS

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 https://github.com/nodejs/docker-node/pull/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.

arcanis avatar Nov 01 '23 18:11 arcanis

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 https://github.com/nodejs/docker-node/pull/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 avatar Nov 01 '23 19:11 blimmer

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": "[email protected]". 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!

blimmer avatar Nov 24 '23 16:11 blimmer

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.

camertron avatar Feb 21 '24 23:02 camertron

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!

skagedal avatar Apr 27 '24 12:04 skagedal

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.

Hi @camertron Did you manage to find a way out of this? I'm currently experiencing this myself and it's been bugging me a lot.

Jolsty avatar Jul 03 '24 07:07 Jolsty

@Jolsty unfortunately it was long enough ago I don't remember. I think I ended up resetting everything by installing a different version of node.

camertron avatar Jul 09 '24 17:07 camertron