corepack
corepack copied to clipboard
Do not auto pin by default COREPACK_ENABLE_AUTO_PIN=0 is now the default
This fixes all issues related to auto-pin mentioned in #485.
Github tells me that you requested changes but I'm not sure what changes you asked for ? (maybe the comments were not commited, I've seen this happen in the past if you don't submit the review)
Without auto-pinning, corepack is the source of significant bugs and failed builds, as the version of the package manager is essentially free floating.
While I do agree, this is very invasive as mentioned in the issue. To sum them up again here:
- Any contributor using corepack will need to either ignore the
package.jsonchanges or try to submit their own version of the packagemanager field, which may not be well tested by the maintainers. - Corepack should not attempt to write read-only files, this defeats the whole purpose of some command parameters of the package managers such as
npm cinpm install -save=falseandpnpm --frozen-lockfile. I should be able to build without triggering "failed to write" issues.
That is why I made it so that a warning will be logged as long as the user does not provide the packageManager field. If it needs more discoverability, we could output it to stderr instead of the logs ?
corepack is the source of significant bugs and failed builds
I mean, that's not any different from people installing their own version of npm, pnpm or yarn. If the package.json does not restrict the package manager version properly, it's an issue whether or not you use corepack.
An alternative to changing COREPACK_ENABLE_AUTO_PIN default, would be to actually prompt the user if they actually want you to do the change.
In general, modifying (especially source) files without user consent is a bad idea IMHO.
So what one could do:
- if user has a non redirected console (basicaly
noTTY() && !CI)) => prompt them to apply the change - if no input or CI or file readonly => don't change the package.json, or fail, except if
COREPACK_ENABLE_AUTO_PINis set (to either0or1explicitly)
I mean, that's not any different from people installing their own version of npm, pnpm or yarn. If the package.json does not restrict the package manager version properly, it's an issue whether or not you use corepack.
Indeed it's the same. The change was requested when it was asked to enable corepack by default with Node.js.
In that specific instance, there is no user will to install a package manager.
If the user does not know that they are actually installing a package manager (and what version that is), I think the only sensible thing to do is to add the packageManager field.
My -1 is that I don't think this should land at all in this form.