pnpm / action-setup

Install pnpm package manager
https://github.com/marketplace/actions/setup-pnpm
MIT License
887 stars 84 forks source link

How does this compare to `run: corepack enable`? #105

Open mmkal opened 9 months ago

mmkal commented 9 months ago

Similar to #30 - I am wondering if there's an advantage to using this vs just running corepack enable, which as far as I understand, will instruct node itself to download and install pnpm, and will respect the packageManager field in package.json.

I'm doing this in my expect-type repo (permalink) and it seems to be working - is there a downside that you know of?

zkochan commented 9 months ago

The advantage is that this installs the "executable" version of pnpm, which is bundled with node.js using vercel/pkg. The "executable" version of pnpm is a bit faster and can run on any version of node.js.

segevfiner commented 9 months ago

Is this really true? Isn't this a flag https://github.com/pnpm/action-setup/tree/v2#standalone ?

paescuj commented 8 months ago

Surely, an advantage over corepack is that the action already takes care of store prune, see https://github.com/pnpm/action-setup#use-cache-to-reduce-installation-time.

jrr commented 7 months ago

One downside is that corepack is still marked as experimental: https://nodejs.org/docs/latest-v20.x/api/corepack.html

zkochan commented 7 months ago

@segevfiner yes, it is turned off by default at the moment.

There is nothing wrong with corepack. In our docs we actually recommend corepack for all CIs except Github actions: https://pnpm.io/continuous-integration. But there is nothing wrong with using corepack instead of this action.

karlhorky commented 4 months ago

Edit: as of pnpm/action-setup@v4, the error described below is surfaced proactively to users with suggestions how to fix the configuration.

One downside of pnpm/action-setup vs corepack enable is that it can lead to ERR_PNPM_BAD_PM_VERSION failures on pnpm install when using the packageManager field in package.json:

  1. Configure a pnpm/action-setup step with version: 'latest'
  2. Configure packageManager in package.json with an old version eg. "packageManager": "pnpm@9.0.3"
  3. Observe the ERR_PNPM_BAD_PM_VERSION error below on pnpm install on CI 💥
$ pnpm install
 ERR_PNPM_BAD_PM_VERSION  This project is configured to use v9.0.3 of pnpm. Your current pnpm is v9.0.4

If you want to bypass this version check, you can set the "package-manager-strict" configuration to "false" or set the "COREPACK_ENABLE_STRICT" environment variable to "0"

Workaround 1

Remove the version: 'latest' config to enable packageManager support:

       - uses: pnpm/action-setup@v3
         with:
-          version: 'latest'

Workaround 2

Switch pnpm/action-setup to corepack enable to resolve this error:

-      - uses: pnpm/action-setup@v3
-        with:
-          version: 'latest'
+      - run: corepack enable

Workaround 3

Always keep packageManager in package.json immediately up to date, eg. using a bot like Renovate bot

karlhorky commented 4 months ago

@zkochan because of the ERR_PNPM_BAD_PM_VERSION error above, maybe pnpm/action-setup should be removed as the recommendation for GitHub Actions?

(to avoid users running into this footgun, especially since packageManager is the recommended way of setting a pnpm version on some platforms like Netlify)

Or, alternatively, if pnpm/action-setup would automatically respect the version in packageManager in package.json, that would be another option (as a feature request to pnpm/action-setup).

segevfiner commented 4 months ago

Doesn't it already have a feature to look at packageManager if you don't specify version? Check the README, I remember I configured it like this.

karlhorky commented 4 months ago

Indeed, I somehow missed this!

As per the pnpm/action-setup docs about the version option, to use the packageManager version, you can just omit the version config:

version

Version of pnpm to install.

Optional when there is a packageManager field in the package.json.

However, this should probably error in the action when packageManager is set and version is specified.

I'll create an issue

karlhorky commented 4 months ago

Ok I ended up creating a PR for this instead - to throw an error if multiple versions of pnpm are specified: