[BUG] corepack enable npm pnpm breaks 32 pnpm commands

[darcyclarke]

Context

pnpm shells out to npm for roughly 32 different commands.

Current Behavior

Enabling both pnpm & npm will break usage of numerous commands in pnpm projects.

Expected Behavior

pnpm commands would not break. Corepack should understand this nuance or remove npm.

Steps to Reproduce

1. corepack enable npm pnpm
2. corepack use pnpm
3. pnpm login get usage error (or pnpm access, pnpm dist-tag, pnpm org, pnpm team & many more)

[aduh95]

Hopefully this will fix itself when devEngines becomes a thing.

[darcyclarke]

Corepack would need to migrate to that & the design of devEngines has only just kicked-off. Also, Corepack would still need to be aware of how these two package managers interact (ex. adding both npm & pnpm to pnpm project's devEngines - which somewhat defeats the purpose of package manager lock-in since you would be able to call npm inside of pnpm projects again). I think, fundamentally, there should be a fix put in place for this scenario if pnpm & npm are going to continue to be supported together. The alternative is for pnpm to drop its dependency on npm but that seems out of the scope of this project & the issue/errors I've run into are from Corepack.

[aduh95]

Corepack would still need to be aware of how these two package managers interact

On the contrary, I think that field would allow Corepack to defer to those tools rather than trying to guess – i.e. COREPACK_ENABLE_STRICT would default to 0.

[darcyclarke]

I'm definitely a big +1 on COREPACK_ENABLE_STRICT being defaulted to 0.

[arcanis]

Why should Corepack change the default when the maintainer of the tool you mention is on the side of fixing it on his side?

[darcyclarke]

@arcanis I don't see any reference to resolving this problem in pnpm itself from the link you referenced.

"pnpm can run npm with the COREPACK_ENABLE_STRICT=0 env variable."
~ zoltan

"Also, I thought npm is not going to be added via corepack, so I don't see what is your point."
~ zoltan

Setting COREPACK_ENABLE_STRICT=0 loosens the requirements of Corepack. It is not a fix to this problem.

The potential fixes I can see here are below:

1. pnpm can remove npm as a dependency (out of the scope of this project)
2. Corepack can add conditions to check for instances where npm is being executed by pnpm in pnpm projects (& not warn/throw in those cases)
3. Corepack can stop managing npm (proposed here)

The first is out of our hands & I haven't heard of support for it yet from that project. The second requires some work & investigation to determine if it's even possible (which I don't have time for & don't think is good to add more project-specific logic). The last is the easiest/cleanest & the work has already been done.

[aduh95]

Why should Corepack change the default

I think it would make sense if the check is implemented by the underlying tool anyway: you get the same UX (an error is thrown if you use the wrong package manager), but the logic is no longer in Corepack. It would also give 2. for free, as pnpm and npm will have to work out those things regardless of Corepack.
Maybe we can add some nuance to that (maybe strict mode is disabled only when devEngines is defined, or something like that), but it seems to me we could avoid the redundant check.

Setting COREPACK_ENABLE_STRICT=0 loosens the requirements of Corepack. It is not a fix to this problem.

The potential fixes I can see here are [...] Corepack can stop managing npm

It seems that removing npm or changing COREPACK_ENABLE_STRICT should equally be "not a fix", no? In both cases, a feature is removed from Corepack.

[styfle]

maybe strict mode is disabled only when devEngines is defined, or something like that

That sounds reasonable since devEngines allows an array of package managers 👍

[arcanis]

I don't see any reference to resolving this problem in pnpm itself from the link you referenced.

Then we have very different reading comprehension. I myself read "pnpm can run npm with the COREPACK_ENABLE_STRICT=0 env variable" as a very clear explanation that pnpm could set the variable in the env when it calls npm.

[darcyclarke]

Maybe we can add some nuance to that (maybe strict mode is disabled only when devEngines is defined, or something like that), but it seems to me we could avoid the redundant check.

Loosing the constraints (ie. COREPACK_ENABLE_STRICT=0) is tangential to ensuring pnpm does not break when COREPACK_ENABLE_STRICT=1.

It seems that removing npm or removing COREPACK_ENABLE_STRICT=0 should equally be "not a fix", no? In both cases, a feature is removed from Corepack.

In the example I provided I'm specifically concerned about pnpm & pnpm projects. Hypothetically npm could have been enabled at any point. The fact is that I should not be able to get into a state where executing pnpm inside a pnpm project throws just because npm was enabled (as it should not be possible to enable npm - ref. https://nodejs.org/docs/latest/api/corepack.html#supported-package-managers).

"Also, I thought npm is not going to be added via corepack, so I don't see what is your point."
~ zoltan

As you can see, even zoltan is surprised I brought this up because they did not think npm was included in Corepack (which is why I'm still recommending #418 lands)

Then we have very different reading comprehension.

I don't think this is a necessary/helpful comment to make.

I myself read "pnpm can run npm with the COREPACK_ENABLE_STRICT=0 env variable" as a very clear explanation that pnpm could set the variable in the env when it calls npm.

You've added conditions which don't meet the requirements. I want Corepack to strictly manage pnpm. pnpm just so happens to have a dependency on npm. When npm is enabled - which shouldn't be possible - executing pnpm via Corepack breaks the numerous commands I've mentioned. There should be no requirement to set COREPACK_ENABLE_STRICT=0 because a package manager is being enabled that shouldn't be possible.

[arcanis]

I want Corepack to strictly manage pnpm

Then don't run corepack enable npm? Why did you run this command if you didn't intend to use npm through Corepack?

[aduh95]

As a workaround, you can either disable npm (by running corepack disable npm), or set COREPACK_ENABLE_STRICT=0 in your env. If you haven't already, I can open try to open a PR to with a fix/workaround on pnpm side.
Another solution that might unblock you is to install pnpm using their standalone script, which does not use Corepack, and you won't get that limitation – I think their standalone script also installs npm for you.

[aduh95]

The fix on pnpm has landed, I think this can be closed. Until the fix makes its way to a pnpm release, you can use any of the workarounds described in my previous comment. Thanks for the report and let me know if you need any more help!

[darcyclarke]

Just want to note that the fix landed in pnpm@>=8.15.5 but all previous versions will still exhibit this issue (~1k versions). If you're running into this problem today, hopefully you land here & now know you should set COREPACK_ENABLE_STRICT=0 when using pnpm@<8.15.5.