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

package-manager-strict enforcement in pnpm 9 is a massive headache #8087

Open
benmccann opened this issue May 15, 2024 · 38 comments
Open

package-manager-strict enforcement in pnpm 9 is a massive headache #8087

benmccann opened this issue May 15, 2024 · 38 comments

Comments

@benmccann
Copy link

Last pnpm version that worked

8

pnpm version

9

Code to reproduce the issue

pnpm install with packageManager set to a different minor or patch version than you have installed

Expected behavior

I would expect to use the engines field to manage restrictions on what versions of package managers.

The packageManager field is used by platforms like Vercel and Netlify to choose what exact version of pnpm to use to run your project, which has been helpful for ensuring they're using a compatible major version.

However, I do not want to have to sync all of my various projects and locally installed version to always be using the same exact version of pnpm - especially with how frequently pnpm updates are released. Now I will have to constantly be updating the packageManager field in all my projects to match whatever the latest pnpm version is causing needless churn and a ton of annoying work that provides close to zero value. If I wanted to do that I already had the ability via engines.

Other users have been frustrated by this new enforcement as well. E.g. see #7956 (comment)

Actual behavior

 ERR_PNPM_BAD_PM_VERSION  This project is configured to use v9.1.1 of pnpm. Your current pnpm is v9.1.0

Additional information

Support for semver ranges is the number two issue in corepack nodejs/corepack#95. Setting package-manager-strict could make more sense after that is available. Though it would still seem to unnecessarily duplicate the engines field, so even then it wouldn't be my preference

Node.js version

20.10.0

Operating System

Linux

@zkochan
Copy link
Member

zkochan commented May 16, 2024

The corepack repo is the right place to discuss this. We did not come up with this field. We just try to adhere it. In the meantime, you can disable this strictness by the setting.

@benmccann
Copy link
Author

benmccann commented May 16, 2024

But pnpm 8 didn't adhere to it. Why start enforcing it now without regard for user impact?

@zkochan
Copy link
Member

zkochan commented May 16, 2024

It is a breaking change, so it couldn't be done in v8.

The users have a choice to not use the field. The users also have a choice to make it less strict with the setting. So, I don't see the issue.

@benmccann
Copy link
Author

benmccann commented May 16, 2024

We really don't have a choice not to use the field. Many hosting providers like Netlifly and Vercel require it to specify what version of pnpm is used. But I really just want to specify what version is used on the hosting provider and not locally. I can do this by updating all of my projects to package-manager-strict=false to get the desired behavior, but it'd be nice if that were the default.

We did not come up with this field. We just try to adhere it.

The point of the field as far as I understand is so that corepack can choose which version of a package manager to use. If a tool wants to adhere to the field by copying corepack's behavior it should automatically download and execute that version instead of bombing out. But why should it affect non-corepack users?

@trevorpfiz
Copy link

I just ran into this issue on Vercel after I had been deploying for days without issue? postinstall:  ERR_PNPM_BAD_PM_VERSION  This project is configured to use v9.1.0 of pnpm. Your current pnpm is v9.0.4. Did Vercel all of a sudden stop respecting "packageManager": "pnpm@9.1.0"? Curious if anyone else started running into this.

@dominikg
Copy link

If a tool wants to adhere to the field by copying corepack's behavior it should automatically download and execute that version instead of bombing out. But why should it affect non-corepack users?

i second this. There are many environments where corepack is not enabled by default and other tools might not use it. As long as pnpm itself is not able to run with the exact specified version on demand, this setting should not be the default.

another example: dependabot/dependabot-core#9522 (looks like dependabot uses pnpm 9.0.6 at the time of writing)

And i might be wrong but didn't corepack not download the exact patch version all the time but looked for a compatible local one first? So even corepack users might be bitten by this

@exiguus
Copy link

exiguus commented May 17, 2024

I just ran into this issue on Vercel after I had been deploying for days without issue? postinstall:  ERR_PNPM_BAD_PM_VERSION  This project is configured to use v9.1.0 of pnpm. Your current pnpm is v9.0.4. Did Vercel all of a sudden stop respecting "packageManager": "pnpm@9.1.0"? Curious if anyone else started running into this.

I have the same issue.
Currently we use in CI

corepack use pnpm@`pnpm -v`

as a work around.

image

Edit: This will set whatever pnpm version is installed on the system as "packageManager" in the package.json file and prevent the ERR_PNPM_BAD_PM_VERSION error.

@wadehammes
Copy link

wadehammes commented May 17, 2024

corepack use pnpm@`pnpm -v`

