Skip to content

feat: skip lockfile writes for legacy packageManager field#11284

Merged
zkochan merged 3 commits into
mainfrom
pm-no-lockfile
Apr 17, 2026
Merged

feat: skip lockfile writes for legacy packageManager field#11284
zkochan merged 3 commits into
mainfrom
pm-no-lockfile

Conversation

@zkochan

@zkochan zkochan commented Apr 17, 2026

Copy link
Copy Markdown
Member

Summary

  • When pnpm is declared via the packageManager field in package.json, the resolved pnpm integrity info is no longer written to pnpm-lock.yaml — unless the pinned version is pnpm v12 or newer.
  • devEngines.packageManager still populates and reuses the packageManagerDependencies section of pnpm-lock.yaml as before.
  • This keeps the v10 → v11 transition quiet by avoiding unrelated lockfile churn for projects that pin pnpm the legacy way.

Implementation notes

  • Added a WantedPackageManager type in @pnpm/config.reader that extends EngineDependency with a fromDevEngines flag. getWantedPackageManager tags devEngines-sourced entries.
  • resolvePackageManagerIntegrities gained a save?: boolean option (defaults to true). When false, the function neither reads from nor writes to pnpm-lock.yaml — it resolves purely in memory and returns the EnvLockfile to the caller.
  • switchCliVersion now passes save: persistLockfile based on the wanted-pm source and (for the legacy field) a semver.major(pm.version) >= 12 check.
  • configDependencies are unaffected — they live in a separate code path (resolveAndInstallConfigDeps) that manages its own lockfile reads/writes.

Test plan

  • pnpm --filter pnpm test -- switchingVersions.test.ts — all 12 tests pass, including the new packageManager field does not write pnpm resolution info to pnpm-lock.yaml.
  • Existing devEngines.packageManager cases continue to write and reuse the lockfile.

When pnpm is pinned via the `packageManager` field in `package.json`, the
resolved pnpm integrity info is no longer written to `pnpm-lock.yaml`
unless the pinned version is pnpm v12 or newer. `devEngines.packageManager`
still populates and reuses `packageManagerDependencies` as before. This
keeps the v10 -> v11 transition quiet by avoiding unrelated lockfile
churn for projects that pin pnpm the legacy way.
Copilot AI review requested due to automatic review settings April 17, 2026 10:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces pnpm-lock.yaml churn by avoiding persisting resolved pnpm integrity metadata when pnpm is declared via the legacy packageManager field (except for pnpm v12+), while keeping the existing persistence behavior for devEngines.packageManager.

Changes:

  • Add a WantedPackageManager type (extends EngineDependency) to tag whether the wanted package manager came from devEngines.
  • Add a save?: boolean option to resolvePackageManagerIntegrities to allow in-memory resolution without reading/writing pnpm-lock.yaml.
  • Update CLI version switching logic + tests to ensure legacy packageManager pins don’t write env lockfile data for pnpm < v12.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pnpm/test/switchingVersions.test.ts Adds coverage ensuring legacy packageManager pins don’t create pnpm-lock.yaml.
pnpm/src/switchCliVersion.ts Controls whether env lockfile is read/written based on source (devEngines vs legacy field) and pnpm major version.
installing/env-installer/src/resolvePackageManagerIntegrities.ts Introduces save flag to skip env lockfile disk I/O while still returning an EnvLockfile.
config/reader/src/index.ts Plumbs WantedPackageManager through config reading and tags devEngines-derived declarations.
config/reader/src/Config.ts Defines WantedPackageManager and updates ConfigContext typing accordingly.
.changeset/packageManager-no-lockfile.md Declares minor bumps for the affected packages and documents the behavioral change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pnpm/src/switchCliVersion.ts
Comment thread installing/env-installer/src/resolvePackageManagerIntegrities.ts
zkochan added 2 commits April 17, 2026 13:56
- Update `configurationalDependencies.test.ts` to assert the new behavior:
  the `packageManager` field no longer writes pnpm resolution info to the
  env lockfile while config dependencies still are.
- Fast-path in `switchCliVersion`: when the lockfile is not persisted and
  the running CLI already matches `pm.version`, skip store access and
  integrity resolution entirely.
- Clarify the `resolvePackageManagerIntegrities` docstring to describe
  the conditional `save` behavior.
Extract the decision logic for persisting pnpm resolution info to the env
lockfile into a dedicated helper so the branches — devEngines source,
legacy `packageManager` field with v11 or older, v12+, and invalid/missing
version — can all be covered without needing an actual pnpm v12 tarball
on the registry.
@zkochan zkochan merged commit ea2a7fb into main Apr 17, 2026
12 checks passed
@zkochan zkochan deleted the pm-no-lockfile branch April 17, 2026 12:45
kairosci pushed a commit to kairosci/pnpm that referenced this pull request Apr 22, 2026
* feat: skip lockfile writes for legacy packageManager field

When pnpm is pinned via the `packageManager` field in `package.json`, the
resolved pnpm integrity info is no longer written to `pnpm-lock.yaml`
unless the pinned version is pnpm v12 or newer. `devEngines.packageManager`
still populates and reuses `packageManagerDependencies` as before. This
keeps the v10 -> v11 transition quiet by avoiding unrelated lockfile
churn for projects that pin pnpm the legacy way.

* fix: address Copilot review and CI failure

