feat(config-deps): support optionalDependencies with platform filtering#11725
Merged
Conversation
Config dependencies can now declare one level of optionalDependencies
with os/cpu/libc constraints, like the esbuild/swc platform-binary
pattern. The env lockfile records all variants so it stays portable
across machines; the installer picks the matching binary at install
time and symlinks it next to the parent in the global virtual store,
making `require('pkg-platform-arch')` resolvable from the parent.
Refs #11723.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughConfig dependencies now record and install one level of optionalDependencies (with platform selectors), include optional-subdep fingerprints in global virtual store leaf paths, and symlink platform-matching optional subdeps into the parent package's node_modules; env lockfile snapshots and packages preserve platform fields. ChangesOptional sub-dependencies for config dependencies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.40785820742,
"stddev": 0.08109477601930336,
"median": 2.43291746692,
"user": 2.70840038,
"system": 3.5976249599999996,
"min": 2.27240797442,
"max": 2.5534866704200003,
"times": [
2.4518136384200004,
2.42842805142,
2.3805501984200004,
2.4374068824200004,
2.44359051742,
2.3067358234200004,
2.4473001614200003,
2.5534866704200003,
2.27240797442,
2.35686215642
]
},
{
"command": "pacquet@main",
"mean": 2.3258925136200004,
"stddev": 0.08219160337914824,
"median": 2.3054538089200003,
"user": 2.72482528,
"system": 3.5923663599999998,
"min": 2.21392083542,
"max": 2.4910551774200003,
"times": [
2.4910551774200003,
2.34641323242,
2.2567211294200002,
2.28727316842,
2.4154430434200003,
2.2734125404200003,
2.32363444942,
2.28365259142,
2.3673989684200003,
2.21392083542
]
},
{
"command": "pnpm",
"mean": 4.5749634424200005,
"stddev": 0.05798504929164784,
"median": 4.569345843420001,
"user": 7.75433458,
"system": 4.0448131599999995,
"min": 4.47485003342,
"max": 4.6696376624200004,
"times": [
4.6696376624200004,
4.62612048042,
4.63022459342,
4.54573454242,
4.51090527642,
4.55986586542,
4.59360428342,
4.47485003342,
4.57126204842,
4.56742963842
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.69340720314,
"stddev": 0.02692371569169816,
"median": 0.6861074705400001,
"user": 0.37953686,
"system": 1.58866364,
"min": 0.67462891804,
"max": 0.76669713504,
"times": [
0.76669713504,
0.68978102104,
0.68464088304,
0.69374573904,
0.69965718904,
0.68108896404,
0.67462891804,
0.68114218404,
0.67511594004,
0.68757405804
]
},
{
"command": "pacquet@main",
"mean": 0.69935460494,
"stddev": 0.018217219309568287,
"median": 0.6927037220400001,
"user": 0.38269755999999994,
"system": 1.5856835400000002,
"min": 0.67886477304,
"max": 0.73505714304,
"times": [
0.7200539890400001,
0.70972795204,
0.68755837604,
0.70723234804,
0.68831624004,
0.68314010904,
0.68650391504,
0.73505714304,
0.69709120404,
0.67886477304
]
},
{
"command": "pnpm",
"mean": 2.5224226513400003,
"stddev": 0.06826096537497021,
"median": 2.5198053760400003,
"user": 2.9647700599999998,
"system": 2.22259024,
"min": 2.4354255580400004,
"max": 2.66925170504,
"times": [
2.5205556600400003,
2.48923982904,
2.56103161104,
2.66925170504,
2.4478524200400003,
2.5190550920400003,
2.4354255580400004,
2.55815343604,
2.4722048500400002,
2.55145635204
]
}
]
} |
… subdep siblings For scoped config dependencies, `path.dirname(pkgDirInGlobalVirtualStore)` returned the scope subdirectory (e.g. `<gvs>/<relPath>/node_modules/@scope`) instead of the actual `node_modules` root. The subsequent `readModulesDir` then returned unscoped names (`['foo']` instead of `['@scope/foo']`), which failed the expected-siblings membership check and caused the parent itself to be rimraf'd as an unexpected sibling. Compute the path from `relPath` directly so the node_modules root is correct for both scoped and unscoped parents.
…vable from parent
There was a problem hiding this comment.
🧹 Nitpick comments (2)
installing/env-installer/test/resolveConfigDeps.test.ts (1)
79-90: ⚡ Quick winAssert
libcpreservation explicitly in env lockfile packages.The test currently proves
os/cpufor one variant, but notlibc. Since this feature relies onos/cpu/libcselectors, add a strict assertion for a musl/glibc package entry.Suggested assertion tightening
- expect(envLockfile!.packages['`@pnpm.e2e/only-linux-x64-musl`@1.0.0']).toBeDefined() + expect(envLockfile!.packages['`@pnpm.e2e/only-linux-x64-musl`@1.0.0']).toStrictEqual({ + resolution: { + integrity: getIntegrity('`@pnpm.e2e/only-linux-x64-musl`', '1.0.0'), + }, + os: ['linux'], + cpu: ['x64'], + libc: ['musl'], + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installing/env-installer/test/resolveConfigDeps.test.ts` around lines 79 - 90, The test must explicitly assert preservation of the libc selector for the linux-musl package: locate the envLockfile usage and the packages entry for '`@pnpm.e2e/only-linux-x64-musl`@1.0.0' (the existing expect on envLockfile!.packages['`@pnpm.e2e/only-linux-x64-musl`@1.0.0']), and tighten the assertion toStrictEqual an object that includes resolution.integrity (use getIntegrity('`@pnpm.e2e/only-linux-x64-musl`','1.0.0')) plus os: ['linux'], cpu: ['x64'], and libc: ['musl'] so the test verifies libc is preserved.installing/env-installer/test/installConfigDeps.ts (1)
127-160: ⚡ Quick winAdd a non-
osmismatch case for platform filtering.This negative test only proves
osfiltering. A regression incpu/libchandling could still pass unnoticed. Please add at least onecpumismatch case (and a Linux-gatedlibcmismatch case if feasible).Suggested test addition
+test('optional subdep that does not match the current cpu is skipped', async () => { + prepareEmpty() + const { storeController, storeDir } = createTempStore() + + const parentName = '`@pnpm.e2e/foo`' + const parentVersion = '100.0.0' + const subdepName = '`@pnpm.e2e/bar`' + const subdepVersion = '100.0.0' + + const lockfile = createEnvLockfile() + const parentKey = `${parentName}@${parentVersion}` + lockfile.importers['.'].configDependencies[parentName] = { specifier: parentVersion, version: parentVersion } + lockfile.packages[parentKey] = { resolution: { integrity: getIntegrity(parentName, parentVersion) } } + lockfile.snapshots[parentKey] = { + optionalDependencies: { [subdepName]: subdepVersion }, + } + lockfile.packages[`${subdepName}@${subdepVersion}`] = { + resolution: { integrity: getIntegrity(subdepName, subdepVersion) }, + os: [process.platform], + cpu: ['this-cpu-does-not-exist'], + } + + await installConfigDeps(lockfile, { + registries: { default: registry }, + rootDir: process.cwd(), + store: storeController, + storeDir, + }) + + const parentRealPath = fs.realpathSync(`node_modules/.pnpm-config/${parentName}`) + const requireFromParent = createRequire(path.join(parentRealPath, 'package.json')) + expect(() => requireFromParent.resolve(`${subdepName}/package.json`)).toThrow(/Cannot find/) +})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installing/env-installer/test/installConfigDeps.ts` around lines 127 - 160, Add a second negative case to the test 'optional subdep that does not match the current platform is skipped' that exercises platform filtering beyond 'os': create another subdependency entry (e.g. subdepCpuName/subdepCpuVersion) in lockfile.packages with a cpu mismatch (set cpu: ['this-cpu-does-not-exist']) and include it in lockfile.snapshots optionalDependencies like the existing os-mismatch package, then assert that resolving that subdependency from the parent also throws; additionally, if process.platform === 'linux' add a third package with a libc mismatch (e.g. libc: ['this-libc-does-not-exist']) and assert it is skipped as well so cpu and Linux-gated libc filtering are covered (use the same patterns/keys as the existing subdep entries and the same requireFromParent.resolve check).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@installing/env-installer/test/installConfigDeps.ts`:
- Around line 127-160: Add a second negative case to the test 'optional subdep
that does not match the current platform is skipped' that exercises platform
filtering beyond 'os': create another subdependency entry (e.g.
subdepCpuName/subdepCpuVersion) in lockfile.packages with a cpu mismatch (set
cpu: ['this-cpu-does-not-exist']) and include it in lockfile.snapshots
optionalDependencies like the existing os-mismatch package, then assert that
resolving that subdependency from the parent also throws; additionally, if
process.platform === 'linux' add a third package with a libc mismatch (e.g.
libc: ['this-libc-does-not-exist']) and assert it is skipped as well so cpu and
Linux-gated libc filtering are covered (use the same patterns/keys as the
existing subdep entries and the same requireFromParent.resolve check).
In `@installing/env-installer/test/resolveConfigDeps.test.ts`:
- Around line 79-90: The test must explicitly assert preservation of the libc
selector for the linux-musl package: locate the envLockfile usage and the
packages entry for '`@pnpm.e2e/only-linux-x64-musl`@1.0.0' (the existing expect on
envLockfile!.packages['`@pnpm.e2e/only-linux-x64-musl`@1.0.0']), and tighten the
assertion toStrictEqual an object that includes resolution.integrity (use
getIntegrity('`@pnpm.e2e/only-linux-x64-musl`','1.0.0')) plus os: ['linux'], cpu:
['x64'], and libc: ['musl'] so the test verifies libc is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d9d5e40-be82-4689-a052-37820150f8d3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/config-deps-optional-subdeps.mdinstalling/env-installer/package.jsoninstalling/env-installer/src/installConfigDeps.tsinstalling/env-installer/src/parseIntegrity.tsinstalling/env-installer/src/resolveAndInstallConfigDeps.tsinstalling/env-installer/src/resolveConfigDeps.tsinstalling/env-installer/src/resolveOptionalSubdeps.tsinstalling/env-installer/test/installConfigDeps.tsinstalling/env-installer/test/resolveConfigDeps.test.tsinstalling/env-installer/tsconfig.json
📜 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: ubuntu-latest / Node.js 24 / Test
🧰 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/src/resolveConfigDeps.tsinstalling/env-installer/src/parseIntegrity.tsinstalling/env-installer/src/resolveOptionalSubdeps.tsinstalling/env-installer/test/installConfigDeps.tsinstalling/env-installer/src/resolveAndInstallConfigDeps.tsinstalling/env-installer/test/resolveConfigDeps.test.tsinstalling/env-installer/src/installConfigDeps.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 (2)
📚 Learning: 2026-05-05T23:03:04.286Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:04.286Z
Learning: The pattern cross-env NODE_OPTIONS="$NODE_OPTIONS ..." in package.json scripts is an established convention in the pnpm/pnpm repository and is used across many packages (e.g., fs/hard-link-dir, worker, __utils__/scripts). Do not flag this as a cross-platform issue in individual files; if a change is needed, apply it as a repo-wide change in a separate PR. Scope this guidance to all package.json files in the repo; use the minimatch pattern '**/package.json' to identify relevant files and review changes at the repository level rather than per-file.
Applied to files:
installing/env-installer/package.json
📚 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/src/resolveConfigDeps.tsinstalling/env-installer/src/parseIntegrity.tsinstalling/env-installer/src/resolveOptionalSubdeps.tsinstalling/env-installer/test/installConfigDeps.tsinstalling/env-installer/src/resolveAndInstallConfigDeps.tsinstalling/env-installer/test/resolveConfigDeps.test.tsinstalling/env-installer/src/installConfigDeps.ts
🔇 Additional comments (18)
installing/env-installer/package.json (1)
36-36: LGTM!installing/env-installer/tsconfig.json (1)
15-17: LGTM!.changeset/config-deps-optional-subdeps.md (1)
1-8: LGTM!installing/env-installer/src/parseIntegrity.ts (2)
3-22: LGTM!
24-39: LGTM!installing/env-installer/src/resolveOptionalSubdeps.ts (3)
1-17: LGTM!
18-72: LGTM!
74-80: LGTM!installing/env-installer/test/installConfigDeps.ts (1)
2-3: LGTM!Also applies to: 85-125
installing/env-installer/test/resolveConfigDeps.test.ts (1)
100-116: LGTM!installing/env-installer/src/resolveConfigDeps.ts (1)
18-18: LGTM!Also applies to: 72-78
installing/env-installer/src/resolveAndInstallConfigDeps.ts (1)
19-19: LGTM!Also applies to: 135-141
installing/env-installer/src/installConfigDeps.ts (6)
78-89: LGTM!
165-176: LGTM!
181-219: LGTM!
246-269: LGTM!
240-244: 🏗️ Heavy liftThe scoped package handling in this code is correct and does not present a concern.
readModulesDirreturns full scoped package names (e.g.,@esbuild/linux-x64), not just scope directories. The implementation infs/read-modules-dir/src/index.ts(line 31) constructs full names when a scope is encountered:const pkgName = scope ?${scope}/${dir.name as string}: dir.name. This meansexpectedSiblingsandexistingSiblingscontain the same data structure—full scoped names—making the filtering logic at lines 242–244 work as intended.> Likely an incorrect or invalid review comment.
231-238: 💤 Low valueNo issues found. The
packageIsInstallablefunction returnsboolean | nullper its type signature. Whenoptional: trueis set, it returnsfalsefor unsupported packages andtruefor supported ones—nevernullor throws. The=== truecomparison is correct and necessary to filter for truly installable packages.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for resolving and installing one level of optionalDependencies for configDependencies, recording all platform variants in the env lockfile while filtering by os/cpu/libc at install time so platform-specific “binary companion” packages (esbuild/swc style) work for config deps.
Changes:
- Resolve and record a config dependency’s one-level
optionalDependenciesinto the env lockfile (preservingos/cpu/libcmetadata). - Install-time platform filtering + sibling symlinking of compatible optional subdeps in the config dep’s GVS leaf to enable Node resolution from inside the parent.
- Add/adjust tests and wire in the new helper module + dependency.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace dependency needed by env-installer. |
| installing/env-installer/tsconfig.json | Adds project reference for config/package-is-installable. |
| installing/env-installer/test/resolveConfigDeps.test.ts | Adds coverage for recording optional subdeps + platform fields and keeping empty snapshots when absent. |
| installing/env-installer/test/installConfigDeps.ts | Adds coverage for install-time filtering and sibling symlink resolution behavior. |
| installing/env-installer/src/resolveOptionalSubdeps.ts | New resolver helper to capture one-level optional subdeps into the env lockfile. |
| installing/env-installer/src/resolveConfigDeps.ts | Wires optional-subdep resolution into config-deps resolution flow. |
| installing/env-installer/src/resolveAndInstallConfigDeps.ts | Wires optional-subdep resolution into “resolve missing config deps then install” flow. |
| installing/env-installer/src/parseIntegrity.ts | Extends normalized config dep shape to include normalized optional subdeps. |
| installing/env-installer/src/installConfigDeps.ts | Installs compatible optional subdeps into their own GVS leaf + symlinks them next to the parent; prunes stale siblings. |
| installing/env-installer/package.json | Adds dependency on @pnpm/config.package-is-installable. |
| .changeset/config-deps-optional-subdeps.md | Declares minor bumps + documents new behavior at a high level. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
Addressed the two CodeRabbit nitpicks in bf8935a:
Written by an agent (Claude Code, claude-opus-4-7). |
…hash
calcLeafGlobalVirtualStorePath hashed with deps:{}, so a config dep with
optionalDependencies always landed in the same GVS leaf regardless of
which subdep versions the lockfile pointed at. Changing a subdep version
while keeping the parent pinned would silently reuse the existing leaf
and overwrite its sibling symlinks. Thread the optional subdeps' full
pkg ids into the hash so the leaf identity matches the lockfile content.
The leaf hash for a subdep itself stays empty-children (one level deep,
matching the config-dep surface).
Range and tag specifiers (^1.0.0, ~1.0.0, latest, etc.) would let the resolved subdep version drift between machines even when the parent is pinned by integrity — defeating the lockfile's reproducibility promise. Reject anything that isn't a valid semver exact version up-front with a clear PnpmError.
…anged The skip-if-already-installed check compared the existing package.json's name/version against the lockfile's. With optional subdeps in play, those can be unchanged while the expected GVS leaf has a different hash (subdep version change, platform change), so the early return left .pnpm-config pointing at the stale leaf with the wrong sibling symlinks. Compare the existing symlink's realpath against the expected leaf instead.
…rent re-link The leaf hash captures parent + subdep identities from the lockfile but not the host's process.arch / process.platform selection, so a host switch between runs (Rosetta x64 vs arm64 on macOS, or any change to the effective platform) keeps the parent symlink target valid while leaving stale sibling links inside the leaf. Run installOptionalSubdeps unconditionally so the prune+relink loop fires; the realpath check now only short-circuits the parent's re-import and re-symlink.
…tale-state repair test The previous test mutated `lockfile.packages[*].os` between two runs to simulate a platform change. The mechanics there were hard to reason about and CI failed in a way that didn't match the production fix's expected behavior (likely a fixture/order interaction with two distinct subdeps). Replace it with a test that directly exercises the property under test: install once, manually `rimraf` the sibling symlink, install again with the same lockfile, assert the symlink is restored. This covers the "skip-check passes but installOptionalSubdeps must still run" path without the cross-package complexity.
…l-subdep resolution failures Mirror the optional-dep error handling from installing/deps-resolver: trust-downgrade errors are a security signal that must abort the install even when the failing dep is optional, while other resolution failures get logged via skippedOptionalDependencyLogger with reason 'resolution_failure' so users can diagnose missing platform binaries.
…n function calcLeafGlobalVirtualStorePath reverts to its original "leaf, no children" semantics. The one-level-with-subdeps computation that the env-installer needs now lives in a dedicated calcGlobalVirtualStorePathWithSubdeps so each function has a single clear contract — and pacquet can mirror them independently when it ports config-dep support.
…eps; fix log version Two follow-ups from PR review: - Switch from packageIsInstallable to checkPackage in the optional-subdep platform filter. packageIsInstallable emits installCheckLogger.warn on every incompatible variant, which would fire on every install for every non-matching platform binary (the env lockfile records all variants for portability). Emit our own skipped-optional-dependency debug log instead. - For resolution_failure logs in resolveOptionalSubdeps, set package.version to the requested bareSpecifier instead of undefined so consumers that format \`name@version\` don't render \`name@undefined\`. Matches the fallback used in installing/deps-resolver.
…ps to preserve libc
Comment on lines
+303
to
+304
| const realPath = await fs.promises.realpath(linkPath) | ||
| return realPath === expectedTarget |
Member
Author
There was a problem hiding this comment.
Fixed in 0a4d552. symlinkPointsTo now realpaths both sides before comparing. Handles the storeDir-is-itself-a-symlink case (e.g. ~/Library/pnpm/store → versioned dir) and the case-insensitive-filesystem case (macOS APFS, Windows NTFS) where two strings can disagree about casing while referring to the same inode. If either path doesn't exist yet, realpath throws and the helper returns false — falling through to the install path is the correct behavior in that case.
Written by an agent (Claude Code, claude-opus-4-7).
KSXGitHub
pushed a commit
that referenced
this pull request
May 19, 2026
Pulls in: - cddf522 feat(pacquet): port lockfile verification (minimumReleaseAge + trustPolicy) (#11729) — adds `default_cache_dir`, which was written against the pre-DI-refactor shape and uses the closure-based `HomeDir: FnOnce(...)` parameter plus a module-level `NoEnv` struct this branch's refactor retired. Followup commit ports it to the trait-based DI seam the PR introduces. - c8d8fde feat(config-deps): support optionalDependencies with platform filtering (#11725) — does not touch files this branch edits. https://claude.ai/code/session_01Btd6rp2MgfXb3ArHwSJpqA
This was referenced May 20, 2026
This was referenced Jun 9, 2026
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.
Summary
Extends
configDependenciesto resolve and install one level ofoptionalDependencies, withos/cpu/libcplatform filtering applied at install time. Closes the prerequisite called out in #11723: this is what makes the esbuild/swc-style platform-binary pattern viable for config dependencies (e.g. shipping pacquet as a config dep with native binaries viaoptionalDependencies).What lands
resolveOptionalSubdeps.ts, wired intoresolveConfigDepsandresolveAndInstallConfigDeps): after each top-level config dep resolves, walks one level ofoptionalDependencies, resolves each, and records them in the env lockfile withos/cpu/libcpreserved. The parent's snapshot getsoptionalDependencies: { … }. All variants are recorded regardless of host platform, so the env lockfile stays portable across machines.installConfigDeps.ts): after the parent is installed into its GVS leaf, fetches each platform-compatible subdep into its own GVS leaf and creates a sibling symlink inside the parent leaf'snode_modules/. Node'srealpath-based resolution then makesrequire('pkg-platform-arch')from inside the parent resolve correctly. Stale siblings are pruned, so platform changes between runs produce a clean layout.calcGlobalVirtualStorePathWithSubdepsingraph-hasher): the parent's GVS leaf hash now folds in the optional subdeps' full pkg ids. Without this, changing a subdep version while keeping the parent pinned would land in the same leaf and silently overwrite the sibling symlinks. The leaf function keeps its original "no children" contract; the new function is a separate entry point that pacquet can mirror cleanly..pnpm-config/{name}symlink'srealpathagainst the expected GVS leaf, not the package.json's name/version. With subdep versions now feeding the leaf hash, name/version alone isn't sufficient. The check only short-circuits the parent's re-import and re-symlink —installOptionalSubdepsalways runs so platform-specific siblings get pruned and relinked when the host's effective platform changes (Rosetta x64 ↔ arm64, etc.)."1.2.3"). Ranges ("^1.0.0") and tags ("latest") are rejected up-front with aCONFIG_DEP_OPTIONAL_NOT_EXACTerror. With the parent pinned by integrity, the subdep's resolved version mustn't drift between machines.skippedOptionalDependencyLoggerwithreason: 'resolution_failure'(same shape asinstalling/deps-resolver) and the install continues — except forERR_PNPM_TRUST_DOWNGRADE, which is a security signal that must still abort the install.Scope
Only one level deep. Transitive
dependenciesand lifecycle scripts remain unsupported — pacquet doesn't need them yet, and they carry meaningful security and complexity tradeoffs that deserve a separate discussion.The env lockfile schema needs no changes:
LockfilePackageInfoalready carriesos/cpu/libc, andLockfilePackageSnapshot.optionalDependenciesalready exists for recording the parent→child edge.Known limitation
If a workspace already had a resolved config dep in the env lockfile (
snapshots[pkgKey] = {}) before this PR, optional subdeps won't be retroactively discovered on subsequent installs. Workaround:pnpm update <pkg>(or remove + re-add). In practice no published package today relies onoptionalDependenciesin a config dep — they couldn't, since the feature didn't exist — so the practical exposure is narrow. See the inline review thread for the design rationale.Test plan
resolveConfigDepsrecords all platform variants withos/cpu/libcpreserved (incl.libcfor musl variants).optionalDependenciesstill produces an empty snapshot.installConfigDepssymlinks the platform-matching subdep and skips non-matching ones (covers bothosandcpumismatches).createRequire(parent).resolve('subdepName')..pnpm-config/{name}.optionalDependenciesare rejected with a clear error.calcLeafGlobalVirtualStorePathunit test covers the leaf-only contract.calcGlobalVirtualStorePathWithSubdepsunit tests: equals leaf path when subdeps is empty; differs on subdep version change, addition, and key-order independence.installConfigDepsandresolveConfigDepstests still pass.configDependenciesin pnpm.io — separate PR to follow.Written by an agent (Claude Code, claude-opus-4-7).