chore: upgrade pnpm to v12 and migrate CI to pnpm/setup - #386
Conversation
The build step in the release workflow ran plain `pnpm build` while every other install/build/test step in CI passes --config.auto-install-peers=true. Since pnpm-workspace.yaml sets autoInstallPeers: false but the preceding peerDependencies install step overrides it to true only for that command, pnpm's dependency-consistency check before running the build script saw a mismatch between the recorded lockfile state and the active config, failing with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH (see release run 34352974702).
There was a problem hiding this comment.
🟡 Changes recommended
pnpm-lock.yaml appears to contain two concatenated YAML documents (duplicate ---/sections), which will likely break pnpm lockfile parsing or cause installs to use incorrect data.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR upgrades the repository’s pinned package manager to pnpm 12.4.0 and updates GitHub Actions workflows to use pnpm/setup (including unified Node+pnpm provisioning in the build/test workflow), while also fixing a release-workflow build command mismatch related to auto-install-peers.
Changes:
- Bump the repo
packageManagerpin topnpm@12.4.0and update the lockfile format to includepackageManagerDependencies. - Migrate CI from
pnpm/action-setuptopnpm/setup, and unify Node + pnpm setup inbuild-test.ymlviaruntime. - Fix the release workflow build command to include
--config.auto-install-peers=trueto avoid lockfile config mismatch failures.
File summaries
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile for pnpm 12 and adds packageManagerDependencies metadata. |
| package.json | Pins the workspace to pnpm@12.4.0 via packageManager. |
| .github/workflows/release.yml | Switches pnpm setup action and fixes release build command config consistency. |
| .github/workflows/build-test.yml | Migrates to pnpm/setup and unifies Node+pnpm setup using runtime with an updated Node matrix. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
panz3r
added
enhancement
The prior --config.auto-install-peers=true CLI overrides worked under pnpm 11, but under pnpm 12.4.0 they no longer cause optional peer dependencies (react-native, expo-modules-core) to be symlinked into the consuming package's own node_modules, even with --force. The override only affects the resolution/download step, not the per-importer linking step, which pnpm 12 apparently ties to the *persisted* autoInstallPeers setting rather than a CLI flag applied to one invocation. Set autoInstallPeers: true directly in pnpm-workspace.yaml (regenerating pnpm-lock.yaml to match) so a single `pnpm i --frozen-lockfile` installs and links everything consistently. This also makes the earlier --config.auto-install-peers=true / separate "Install peerDependencies" step unnecessary, so both are removed from build-test.yml and release.yml, along with the now-redundant flag added in the previous commit. Verified locally: fresh install links react-native/expo-modules-core into packages/google-signin and packages/apple-signin's node_modules, and all three packages build and test successfully (46+25+24 tests passing).
There was a problem hiding this comment.
🟡 Changes recommended
The lockfile appears to be malformed (duplicate YAML documents) and the CI/peer-deps behavior changes conflict with the PR description and intended lockfile-config consistency.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
pnpm-lock.yaml:5
pnpm-lock.yamlcurrently contains two YAML documents (there is a second---at line ~158 followed by anotherlockfileVersionandimportersblock). pnpm lockfiles are expected to be a single coherent document; this looks like an accidental concatenation that is likely to breakpnpm install/CI parsing. Please regenerate the lockfile with pnpm 12 (or otherwise merge thepackageManagerDependenciessection into the main document) so there is only onelockfileVersion/importerstree.
- Files reviewed: 4/5 changed files
- Comments generated: 4
- Review effort level: Lite
true is pnpm's default, so the explicit setting in pnpm-workspace.yaml was a no-op once it matched the default. pnpm-lock.yaml still records settings.autoInstallPeers: true (the resolved effective value), confirming behavior is unchanged.
|
Addressing the review comments from Copilot:
Missing
All resolved — happy to re-request a Copilot review. |
There was a problem hiding this comment.
🟡 Changes recommended
The committed pnpm-lock.yaml is structurally invalid (two YAML documents / duplicated sections), which is likely to break deterministic installs in CI and for contributors.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
pnpm-lock.yaml:163
pnpm-lock.yamlappears to contain two YAML documents (there is a second---separator and duplicated top-level sections likelockfileVersion/importers). pnpm expects a single-document lockfile; this structure is likely to break installs or produce non-deterministic behavior. Regenerate the lockfile so it contains exactly one document with a singlesettings/importers/packages/snapshotsset.
- Files reviewed: 4/5 changed files
- Comments generated: 1
- Review effort level: Lite
Reverts the previous removal. Copilot review correctly flagged that relying on pnpm's current default is fragile: this PR exists precisely because a pnpm-version-specific behavior change around autoInstallPeers broke CI. Declaring it explicitly documents the repo's dependency on peer auto-install (needed for the optional react-native/expo-modules-core peers in google-signin/apple-signin) and protects against pnpm changing its default in a future major version. pnpm-lock.yaml is unchanged since the resolved value was already true either way.
|
Fair point — re-added |
There was a problem hiding this comment.
🟢 Approval recommended
The workflow and workspace config changes are internally consistent (pnpm pin + lockfile + autoInstallPeers + CI install steps) with no issues found in the modified hunks.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 4/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The updated pnpm-lock.yaml contains multiple YAML documents / duplicated root keys, which can break pnpm’s lockfile parsing and installs.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
pnpm-lock.yaml:160
- pnpm-lock.yaml currently contains a second YAML document separator (
---) and repeats top-level keys (lockfileVersion,importers, etc.). pnpm lockfiles are expected to be a single YAML document; multiple documents / duplicated root keys can cause the lockfile to be unreadable or only partially parsed, breaking installs in CI and locally. Please regenerate the lockfile with pnpm@12.4.0 and ensure it serializes as a single document with a single rootimporterssection (including thepackageManagerDependenciesinfo) and no extra---separators.
- Files reviewed: 4/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
packageManagerin package.json, regeneratedpnpm-lock.yaml).pnpm/action-setuptopnpm/setupper its migration guide, pinned by commit SHA (v2.1.0).build-test.ymlinto a singlepnpm/setupstep using itsruntimeinput, replacing the separateactions/setup-nodestep. Node matrix changed from[lts/-1, lts/*, latest]to[22, lts, latest](no "previous LTS" alias exists inpnpm/setup;22will need a manual bump when Node's LTS rotates).release.ymlkeepsactions/setup-nodealongsidepnpm/setup, since its publish job needsregistry-urlfor npm's OIDC trusted-publishing flow.google-signin/apple-signinpackages. These packages declarereact-native/expo-modules-coreas optional peer dependencies. CI worked aroundpnpm-workspace.yaml'sautoInstallPeers: falseby running a second install step with--config.auto-install-peers=true. Under pnpm 11 this correctly linked the optional peers into each package's ownnode_modules. Under pnpm 12.4.0, that same CLI override no longer performs the linking step (confirmed even with--force), sotscfailed withCannot find module 'react-native'. It's tied to the value persisted inpnpm-workspace.yaml, not a one-off CLI flag.autoInstallPeers: truedirectly inpnpm-workspace.yaml(regenerating the lockfile to match), and drop the now-unnecessary--config.auto-install-peers=trueoverrides and separate "Install peerDependencies" step from all three workflows — a singlepnpm i --frozen-lockfilenow installs and links everything consistently.ERR_PNPM_LOCKFILE_CONFIG_MISMATCHfailure onmain's release workflow (config and lockfile settings can no longer disagree), making the flag added in an earlier commit on this branch redundant — removed as part of this fix.Verification
google-signin/apple-signinbuild failure locally under pnpm 12.4.0 (confirmedreact-native/expo-modules-corewere resolved into the store but never symlinked into the package'snode_modules, even with--force).pnpm i --frozen-lockfilelinks both packages' optional peers correctly.Commits
chore: upgrade pnpm to v12.4.0ci: migrate from pnpm/action-setup to pnpm/setupci: unify pnpm and Node.js setup via pnpm/setup runtime inputfix(ci): pass auto-install-peers config to release build stepfix(ci): persist autoInstallPeers:true instead of per-command overrides