install: drop packages held only by optional-peer resolution slots from bun.lock

Merged
RO
3mo ago
+150-0

Repro

bun add -D optional-peer-deps   # has an optional peer on no-deps; no-deps is NOT in bun.lock
bun add no-deps                 # no-deps now in bun.lock as a direct dep
bun remove no-deps

After the remove, bun.lock still contains a packages entry for no-deps:

"packages": {
  "no-deps": ["no-deps@1.0.0", "...", {}, "sha512-..."],
  "optional-peer-deps": ["optional-peer-deps@1.0.0", "...", { "peerDependencies": { "no-deps": "*" }, "optionalPeers": ["no-deps"] }, "sha512-..."]
}

Same thing happens when no-deps is removed from package.json by hand and bun install is re-run. npm and yarn 1.x both return the lockfile to its pre-add state. Reported as the second repro on #8662 (https://github.com/oven-sh/bun/issues/8662#issuecomment-3379529330); distinct from the node_modules pruning part of that issue, which is #32974.

Cause

Lockfile::clean_with_logger rebuilds the package list by a reachability walk from the root (Package::clone over each resolution slot). Once hoist has filled an optional-peer slot with a valid PackageID, Package::clone treats it exactly like a hard dependency edge: it is enqueued and the target survives the clean. During fresh resolve the slot is never populated independently (enqueue_dependency_with_main_and_success_fn returns early for optional peers) and is only filled by hoist when the target is already in the tree via some other edge; the clean step did not honour that invariant.

Fix

In Package::clone, write invalid_package_id to an optional-peer resolution slot instead of enqueuing the old target. Cloner::flush finishes with resolve()/hoist(), which already walks the freshly cloned tree and fills an optional-peer slot only when the target package was kept alive by some non-optional-peer edge, so a still-referenced peer target is re-bound and an unreferenced one is dropped together with its transitives.

Because the slot is now always re-derived from the post-clean tree instead of carried over, a lockfile that contains a history-dependent nested optional-peer placement (the old carried value diverged from what a fresh install would place) may be rewritten once on the first bun install after upgrading. The rewritten value is the fresh-install value, so this is a one-time convergence rather than churn.

Verification

Four tests in test/cli/install/bun-lock.test.ts:

  • bun remove of a package that also satisfied an optional peer returns bun.lock to its pre-add state and round-trips --frozen-lockfile
  • bun install after editing the same removal into package.json does the same
  • control: when a hard dependency elsewhere still pulls the package in, bun.lock is byte-identical before and after the add/remove pair
  • idempotence guard: a non-wildcard optional peer (no-deps@^1.0.0) with two versions of no-deps in the tree (1.0.1 and 2.0.0) produces a byte-identical lockfile across two consecutive installs and passes --frozen-lockfile

The first two fail on the released build with the no-deps entry quoted above and pass with this change; the control and idempotence tests pass on both. Also ran the peer/optional filters of bun-install-registry.test.ts, isolated-install.test.ts, hoist.test.ts, bun-remove.test.ts and bun-lockb.test.ts with no new failures.

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-lock.test.ts