This worked for me

corepack use pnpm@`pnpm -v` && pnpm i

Thanks @exiguus

@benmccann
Copy link
Author

Most deployments using pnpm 9 on Vercel are failing as a result of this issue because they use pnpm 9.0.4 on their platform, so you need to set exactly "packageManager": "pnpm@9.0.4" or the deployment will fail. Thanks to @EndangeredMassa for providing these details. See vercel/vercel#11607 for more info

I think there are a few potential issues here:

  • Vercel isn't following the packageManager field by default. I think it's not completely unreasonable for them to say they're going to use a specific version of pnpm, but I'm not sure they should reuse corepack's field for this if they're not also using corepack.
  • By default, pnpm is enforcing that projects using the packageManager field can only be run using that specific version and basically disallowing you from running the project without corepack. I don't see why pnpm should have to enforce that you use corepack. I think it'd be nicer if pnpm knew nothing about corepack and corepack were a tool that sat on top of the various package managers
  • Corepack doesn't provide a way to say you want pnpm 9 without caring about the exact version. Though I think that specifying an exact version is good practice, so I wouldn't necessarily suggest they change that. That's why we use lockfiles after all. But it does add an extra layer of annoyance to the two issues above

@EndangeredMassa
Copy link

Just dropping in to add some clarity/context to Vercel-specific support.

Many hosting providers like ... Vercel require it to specify what version of pnpm is used.

Vercel does not require this value. If you don't provide it, we'll detect the appropriate version of package manger to use based on the lockfile version. We have the most detailed mapping for pnpm, specifically.

There were some edge cases with this recently, but if you try again without the packageManager value, it should pick up the right major version of pnpm to use.

I just ran into this issue on Vercel

We did notice a lot of users started seeing this after we rolled out more complete pnpm@9 support. There are workarounds on the user end, which I described here: vercel/vercel#11607 (comment)

Vercel isn't following the packageManager field by default.

This is correct. Users can opt in to corepack support if they want, but by default, we do not use the package.json#packageManager value.

@benmccann
Copy link
Author

Vercel does not require this value. If you don't provide it, we'll detect the appropriate version of package manger to use based on the lockfile version. We have the most detailed mapping for pnpm, specifically.

Ah, thanks! That's helpful to know. I tested it out and probably won't go that route because pnpm/action-setup also uses that field, so if I were to remove it then I'd have to duplicate the version in half a dozen different places in our GitHub workflows. Plus, with Netlify and other providers using that field still it's simpler just to have a standard solution of disabling package-manager-strict. I'll probably end up adding an .npmrc to the SvelteKit templates with package-manager-strict=false if the default isn't changed in pnpm

@kirso
Copy link

kirso commented May 19, 2024

Just dropping in to add some clarity/context to Vercel-specific support.

Many hosting providers like ... Vercel require it to specify what version of pnpm is used.

Vercel does not require this value. If you don't provide it, we'll detect the appropriate version of package manger to use based on the lockfile version. We have the most detailed mapping for pnpm, specifically.

There were some edge cases with this recently, but if you try again without the packageManager value, it should pick up the right major version of pnpm to use.

I just ran into this issue on Vercel

We did notice a lot of users started seeing this after we rolled out more complete pnpm@9 support. There are workarounds on the user end, which I described here: vercel/vercel#11607 (comment)

Vercel isn't following the packageManager field by default.

This is correct. Users can opt in to corepack support if they want, but by default, we do not use the package.json#packageManager value.

Just wanted to com here and say that after a long time setting the corepack ENV variable worked for me and build finally did not fail. Thank you.

@zkochan
Copy link
Member

zkochan commented May 24, 2024

I have made a poll: https://x.com/pnpmjs/status/1793795772921573544

@GeoffreyBooth
Copy link

The packageManager field is still experimental and subject to breaking changes. It’s also intended for Corepack’s internal use, not as a field for other tools.

Perhaps pnpm could instead implement the devEngines field? It was designed to be used by all package managers, and it supports version ranges.

@Dhruv97Sharma
Copy link

corepack use pnpm@`pnpm -v`

This worked for me

corepack use pnpm@`pnpm -v` && pnpm i

Thanks @exiguus

Just Overriding this with the above-given command in the install command step on Vercel deployment settings for my application really helped me. Thanks.

@kms0219kms
Copy link

I solved this problem by doing this:
https://github.com/orgs/vercel/discussions/6865#discussioncomment-9468470

@benmccann
Copy link
Author

I filed pnpm/action-setup#124 and netlify/build#5663, which would reduce the need for changing this

