-
-
Notifications
You must be signed in to change notification settings - Fork 941
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
feat(exec): allow scripts to call the right pnpm #7723
base: main
Are you sure you want to change the base?
Conversation
When there are multiple pnpm versions installed in different places (such as `corepack`, distro specific package manager, or combination of different methods), calling `pnpm` from within a script (in `package.json#scripts`) often leads to wrong version of pnpm. By adding the installation directory to the start of `PATH`, this problem should be resolved.
if (require.main?.filename) { | ||
prependPaths.unshift(path.dirname(require.main.filename)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It won't work correctly because this is the pnpm js file but the command shim itself (a bash or powershell file) could be in another directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command shim from npm install pnpm
is a JavaScript file has require('../dist/pnpm.cjs')
.
Is there still bash/powershell command shim? If there is, then I can edit PATH
in them, maybe I should edit the JS command shim too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command shim from npm install pnpm is a JavaScript file has require('../dist/pnpm.cjs').
That's only on POSIX. On windows there's be a shim. On POSIX it is a symlink, which still means that the real location will be in a different place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there still bash/powershell command shim? If there is, then I can edit PATH in them, maybe I should edit the JS command shim too.
How would you edit it? pnpm could be installed by corepack, for instance. You don't have control over the command shim
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would process.argv[1]
be consistent? How does yarn
do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just print a warning if pnpm is executed with a different pnpm
If it can do it then it can already do the original intention.
What problem are you trying to solve?
Just some annoyance when switching projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you could set an env variable with the pnpm script location and then in a child process pnpm can check the env variable and if the paths don't match, fall to running the pnpm set in the env variable. This will make it a bit slower. Also, I don't know if it is good practice or not. And there is already corepack, which does pnpm switching, so it might complicate things further or even makes something like an infinite loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's too complicated then we can skip it.
The ideal solution is we control the shims we release. As for corepack, it sets an environment variable named COREPACK_ROOT
, so we know to add $COREPACK_ROOT/shims
to PATH
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure we can control it in all scenarios as there are ways to install pnpm with other package managers (like Winget, Homebrew, etc). In that case they control the command shim (if they use a command shim).
We could dynamically create our own (internal) command shim that would be in a directory that pnpm creates at startup and that directory would be added to the PATH. If we want to solve this issue, then I think this should be the way. I guess it would work with corepack too as it would just go around corepack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mindset is that they way pnpm is executed is a blackbox invisible to the end-user. We should emphasize that the only official way to use pnpm is to call via the official shim or corepack.
Alternatively, we can add to the documentation the necessary environment variables that need to be defined by the shim to correctly execute pnpm.
If they use their own command shim to directly call pnpm.cjs
, then they essentially forked pnpm, so it is their responsibility to define the envs.
When there are multiple pnpm versions installed in different places (such as
corepack
, distro specific package manager, or combination of different methods), callingpnpm
from within a script (inpackage.json#scripts
) often leads to wrong version of pnpm.By adding the installation directory to the start of
PATH
, this problem should be resolved.