fix(env-installer): mark optional config subdep snapshots with optional: true by zkochan · Pull Request #11770 · pnpm/pnpm

zkochan · 2026-05-20T13:12:59Z

Summary

When resolveOptionalSubdeps records a subdep snapshot in the env lockfile, it now writes { optional: true } instead of {}, matching how all other optional packages are recorded in pnpm-lock.yaml.
Addresses the Copilot review comment on chore: update pnpm and add pacquet to config deps #11765: the @pacquet/* platform packages pulled in via @pnpm/pacquet's optionalDependencies were recorded as plain empty snapshots, inconsistent with how @reflink/*, @esbuild/*, and friends are marked elsewhere in the lockfile.

Notes

Pure serialization fix on the writer side; the reader (installConfigDeps) doesn't consume this flag.
Existing env lockfiles that already have empty-object snapshots for config-dep optional subdeps will need a regen (pnpm update <config-dep> or remove + re-add) for the flag to land.

Test plan

Updated resolveConfigDeps.test.ts to assert { optional: true } on the subdep snapshot.
CI: build + jest.
Once chore: update pnpm and add pacquet to config deps #11765 lands, regenerate pnpm-lock.yaml so the @pacquet/* snapshots pick up optional: true.

Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

Release Notes

Bug Fixes

Resolved inconsistency in lockfile snapshot generation for optional subdependencies. Platform-specific optional dependency snapshots are now properly marked with metadata instead of being emitted as empty objects, ensuring consistency across the lockfile.

…al: true Match how optional packages are recorded elsewhere in pnpm-lock.yaml so non-host platform variants pulled in via a config dep's optionalDependencies aren't treated as required.

coderabbitai · 2026-05-20T13:13:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9bc28c37-cd0f-4162-bf45-91db926a4608

📥 Commits

Reviewing files that changed from the base of the PR and between ef87f3c and 84e7a21.

📒 Files selected for processing (3)

.changeset/config-deps-optional-subdep-snapshot-flag.md
installing/env-installer/src/resolveOptionalSubdeps.ts
installing/env-installer/test/resolveConfigDeps.test.ts

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Agent
GitHub Check: Compile & Lint

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Follow Standard Style with trailing commas, prefer functions over classes, declare functions after they are used (rely on hoisting), and use a single options object for functions with more than two or three arguments
Sort imports in three groups: standard libraries, external dependencies (alphabetically), then relative imports
Write code that explains itself through clear naming and types — do not write comments that merely restate what the code already says; use comments only for non-obvious reasons, hidden invariants, or workarounds

Files:

installing/env-installer/test/resolveConfigDeps.test.ts
installing/env-installer/src/resolveOptionalSubdeps.ts

**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use util.types.isNativeError() instead of instanceof Error when type-checking errors in Jest tests, as instanceof checks can fail across VM realms

Files:

installing/env-installer/test/resolveConfigDeps.test.ts

🧠 Learnings (1)

📚 Learning: 2026-05-14T09:04:00.133Z

Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.

Applied to files:

installing/env-installer/test/resolveConfigDeps.test.ts
installing/env-installer/src/resolveOptionalSubdeps.ts

🔇 Additional comments (3)

installing/env-installer/src/resolveOptionalSubdeps.ts (1)

99-101: LGTM!

installing/env-installer/test/resolveConfigDeps.test.ts (1)

80-82: LGTM!

Also applies to: 89-89

.changeset/config-deps-optional-subdep-snapshot-flag.md (1)

1-7: LGTM!

📝 Walkthrough

Walkthrough

The PR updates env-lockfile snapshot recording for optional subdependencies of config dependencies. Previously, these snapshots were empty objects; they now include { optional: true } to align with the representation of other optional dependencies. The implementation change, test assertion update, and release changeset are included together.

Changes

Optional subdependency snapshot flagging

Layer / File(s)	Summary
Mark optional subdependencies with `optional: true` flag in env-lockfile snapshots `installing/env-installer/src/resolveOptionalSubdeps.ts`, `installing/env-installer/test/resolveConfigDeps.test.ts`, `.changeset/config-deps-optional-subdep-snapshot-flag.md`	Snapshot entries for optional subdependencies are now initialized with `{ optional: true }` instead of `{}`. Test expectations for `@pnpm.e2e/only-darwin-arm64@1.0.0` snapshot are updated to reflect this change. A changeset documents the behavior change with `patch` version bumps for `@pnpm/installing.env-installer` and `pnpm`.

Possibly related PRs

pnpm/pnpm#11725: The parent PR adding optionalDependencies snapshotting for config deps, which this PR refines by marking optional subdependencies with the optional flag.

Suggested labels

area: lockfile, area: config dependencies

Poem

🐰 A snapshot once bare, now wears a new flag,
optional: true — no more empty bag,
Config deps shine with clarity bright,
Consistency reigns, everything's right! ✨

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: marking optional config subdependency snapshots with the `optional: true` flag in the env-installer.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch config-optional

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Adjusts env lockfile serialization for optional subdependencies of config dependencies so they are marked consistently as optional (aligning with pnpm-lock.yaml semantics).

Changes:

Write { optional: true } into env lockfile snapshots for optional subdeps recorded by resolveOptionalSubdeps.
Update resolveConfigDeps test expectations and inline commentary to match the new snapshot shape.
Add a changeset to release the behavior change as a patch.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
installing/env-installer/test/resolveConfigDeps.test.ts	Updates assertions/comments to expect `{ optional: true }` snapshots for optional subdeps.
installing/env-installer/src/resolveOptionalSubdeps.ts	Changes optional subdep snapshot serialization from `{}` to `{ optional: true }`.
.changeset/config-deps-optional-subdep-snapshot-flag.md	Declares patch releases and documents the lockfile serialization adjustment.

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

      ...pickPlatformFields(resolution.manifest),
    }
    if (opts.envLockfile.snapshots[subdepKey] == null) {
-      opts.envLockfile.snapshots[subdepKey] = {}
+      opts.envLockfile.snapshots[subdepKey] = { optional: true }
    }


fix(env-installer): mark optional config subdep snapshots with option…

84e7a21

…al: true Match how optional packages are recorded elsewhere in pnpm-lock.yaml so non-host platform variants pulled in via a config dep's optionalDependencies aren't treated as required.

Copilot AI review requested due to automatic review settings May 20, 2026 13:13

Copilot started reviewing on behalf of zkochan May 20, 2026 13:13 View session

coderabbitai Bot added area: lockfile area: config dependencies Changes related to configDependencies. labels May 20, 2026

coderabbitai Bot approved these changes May 20, 2026

View reviewed changes

Copilot AI reviewed May 20, 2026

View reviewed changes

zkochan merged commit 2061c55 into main May 20, 2026
19 checks passed

zkochan deleted the config-optional branch May 20, 2026 13:40

coderabbitai Bot mentioned this pull request May 20, 2026

chore(release): 11.2.1 #11777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(env-installer): mark optional config subdep snapshots with optional: true#11770

fix(env-installer): mark optional config subdep snapshots with optional: true#11770
zkochan merged 1 commit into
mainfrom
config-optional

zkochan commented May 20, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 20, 2026 •

edited

Loading

Walkthrough

Changes

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zkochan commented May 20, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zkochan commented May 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 20, 2026 •

edited

Loading