chore: migrate from npm to pnpm 10#659
Merged
Merged
Conversation
Closes #658. The driver is `pnpm-workspace.yaml`'s `minimumReleaseAge: 1440` — a 24-hour quarantine on newly published versions blunting supply-chain attacks. npm has no equivalent; this is why the issue requested the migration. ## Lockfile / package manager - `packageManager: pnpm@11.1.3` + matching `devEngines.packageManager` (`onFail: error` — pnpm v11 reimplemented `config / version / pkg` as native subcommands, so `error` is safe). - Bump `engines.node` `>=20.11` → `>=22` (pnpm 11 requires Node 22+). - Commit `pnpm-lock.yaml` (the previous setup had `package-lock.json` gitignored, so CI was effectively unpinned — every run re-resolved deps from the registry, exactly the surface this PR closes). - `prepublishOnly`: `npm run build` → `pnpm run build`. - `before-beta-release.cjs`: `npm show` → `pnpm view` (npm CLI is blocked under devEngines `onFail: error`; `pnpm view` is functionally equivalent). ## `pnpm-workspace.yaml` - `minimumReleaseAge: 1440` (24 h) with `@apify/*` excluded. - `allowBuilds`: `puppeteer` (Chromium download, required by e2e), `esbuild` (platform binary, required by tsx); `core-js` denied (postinstall is just a donation banner). - `strictDepBuilds: false` so a new build-script transitive doesn't fail `--frozen-lockfile` installs. ## CI (`check.yaml`, `release.yaml`) - Every install step collapses to: ```yaml - uses: apify/actions/pnpm-install@v1.1.2 ``` - Test matrix `[20, 22, 24]` → `[22, 24]`. Node 20 drops because pnpm 11 needs ≥22. Node 26 is intentionally NOT added: a pre-existing tsx + yargs (16.x CJS bin) + Node 26 ESM-loader incompatibility breaks `mocha` under the repo's `node-option: ["import=tsx"]` config (reproducible on `master` with plain `npm install`, so not a migration regression — but the fix belongs in a separate change). - `release.yaml`: `npm publish` → `pnpm publish --tag <…> --no-git-checks`. npm publish would be rejected under devEngines `onFail: error`; pnpm publish honours the dist-tag and inherits OIDC auth from `setup-node`'s `registry-url`. Verified locally on Node 24: `pnpm install` clean, `pnpm run build`, `pnpm run lint`, `pnpm run test:unit` (4 passing). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
B4nan
commented
May 19, 2026
pnpm 11 requires Node ≥22, which would force-drop Node 20 from the supported matrix. Staying on Node 20 means staying on pnpm 10. - `packageManager`: `pnpm@11.1.3` → `pnpm@10.33.4` - `devEngines.packageManager.version`: same - `engines.node`: `>=22` → `>=20.11` (restoring the previous floor) - Test matrix: `[22, 24]` → `[20, 22, 24]` - `pnpm-lock.yaml` regenerated under pnpm 10 Note: pnpm 10 shells out to npm for `pnpm config` / `pnpm version` / `pnpm pkg`, so those break under `devEngines onFail: error`. We don't invoke any of them in scripts or CI (`pnpm publish`, `pnpm view`, `pnpm install`, `pnpm run`, `pnpm deploy` are all native and safe), so the guard stays useful. Kept as a separate commit so it can be reverted cleanly the moment we drop Node 20 support and move to pnpm 11. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A pass through the comments added in this PR caught three claims that sounded plausible but don't survive testing on pnpm 10.33.4: - `release.yaml`: the rationale for `pnpm publish` said "`npm publish` would be rejected" under `devEngines.onFail: error`. It isn't — only `pnpm config`/`version`/`pkg` shell-outs break. Reframed the comment around the real reason: toolchain consistency, plus `--no-git-checks` for the detached-HEAD release-tag checkout. - `before-beta-release.cjs`: same pattern. `npm view` is not blocked by devEngines either. `pnpm view` substitution stays (toolchain consistency) but the justification is corrected. - `pnpm-workspace.yaml`: the `strictDepBuilds` comment claimed the default is `true` (per pnpm docs). On 10.33.4 it's empirically warn-only. Rewrote the comment around the actual failure mode that motivated the explicit `false` setting. Also dropped the "see issue #658" task-reference line from the workspace header — that context belongs in the PR description, not in code. No functional changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pnpm's content-addressable store runs `postinstall` once when a package is first fetched, then symlinks on subsequent installs. GH Actions caches the pnpm store but not puppeteer's separate browser cache (`~/.cache/puppeteer/`). On warm-cache CI runs — i.e. every run after the first against a given lockfile hash — pnpm just symlinks from the store and skips `postinstall`, so the Chromium download never happens. Result: 52 e2e failures with `Error: Could not find Chromium (rev. 1108766)`. Triggered immediately on this PR: the third commit (a docs-only change with the same `pnpm-lock.yaml` hash as the second) hit the cache for the first time and surfaced the bug. Fix: run `pnpm rebuild puppeteer` after install in the two jobs that need the browser. Idempotent — no-op once the binary is in place — and scoped to the e2e jobs (lint, unit, type-check don't drive puppeteer). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bliuchak
approved these changes
May 21, 2026
This was referenced May 21, 2026
B4nan
added a commit
that referenced
this pull request
May 21, 2026
`error` plus pnpm 10 is fragile because many pnpm 10 subcommands shell out to npm (`view`, `version`, `pkg`, `info`, `search`, `dist-tag`, ...) — they're only reimplemented natively in pnpm 11. Each one trips `EBADDEVENGINES` under npm 11+. Hit in CI on the first beta release after #659 merged: the publish workflow's `Bump pre-release version` step runs `pnpm view` via `before-beta-release.cjs`, which dies. Run: https://github.com/apify/proxy-chain/actions/runs/26219338774/job/77150345843 The pnpm CLIs we actually invoke from CI (`install`, `publish`, `run`, `rebuild`) are all native, so CI itself stays put. But human contributors running `pnpm view foo` / `pnpm version patch` locally would see the same confusing crash. `warn` keeps the human signal against accidental `npm install` / `yarn install` without the cliff. Bump back to `error` once Node 20 support is dropped and we move to pnpm 11. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #658.
The driver is
pnpm-workspace.yaml'sminimumReleaseAge: 1440— a 24-hour quarantine on newly published versions blunting supply-chain attacks. npm has no equivalent; this is why the issue requested the migration.Two commits, intentionally separated
b789ce6).3396fdc) — keeps Node 20 in the supported matrix. pnpm 11 requires Node ≥22, which would force-drop Node 20. Kept separate so it can be reverted cleanly the moment we drop Node 20 support.Lockfile / package manager
packageManager: pnpm@10.33.4+ matchingdevEngines.packageManagerwithonFail: error.engines.nodestays at>=20.11(matching the previous floor).pnpm-lock.yaml. The previous setup hadpackage-lock.jsonin.gitignore, so CI was effectively unpinned — everynpm installre-resolved deps from the registry. That's exactly the surface this PR closes.prepublishOnly:npm run build→pnpm run build.before-beta-release.cjs:npm show→pnpm view. npm CLI is blocked under devEnginesonFail: error;pnpm viewis functionally equivalent.pnpm-workspace.yamlminimumReleaseAge: 1440(24 h) with@apify/*excluded.allowBuilds:puppeteer(Chromium download, required by e2e),esbuild(platform binary, required by tsx);core-jsdenied (postinstall is just a donation banner).strictDepBuilds: falseso a new build-script transitive doesn't fail--frozen-lockfileinstalls.CI (
check.yaml,release.yaml)[20, 22, 24]. Node 26 is intentionally NOT added: a pre-existing tsx + yargs (16.x CJS bin) + Node 26 ESM-loader incompatibility breaksmochaunder the repo'snode-option: ["import=tsx"]config (reproducible onmasterwith plainnpm install, so not a migration regression — fix belongs in a separate change).release.yaml:npm publish→pnpm publish --tag <…> --no-git-checks. npm publish would be rejected under devEnginesonFail: error; pnpm publish honours the dist-tag and inherits OIDC auth fromsetup-node'sregistry-url.Notes for review
github-registry-tokeninput onapify/actions/pnpm-installis not passed — this repo has no@apify-packages/*deps (only public-registry packages).--frozen-lockfile-style strictness is new here. Renovate / dependency updates will start writing topnpm-lock.yamlgoing forward.pnpm installclean on both Node 20 and Node 24;pnpm run build,pnpm run lint,pnpm run test:unit(4 passing) all green.🤖 Generated with Claude Code