From 624c4dcac924d4bfa5fff54d3fbd39450185564d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 05:20:43 +0000 Subject: [PATCH 1/4] install: keep optional peer bindings when cleaning the lockfile Loading bun.lock binds every optional peer to the package next to it before hoisting, but since #35681 Package::clone dropped those bindings again, so the tree built while cleaning could differ from the tree built while loading. For a graph where the peer target's subtree competes with another package for a hoisted slot, --frozen-lockfile then failed on an unchanged project, including on the lockfile bun had just written, and a re-save moved packages around (test/bun.lock: jsbn). Bind the slot after the clone queue drains instead, and only when a non-peer edge cloned the target, so a target held only by peer slots is still dropped. Hoist a second time when a peer was bound by a package placed after its dependent, so a fresh install writes the same layout a reload builds. --- src/install/lockfile.rs | 48 ++++++- src/install/lockfile/Package.rs | 20 +-- src/install/lockfile/Tree.rs | 6 + test/cli/install/bun-lock.test.ts | 134 +++++++++++++++++- .../create-optional-peer-hoist-packages.ts | 93 ++++++++++++ .../optional-peer-hoist-consumer-1.0.0.tgz | Bin 0 -> 234 bytes .../optional-peer-hoist-consumer/package.json | 27 ++++ .../optional-peer-hoist-deep-child-1.0.0.tgz | Bin 0 -> 214 bytes .../package.json | 23 +++ .../optional-peer-hoist-deep-1.0.0.tgz | Bin 0 -> 203 bytes .../optional-peer-hoist-deep/package.json | 22 +++ .../optional-peer-hoist-leaf-1.0.0.tgz | Bin 0 -> 171 bytes .../optional-peer-hoist-leaf-2.0.0.tgz | Bin 0 -> 171 bytes .../optional-peer-hoist-leaf/package.json | 29 ++++ .../optional-peer-hoist-provider-1.0.0.tgz | Bin 0 -> 207 bytes .../optional-peer-hoist-provider/package.json | 22 +++ .../optional-peer-hoist-target-1.0.0.tgz | Bin 0 -> 205 bytes .../optional-peer-hoist-target-2.0.0.tgz | Bin 0 -> 171 bytes .../optional-peer-hoist-target/package.json | 32 +++++ 19 files changed, 440 insertions(+), 16 deletions(-) create mode 100644 test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-consumer/optional-peer-hoist-consumer-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-deep/optional-peer-hoist-deep-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-deep/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-provider/optional-peer-hoist-provider-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-provider/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target/package.json diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 5e5b8ec418f6..8dbcb929ad2c 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -1020,6 +1020,7 @@ impl Lockfile { lockfile: &mut *new, mapping: &mut package_id_mapping, clone_queue: clone_queue_, + optional_peers: PendingResolutions::new(), log, old_preinstall_state, manager: &mut *manager, @@ -1292,6 +1293,9 @@ impl Lockfile { pub struct Cloner<'a> { pub(crate) clone_queue: PendingResolutions, + /// Optional-peer slots, bound in `flush` once every package reachable + /// through a non-peer edge has been cloned. + pub(crate) optional_peers: PendingResolutions, pub lockfile: &'a mut Lockfile, pub(crate) old: &'a mut Lockfile, pub(crate) mapping: &'a mut [PackageID], @@ -1318,6 +1322,19 @@ impl<'a> Cloner<'a> { self.lockfile.buffers.resolutions[to_clone.resolve_id as usize] = new_id; } + // An optional peer stays bound to its target if the target survived the + // clean. Loading a lockfile binds these slots before hoisting, so the + // `resolve` below has to see them bound as well, or it builds a + // different tree than the one `--frozen-lockfile` compares against and + // a re-save moves packages around. A target only reachable through + // peer slots was never cloned and the slots stay unresolved. + for pending in self.optional_peers.drain(..) { + let mapping = self.mapping[pending.old_resolution as usize]; + if (mapping as usize) < max_package_id { + self.lockfile.buffers.resolutions[pending.resolve_id as usize] = mapping; + } + } + // cloning finished, items in lockfile buffer might have a different order, meaning // package ids and dependency ids have changed self.manager @@ -1345,8 +1362,22 @@ impl<'a> Cloner<'a> { // ──────────────────────────────────────────────────────────────────────────── impl Lockfile { + /// Builds the tree that is saved to disk. + /// + /// The resolver leaves optional peers unresolved; hoisting binds each one + /// to the same-named package that ends up next to it. When that package is + /// placed by an edge processed after the peer's dependent, its subtree is + /// queued from that later edge. A lockfile loaded from disk has the binding + /// up front and queues the subtree from the dependent instead, which can + /// hoist the subtree's dependencies differently. Hoist again in that case + /// so the tree is the one a reload builds; otherwise `--frozen-lockfile` + /// rejects the lockfile we are about to save. The second pass starts with + /// every binding the first one made and cannot bind anything late. pub(crate) fn resolve(&mut self, log: &mut bun_ast::Log) -> Result<(), tree::SubtreeError> { - self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None) + if self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)? { + self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)?; + } + Ok(()) } pub(crate) fn filter( @@ -1357,16 +1388,21 @@ impl Lockfile { workspace_filters: &[WorkspaceFilter], packages_to_install: Option<&[PackageID]>, ) -> Result<(), tree::SubtreeError> { + // Runs after `resolve` bound the optional peers, so there is nothing + // left to bind late here. self.hoist::<{ tree::BuilderMethod::Filter }>( log, Some(manager), install_root_dependencies, workspace_filters, packages_to_install, - ) + )?; + Ok(()) } - /// Sets `buffers.trees` and `buffers.hoisted_dependencies` + /// Sets `buffers.trees` and `buffers.hoisted_dependencies`. Returns whether + /// an optional peer was bound after its dependent had been placed + /// (`tree::Builder::late_bound_optional_peer`). pub(crate) fn hoist( &mut self, log: &mut bun_ast::Log, @@ -1376,7 +1412,7 @@ impl Lockfile { install_root_dependencies: bool, workspace_filters: &[WorkspaceFilter], packages_to_install: Option<&[PackageID]>, - ) -> Result<(), tree::SubtreeError> { + ) -> Result { let slice = self.packages.slice(); // `tree::Builder` stores `lockfile: ParentRef` so @@ -1398,6 +1434,7 @@ impl Lockfile { workspace_filters, packages_to_install, pending_optional_peers: Default::default(), + late_bound_optional_peer: false, list: Default::default(), sort_buf: Default::default(), }; @@ -1416,9 +1453,10 @@ impl Lockfile { } let cleaned = builder.clean()?; + let late_bound_optional_peer = builder.late_bound_optional_peer; self.buffers.trees = cleaned.trees; self.buffers.hoisted_dependencies = cleaned.dep_ids; - Ok(()) + Ok(late_bound_optional_peer) } } diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index da74ede1d3b9..7f34d4b81764 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -606,14 +606,20 @@ impl Package { .zip(resolutions.iter_mut()) .enumerate() { - // Optional-peer slots are re-derived by `hoist` (Cloner::flush), not carried over. - if old_dependencies[i].behavior.is_optional_peer() { + if *old_resolution >= max_package_id { *resolution = invalid_package_id; continue; } - if *old_resolution >= max_package_id { - *resolution = invalid_package_id; + let pending = PendingResolution { + old_resolution: *old_resolution, + resolve_id: new_package.resolutions.off + PackageID::try_from(i).expect("int cast"), + }; + + // An optional peer does not keep its target alive. `Cloner::flush` + // binds the slot again only if a non-peer edge cloned the target. + if old_dependencies[i].behavior.is_optional_peer() { + cloner.optional_peers.push(pending); continue; } @@ -621,11 +627,7 @@ impl Package { if mapped < max_package_id { *resolution = mapped; } else { - cloner.clone_queue.push(PendingResolution { - old_resolution: *old_resolution, - resolve_id: new_package.resolutions.off - + PackageID::try_from(i).expect("int cast"), - }); + cloner.clone_queue.push(pending); } } diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index 65c717839caa..c94627e9724d 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -442,6 +442,11 @@ pub struct Builder<'a, const METHOD: BuilderMethod> { // could be visited multiple times before it's resolved. pub(crate) pending_optional_peers: ArrayHashMap>, + /// Set when an unresolved optional peer got bound by a package placed + /// after the peer's dependent (`HoistDependencyResult::ResolveReplace`), + /// so the target's subtree was queued later than it will be once the + /// binding is known up front. See `Lockfile::resolve`. + pub(crate) late_bound_optional_peer: bool, pub(crate) manager: Option<&'a PackageManager>, pub(crate) sort_buf: Vec, pub(crate) workspace_filters: &'a [WorkspaceFilter], @@ -874,6 +879,7 @@ impl Tree { } HoistDependencyResult::ResolveReplace(replace) => { debug_assert!(pkg_id != invalid_package_id); + builder.late_bound_optional_peer = true; builder.resolutions[replace.dep_id as usize] = pkg_id; if let Some(entry) = builder .pending_optional_peers diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 8180f837292a..7645390aab51 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1004,11 +1004,141 @@ it("optional peer with a non-wildcard range is idempotent with two versions of t expect(first).toContain('"no-deps": ["no-deps@'); // A second install over the same lockfile must be a byte-for-byte no-op: the - // cleared optional-peer slot re-derives to the same value hoist produced on - // fresh install. + // optional peer stays bound to the no-deps the fresh install bound it to. await run(["install"]); expect(await file(join(packageDir, "bun.lock")).text()).toBe(first); await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); await run(["install", "--frozen-lockfile"]); }); + +// The optional-peer-hoist-* fixtures (registry/packages/create-optional-peer-hoist-packages.ts): +// +// consumer optional peer on target +// deep -> deep-child -> leaf@1.0.0, target@1.0.0 +// target@1.0.0 -> leaf@2.0.0 +// provider -> target@2.0.0 +// +// Hoisting is breadth-first, so which leaf ends up in the root node_modules +// depends on whether consumer's peer is already bound to target when the tree +// is built: bound, target is placed from consumer and its leaf@2.0.0 reaches +// the root before deep-child's leaf@1.0.0; unbound, target is only placed once +// deep-child is reached and leaf@1.0.0 wins. A loaded bun.lock always has the +// peer bound, so that is the tree every install has to build, otherwise +// --frozen-lockfile compares two different trees. +const optionalPeerHoistDeps = { + "optional-peer-hoist-consumer": "1.0.0", + "optional-peer-hoist-deep": "1.0.0", +}; + +it("a fresh install hoists around an optional peer the same way a reinstall does", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } }); + const run = makeInstallRunner(packageDir); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: optionalPeerHoistDeps })); + await run(["install"]); + const fresh = await file(join(packageDir, "bun.lock")).text(); + expect(fresh).toContain('"optional-peer-hoist-leaf": ["optional-peer-hoist-leaf@2.0.0"'); + expect(fresh).toContain( + '"optional-peer-hoist-deep-child/optional-peer-hoist-leaf": ["optional-peer-hoist-leaf@1.0.0"', + ); + + await run(["install", "--frozen-lockfile"]); + + // --lockfile-only always writes, so this checks the tree a reload builds + // prints back to the same text. + await run(["install", "--lockfile-only"]); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh); +}); + +it.each([ + [ + "leaf@2.0.0 hoisted (target placed from consumer)", + { + "optional-peer-hoist-leaf": "2.0.0", + "optional-peer-hoist-deep-child/optional-peer-hoist-leaf": "1.0.0", + }, + ], + [ + // What a fresh install wrote before the peer binding was carried over. + "leaf@1.0.0 hoisted (target placed from deep-child)", + { + "optional-peer-hoist-leaf": "1.0.0", + "optional-peer-hoist-target/optional-peer-hoist-leaf": "2.0.0", + }, + ], +])("--frozen-lockfile accepts an existing bun.lock with %s", async (_, leafPlacement) => { + const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } }); + const run = makeInstallRunner(packageDir); + + const pkg = (name: string, version: string, info: object = {}) => [ + `${name}@${version}`, + `${registry.registryUrl()}${name}/-/${name}-${version}.tgz`, + info, + "", + ]; + const packages: Record = { + "optional-peer-hoist-consumer": pkg("optional-peer-hoist-consumer", "1.0.0", { + peerDependencies: { "optional-peer-hoist-target": "*" }, + optionalPeers: ["optional-peer-hoist-target"], + }), + "optional-peer-hoist-deep": pkg("optional-peer-hoist-deep", "1.0.0", { + dependencies: { "optional-peer-hoist-deep-child": "1.0.0" }, + }), + "optional-peer-hoist-deep-child": pkg("optional-peer-hoist-deep-child", "1.0.0", { + dependencies: { "optional-peer-hoist-leaf": "1.0.0", "optional-peer-hoist-target": "1.0.0" }, + }), + "optional-peer-hoist-target": pkg("optional-peer-hoist-target", "1.0.0", { + dependencies: { "optional-peer-hoist-leaf": "2.0.0" }, + }), + }; + for (const [path, version] of Object.entries(leafPlacement)) { + packages[path] = pkg("optional-peer-hoist-leaf", version); + } + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: optionalPeerHoistDeps })); + await write( + join(packageDir, "bun.lock"), + JSON.stringify({ + lockfileVersion: 2, + configVersion: 1, + workspaces: { "": { name: "foo", dependencies: optionalPeerHoistDeps } }, + packages, + }), + ); + + await run(["install", "--frozen-lockfile"]); +}); + +it("adding a dependency keeps an optional peer bound to the package bun.lock bound it to", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } }); + const run = makeInstallRunner(packageDir); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: optionalPeerHoistDeps })); + await run(["install"]); + expect(await file(join(packageDir, "bun.lock")).text()).toContain( + '"optional-peer-hoist-target": ["optional-peer-hoist-target@1.0.0"', + ); + + // provider brings in target@2.0.0, which consumer's peer range would accept + // too. The lockfile already binds consumer to target@1.0.0, so that binding + // (and the hoisting that follows from it) is kept and the new version nests. + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { ...optionalPeerHoistDeps, "optional-peer-hoist-provider": "1.0.0" }, + }), + ); + await run(["install"]); + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile).toContain('"optional-peer-hoist-target": ["optional-peer-hoist-target@1.0.0"'); + expect(lockfile).toContain( + '"optional-peer-hoist-provider/optional-peer-hoist-target": ["optional-peer-hoist-target@2.0.0"', + ); + expect(lockfile).toContain('"optional-peer-hoist-leaf": ["optional-peer-hoist-leaf@2.0.0"'); + + await run(["install", "--frozen-lockfile"]); + await run(["install", "--lockfile-only"]); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); +}); diff --git a/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts b/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts new file mode 100644 index 000000000000..42488404f6f9 --- /dev/null +++ b/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts @@ -0,0 +1,93 @@ +#!/usr/bin/env bun +/** + * Generates the `optional-peer-hoist-*` fixtures used by bun-lock.test.ts. + * + * The shape makes the hoisted position of `optional-peer-hoist-leaf` depend on + * whether `consumer`'s optional peer is already bound to `target` when the + * tree is hoisted: + * + * - optional-peer-hoist-consumer@1.0.0 optional peer on optional-peer-hoist-target (any version) + * - optional-peer-hoist-deep@1.0.0 depends on optional-peer-hoist-deep-child@1.0.0 + * - optional-peer-hoist-deep-child@1.0.0 depends on optional-peer-hoist-leaf@1.0.0 and optional-peer-hoist-target@1.0.0 + * - optional-peer-hoist-target@1.0.0 depends on optional-peer-hoist-leaf@2.0.0 + * - optional-peer-hoist-target@2.0.0 no dependencies + * - optional-peer-hoist-leaf@1.0.0/2.0.0 no dependencies + * - optional-peer-hoist-provider@1.0.0 depends on optional-peer-hoist-target@2.0.0 + * + * Hoisting is breadth-first and `consumer` sorts before `deep` (and `provider`), + * so with the peer bound, `target@1.0.0` is placed from `consumer` and its + * `leaf@2.0.0` reaches the root before `deep-child`'s `leaf@1.0.0`. With the + * peer unbound, `target` is only placed once `deep-child` is reached, and + * `leaf@1.0.0` wins the root instead. + */ + +import { mkdir, writeFile } from "fs/promises"; +import { join } from "path"; + +const packagesDir = import.meta.dir; + +const prefix = "optional-peer-hoist-"; + +type Manifest = { + version: string; + dependencies?: Record; + peerDependencies?: Record; + peerDependenciesMeta?: Record; +}; + +const packages: Record = { + consumer: [ + { + version: "1.0.0", + peerDependencies: { [`${prefix}target`]: "*" }, + peerDependenciesMeta: { [`${prefix}target`]: { optional: true } }, + }, + ], + deep: [{ version: "1.0.0", dependencies: { [`${prefix}deep-child`]: "1.0.0" } }], + "deep-child": [ + { + version: "1.0.0", + dependencies: { [`${prefix}leaf`]: "1.0.0", [`${prefix}target`]: "1.0.0" }, + }, + ], + target: [{ version: "1.0.0", dependencies: { [`${prefix}leaf`]: "2.0.0" } }, { version: "2.0.0" }], + leaf: [{ version: "1.0.0" }, { version: "2.0.0" }], + provider: [{ version: "1.0.0", dependencies: { [`${prefix}target`]: "2.0.0" } }], +}; + +for (const [suffix, manifests] of Object.entries(packages)) { + const name = prefix + suffix; + const dir = join(packagesDir, name); + await mkdir(dir, { recursive: true }); + + const versions: Record = {}; + let latest = ""; + for (const manifest of manifests) { + const pkgJson = { name, ...manifest }; + const tarball = join(dir, `${name}-${manifest.version}.tgz`); + await Bun.Archive.write( + tarball, + { "package/package.json": JSON.stringify(pkgJson, null, 2) }, + { compress: "gzip" }, + ); + + const bytes = await Bun.file(tarball).bytes(); + versions[manifest.version] = { + ...pkgJson, + _id: `${name}@${manifest.version}`, + dist: { + integrity: `sha512-${Buffer.from(new Bun.CryptoHasher("sha512").update(bytes).digest()).toString("base64")}`, + shasum: new Bun.CryptoHasher("sha1").update(bytes).digest("hex"), + tarball: `http://localhost:4873/${name}/-/${name}-${manifest.version}.tgz`, + }, + }; + latest = manifest.version; + } + + await writeFile( + join(dir, "package.json"), + JSON.stringify({ _id: name, name, "dist-tags": { latest }, versions }, null, 2), + ); +} + +console.log("Created optional-peer-hoist test packages"); diff --git a/test/cli/install/registry/packages/optional-peer-hoist-consumer/optional-peer-hoist-consumer-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-consumer/optional-peer-hoist-consumer-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..384361d56306832bca7d4e5e4ae26f881e6ac727 GIT binary patch literal 234 zcmb2|=3oGW|8LLxda)P^us&E*+_h+0(ss}PoRPb~3M|w;7N*YWdF$+ja`9#6MrWJ? zSf19-ZhG{w%d4*LEBj9G-#PJKJg?d%lb0nbq@L4TZuqyrzwFJg%s0DEmZjw6zg=-+ zMf`?ouQnUq-&gwcj(p)I7nwhSHw2G|Oqci*J=vw`v&{6M*_LgaU(Yrc80r1M2p^4FgVQ)NH$W-of1b#!sa|D&^PKWiTC%Zhut c=bp*MW#+69pCFMR?$<{9 literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json b/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json new file mode 100644 index 000000000000..e9368a05125e --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json @@ -0,0 +1,27 @@ +{ + "_id": "optional-peer-hoist-consumer", + "name": "optional-peer-hoist-consumer", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "optional-peer-hoist-consumer", + "version": "1.0.0", + "peerDependencies": { + "optional-peer-hoist-target": "*" + }, + "peerDependenciesMeta": { + "optional-peer-hoist-target": { + "optional": true + } + }, + "_id": "optional-peer-hoist-consumer@1.0.0", + "dist": { + "integrity": "sha512-rq7OhIYtUaZVmYZs+XRxr7o7cOfH/Cy0X9+o5CE4yTvpHfDdQmqOvSkS4KQMvVp4y5d+8byAVmRg0kNKS6Yh3w==", + "shasum": "92907b252bd102a17aedc94837a27497a6baff37", + "tarball": "http://localhost:4873/optional-peer-hoist-consumer/-/optional-peer-hoist-consumer-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..c257121b2830d0563b24881ce8857e0b887d212a GIT binary patch literal 214 zcmb2|=3oGW|8LJ9>|!$HVS6y|x5&chb8l{WrE})?l$mSF%D3rWUMK$XE~{V+o#{qXwwrfZUjF`3x;&)>&i?Wt7Im*B~+&(bD4tvstSX?CLUvWqntN}H$dY!UJG zKep}8ry_dV;kRGo7$MF^A`e_-WG(OZ JWYAz>006b8Wyb&j literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json new file mode 100644 index 000000000000..bce75aa233c5 --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json @@ -0,0 +1,23 @@ +{ + "_id": "optional-peer-hoist-deep-child", + "name": "optional-peer-hoist-deep-child", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "optional-peer-hoist-deep-child", + "version": "1.0.0", + "dependencies": { + "optional-peer-hoist-leaf": "1.0.0", + "optional-peer-hoist-target": "1.0.0" + }, + "_id": "optional-peer-hoist-deep-child@1.0.0", + "dist": { + "integrity": "sha512-VQgi9/nSZfe/SB9ahOpe2DOQQa4/EhUmPzO93nFysRlggSz+uiScCyjZ7CV75p3GSpUjZfkR0RoLxhFMWb1ryg==", + "shasum": "56b39a1832a359340949718d6360ff267583175b", + "tarball": "http://localhost:4873/optional-peer-hoist-deep-child/-/optional-peer-hoist-deep-child-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep/optional-peer-hoist-deep-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-deep/optional-peer-hoist-deep-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..1a059d233cc0534e271e0c5fdd0dbdf35852796b GIT binary patch literal 203 zcmb2|=3oGW|8LJ5W*v4AXnnZ0v@7wohvdm=TzB0(ocDBY-kmYk*Lp0>`ID(_xu{zN)I_C~{$v+M0Jn`Q5YYoTor^^=!S%#;xL);4`f2`L3qOr7_L4$z-06y|XgxfkQLolB~~WivKycPDrqt_wkiu_so)Fm(9HOBoE>o R7-|2BA-eVH6$T9k1^{MAPC){3!~&^uOcez7-nvEk4CPv!mn z$Hvd`ZRbSP-x8YZB{XSJ0xV>nd$)ac6dXGie7U$=m zJ^!~eS}kdJLEkp9*}qnL#%6rBJEmyAByG+ov$nahDN5g@+BbU6U3>JG6Zh7ecOlM! Rk#$cP6$OtTXV73^006mMO^*No literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json b/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json new file mode 100644 index 000000000000..1b39b6e45b31 --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json @@ -0,0 +1,29 @@ +{ + "_id": "optional-peer-hoist-leaf", + "name": "optional-peer-hoist-leaf", + "dist-tags": { + "latest": "2.0.0" + }, + "versions": { + "1.0.0": { + "name": "optional-peer-hoist-leaf", + "version": "1.0.0", + "_id": "optional-peer-hoist-leaf@1.0.0", + "dist": { + "integrity": "sha512-fqljP59V2vz45cC0Zan3B9MFl9YqzOkr3yIwFVF8RnI9A/dtx3f8MeLMw75pd18KBY/1aIYCIGN5cvaxwjMiMw==", + "shasum": "d34a5ad0681194b5ea936a7db014a9fad1ea8e3d", + "tarball": "http://localhost:4873/optional-peer-hoist-leaf/-/optional-peer-hoist-leaf-1.0.0.tgz" + } + }, + "2.0.0": { + "name": "optional-peer-hoist-leaf", + "version": "2.0.0", + "_id": "optional-peer-hoist-leaf@2.0.0", + "dist": { + "integrity": "sha512-yfqEs/442yivAG4zTY4u1DFAf8W/ZjIZTI1elABJFvqfaVQiDxmwo+yoDrIXNiJVz7vVrNKIDjhOeU0vToAZNg==", + "shasum": "d7f6cf082fcfb677ee9217b58d25c53b2edee85c", + "tarball": "http://localhost:4873/optional-peer-hoist-leaf/-/optional-peer-hoist-leaf-2.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-provider/optional-peer-hoist-provider-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-provider/optional-peer-hoist-provider-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..8248bb07a20881d387d4d688e35d507fec783c2a GIT binary patch literal 207 zcmb2|=3oGW|8LLRb}<|BuqIr$YYo_ItZhA0^O>2N@}_(8w{P4tQ7n)5*tNmegM;PX zbsGVXM^iWa=BlpMpDtq`?f2}UOUk{xlP7MLc2&$g;ag|yfA9Rc$vb){-<&W*{YS2C zh;sS=U3%Y@`lfXR{|JohD1K?U{DZB?RjZmOH+}_V*&PYh-XE!Cmyv6z9@lYKV{y!~ x)ndM@m&IN=#x1KW__uy-`>yNfxHsCbk6tv{RubY|WOBz<`M7=7)eIU83;^g&R#X50 literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json b/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json new file mode 100644 index 000000000000..5ccab221ad44 --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json @@ -0,0 +1,22 @@ +{ + "_id": "optional-peer-hoist-provider", + "name": "optional-peer-hoist-provider", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "optional-peer-hoist-provider", + "version": "1.0.0", + "dependencies": { + "optional-peer-hoist-target": "2.0.0" + }, + "_id": "optional-peer-hoist-provider@1.0.0", + "dist": { + "integrity": "sha512-tAHlT0reFQqTCgu9fAMlAFluINA6RDKlxD1sBHDBCLQ+ra8GX1e45fDGHFhKY8z+ocrrwC2NT6OYx5O3ydr4RQ==", + "shasum": "5d6cc472a7fd1feac9d883fd688113640ad423af", + "tarball": "http://localhost:4873/optional-peer-hoist-provider/-/optional-peer-hoist-provider-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..93d6285c7ae4961cc7e92e712fe32de250fe2b29 GIT binary patch literal 205 zcmb2|=3oGW|8LJbdNUgeus)dcSzE5S?7HWzcZvUfGmh_yn!KSi&g=i~3-`LzMI9^d zKRtNh%!jO7`5$S}c@Mw5vI||z))ITiXSc+1frW{}CRQtcGrwQE z{OZg8`n%JAtDlpbFz236n#amoP0P7$S=;!|r)Ji^$h3``&dz_LeQmC0WSWOW>6P1G v60Uwa_^EEAn(;Zg1J}3vO`Dg}cI5RYcD;X(IxxTsYdc1k1*~NZ8Vn2oYEWj! literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-2.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..8713d31235967f4d70c15efb76d212b927286bef GIT binary patch literal 171 zcmb2|=3oGW|8LLlvm0*d_7)u8VnC@3AMf)5`yR z>P;-?iH+%BAJ%Tk`|NPY)|+`pUtHd_$}cqLt-0^p+Q)lV+_nf+&f53D=(&MpZFP3# zJO4Xeg4Qj?Th`t%d1lA^Fr?4W@2TC_V!h%SyJoyyC9v_j#o1b^9PMl2Q7d)du|eDe RB|ixBe>=EoJA(!T0|4$COd9|I literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target/package.json b/test/cli/install/registry/packages/optional-peer-hoist-target/package.json new file mode 100644 index 000000000000..3274d8eddd9c --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-target/package.json @@ -0,0 +1,32 @@ +{ + "_id": "optional-peer-hoist-target", + "name": "optional-peer-hoist-target", + "dist-tags": { + "latest": "2.0.0" + }, + "versions": { + "1.0.0": { + "name": "optional-peer-hoist-target", + "version": "1.0.0", + "dependencies": { + "optional-peer-hoist-leaf": "2.0.0" + }, + "_id": "optional-peer-hoist-target@1.0.0", + "dist": { + "integrity": "sha512-2W7OticAYM3dUYnb7H9mj8Pq7uL9Tx1NlvNGqKSJrkw6ClAmUwcMP5Xc08IRS+BQnyKOD8fKoDB4J53ta5fJsw==", + "shasum": "1f38ce1bd407c4f8cf2906126f032ef5e84d9090", + "tarball": "http://localhost:4873/optional-peer-hoist-target/-/optional-peer-hoist-target-1.0.0.tgz" + } + }, + "2.0.0": { + "name": "optional-peer-hoist-target", + "version": "2.0.0", + "_id": "optional-peer-hoist-target@2.0.0", + "dist": { + "integrity": "sha512-tNvzUUL8t4tOZAkyCp1GScCB2V/uWjv15jyvl26QqmkCDcUa/SDQs6BlnqFbHHnYPhnjb9j1sG50jLauLxw6Aw==", + "shasum": "18693529c8e1c9f24fe23f8f313c4f422a9d2bdd", + "tarball": "http://localhost:4873/optional-peer-hoist-target/-/optional-peer-hoist-target-2.0.0.tgz" + } + } + } +} \ No newline at end of file From 3685a290666b486985bd1483fd7f8d2aa5eadb18 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:01:15 +0000 Subject: [PATCH 2/4] install: rebind an optional peer that dedupes onto another version, hoist until settled A carried-over binding is only kept if the bound package is still the one placed next to the dependent. When another version of it already holds that slot and the peer range accepts it, the dependent dedupes onto it, so repoint the binding there as well: that is what a reload binds from the saved tree, and the isolated linker keys the dependent's store entry by it, so the install that wrote bun.lock and the next install from it now link the same entry. Binding one peer can move a dependent and put a target within reach of a second peer the previous pass could not bind, so resolve() hoists until a pass binds nothing late instead of exactly twice. Each extra pass fills at least one more slot and none is ever emptied, so it ends. --- src/install/lockfile.rs | 16 +-- src/install/lockfile/Tree.rs | 30 +++++- test/cli/install/bun-lock.test.ts | 101 +++++++++++++++--- .../create-optional-peer-hoist-packages.ts | 101 +++++++++++++----- .../optional-peer-hoist-consumer-1.0.0.tgz | Bin 234 -> 234 bytes .../optional-peer-hoist-consumer/package.json | 4 +- .../optional-peer-hoist-consumer2-1.0.0.tgz | Bin 0 -> 235 bytes .../package.json | 27 +++++ .../optional-peer-hoist-deep-child-1.0.0.tgz | Bin 214 -> 214 bytes .../optional-peer-hoist-deep-child-2.0.0.tgz | Bin 0 -> 224 bytes .../package.json | 21 +++- .../optional-peer-hoist-deep-1.0.0.tgz | Bin 203 -> 203 bytes .../optional-peer-hoist-deep-2.0.0.tgz | Bin 0 -> 203 bytes .../optional-peer-hoist-deep/package.json | 19 +++- .../optional-peer-hoist-leaf-1.0.0.tgz | Bin 171 -> 171 bytes .../optional-peer-hoist-leaf-2.0.0.tgz | Bin 171 -> 171 bytes .../optional-peer-hoist-leaf-3.0.0.tgz | Bin 0 -> 206 bytes .../optional-peer-hoist-leaf/package.json | 23 +++- .../optional-peer-hoist-provider-1.0.0.tgz | Bin 207 -> 208 bytes .../optional-peer-hoist-provider/package.json | 4 +- .../optional-peer-hoist-tail-1.0.0.tgz | Bin 0 -> 170 bytes .../optional-peer-hoist-tail-2.0.0.tgz | Bin 0 -> 171 bytes .../optional-peer-hoist-tail/package.json | 29 +++++ .../optional-peer-hoist-target-1.0.0.tgz | Bin 205 -> 204 bytes .../optional-peer-hoist-target-2.0.0.tgz | Bin 171 -> 172 bytes .../optional-peer-hoist-target-3.0.0.tgz | Bin 0 -> 278 bytes .../optional-peer-hoist-target/package.json | 27 ++++- .../optional-peer-hoist-target2-0.0.1.tgz | Bin 0 -> 172 bytes .../optional-peer-hoist-target2-1.0.0.tgz | Bin 0 -> 204 bytes .../optional-peer-hoist-target2/package.json | 32 ++++++ 30 files changed, 362 insertions(+), 72 deletions(-) create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-consumer2/optional-peer-hoist-consumer2-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-consumer2/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-deep/optional-peer-hoist-deep-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-3.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-tail/optional-peer-hoist-tail-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-tail/optional-peer-hoist-tail-2.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-tail/package.json create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-3.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target2/optional-peer-hoist-target2-0.0.1.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target2/optional-peer-hoist-target2-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/optional-peer-hoist-target2/package.json diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 8dbcb929ad2c..823554b0278f 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -1369,14 +1369,16 @@ impl Lockfile { /// placed by an edge processed after the peer's dependent, its subtree is /// queued from that later edge. A lockfile loaded from disk has the binding /// up front and queues the subtree from the dependent instead, which can - /// hoist the subtree's dependencies differently. Hoist again in that case - /// so the tree is the one a reload builds; otherwise `--frozen-lockfile` - /// rejects the lockfile we are about to save. The second pass starts with - /// every binding the first one made and cannot bind anything late. + /// hoist the subtree's dependencies differently. So hoist again until a + /// pass binds nothing late: that pass built its tree from bindings it had + /// up front, which is what a reload does, and anything else would make + /// `--frozen-lockfile` reject the lockfile we are about to save. A pass + /// reports `true` only after filling an empty optional peer slot (moving a + /// dependent can put a target within reach of a peer the previous pass + /// could not bind) and no pass empties one, so this ends after at most + /// one pass per optional peer. pub(crate) fn resolve(&mut self, log: &mut bun_ast::Log) -> Result<(), tree::SubtreeError> { - if self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)? { - self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)?; - } + while self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)? {} Ok(()) } diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index c94627e9724d..ef1b80534279 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -135,6 +135,11 @@ enum HoistDependencyResult { Resolve(PackageID), ResolveReplace(ResolveReplace), ResolveLater, + /// Like `Hoisted`, for an optional peer that was bound to one package but + /// deduplicated onto another version of it: the slot is repointed at the + /// package the dependent will find next to itself, which is also what + /// loading the saved tree binds the edge to (`bun.lock.rs`). + Rebind(PackageID), Placement(Placement), } @@ -915,6 +920,10 @@ impl Tree { })?; } } + HoistDependencyResult::Rebind(res_id) => { + debug_assert!(dependency.behavior.is_optional_peer()); + builder.resolutions[dep_id as usize] = res_id; + } HoistDependencyResult::ResolveLater => { // `dep_id` is an unresolved optional peer. while hoisting it deduplicated // with another unresolved optional peer. save it so we remember resolve it @@ -1052,6 +1061,23 @@ impl Tree { // or hoist if peer version allows it if dependency.behavior.is_peer() { + // An optional peer is bound to whatever ends up next to it, so + // a binding that came in pointing elsewhere (carried over by + // `Package::clone`, or made by an earlier pass of + // `Lockfile::resolve`) moves to `res_id`. Only the tree being + // saved decides this; `filter` runs afterwards for installing + // and leaves the bindings alone. Required peers keep the + // version the resolver picked; `bun.lock.rs` re-derives that + // one by version rather than from the tree. + let dedupe = || { + if METHOD == BuilderMethod::Resolvable && dependency.behavior.is_optional_peer() + { + HoistDependencyResult::Rebind(res_id) + } else { + HoistDependencyResult::Hoisted + } + }; + if dependency.version.tag == crate::dependency::VersionTag::Npm { let resolution: Resolution = builder.lockfile().packages.items_resolution()[res_id as usize]; @@ -1059,7 +1085,7 @@ impl Tree { if resolution.tag == crate::resolution::Tag::Npm && version.satisfies(resolution.npm().version, builder.buf(), builder.buf()) { - return Ok(HoistDependencyResult::Hoisted); // 1 + return Ok(dedupe()); // 1 } } @@ -1067,7 +1093,7 @@ impl Tree { // to hoist other peers even if they don't satisfy the version if builder.lockfile().is_workspace_root_dependency(dep_id) { // TODO: warning about peer dependency version mismatch - return Ok(HoistDependencyResult::Hoisted); // 1 + return Ok(dedupe()); // 1 } } diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 7645390aab51..11ba7ce3a7f6 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1,5 +1,6 @@ import { file, spawn, write } from "bun"; import { afterAll, beforeAll, expect, it } from "bun:test"; +import { readlinkSync } from "fs"; import { access, copyFile, cp, exists, open, rm, writeFile } from "fs/promises"; import { bunExe, @@ -1012,19 +1013,13 @@ it("optional peer with a non-wildcard range is idempotent with two versions of t await run(["install", "--frozen-lockfile"]); }); -// The optional-peer-hoist-* fixtures (registry/packages/create-optional-peer-hoist-packages.ts): -// -// consumer optional peer on target -// deep -> deep-child -> leaf@1.0.0, target@1.0.0 -// target@1.0.0 -> leaf@2.0.0 -// provider -> target@2.0.0 -// -// Hoisting is breadth-first, so which leaf ends up in the root node_modules -// depends on whether consumer's peer is already bound to target when the tree -// is built: bound, target is placed from consumer and its leaf@2.0.0 reaches -// the root before deep-child's leaf@1.0.0; unbound, target is only placed once -// deep-child is reached and leaf@1.0.0 wins. A loaded bun.lock always has the -// peer bound, so that is the tree every install has to build, otherwise +// The optional-peer-hoist-* fixtures are described in +// registry/packages/create-optional-peer-hoist-packages.ts. In short: consumer +// has an optional peer on target, and deep -> deep-child reaches target@1.0.0 +// (which depends on leaf@2.0.0) as well as leaf@1.0.0. Hoisting is +// breadth-first, so leaf@2.0.0 only wins the root slot if consumer's peer is +// already bound to target when the tree is built. A loaded bun.lock always has +// the peer bound, so that is the tree every install has to build, otherwise // --frozen-lockfile compares two different trees. const optionalPeerHoistDeps = { "optional-peer-hoist-consumer": "1.0.0", @@ -1051,6 +1046,37 @@ it("a fresh install hoists around an optional peer the same way a reinstall does expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh); }); +it("a fresh install settles hoisting around a peer that only becomes bindable once another peer is bound", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } }); + const run = makeInstallRunner(packageDir); + + // Shape 2 in the fixture generator: binding consumer's peer hoists leaf@3.0.0 + // out from under target, which is what lets target2@1.0.0 reach consumer2's + // peer, and only with that one bound too does target2's tail@2.0.0 beat + // deep-child's tail@1.0.0 to the root, the way it does on every reload. + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "optional-peer-hoist-consumer": "1.0.0", + "optional-peer-hoist-consumer2": "1.0.0", + "optional-peer-hoist-deep": "2.0.0", + }, + }), + ); + await run(["install"]); + const fresh = await file(join(packageDir, "bun.lock")).text(); + expect(fresh).toContain('"optional-peer-hoist-tail": ["optional-peer-hoist-tail@2.0.0"'); + expect(fresh).toContain( + '"optional-peer-hoist-deep-child/optional-peer-hoist-tail": ["optional-peer-hoist-tail@1.0.0"', + ); + + await run(["install", "--frozen-lockfile"]); + await run(["install", "--lockfile-only"]); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(fresh); +}); + it.each([ [ "leaf@2.0.0 hoisted (target placed from consumer)", @@ -1110,7 +1136,7 @@ it.each([ await run(["install", "--frozen-lockfile"]); }); -it("adding a dependency keeps an optional peer bound to the package bun.lock bound it to", async () => { +it("adding a dependency keeps an optional peer on the package bun.lock bound it to while that package stays next to it", async () => { const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } }); const run = makeInstallRunner(packageDir); @@ -1121,8 +1147,9 @@ it("adding a dependency keeps an optional peer bound to the package bun.lock bou ); // provider brings in target@2.0.0, which consumer's peer range would accept - // too. The lockfile already binds consumer to target@1.0.0, so that binding - // (and the hoisting that follows from it) is kept and the new version nests. + // too. bun.lock binds consumer to target@1.0.0, and consumer sorts before + // provider, so target@1.0.0 is placed from consumer first, keeps the root + // slot and the binding, and target@2.0.0 nests under provider. await write( packageJson, JSON.stringify({ @@ -1142,3 +1169,45 @@ it("adding a dependency keeps an optional peer bound to the package bun.lock bou await run(["install", "--lockfile-only"]); expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); }); + +it("an optional peer is rebound when another version of its package takes the slot next to it", async () => { + // The isolated linker is the one consumer of the binding itself: consumer's + // store entry is keyed by the target it was linked against. + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { saveTextLockfile: true, linker: "isolated" }, + }); + const run = makeInstallRunner(packageDir); + const consumerLink = () => readlinkSync(join(packageDir, "node_modules", "optional-peer-hoist-consumer")); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: optionalPeerHoistDeps })); + await run(["install"]); + const boundToTarget1 = consumerLink(); + + // Same as the previous test, but aliased so the provider sorts before + // consumer: target@2.0.0 takes the root slot before consumer's bound + // target@1.0.0 can be placed, and since the peer range accepts it, consumer + // dedupes onto it. That is what a reload of this bun.lock binds consumer to, + // so it is also what this install has to link consumer against. + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { "a-provider": "npm:optional-peer-hoist-provider@1.0.0", ...optionalPeerHoistDeps }, + }), + ); + await run(["install"]); + const lockfile = await file(join(packageDir, "bun.lock")).text(); + expect(lockfile).toContain('"optional-peer-hoist-target": ["optional-peer-hoist-target@2.0.0"'); + expect(lockfile).toContain( + '"optional-peer-hoist-deep-child/optional-peer-hoist-target": ["optional-peer-hoist-target@1.0.0"', + ); + const linkedByThisInstall = consumerLink(); + expect(linkedByThisInstall).not.toBe(boundToTarget1); + + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await run(["install", "--frozen-lockfile"]); + expect(consumerLink()).toBe(linkedByThisInstall); + + await run(["install", "--lockfile-only"]); + expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile); +}); diff --git a/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts b/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts index 42488404f6f9..15a9d0c9ca1f 100644 --- a/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts +++ b/test/cli/install/registry/packages/create-optional-peer-hoist-packages.ts @@ -2,23 +2,46 @@ /** * Generates the `optional-peer-hoist-*` fixtures used by bun-lock.test.ts. * - * The shape makes the hoisted position of `optional-peer-hoist-leaf` depend on - * whether `consumer`'s optional peer is already bound to `target` when the - * tree is hoisted: + * Hoisting is breadth-first, so where a package lands can depend on whether an + * optional peer is already bound to its target when the tree is built: bound, + * the target's subtree is queued right after the dependent; unbound, only once + * a regular dependency edge reaches the target. These packages make that + * difference visible in bun.lock. + * + * Shape 1 (consumer + deep@1.0.0, optionally provider): * * - optional-peer-hoist-consumer@1.0.0 optional peer on optional-peer-hoist-target (any version) - * - optional-peer-hoist-deep@1.0.0 depends on optional-peer-hoist-deep-child@1.0.0 - * - optional-peer-hoist-deep-child@1.0.0 depends on optional-peer-hoist-leaf@1.0.0 and optional-peer-hoist-target@1.0.0 - * - optional-peer-hoist-target@1.0.0 depends on optional-peer-hoist-leaf@2.0.0 + * - optional-peer-hoist-deep@1.0.0 -> deep-child@1.0.0 + * - optional-peer-hoist-deep-child@1.0.0 -> leaf@1.0.0, target@1.0.0 + * - optional-peer-hoist-target@1.0.0 -> leaf@2.0.0 * - optional-peer-hoist-target@2.0.0 no dependencies * - optional-peer-hoist-leaf@1.0.0/2.0.0 no dependencies - * - optional-peer-hoist-provider@1.0.0 depends on optional-peer-hoist-target@2.0.0 + * - optional-peer-hoist-provider@1.0.0 -> target@2.0.0 + * + * `consumer` sorts before `deep`, so with the peer bound, target@1.0.0 is placed + * from consumer and its leaf@2.0.0 reaches the root before deep-child's + * leaf@1.0.0; unbound, target is only placed once deep-child is reached and + * leaf@1.0.0 wins the root. + * + * Shape 2 (consumer + consumer2 + deep@2.0.0) takes more than one extra hoist + * pass to settle: * - * Hoisting is breadth-first and `consumer` sorts before `deep` (and `provider`), - * so with the peer bound, `target@1.0.0` is placed from `consumer` and its - * `leaf@2.0.0` reaches the root before `deep-child`'s `leaf@1.0.0`. With the - * peer unbound, `target` is only placed once `deep-child` is reached, and - * `leaf@1.0.0` wins the root instead. + * - optional-peer-hoist-consumer2@1.0.0 optional peer on optional-peer-hoist-target2 (any version) + * - optional-peer-hoist-deep@2.0.0 -> deep-child@2.0.0 + * - optional-peer-hoist-deep-child@2.0.0 -> leaf@1.0.0, target@3.0.0, tail@1.0.0 + * - optional-peer-hoist-target@3.0.0 -> leaf@3.0.0, and bundles target2@0.0.1 + * - optional-peer-hoist-leaf@3.0.0 -> target2@1.0.0 + * - optional-peer-hoist-target2@0.0.1 no dependencies (the bundled copy) + * - optional-peer-hoist-target2@1.0.0 -> tail@2.0.0 + * - optional-peer-hoist-tail@1.0.0/2.0.0 no dependencies + * + * While consumer's peer is unbound, leaf@3.0.0 nests under target (deep-child's + * leaf@1.0.0 holds the root), so its target2@1.0.0 runs into the bundled + * target2@0.0.1 in target's node_modules and nests there too, out of reach of + * consumer2's peer. Once consumer's peer is bound, leaf@3.0.0 is hoisted to the + * root and target2@1.0.0 does reach consumer2's peer, but only after deep-child + * has put tail@1.0.0 at the root. Once consumer2's peer is bound as well, + * target2's subtree is queued from consumer2 and tail@2.0.0 takes the root. */ import { mkdir, writeFile } from "fs/promises"; @@ -33,25 +56,45 @@ type Manifest = { dependencies?: Record; peerDependencies?: Record; peerDependenciesMeta?: Record; + bundleDependencies?: string[]; }; +const optionalPeerOn = (suffix: string): Manifest => ({ + version: "1.0.0", + peerDependencies: { [prefix + suffix]: "*" }, + peerDependenciesMeta: { [prefix + suffix]: { optional: true } }, +}); + const packages: Record = { - consumer: [ + consumer: [optionalPeerOn("target")], + consumer2: [optionalPeerOn("target2")], + deep: [ + { version: "1.0.0", dependencies: { [`${prefix}deep-child`]: "1.0.0" } }, + { version: "2.0.0", dependencies: { [`${prefix}deep-child`]: "2.0.0" } }, + ], + "deep-child": [ + { version: "1.0.0", dependencies: { [`${prefix}leaf`]: "1.0.0", [`${prefix}target`]: "1.0.0" } }, { - version: "1.0.0", - peerDependencies: { [`${prefix}target`]: "*" }, - peerDependenciesMeta: { [`${prefix}target`]: { optional: true } }, + version: "2.0.0", + dependencies: { [`${prefix}leaf`]: "1.0.0", [`${prefix}target`]: "3.0.0", [`${prefix}tail`]: "1.0.0" }, }, ], - deep: [{ version: "1.0.0", dependencies: { [`${prefix}deep-child`]: "1.0.0" } }], - "deep-child": [ + target: [ + { version: "1.0.0", dependencies: { [`${prefix}leaf`]: "2.0.0" } }, + { version: "2.0.0" }, { - version: "1.0.0", - dependencies: { [`${prefix}leaf`]: "1.0.0", [`${prefix}target`]: "1.0.0" }, + version: "3.0.0", + dependencies: { [`${prefix}leaf`]: "3.0.0", [`${prefix}target2`]: "0.0.1" }, + bundleDependencies: [`${prefix}target2`], }, ], - target: [{ version: "1.0.0", dependencies: { [`${prefix}leaf`]: "2.0.0" } }, { version: "2.0.0" }], - leaf: [{ version: "1.0.0" }, { version: "2.0.0" }], + target2: [{ version: "0.0.1" }, { version: "1.0.0", dependencies: { [`${prefix}tail`]: "2.0.0" } }], + leaf: [ + { version: "1.0.0" }, + { version: "2.0.0" }, + { version: "3.0.0", dependencies: { [`${prefix}target2`]: "1.0.0" } }, + ], + tail: [{ version: "1.0.0" }, { version: "2.0.0" }], provider: [{ version: "1.0.0", dependencies: { [`${prefix}target`]: "2.0.0" } }], }; @@ -64,12 +107,16 @@ for (const [suffix, manifests] of Object.entries(packages)) { let latest = ""; for (const manifest of manifests) { const pkgJson = { name, ...manifest }; + const files: Record = { "package/package.json": JSON.stringify(pkgJson, null, 2) }; + for (const bundled of manifest.bundleDependencies ?? []) { + files[`package/node_modules/${bundled}/package.json`] = JSON.stringify( + { name: bundled, version: manifest.dependencies![bundled] }, + null, + 2, + ); + } const tarball = join(dir, `${name}-${manifest.version}.tgz`); - await Bun.Archive.write( - tarball, - { "package/package.json": JSON.stringify(pkgJson, null, 2) }, - { compress: "gzip" }, - ); + await Bun.Archive.write(tarball, files, { compress: "gzip" }); const bytes = await Bun.file(tarball).bytes(); versions[manifest.version] = { diff --git a/test/cli/install/registry/packages/optional-peer-hoist-consumer/optional-peer-hoist-consumer-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-consumer/optional-peer-hoist-consumer-1.0.0.tgz index 384361d56306832bca7d4e5e4ae26f881e6ac727..12b8c53939cdaebce9c70ce9dfd31f145bb0c4e5 100644 GIT binary patch literal 234 zcmb2|=3oGW|8LJ9^g3c7!1iFyXOY7)N3XryR;HM~-p*yk;i%Hd6PCnH+qZke-V>YH z9R;WS-_65g|KH%G|N4Im$}?xb_3k|Q$3RfBy|HIbtlz}C8^PuKUmp)_+jo0%N$qxF z=cKxWL95QY?W=wLX{Y_8lunBui#I9vtvqM=W4qeQXU{CpEj0_~Nwv&+7VUlH{jAe6 z%+`U(Yrc80r1M2p^4FgVQ)NH$W-of1b#!sa|D&^PKWiTC%Zhut c=bp*MW#+69pCFMR?$<{9 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json b/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json index e9368a05125e..5e10ffcbf135 100644 --- a/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json +++ b/test/cli/install/registry/packages/optional-peer-hoist-consumer/package.json @@ -18,8 +18,8 @@ }, "_id": "optional-peer-hoist-consumer@1.0.0", "dist": { - "integrity": "sha512-rq7OhIYtUaZVmYZs+XRxr7o7cOfH/Cy0X9+o5CE4yTvpHfDdQmqOvSkS4KQMvVp4y5d+8byAVmRg0kNKS6Yh3w==", - "shasum": "92907b252bd102a17aedc94837a27497a6baff37", + "integrity": "sha512-sVIugo50KczyFT1h3oEAZTIvptOi5hfwwveKVbLw3MjOsQC4XBkB8FYFpgQH2ltjpU88VhHpN6g3LSF+OuQOYQ==", + "shasum": "b96f9a0bb75bbcf182c94187b007131b52534742", "tarball": "http://localhost:4873/optional-peer-hoist-consumer/-/optional-peer-hoist-consumer-1.0.0.tgz" } } diff --git a/test/cli/install/registry/packages/optional-peer-hoist-consumer2/optional-peer-hoist-consumer2-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-consumer2/optional-peer-hoist-consumer2-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..53654651773be438bdbe70def698ecb0cfdd29d3 GIT binary patch literal 235 zcmb2|=3oGW|8LJ9^g3c7!1iFyXOY7)N7qi?7QHZiy`9U7!%?M^CoGAZwr}@_y(c!Y zI|@$uznh20{=dOV|MmYClxNO9>)mTVUOv@0&M)-EHrQe|JF_m;S~6<1#EzqPs5?r6o5 dS>m?q!@alMx(e|MGWkba-lZ(DfI)+S0RT#(Y$gBz literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-consumer2/package.json b/test/cli/install/registry/packages/optional-peer-hoist-consumer2/package.json new file mode 100644 index 000000000000..a37b4cd92c38 --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-consumer2/package.json @@ -0,0 +1,27 @@ +{ + "_id": "optional-peer-hoist-consumer2", + "name": "optional-peer-hoist-consumer2", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "optional-peer-hoist-consumer2", + "version": "1.0.0", + "peerDependencies": { + "optional-peer-hoist-target2": "*" + }, + "peerDependenciesMeta": { + "optional-peer-hoist-target2": { + "optional": true + } + }, + "_id": "optional-peer-hoist-consumer2@1.0.0", + "dist": { + "integrity": "sha512-9cKPjtskNfVsoLMh7v3AKDZxX3zI0ayq4lD5bCtQzpcscobikU1WNK9PWrPir/xjvGQ8P1YsleCO2OacONQ1iA==", + "shasum": "1d3aefe2e6828fa609bfe8b40c35ea541d9846b9", + "tarball": "http://localhost:4873/optional-peer-hoist-consumer2/-/optional-peer-hoist-consumer2-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-1.0.0.tgz index c257121b2830d0563b24881ce8857e0b887d212a..be42b51658abd2a9b238317a14a0aa15a6d658ed 100644 GIT binary patch literal 214 zcmb2|=3oGW|8LJ9>^fv1!1iFyXA$Rfx8E&!TedJ?_*UfEJabOpFG~A%FJKS+vV`N% z|5&!R`m`BOj@fUYbiQQO*Ze+5TS?>ZvF=^{>$Hn68{b^N>}~bCsNcRh(b>|jYvK|n zcSnoZ*S!9<(|qG)7quPrhRic(boC!R8!8mJs`SgwkjWmaj%}T<{&`{NnXqYZk`0(8 zql?cjf2ZPkNG0*hq_YmzzAoFppM6xP*Z!WRJ31`rT%hlk>(#6fXCssD>;ggk{~0tG F7yxr|Unu|p literal 214 zcmb2|=3oGW|8LJ9>|!$HVS6y|x5&chb8l{WrE})?l$mSF%D3rWUMK$XE~{V+o#{qXwwrfZUjF`3x;&)>&i?Wt7Im*B~+&(bD4tvstSX?CLUvWqntN}H$dY!UJG zKep}8ry_dV;kRGo7$MF^A`e_-WG(OZ JWYAz>006b8Wyb&j diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-2.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/optional-peer-hoist-deep-child-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..3e0df4238cadb09b29eeb5ce0edbac836ca5bb97 GIT binary patch literal 224 zcmb2|=3oGW|8LJ9^kOmOVS6y|xu(;(b2IXe3;r@oReKk9e4Eee>8bZ)3mg*zZ8Vxb z)lYZu_%}i5S^I|TuZz|lu04D5kAdLKV-tJ!>{_FAE^?E(wYjg6Rke50+^yFp%!vOG z5`Ckm=J&Oy#q~!vDnAmDn`&FAr~6Fg+P)Qee-)44`sKRy%V))P-0`I@f7V3zT5eRo z$7^(A=kAP)TQX)X4*OQ}rsKuqJ)wtXtCN0j`*SjZ`*xgkb<+2*88P>btrrEC8$sNU RO#Z*keNF4$H3kg^1^`bEX?g$v literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json index bce75aa233c5..8048eb3f5102 100644 --- a/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json +++ b/test/cli/install/registry/packages/optional-peer-hoist-deep-child/package.json @@ -2,7 +2,7 @@ "_id": "optional-peer-hoist-deep-child", "name": "optional-peer-hoist-deep-child", "dist-tags": { - "latest": "1.0.0" + "latest": "2.0.0" }, "versions": { "1.0.0": { @@ -14,10 +14,25 @@ }, "_id": "optional-peer-hoist-deep-child@1.0.0", "dist": { - "integrity": "sha512-VQgi9/nSZfe/SB9ahOpe2DOQQa4/EhUmPzO93nFysRlggSz+uiScCyjZ7CV75p3GSpUjZfkR0RoLxhFMWb1ryg==", - "shasum": "56b39a1832a359340949718d6360ff267583175b", + "integrity": "sha512-s+joWU9Q0rD813sf/B6wMc3b9wNT+gaKC0Z7+nFwFXx81J1mA+XIJ1ppLZg8JJup4f+7lBuz0viJDHudI/U08Q==", + "shasum": "ea89ffe7f38b2f18c9ca3d02f72030741a8347f4", "tarball": "http://localhost:4873/optional-peer-hoist-deep-child/-/optional-peer-hoist-deep-child-1.0.0.tgz" } + }, + "2.0.0": { + "name": "optional-peer-hoist-deep-child", + "version": "2.0.0", + "dependencies": { + "optional-peer-hoist-leaf": "1.0.0", + "optional-peer-hoist-target": "3.0.0", + "optional-peer-hoist-tail": "1.0.0" + }, + "_id": "optional-peer-hoist-deep-child@2.0.0", + "dist": { + "integrity": "sha512-YocWmQN6LECrF6fsAnQlTn63MxR1tmQiPmk2TfFe9UlTOHg4cJaLcxoRAx3ENFqiDA23z2O2YXXEQfH7CwgnDQ==", + "shasum": "590338e91f3aeb5e59a3f501d40e5244d4f3650c", + "tarball": "http://localhost:4873/optional-peer-hoist-deep-child/-/optional-peer-hoist-deep-child-2.0.0.tgz" + } } } } \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep/optional-peer-hoist-deep-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-deep/optional-peer-hoist-deep-1.0.0.tgz index 1a059d233cc0534e271e0c5fdd0dbdf35852796b..e57632f6315a4d8c68883871953366c313796c4a 100644 GIT binary patch literal 203 zcmb2|=3oGW|8LJ9^g8Sy()uv+xu)J`vyXn0-A>$Yd6KlaH0^|o`O^1#H(CP<6ulOH zsGrSTUZ|(ufBE%~oc^Da&U$-Z{A(QgcDLW0x}APk=e|>YQ=WT2`+A%D#S3i$b5`tb z{JyDo^=142yX4B%{ib&GKc4;5vF6n6NslVlY`z&Ke<^5NmD%Zk3xz()Z=0s-YTv=% zYgV=M+%(;DV#|+q$v+9{JaOLlYe_`k=W*Lp0>`ID(_xu{zN)I_C~{$v+M0Jn`Q5YYoTor^^=!S%#;xL);4`f2`L3qOr7_L4$z-06yT*P&~?;1ZN!J^p{MEpwLW;Tf6wpm0Fs+;+L&*zTbVBfBVW?uN)68eB0*uThFGWO|RvF%U$^j z-QQa*|9)J5yYT;!okAZAs|@YC{43*w=lGSH-F}rEbvjNz?dJVZrGIC)ZCdui{bTOp z-njMMsomX2E4My!w+vVMR2pn6fBo{O$iU8+Ro%=G_d>~<>(=65;_fqOFfafB?@3;q literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-deep/package.json b/test/cli/install/registry/packages/optional-peer-hoist-deep/package.json index d23e64e1873b..9fc336d3eada 100644 --- a/test/cli/install/registry/packages/optional-peer-hoist-deep/package.json +++ b/test/cli/install/registry/packages/optional-peer-hoist-deep/package.json @@ -2,7 +2,7 @@ "_id": "optional-peer-hoist-deep", "name": "optional-peer-hoist-deep", "dist-tags": { - "latest": "1.0.0" + "latest": "2.0.0" }, "versions": { "1.0.0": { @@ -13,10 +13,23 @@ }, "_id": "optional-peer-hoist-deep@1.0.0", "dist": { - "integrity": "sha512-ytgvRJWhHiAPJbQhFw3iuAZgyxEr/jPJK8AeKzXZxQGcbHHZApeWtULf8t1KSF1fUKYC9pl19rlyC9axWiAVDw==", - "shasum": "88938b872ea61cd2fa96da6ed9b9f43db3b01745", + "integrity": "sha512-Nl/l5CFE2ostDfGRN+FKlbWP0HgYdjZ+rUZHT5kKwmVGMyKUPsParH9qCHGVaWMcxwQfqzPqUJyAMaHXW+ci1w==", + "shasum": "2fc39ef50643af6eac1abaf7ad78f1ed48806e06", "tarball": "http://localhost:4873/optional-peer-hoist-deep/-/optional-peer-hoist-deep-1.0.0.tgz" } + }, + "2.0.0": { + "name": "optional-peer-hoist-deep", + "version": "2.0.0", + "dependencies": { + "optional-peer-hoist-deep-child": "2.0.0" + }, + "_id": "optional-peer-hoist-deep@2.0.0", + "dist": { + "integrity": "sha512-UJk6zpTjUOU812aFbNk2J3UyY072P6rDAVfbDKSNheRzStX2U+fdA05CQyFlGd3SHk5HWJojFvyF1ssLjW9QhA==", + "shasum": "282a38f52b9ef982c7c5bed3be0312b96c8bd83c", + "tarball": "http://localhost:4873/optional-peer-hoist-deep/-/optional-peer-hoist-deep-2.0.0.tgz" + } } } } \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-1.0.0.tgz index 8cc3654725b0f2228769789f8067f2ceacb83ec7..1eee72aa3c603d794bbf98a4faec8e46d4b0f70f 100644 GIT binary patch literal 171 zcmb2|=3oGW|8LK43=^ZOKzt|hP*zo86r}F;i z9&A_N&wu*i)kiau^6h*=U#}8c?EiUHR&aOp`cu2Ett#nVp*=UdXT{GZ?^%B`zPw%g zbEkdbX_uV$=MUx`IhL4}{C;;0N8%a&jlRM6Sod%4_Bj%Fc*d<8w|gVPGZvcdmWDV7 SN*?I9VOaAka5IAj0|NjHF;Hv( literal 171 zcmb2|=3oGW|8LLRavgFIaecUQbJy`JYWq&kpKzDQD9a?xNhtToUhZ2|XgxfkQLolB~~WivKycPDrqt_wkiu_so)Fm(9HOBoE>o R7-|2BA-eVH6$T9k1^{MAPC)Em$}Srr!>Sl SQ1U>(4a2p(!UYT(3=9CSn^2tq literal 171 zcmb2|=3oGW|8LK4{3!~&^uOcez7-nvEk4CPv!mn z$Hvd`ZRbSP-x8YZB{XSJ0xV>nd$)ac6dXGie7U$=m zJ^!~eS}kdJLEkp9*}qnL#%6rBJEmyAByG+ov$nahDN5g@+BbU6U3>JG6Zh7ecOlM! Rk#$cP6$OtTXV73^006mMO^*No diff --git a/test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-3.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-leaf/optional-peer-hoist-leaf-3.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..b64f89391d63410bd3a911d272454a3b69841d22 GIT binary patch literal 206 zcmb2|=3oGW|8LJbdLMQWV0~cwetpH6n@4V{xSiM^d26HF>vxl4_fGj&zHqOozgE+S zeW#lnXFi;|qq>)UxBm4v@mYTFrmuWu<6BTwID1ty9|!+oi}O~!4+W);E!f<=%l<=L z^qo2Xeq7JpS^ww>qvyx7DUDAqU7Pc0qjp;K_c_;Z?OJ?Pedp(fnj2x)=cF|D?EO|X y_ublvThX`HhNT`n$^7G2Q1|VFw-mTlV!qiHd`owC!2tI^a2m#L$z#x9U;qFNcWQJ1 literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json b/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json index 1b39b6e45b31..d1cb1eccd9f3 100644 --- a/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json +++ b/test/cli/install/registry/packages/optional-peer-hoist-leaf/package.json @@ -2,7 +2,7 @@ "_id": "optional-peer-hoist-leaf", "name": "optional-peer-hoist-leaf", "dist-tags": { - "latest": "2.0.0" + "latest": "3.0.0" }, "versions": { "1.0.0": { @@ -10,8 +10,8 @@ "version": "1.0.0", "_id": "optional-peer-hoist-leaf@1.0.0", "dist": { - "integrity": "sha512-fqljP59V2vz45cC0Zan3B9MFl9YqzOkr3yIwFVF8RnI9A/dtx3f8MeLMw75pd18KBY/1aIYCIGN5cvaxwjMiMw==", - "shasum": "d34a5ad0681194b5ea936a7db014a9fad1ea8e3d", + "integrity": "sha512-57x3rWhz3tBcdNnWCudX5V6XvzIHn7taFxSdYzNgAUCAkYIrc+945lIT5CziLvmeXDh4EbRxamaYFHbIDA1BeA==", + "shasum": "37a38558bbefe6dbfdc785c4c61841b0e704caac", "tarball": "http://localhost:4873/optional-peer-hoist-leaf/-/optional-peer-hoist-leaf-1.0.0.tgz" } }, @@ -20,10 +20,23 @@ "version": "2.0.0", "_id": "optional-peer-hoist-leaf@2.0.0", "dist": { - "integrity": "sha512-yfqEs/442yivAG4zTY4u1DFAf8W/ZjIZTI1elABJFvqfaVQiDxmwo+yoDrIXNiJVz7vVrNKIDjhOeU0vToAZNg==", - "shasum": "d7f6cf082fcfb677ee9217b58d25c53b2edee85c", + "integrity": "sha512-tfK30752Xwe6oDLE4Iv0cnDxJILQVmb+HiGrOHDHqnn/Oi3g+RIGH3NuSqX2WmluXbH0C/bkJOrjqgTlGk6wNg==", + "shasum": "510479d612daa8950f98e5a63238e6916a2f7ee9", "tarball": "http://localhost:4873/optional-peer-hoist-leaf/-/optional-peer-hoist-leaf-2.0.0.tgz" } + }, + "3.0.0": { + "name": "optional-peer-hoist-leaf", + "version": "3.0.0", + "dependencies": { + "optional-peer-hoist-target2": "1.0.0" + }, + "_id": "optional-peer-hoist-leaf@3.0.0", + "dist": { + "integrity": "sha512-xSPMrxzzfuv9sFRWtGGfB9+DOZj8UOB8i7BUt+0xul0M+GgZV6QsM/PtWtybEMOfr45ASiN3HRuVUHF2Tix4Gw==", + "shasum": "dc5163d16ef66c63acbc8c1eb151a43c7abe076d", + "tarball": "http://localhost:4873/optional-peer-hoist-leaf/-/optional-peer-hoist-leaf-3.0.0.tgz" + } } } } \ No newline at end of file diff --git a/test/cli/install/registry/packages/optional-peer-hoist-provider/optional-peer-hoist-provider-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-provider/optional-peer-hoist-provider-1.0.0.tgz index 8248bb07a20881d387d4d688e35d507fec783c2a..cc228d7ce52c74b8aac90dc2d3a9ca79d3540607 100644 GIT binary patch literal 208 zcmb2|=3oGW|8LLxb{#h0VNF2N@}_(8w{P4tQ7n)5*tNmegM;PX zbsGVXM^iWa=BlpMpDtq`?f2}UOUk{xlP7MLc2&$g;ag|yfA9Rc$vb){-<&W*{YS2C zh;sS=U3%Y@`lfXR{|JohD1K?U{DZB?RjZmOH+}_V*&PYh-XE!Cmyv6z9@lYKV{y!~ x)ndM@m&IN=#x1KW__uy-`>yNfxHsCbk6tv{RubY|WOBz<`M7=7)eIU83;^g&R#X50 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json b/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json index 5ccab221ad44..fe42ae163bb9 100644 --- a/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json +++ b/test/cli/install/registry/packages/optional-peer-hoist-provider/package.json @@ -13,8 +13,8 @@ }, "_id": "optional-peer-hoist-provider@1.0.0", "dist": { - "integrity": "sha512-tAHlT0reFQqTCgu9fAMlAFluINA6RDKlxD1sBHDBCLQ+ra8GX1e45fDGHFhKY8z+ocrrwC2NT6OYx5O3ydr4RQ==", - "shasum": "5d6cc472a7fd1feac9d883fd688113640ad423af", + "integrity": "sha512-h9jQ95AgpiWboNU8u/+OtU9GFMySNeOg3sRQUipTUWJzOom4NFJjYnpSYC7CGG1zVVTfuFaMNtJSxocFfGsG0g==", + "shasum": "f195ae5906efce71a33a94ee982f50ea263aad69", "tarball": "http://localhost:4873/optional-peer-hoist-provider/-/optional-peer-hoist-provider-1.0.0.tgz" } } diff --git a/test/cli/install/registry/packages/optional-peer-hoist-tail/optional-peer-hoist-tail-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-tail/optional-peer-hoist-tail-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..1a0927fd37bec97e3f50e2d85f8105ce7ccc520c GIT binary patch literal 170 zcmb2|=3oGW|8LJ9N2tIcc+xD4SgvU7;bf4BJeZobjzP7U)+v9 zU0nZYTIY>}^P6*z9y2U0{7`GxX7IHA##v4OeXrkY32(e^a5h#ZNBf#+)QYq3*dWe< Rk{^`$7xZ7xW6)q=001$ZO~3#E literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-tail/optional-peer-hoist-tail-2.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-tail/optional-peer-hoist-tail-2.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..b0a48ffc6ec39bc6dcc2f08e2bb2ef46e9443628 GIT binary patch literal 171 zcmb2|=3oGW|8LJ9N2tIcc+xD4SgvU7;bf4BJeZobjzP7U)+v9 zU0nZYTIY>}^P6*z9y2U0{7`GxX7IHA##v4OeXrkY32(e^a5h#ZN4qRIW1-n@X^3;6 R%Zo|i_9Kj;{WyE!Q#o_9oR`H@X=iZbc#+!ebZ>TUM7aXfQAU05MWmZU6uP literal 205 zcmb2|=3oGW|8LJbdNUgeus)dcSzE5S?7HWzcZvUfGmh_yn!KSi&g=i~3-`LzMI9^d zKRtNh%!jO7`5$S}c@Mw5vI||z))ITiXSc+1frW{}CRQtcGrwQE z{OZg8`n%JAtDlpbFz236n#amoP0P7$S=;!|r)Ji^$h3``&dz_LeQmC0WSWOW>6P1G v60Uwa_^EEAn(;Zg1J}3vO`Dg}cI5RYcD;X(IxxTsYdc1k1*~NZ8Vn2oYEWj! diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-2.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-2.0.0.tgz index 8713d31235967f4d70c15efb76d212b927286bef..e0e9d1b3f362e08ed32d35561995f0d86038e088 100644 GIT binary patch literal 172 zcmb2|=3oGW|8LKq&k4yUt89!3Kre2xq8{fT}9D>;g)NAR{U%_J?Br#m$z%5 z?zAsF?egZq`Geb%9?$q?@gvx-&EVs~o+TJ3Wr?y$-C!VPP0Y`Wl-TV@V% T50q?}U%|k*kL?131_J{CyJk`F literal 171 zcmb2|=3oGW|8LLlvm0*d_7)u8VnC@3AMf)5`yR z>P;-?iH+%BAJ%Tk`|NPY)|+`pUtHd_$}cqLt-0^p+Q)lV+_nf+&f53D=(&MpZFP3# zJO4Xeg4Qj?Th`t%d1lA^Fr?4W@2TC_V!h%SyJoyyC9v_j#o1b^9PMl2Q7d)du|eDe RB|ixBe>=EoJA(!T0|4$COd9|I diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-3.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-target/optional-peer-hoist-target-3.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..aafe670eea31344dd74c14954fef448d5d8778b2 GIT binary patch literal 278 zcmb2|=3oGW|8K7x^g8Sy!1iFycg;52+bL$V-A>%xE--PcWVXp&-kV3fU+(74)$hF2 z<+b!f{q)AxxryTcUfy|icCG4bKGWLXgX;A?GfyvC(DMD+)SG(VMyKygQx(5IL&IkNw{A zvmcbY-dZ;2)59&({AzUTu6kS%%@q}_y}MfUyUkvo&k4yUt89!3Rcex4)a}H`gGf(bwyFqfuC8YTmC%x;&$}u z;`&F^I&U1D-<*5&nBmvL56f$K5>I{KI6rvapDV2)w?fsD)+f&TZE+)PjaZb$?7O@W S2SLdP;&N{5vnm)g7#IKoWKS6Y literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target2/optional-peer-hoist-target2-1.0.0.tgz b/test/cli/install/registry/packages/optional-peer-hoist-target2/optional-peer-hoist-target2-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..783235da95f4fdc20d763ed25a9889c0937ddb28 GIT binary patch literal 204 zcmb2|=3oGW|8LJ9%wjSWV0$p^GAeV%t7q!_6@V=dd- zdi2EM{p(Ke?QB+kB=T=+`lFbrGnzTO7wsysx4d?3>xP^ymH#%l?76=5*qjQrvb|r5 wX0F?s5tSX~y4L3ML7|IllM7E9Z?xBsUbNU&3gT8|a>rrLm#lJ288jFe07t=H+W-In literal 0 HcmV?d00001 diff --git a/test/cli/install/registry/packages/optional-peer-hoist-target2/package.json b/test/cli/install/registry/packages/optional-peer-hoist-target2/package.json new file mode 100644 index 000000000000..ce98e15ba5ae --- /dev/null +++ b/test/cli/install/registry/packages/optional-peer-hoist-target2/package.json @@ -0,0 +1,32 @@ +{ + "_id": "optional-peer-hoist-target2", + "name": "optional-peer-hoist-target2", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "0.0.1": { + "name": "optional-peer-hoist-target2", + "version": "0.0.1", + "_id": "optional-peer-hoist-target2@0.0.1", + "dist": { + "integrity": "sha512-xY6X5B7Kf871dRTuI3LmN7kjI+MylkvmStfISbJUodW45kVfJBZqbvA0rAIy2ZRs6hfjwRkc5pDMZSxrRwp8rg==", + "shasum": "bd2acff2d6b98ce81b856a2c1eb8b5b220c2e3e9", + "tarball": "http://localhost:4873/optional-peer-hoist-target2/-/optional-peer-hoist-target2-0.0.1.tgz" + } + }, + "1.0.0": { + "name": "optional-peer-hoist-target2", + "version": "1.0.0", + "dependencies": { + "optional-peer-hoist-tail": "2.0.0" + }, + "_id": "optional-peer-hoist-target2@1.0.0", + "dist": { + "integrity": "sha512-k2m7NpKAOXV0lYfAicW/eCTRFYlAJcoSkGp0z/qL7GlaQEtRqtLitQJSsb5p5cXTYezuuLe61PsbGggVWgS65A==", + "shasum": "3512fb5ad917d139ce5af219d14673ddc4d4b9e9", + "tarball": "http://localhost:4873/optional-peer-hoist-target2/-/optional-peer-hoist-target2-1.0.0.tgz" + } + } + } +} \ No newline at end of file From 28d913b585fcb00252b3483a77dd3b4927d2da0f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:09:24 +0000 Subject: [PATCH 3/4] install: trim comments around the optional peer binding changes --- src/install/lockfile.rs | 41 +++++++++++---------------------- src/install/lockfile/Package.rs | 3 +-- src/install/lockfile/Tree.rs | 24 +++++++------------ 3 files changed, 22 insertions(+), 46 deletions(-) diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 823554b0278f..df152e7607f7 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -1293,8 +1293,7 @@ impl Lockfile { pub struct Cloner<'a> { pub(crate) clone_queue: PendingResolutions, - /// Optional-peer slots, bound in `flush` once every package reachable - /// through a non-peer edge has been cloned. + /// Bound in `flush`, once `clone_queue` has decided which targets survive. pub(crate) optional_peers: PendingResolutions, pub lockfile: &'a mut Lockfile, pub(crate) old: &'a mut Lockfile, @@ -1322,12 +1321,9 @@ impl<'a> Cloner<'a> { self.lockfile.buffers.resolutions[to_clone.resolve_id as usize] = new_id; } - // An optional peer stays bound to its target if the target survived the - // clean. Loading a lockfile binds these slots before hoisting, so the - // `resolve` below has to see them bound as well, or it builds a - // different tree than the one `--frozen-lockfile` compares against and - // a re-save moves packages around. A target only reachable through - // peer slots was never cloned and the slots stay unresolved. + // Loading a lockfile binds optional peers before hoisting, so the hoist + // below has to see them bound too or `--frozen-lockfile` compares two + // different trees. A target nothing else cloned leaves its slots unbound. for pending in self.optional_peers.drain(..) { let mapping = self.mapping[pending.old_resolution as usize]; if (mapping as usize) < max_package_id { @@ -1362,21 +1358,12 @@ impl<'a> Cloner<'a> { // ──────────────────────────────────────────────────────────────────────────── impl Lockfile { - /// Builds the tree that is saved to disk. - /// - /// The resolver leaves optional peers unresolved; hoisting binds each one - /// to the same-named package that ends up next to it. When that package is - /// placed by an edge processed after the peer's dependent, its subtree is - /// queued from that later edge. A lockfile loaded from disk has the binding - /// up front and queues the subtree from the dependent instead, which can - /// hoist the subtree's dependencies differently. So hoist again until a - /// pass binds nothing late: that pass built its tree from bindings it had - /// up front, which is what a reload does, and anything else would make - /// `--frozen-lockfile` reject the lockfile we are about to save. A pass - /// reports `true` only after filling an empty optional peer slot (moving a - /// dependent can put a target within reach of a peer the previous pass - /// could not bind) and no pass empties one, so this ends after at most - /// one pass per optional peer. + /// Builds the tree that is saved to disk: hoists until a pass binds no + /// optional peer late. A peer bound mid-pass had its target's subtree + /// queued from a later edge than a reload (which has the binding up + /// front) queues it from, so that pass can hoist differently than the + /// reload `--frozen-lockfile` compares against. Repeating only ever fills + /// more slots, so this ends. pub(crate) fn resolve(&mut self, log: &mut bun_ast::Log) -> Result<(), tree::SubtreeError> { while self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)? {} Ok(()) @@ -1390,8 +1377,7 @@ impl Lockfile { workspace_filters: &[WorkspaceFilter], packages_to_install: Option<&[PackageID]>, ) -> Result<(), tree::SubtreeError> { - // Runs after `resolve` bound the optional peers, so there is nothing - // left to bind late here. + // `resolve` already bound every optional peer; nothing binds late here. self.hoist::<{ tree::BuilderMethod::Filter }>( log, Some(manager), @@ -1402,9 +1388,8 @@ impl Lockfile { Ok(()) } - /// Sets `buffers.trees` and `buffers.hoisted_dependencies`. Returns whether - /// an optional peer was bound after its dependent had been placed - /// (`tree::Builder::late_bound_optional_peer`). + /// Sets `buffers.trees` and `buffers.hoisted_dependencies`. Returns + /// `tree::Builder::late_bound_optional_peer`. pub(crate) fn hoist( &mut self, log: &mut bun_ast::Log, diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index 7f34d4b81764..a183ccf2ac6e 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -616,8 +616,7 @@ impl Package { resolve_id: new_package.resolutions.off + PackageID::try_from(i).expect("int cast"), }; - // An optional peer does not keep its target alive. `Cloner::flush` - // binds the slot again only if a non-peer edge cloned the target. + // Peer slots must not keep their target alive; bound in `Cloner::flush`. if old_dependencies[i].behavior.is_optional_peer() { cloner.optional_peers.push(pending); continue; diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index ef1b80534279..9e85f098dac0 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -135,10 +135,8 @@ enum HoistDependencyResult { Resolve(PackageID), ResolveReplace(ResolveReplace), ResolveLater, - /// Like `Hoisted`, for an optional peer that was bound to one package but - /// deduplicated onto another version of it: the slot is repointed at the - /// package the dependent will find next to itself, which is also what - /// loading the saved tree binds the edge to (`bun.lock.rs`). + /// `Hoisted`, and the optional peer's slot is repointed at the version it + /// deduplicated onto, which is what loading the saved tree binds it to. Rebind(PackageID), Placement(Placement), } @@ -447,10 +445,8 @@ pub struct Builder<'a, const METHOD: BuilderMethod> { // could be visited multiple times before it's resolved. pub(crate) pending_optional_peers: ArrayHashMap>, - /// Set when an unresolved optional peer got bound by a package placed - /// after the peer's dependent (`HoistDependencyResult::ResolveReplace`), - /// so the target's subtree was queued later than it will be once the - /// binding is known up front. See `Lockfile::resolve`. + /// A `ResolveReplace` bound an optional peer after its dependent was + /// placed. See `Lockfile::resolve`. pub(crate) late_bound_optional_peer: bool, pub(crate) manager: Option<&'a PackageManager>, pub(crate) sort_buf: Vec, @@ -1061,14 +1057,10 @@ impl Tree { // or hoist if peer version allows it if dependency.behavior.is_peer() { - // An optional peer is bound to whatever ends up next to it, so - // a binding that came in pointing elsewhere (carried over by - // `Package::clone`, or made by an earlier pass of - // `Lockfile::resolve`) moves to `res_id`. Only the tree being - // saved decides this; `filter` runs afterwards for installing - // and leaves the bindings alone. Required peers keep the - // version the resolver picked; `bun.lock.rs` re-derives that - // one by version rather than from the tree. + // An optional peer follows the version it dedupes onto (its + // incoming binding may be carried over from an older tree), and + // only the saved tree decides that. Required peers keep the + // resolver's pick, which `bun.lock.rs` re-derives by version. let dedupe = || { if METHOD == BuilderMethod::Resolvable && dependency.behavior.is_optional_peer() { From 5755a5d2215128a8473e1c6c1d80041c64511503 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:15:28 +0000 Subject: [PATCH 4/4] install: one-line comments for the optional peer binding changes --- src/install/lockfile.rs | 15 +++------------ src/install/lockfile/Tree.rs | 11 +++-------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index df152e7607f7..f4c78a03cf18 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -1321,9 +1321,7 @@ impl<'a> Cloner<'a> { self.lockfile.buffers.resolutions[to_clone.resolve_id as usize] = new_id; } - // Loading a lockfile binds optional peers before hoisting, so the hoist - // below has to see them bound too or `--frozen-lockfile` compares two - // different trees. A target nothing else cloned leaves its slots unbound. + // bun.lock.rs binds these before hoisting; the hoist below has to see the same bindings. for pending in self.optional_peers.drain(..) { let mapping = self.mapping[pending.old_resolution as usize]; if (mapping as usize) < max_package_id { @@ -1358,12 +1356,7 @@ impl<'a> Cloner<'a> { // ──────────────────────────────────────────────────────────────────────────── impl Lockfile { - /// Builds the tree that is saved to disk: hoists until a pass binds no - /// optional peer late. A peer bound mid-pass had its target's subtree - /// queued from a later edge than a reload (which has the binding up - /// front) queues it from, so that pass can hoist differently than the - /// reload `--frozen-lockfile` compares against. Repeating only ever fills - /// more slots, so this ends. + /// Re-hoists while a pass bound an optional peer late; a reload has that binding up front. pub(crate) fn resolve(&mut self, log: &mut bun_ast::Log) -> Result<(), tree::SubtreeError> { while self.hoist::<{ tree::BuilderMethod::Resolvable }>(log, None, true, &[], None)? {} Ok(()) @@ -1377,7 +1370,6 @@ impl Lockfile { workspace_filters: &[WorkspaceFilter], packages_to_install: Option<&[PackageID]>, ) -> Result<(), tree::SubtreeError> { - // `resolve` already bound every optional peer; nothing binds late here. self.hoist::<{ tree::BuilderMethod::Filter }>( log, Some(manager), @@ -1388,8 +1380,7 @@ impl Lockfile { Ok(()) } - /// Sets `buffers.trees` and `buffers.hoisted_dependencies`. Returns - /// `tree::Builder::late_bound_optional_peer`. + /// Sets `buffers.trees`/`hoisted_dependencies`; returns `Builder::late_bound_optional_peer`. pub(crate) fn hoist( &mut self, log: &mut bun_ast::Log, diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index 9e85f098dac0..66d7c3c067ce 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -135,8 +135,7 @@ enum HoistDependencyResult { Resolve(PackageID), ResolveReplace(ResolveReplace), ResolveLater, - /// `Hoisted`, and the optional peer's slot is repointed at the version it - /// deduplicated onto, which is what loading the saved tree binds it to. + /// `Hoisted`, plus the optional peer's slot now points at the version it deduplicated onto. Rebind(PackageID), Placement(Placement), } @@ -445,8 +444,7 @@ pub struct Builder<'a, const METHOD: BuilderMethod> { // could be visited multiple times before it's resolved. pub(crate) pending_optional_peers: ArrayHashMap>, - /// A `ResolveReplace` bound an optional peer after its dependent was - /// placed. See `Lockfile::resolve`. + /// An optional peer got bound after its dependent was placed; see `Lockfile::resolve`. pub(crate) late_bound_optional_peer: bool, pub(crate) manager: Option<&'a PackageManager>, pub(crate) sort_buf: Vec, @@ -1057,10 +1055,7 @@ impl Tree { // or hoist if peer version allows it if dependency.behavior.is_peer() { - // An optional peer follows the version it dedupes onto (its - // incoming binding may be carried over from an older tree), and - // only the saved tree decides that. Required peers keep the - // resolver's pick, which `bun.lock.rs` re-derives by version. + // An optional peer's binding follows the dedupe, but only in the tree being saved. let dedupe = || { if METHOD == BuilderMethod::Resolvable && dependency.behavior.is_optional_peer() {