- Update `configurationalDependencies.test.ts` to assert the new behavior:
  the `packageManager` field no longer writes pnpm resolution info to the
  env lockfile while config dependencies still are.
- Fast-path in `switchCliVersion`: when the lockfile is not persisted and
  the running CLI already matches `pm.version`, skip store access and
  integrity resolution entirely.
- Clarify the `resolvePackageManagerIntegrities` docstring to describe
  the conditional `save` behavior.

* test: add unit tests for shouldPersistLockfile

Extract the decision logic for persisting pnpm resolution info to the env
lockfile into a dedicated helper so the branches — devEngines source,
legacy `packageManager` field with v11 or older, v12+, and invalid/missing
version — can all be covered without needing an actual pnpm v12 tarball
on the registry.
lacolaco added a commit to lacolaco/blog.lacolaco.net that referenced this pull request Apr 26, 2026
#1560)

pnpm/action-setup@v6.0.1 は内部で pnpm v11.0.0-rc.2 を bootstrap し、
actions/setup-node の cache:'pnpm' との連携時に
pnpm-lock.yaml の packageManagerDependencies セクションを書き戻していた。
これは pnpm install --frozen-lockfile の前に発生するため
--frozen-lockfile では防げず、sync-with-notion ワークフロー経由で
PR に流出して継続的なノイズの原因となっていた (#1538 参照)。

pnpm/pnpm#11284 が「packageManager フィールド (< v12) 指定時には
lockfile に packageManagerDependencies を書き込まない」修正を導入し、
これを含む pnpm v11.0.0-rc.5 を内蔵する pnpm/action-setup@v6.0.3 が
リリースされている。

全ワークフローを v6.0.3 (8b2eead6) に更新する。

参考:
- pnpm/action-setup#226
- pnpm/action-setup#228
- pnpm/pnpm#11284

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lacolaco added a commit to lacolaco/blog.lacolaco.net that referenced this pull request Apr 26, 2026
…ation

セッション #1560 のレトロスペクティブで判明した2つの規律を追加:

1. 環境差・バージョン差・CIだけで再現する挙動を「仕様」と推測で断定するな
   - 今回 pnpm/action-setup@v6.0.1 内蔵 pnpm v11RC の lockfile 書き戻しを
     「pnpm の正規仕様」と誤認した
   - 実際は pnpm/action-setup#226, #228, pnpm/pnpm#11284 で報告・修正済みの
     既知バグだった
   - 一次情報 (上流 issue/PR/changelog) を当てる前に、二次情報 (公式設定
     ドキュメントの解釈) で因果推論したのが原因

2. 「即時対応」と「根本対応」を分けて両方 push するな
   - 今回 PR #1538 で pnpm-lock.yaml を main から手動 revert して push
     したが、根本対応 (#1560) が確定する前に即時対応を先行させた
   - 根本対応の方針が変われば即時対応 push は無駄になる、または判断を
     歪める

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lacolaco added a commit to lacolaco/blog.lacolaco.net that referenced this pull request Apr 26, 2026
…ation (#1562)

セッション #1560 のレトロスペクティブで判明した2つの規律を追加:

1. 環境差・バージョン差・CIだけで再現する挙動を「仕様」と推測で断定するな
   - 今回 pnpm/action-setup@v6.0.1 内蔵 pnpm v11RC の lockfile 書き戻しを
     「pnpm の正規仕様」と誤認した
   - 実際は pnpm/action-setup#226, #228, pnpm/pnpm#11284 で報告・修正済みの
     既知バグだった
   - 一次情報 (上流 issue/PR/changelog) を当てる前に、二次情報 (公式設定
     ドキュメントの解釈) で因果推論したのが原因

2. 「即時対応」と「根本対応」を分けて両方 push するな
   - 今回 PR #1538 で pnpm-lock.yaml を main から手動 revert して push
     したが、根本対応 (#1560) が確定する前に即時対応を先行させた
   - 根本対応の方針が変われば即時対応 push は無駄になる、または判断を
     歪める

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jimCresswell added a commit to oaknational/oak-open-curriculum-ecosystem that referenced this pull request Apr 30, 2026
Vercel production was failing on every release commit. Root cause:
pnpm/action-setup@v6.0.2 installed pnpm 11 as the launcher, which
writes its env-lockfile as a separate first YAML document into
pnpm-lock.yaml before delegating to packageManager-pinned pnpm 10.33.2.
The dual-document lockfile is rejected by pnpm 10's full-parse path on
fresh installs (Vercel), which falls back to npm registry fetches that
trip Node 24's strict URLSearchParams (ERR_INVALID_THIS).

Pin chosen by following GitHub's /releases/latest rather than the
highest version number. v5.0.0 is the maintainer-flagged Latest;
v6.0.x is an active stability saga (pnpm/action-setup#228 open). The
principle: pin SHAs from the maintainer's Latest signal, not from the
highest tag - they diverge precisely when a release line is unstable.

Also regenerates pnpm-lock.yaml as single-document by removing the
orphan first document (94-line self-management preamble). Verified
locally: pnpm install --frozen-lockfile completes with shasum unchanged.

Upstream context: pnpm/action-setup#228 (multi-doc breaks pnpm 10),
pnpm/pnpm#11264 (pnpm 11 packageManager downgrade path),
pnpm/pnpm#11284 (upstream fix in pnpm 11.0.0-rc.5).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants