Pnpm Support #46
Pnpm Support #46
Conversation
|
🤩 I'm super excited to see this opened! I will take a deep dive look when I'm back from vacation next week. 😄 |
There was a problem hiding this comment.
This is awesome, thank you for taking the time to compile it all @mikrostew! I think the biggest questions are around what versions we want to support and then how that impacts our ability to manage the global installs.
| lrwxr-xr-x 1 mikrostew staff 38 May 5 23:01 npx@ -> ../lib/node_modules/npm/bin/npx-cli.js | ||
| ``` | ||
|
|
||
| I don't see an easy way to change where those schell scripts are written. We need to have `node` on the `PATH` to run pnpm. |
There was a problem hiding this comment.
I believe we may be able to work around this: Currently, all global packages are installed into the $VOLTA_HOME/tmp directory first, then moved to their final location. We already have some logic for creating sub-paths so that relative symlinks line up. What we can try is to make one of those sub-directories named node, and then add the target directory we want to use for the scripts to the PATH before executing the command. That shouldn't break anything (because there won't be anything there yet), but should fit into pnpm's logic for locating the bin directory.
| There should be a node, nodejs, npm, or pnpm directory in the "PATH" environment variable | ||
| ``` | ||
|
|
||
| This approach seems brittle, because manipulating the `PATH` relies on internal behavior that may change in the future. |
There was a problem hiding this comment.
I suspect that any solution will have some amount of brittleness, since they ultimately will have to work with the internal details of where pnpm decides to put those files. One way we might be able to mitigate that would be to engage with the pnpm maintainers (they've been pretty open from what I've seen, even participating in the issue here on our repo) to add a flag / config setting of some kind to allow us to directly set the bin directory.
Then we could limit our support to e.g. pnpm 4.2 and higher, use the hacky PATH approach for existing versions and the more stable approach for versions that support it. Alternatively, we could even work to add that config setting and then only support pnpm at that version or higher. The (admittedly anecdotal) indication I got from the Volta issue was that back support wouldn't be a major blocker.
We could also do it in stages: Add support for the recent versions once we get a flag added, then expand to include the older ones with the hack later, if there's a community need for it. That way we could avoid any hacky workarounds until they're definitely needed.
|
|
||
| What versions of pnpm should we support? Should we only support certain versions? | ||
| - versions of pnpm prior to v3 don't support the current Node LTS versions (see <https://pnpm.io/installation#compatibility>) | ||
| - versions of pnpm prior to v4.2 don't support `NPM_CONFIG_GLOBAL_DIR` and `--global-dir`, for global package installation |
There was a problem hiding this comment.
4.2 was released in Nov 2019, which is relatively old as these things go. That said, it looks like there was a similar setting prior to version 4.2: pnpm/pnpm#2121 --global-dir was renamed from --pnpm-prefix, so if we set both to the same value, we can probably push that support back even further.
There was a problem hiding this comment.
pnpm has fairly regular updates. I would de-prioritize backwards compatibility.
| - the `NPM_CONFIG_GLOBAL_DIR` environment variable | ||
| - the `--global-dir` command-line option |
There was a problem hiding this comment.
I would lean towards setting the environment variable. That's what we've done for npm and yarn to set the similar settings in those global install interceptions. See https://github.com/volta-cli/volta/blob/main/crates/volta-core/src/tool/package/manager.rs#L84
| +++++++++++++++++++++++++++++++++ | ||
| Progress: resolved 33, reused 33, downloaded 0, added 0, done | ||
|
|
||
| /Users/mikrostew/.volta/tools/image/node/14.16.1/pnpm-global/5: |
There was a problem hiding this comment.
One other small complication here, which I believe is fairly easy to work around, but worth noting: The pnpm global directory (wherever you tell it to install) appends a versioned directory (the 5 in this path), then puts all of the files under that directory. The version isn't tied to the pnpm version, but rather their internal directory layout version. In our case, I don't think it's a big issue, because we can find the single directory and step into it, but it's something to be aware of.
It's also nice because if their directory layout changes in the future, we'll get a direct indication of it via the number changing, so we can likely have a better integration moving forward.
There was a problem hiding this comment.
This all seems reasonable enough to me; mainly I'd be really interested to hear from @zkochan – obviously this is happening later than we'd like and the existence of corepack (though still experimental) adds a wrinkle here, but Volta is still a go-to tool for a lot of folks (including LinkedIn and me personally!) and we'd really like it to have top-notch pnpm support.
|
|
||
| ## Shims | ||
|
|
||
| pnpm includes two executables, `pnpm` and `pnpx`. These will need to be supported in `crates/volta-core/src/run/`, the same way that we support `yarn` and `npx`. There is probably some opportunity for refactoring duplicated code in this section as well. |
There was a problem hiding this comment.
We are deprecating pnpx in v7. I don't think support for pnpx is needed.
pnpx is changed to pnpm dlx
There was a problem hiding this comment.
Good to know, thanks!
|
|
||
| ## Installing Global Packages | ||
|
|
||
| Currently, pnpm installs global packages based on the `PATH` (see [the discussion in this issue](https://github.com/pnpm/pnpm/issues/1360)). The first directory that has `node`, `npm`, or some other node-related binaries is chosen, or failing that, the first directory that is writeable. |
There was a problem hiding this comment.
This is changed in pnpm v7. Now we only write global bin files into the directory which specified through the PNPM_HOME env variable.
There was a problem hiding this comment.
Oh very cool and good to know @zkochan! Maybe the way forward here for us would be to start with support for pnpm v7, and hold off on the special-casing for previous versions until / unless we get significant user feedback about needing them.
There was a problem hiding this comment.
I'd be super interested in this approach as well!
There was a problem hiding this comment.
This is changed in pnpm v7. Now we only write global bin files into the directory which specified through the PNPM_HOME env variable.
I could change this for v7 by setting the PNPM_HOME env. But is there any way to modify global bin files writing for pnpm version 6.x down to version 4.2?
There was a problem hiding this comment.
global-bin-dir https://pnpm.io/6.x/npmrc#global-bin-dir
There was a problem hiding this comment.
global-bin-dir https://pnpm.io/6.x/npmrc#global-bin-dir
Thanks, noticed it's added in v6.15.0, what about older versions below v6.15.0?
There was a problem hiding this comment.
I don't remember whether there was a way to do it in prior versions. I also don't see any value in researching it. Just don't support older versions of pnpm.
|
Looking forward to pnpm support. Any update on when we can expect this to be included.. |
Rendered RFC