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

Usage Error: This project is configured to use <pkgmgr> #157

Closed
styfle opened this issue Aug 7, 2022 · 8 comments · Fixed by #167
Closed

Usage Error: This project is configured to use <pkgmgr> #157

styfle opened this issue Aug 7, 2022 · 8 comments · Fixed by #167

Comments

@styfle
Copy link
Member

styfle commented Aug 7, 2022

One of the biggest problems I have today with Corepack is the Usage Error: This project is configured to use <pkgmgr>.

This happens when I run npm i -g vercel@latest in a project that is configured to use yarn or pnpm. This seems a little too strict because the global flag (-g) is not scoped to my current project.

Similarly, vercel dev will shell out to npm or npx and corepack being enabled could block it from working properly, even if the system has npm installed.

While I think the intention is good (catch accidental typos), I think this shouldn't be the default behavior because it is too restrictive. Perhaps making this strict mode be opt-in would help Corepack go stable (see #104).

@aduh95
Copy link
Contributor

aduh95 commented Aug 8, 2022

Or it could spawn a prompt: You are running a command the uses <pkgmng> in a directory configured to use <pkgmng>. Do you want to continue?.

@mehulkar
Copy link

mehulkar commented Aug 9, 2022

I would also like to be able to run things like yarn --version or npm --version in a situation where corepack is enabled. This early UsageError prevents me from doing that. While debugging @nathanhammond and I noticed that config.json contains an allowlist of commands that are passed through:

corepack/sources/main.ts

Lines 46 to 55 in 312d9ea

// If all leading segments match one of the patterns defined in the `transparent`
// key, we tolerate calling this binary even if the local project isn't explicitly
// configured for it, and we use the special default version if requested.
let isTransparentCommand = false;
for (const transparentPath of definition.transparent.commands) {
if (transparentPath[0] === binaryName && transparentPath.slice(1).every((segment, index) => segment === args[index])) {
isTransparentCommand = true;
break;
}
}

I'm not sure if my case is a good use case for this config, but may be a good place to start?

chris-olszewski added a commit to chris-olszewski/turborepo that referenced this issue Aug 10, 2022
The corepack step as it exists causes issues if a machine already has corepack enabled and is using the npm shim:
```
olszewski@chriss-mbp cli % make corepack
npm install -g corepack@latest
Usage Error: This project is configured to use pnpm

$ npm ...
make: *** [corepack] Error 1
```

I think we could get around this by changing to a working directory without a packageManager, but that feels very icky.  Since we were only upgrading corepack in order to avoid [#110](nodejs/corepack#110) which only happens the first time a user sets up a package manager on a machine, I think this an acceptable regression in order to unblock development.

We should follow what comes out of [vercel#157](nodejs/corepack#157) to see if we can add this back eventually.
styfle added a commit to styfle/corepack that referenced this issue Aug 10, 2022
styfle added a commit to styfle/corepack that referenced this issue Aug 10, 2022
@styfle
Copy link
Member Author

styfle commented Aug 10, 2022

Or it could spawn a prompt:

I think a prompt would be annoying and cause more problems because you need a way to pass --yes to corepack without it passing --yes to the underlying package manager. Maybe a warning would be sufficient, but the hard block is just too much of a breaking change.

The only thing that might be safe to hard block is <pkg> install. So perhaps the allowlist needs to be inverted to a denylist so most commands can be allowed.

chris-olszewski added a commit to vercel/turbo that referenced this issue Aug 11, 2022
The corepack step as it exists causes issues if a machine already has corepack enabled and is using the npm shim:
```
olszewski@chriss-mbp cli % make corepack
npm install -g corepack@latest
Usage Error: This project is configured to use pnpm

$ npm ...
make: *** [corepack] Error 1
```

I think we could get around this by changing to a working directory without a packageManager, but that feels very icky.  Since we were only upgrading corepack in order to avoid [#110](nodejs/corepack#110) which only happens the first time a user sets up a package manager on a machine, I think this an acceptable regression in order to unblock development.

We should follow what comes out of [#157](nodejs/corepack#157) to see if we can add this back eventually.
@aduh95
Copy link
Contributor

aduh95 commented Aug 15, 2022

I think a prompt would be annoying and cause more problems because you need a way to pass --yes to corepack without it passing --yes to the underlying package manager.

Using an env variable would be more practical than a --yes flag IMO.

@styfle
Copy link
Member Author

styfle commented Aug 24, 2022

I think an env var would be good.

I also think corepack needs to be smart enough to ignore -g or --global flags automatically.

@kenrick95
Copy link
Contributor

One other use case I face while using it was that some external dependencies of my repo might have pre/postinstall scripts that calls yarn run something or npm run something. But if my repo is using pnpm, I set "packageManager" field to pnpm, and I enabled Corepack, my repo installation step (pnpm install) will fail on those pre/postinstall scripts.

@nathanhammond
Copy link

Hey y'all, this particular feature (command invocation prevention) has been an absolutely tremendous thorn in our side for the entire time we've been using corepack.

In that time this feature has:

  • required us to rework the entirety of our CI setup
  • broken our local development workflow for debugging
  • required manual local changes to our provided sample code (which doesn't specify packageManager because they're intended to be agnostic) prior to being able to test them

We find ourselves regularly using corepack disable (to expose a global version in $PATH listed behind it) to sidestep this issue. This happens because our tool (vercel/turborepo) must test against each package manager. We would hope that corepack would make it easy to support our needs of regularly switching package managers, but our experience has been that global installs (though more configuration up front) have allowed us to sidestep this one particular feature and make our development experience much smoother.

I hypothesize that we're going to see more and more of this as CI and PaaS vendors begin to roll out tooling to support corepack.

I would like to see exploration of different ways to constrain this behavior:

  • Workspace awareness. You're running this command in a place where the closest "find up" package.json does not specify packageManager. If the current package can be detected as a workspace of a parent package.json that does specify a packageManager, throw an error. If not, log a warning about not specifying packageManager and proceed.
  • Significant expansion of allow/deny lists to enable things like existence checks with -v, npm config set registry foo to run to completion. There exists a subset of commands that are going to be safe and/or compatible.
  • Opt in by specifying packageManager in only the closest package.json?

@nathanhammond
Copy link

nathanhammond commented Sep 1, 2022

This comment is at least partially wrong; corepack checks to make sure there is not an ancestor node_modules directory. It may fail if for some reason a user isn't using node_modules as their require path (again making vercel/turborepo's workspaces setup a pain) but it's not a catastrophically bad situation.


Worth mentioning, esbuild and turbo both will encounter @kenrick95's noted issue:

https://github.com/vercel/turborepo/blob/add338a8d664544118178175281059878acd8fe6/packages/turbo/install.js#L120-L123

https://github.com/evanw/esbuild/blob/41c45af627cd72e86e6389434547d3d9a15b4ab2/lib/npm/node-install.ts#L97-L98

We can (and will) adjust our install method for future versions but we can't forcibly move everybody off of old versions.

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

Successfully merging a pull request may close this issue.

5 participants