@benmccann
Copy link
Author

I sent pnpm/action-setup#126 to help with avoiding using the packageManager field. While we can avoid it already with the version field, you need to duplicate that field in a dozen places for any moderately complex workflow

@benmccann
Copy link
Author

benmccann commented May 28, 2024

Sigh. I've just been told by a corepack user that I can't simply omit the packageManager field because corepack will automatically add it unless you've set COREPACK_ENABLE_AUTO_PIN=0, so I don't know how much it's going to help to try to avoid using the packageManager field. We can leave it out of our project, but that's going to be a pain for external contributors then unless they've all set that variable. I anticipate receiving a lot of PRs that accidentally add that field. I've filed nodejs/corepack#485 about this

I've set package-manager-strict=false in another project of mine, but that's causing annoying warnings (#8145), so I can't even ignore this setting:

 WARN  This project is configured to use v9.0.4 of pnpm. Your current pnpm is v9.0.6

@dominikg
Copy link

dominikg commented May 28, 2024

i'm not on twitter so i'd like to vote here for "don't crash".

corepacks behavior does not work for me so i don't have it enabled. The new default in pnpm9 would cause a massive headache for me when switching between multiple open source projects i work with.

my workaround for now is to put

package-manager-strict=false

in my ~/.npmrc and live with the warnings.

What's the reasoning behind this default? different patch releases of pnpm@9 should still resolve to the same outcome given a lockfile. Breaking changes in resolving would require a major release, so why is pinning to the patch default? And why do it in a way that requires you to use corepack?

@dominikg
Copy link

Also what would happen if you release an important bugfix in pnpm@9.1.5 and i installed that locally, but a project i want to send a fix to still has "packageManager":"pnpm@9.1.3" in their package.json.

Am i supposed to send a PR to update that first ?

@benmccann
Copy link
Author

The Twitter poll seems to be broken. It appears to be permanently stuck at 586 votes in recent days despite sooo many people having told me they've voted "don't crash" since then.

I will note that while "don't crash" is currently behind by a mere 6 votes or so, nearly all the tweets in reply to the poll are write-ins for "don't crash", which means a sizable portion of the "other (comment)" vote should be registered towards "don't crash".

@zkochan
Copy link
Member

zkochan commented May 29, 2024

Look, I don't like it either, that is why I don't use the "packageManager" field in my repos. According to the results of the poll most people are fine with how it works now. The comments are mostly about making it accept ranges, which is up to corepack to decide.

I was fine with any solution that is why I created the poll.

@antfu
Copy link

antfu commented May 30, 2024

To me, the strictness makes total sense. It's like you lock every package to its exact patch version in the lockfile, so you should lock the package manager version to make the state of the commit ~100% reproducible. It's weird to say I specified the exact version of packageManager, but end up using a different version to run. It's like you have the option to not use lock file, so you have the option to not use packageManager - but when they are presented, they should be respected. While I totally agree it could be more loosely to have range support, it's more like a Corepack's problem, not pnpm's.

I guess maybe in the error message, pnpm could say "Consider using corepack(link) to switching to the correct version automatically"? I see probably the majority of the user confusing about this is because they don't aware corepack exists, but happend to come by to the project that enforce packageManager?

@zkochan
Copy link
Member

zkochan commented May 30, 2024

The poll might be broken indeed. I have retweeted the poll and posted to more channels but the number of votes is still 586

@zkochan
Copy link
Member

zkochan commented May 30, 2024

8 hours left. Now the results are different:

image

@benmccann
Copy link
Author

lol. wtf. even if all 25 new votes were for "don't crash" it wouldn't result in an extra 11-12% for that option. i'm not complaining as that's my preference, but I have no idea how twitter does its math

@FleetAdmiralJakob
Copy link

lol. wtf. even if all 25 new votes were for "don't crash" it wouldn't result in an extra 11-12% for that option. i'm not complaining as that's my preference, but I have no idea how twitter does its math

Yeah, I think now the poll is not correct the other way around

@GeoffreyBooth
Copy link

pnpm is welcome to implement the devEngines field. It includes two things that I think would help in this situation:

  • Support for semver ranges
  • The ability for the user to define what should be done if validation fails

So in package.json, for example:

"devEngines": {
  "packageManager": {
    "name": "pnpm",
    "version": ">= 9.0.0",
    "onFail": "warn"
  }
}

So in other words, pnpm wouldn’t need to decide between warning or exiting; the user defining the version in package.json could tell pnpm what to do via the onFail sub-field.

If you read down that thread, the npm team is interested in supporting this field, so pnpm doing so as well would improve interoperability between projects.

@zkochan
Copy link
Member

zkochan commented May 30, 2024

@GeoffreyBooth the new field that you suggest is not related to this. Regardless of your new field, the packageManager field already exists and doesn't go anywhere. If you want to discuss the new field, do it in a separate issue.

@zkochan
Copy link
Member

zkochan commented May 31, 2024

According to the final results, 58.8% of pnpm users would prefer pnpm to not fail in this case. Hence, I agree to change the behaviour.

What if pnpm will only fail if the version is a different major version than the one specified in packageManager? In case if it is the same major version but an older version than the one in packageManager, we could either fail it or print a warning.

To solve the issue on Vercel we should allow older versions from the same major?

@zkochan zkochan pinned this issue May 31, 2024
@benmccann
Copy link
Author

What if pnpm will only fail if the version is a different major version than the one specified in packageManager?

I would prefer to use engines for this as it's much more flexible and I can specify the exact ranges to allow

@dominikg
Copy link

Thank you ♥️

Changing the default of package-manager-strict to false ( projects can still opt in to strict behavior by adding an .npmrc to their repo with package-manager-strict=true ) is going to make it much easier to use pnpm in environments without corepack.

Only failing on a different (older?) major in packageManager is an interesting compromise. In case of older majors the preferred lockfile format could be different and breaking changes can lead to different node_modules output. (eg resolution_mode). If not failing, at least a warning like

Your version of pnpm (pnpm@x.y.z) is a different major than specified in the 'packageManager' field of this project (pnpm@a.b.c) - to avoid compatibility issues, please switch to that version.

would be nice.

In monorepos this can be achieved today with the engines.pnpm field, but single package repos would have to be careful not to publish it otherwise it could break when the package is used in a project that has a different major of pnpm as requirement. In that scenario - and for more fine grained control for projects - the devEngines field mentioned here makes sense to support long term. But that is also experimental still and not widely used. Maybe something to consider for pnpm@10?

@FleetAdmiralJakob
Copy link

FleetAdmiralJakob commented May 31, 2024

Hi, if this is now done in the .npmrc files how does this work? Is there documentation about this? Does it work on Windows too?

@dominikg
Copy link

https://pnpm.io/npmrc and yes it works on windows too.

example from this repo https://github.com/pnpm/pnpm/blob/main/.npmrc

@antfu
Copy link

antfu commented May 31, 2024

Default not to fail (or only check major) + warning on version misalignment sounds reasonable to me giving the current situation that packageManager is still experimental.

However, I want to mention that in the case of Vercel et al, they should either ignore packageManager at all or strictly follow the exact version specified in it. Especially for being a CI environment where the goal is to be reproducible (similar to that you'd freeze the lockfile in CI). Partially implementing that support sounds a bit harmful to me.

Really wish "packageManager": "pnpm@^8" could be supported and then we don't have this issue at all.

@benmccann
Copy link
Author

Thank you ♥️

+1 thank you for all your hard work on pnpm, Zoltan!

In monorepos this can be achieved today with the engines.pnpm field, but single package repos would have to be careful not to publish it otherwise it could break when the package is used in a project that has a different major of pnpm as requirement. In that scenario - and for more fine grained control for projects - the devEngines field mentioned here makes sense to support long term

Great point, Dominik. engines works for most of my projects, which are monorepos, but devEngines would be a great help here as well and probably the better solution

Default not to fail (or only check major) + warning on version misalignment sounds reasonable to me giving the current situation that packageManager is still experimental.

+1

However, I want to mention that in the case of Vercel et al, they should either ignore packageManager at all or strictly follow the exact version specified in it.

Agree that they should strictly follow the exact version. Ignoring it would cause lots of existing projects to break though

Really wish "packageManager": "pnpm@^8" could be supported and then we don't have this issue at all.

This is the one part I'm not sure I agree with. I think specifying an exact version to deploy to production with makes sense. Just like lockfiles specify exact versions I want to specify the exact version of the package manager to use in production. I just don't want usage of this field to stop me from running the project locally when I'm not using corepack just like pnpm won't stop me from running the project locally with pnpm if there's a yarn.lock or package-lock.json present. It's a field for a different tool and I don't think pnpm needs to be aware of it.

@zkochan
Copy link
Member

zkochan commented Jun 1, 2024

I guess we could make it fail only when the package manager differs. When the version differs we can either print a warning or just don't do anything.

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

No branches or pull requests