Skip to content
14 changes: 14 additions & 0 deletions src/install/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,20 @@ impl<'a> DepSorter<'a> {
Ordering::Less => true,
Ordering::Greater => false,
Ordering::Equal => {
// Match npm: order workspaces by relative path so the path-first
// workspace's deps win the root node_modules slot.
Comment thread
robobun marked this conversation as resolved.
if l_dep.behavior.is_workspace() {
if let (Some(l_path), Some(r_path)) = (
self.lockfile.workspace_paths.get(&l_dep.name_hash),
self.lockfile.workspace_paths.get(&r_dep.name_hash),
) {
match strings::order(l_path.slice(string_buf), r_path.slice(string_buf)) {
Ordering::Less => return true,
Ordering::Greater => return false,
Ordering::Equal => {}
}
}
}
strings::order(l_dep.name.slice(string_buf), r_dep.name.slice(string_buf))
== Ordering::Less
}
Expand Down
21 changes: 14 additions & 7 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2801,6 +2801,9 @@ pub(crate) fn parse_into_binary_lockfile(
let dep_id: DependencyID = _dep_id;
let dep = &mut dependencies[dep_id as usize];

if dep.behavior.is_optional_peer() {
continue;
}
let peer_res_id = if is_deferred_peer(dep) {
resolve_peer_dep_version_based(
dep,
Expand Down Expand Up @@ -2865,6 +2868,10 @@ pub(crate) fn parse_into_binary_lockfile(
let dep = &mut dependencies[dep_id as usize];
let dep_name = dep.name.slice(string_buf);

if dep.behavior.is_optional_peer() {
continue;
}
Comment thread
robobun marked this conversation as resolved.

let workspace_node_modules = {
let buf_slice = &mut path_buf[..];
let needed = workspace_name.len() + 1 + dep_name.len();
Expand Down Expand Up @@ -2955,6 +2962,9 @@ pub(crate) fn parse_into_binary_lockfile(
let dep_id: DependencyID = _dep_id;
let dep = &mut dependencies[dep_id as usize];

if dep.behavior.is_optional_peer() {
continue 'deps;
}
Comment thread
robobun marked this conversation as resolved.
let peer_res_id = if is_deferred_peer(dep) {
resolve_peer_dep_version_based(
dep,
Expand Down Expand Up @@ -3017,15 +3027,12 @@ pub(crate) fn parse_into_binary_lockfile(
}

/// True for peer edges the fresh resolver defers to its second phase
/// (`install_peer`) and binds by version there. Two exemptions, matching
/// `enqueue_dependency_with_main_and_success_fn`: optional peers return
/// before the deferred phase and are bound to the hoisted-tree sibling by
/// `process_subtree` instead, and `*` peers express no version preference
/// and bind to whatever sibling pin existed first. Both of those are
/// exactly what the printed tree's path walk reproduces, so they keep it.
/// (`install_peer`) and binds by version there. Callers skip optional peers
/// entirely; `*` peers express no version preference so the printed tree's
/// path walk binds them.
Comment thread
robobun marked this conversation as resolved.
fn is_deferred_peer(dep: &Dependency) -> bool {
debug_assert!(!dep.behavior.is_optional_peer());
dep.behavior.is_peer()
&& !dep.behavior.is_optional_peer()
&& !(dep.version.tag == DependencyVersionTag::Npm && dep.version.npm().version.is_star())
}

Expand Down
140 changes: 140 additions & 0 deletions test/cli/install/bun-install-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3923,6 +3923,146 @@ describe("hoisting", async () => {
assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl());
});

// https://github.com/oven-sh/bun/issues/9838
test("conflicting workspace deps are hoisted in workspace-path order, not package-name order", async () => {
await Promise.all([
write(
packageJson,
JSON.stringify({
name: "root",
private: true,
workspaces: ["libs/*", "apps/*"],
}),
),
write(
join(packageDir, "apps", "app", "package.json"),
JSON.stringify({
name: "my-app",
version: "1.0.0",
dependencies: {
"no-deps": "2.0.0",
"strict-peer-dep": "1.0.0",
"optional-peer-deps": "1.0.0",
},
}),
),
write(
join(packageDir, "libs", "lib", "package.json"),
JSON.stringify({
name: "@libs/lib",
version: "1.0.0",
dependencies: {
"no-deps": "1.0.0",
},
}),
),
]);

async function checkLayout() {
expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toMatchObject({
name: "no-deps",
version: "2.0.0",
});
expect(
await file(join(packageDir, "libs", "lib", "node_modules", "no-deps", "package.json")).json(),
).toMatchObject({
name: "no-deps",
version: "1.0.0",
});
expect(await exists(join(packageDir, "apps", "app", "node_modules"))).toBeFalse();
expect(await exists(join(packageDir, "node_modules", "strict-peer-dep", "node_modules"))).toBeFalse();
expect(await exists(join(packageDir, "node_modules", "optional-peer-deps", "node_modules"))).toBeFalse();
}

async function install(cwd: string, ...args: string[]) {
const { stderr, exited } = spawn({
cmd: [bunExe(), "install", ...args],
cwd,
stdout: "ignore",
stderr: "pipe",
env,
});
const [err, exitCode] = await Promise.all([stderr.text(), exited]);
expect(err).not.toContain("error:");
expect(err).not.toContain("lockfile had changes");
expect(exitCode).toBe(0);
}

for (const cwd of [packageDir, join(packageDir, "apps", "app")]) {
await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await rm(join(packageDir, "apps", "app", "node_modules"), { recursive: true, force: true });
await rm(join(packageDir, "libs", "lib", "node_modules"), { recursive: true, force: true });
await rm(join(packageDir, "bun.lockb"), { force: true });
await rm(join(packageDir, "bun.lock"), { force: true });

await install(cwd);
await checkLayout();

await install(cwd, "--frozen-lockfile");

await rm(join(packageDir, "node_modules"), { recursive: true, force: true });
await rm(join(packageDir, "libs", "lib", "node_modules"), { recursive: true, force: true });

await install(cwd);
await checkLayout();
}
});

test("conflicting workspace deps are hoisted in workspace-path order (reversed)", async () => {
await Promise.all([
write(
packageJson,
JSON.stringify({
name: "root",
private: true,
workspaces: ["zed/*", "aah/*"],
}),
),
write(
join(packageDir, "aah", "a", "package.json"),
JSON.stringify({
name: "zzz-lib",
version: "1.0.0",
dependencies: {
"no-deps": "1.0.0",
},
}),
),
write(
join(packageDir, "zed", "z", "package.json"),
JSON.stringify({
name: "aaa-app",
version: "1.0.0",
dependencies: {
"no-deps": "2.0.0",
},
}),
),
]);

const { stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: packageDir,
stdout: "ignore",
stderr: "pipe",
env,
});

const [err, exitCode] = await Promise.all([stderr.text(), exited]);
expect(err).not.toContain("error:");
expect(exitCode).toBe(0);

expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toMatchObject({
name: "no-deps",
version: "1.0.0",
});
expect(await file(join(packageDir, "zed", "z", "node_modules", "no-deps", "package.json")).json()).toMatchObject({
name: "no-deps",
version: "2.0.0",
});
expect(await exists(join(packageDir, "aah", "a", "node_modules"))).toBeFalse();
});

test("hoisting/using incorrect peer dep on initial install", async () => {
await writeFile(
packageJson,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26725,7 +26725,7 @@ exports[`ssr works for 100-ish requests 1`] = `
"package_id": 315,
},
"jiti": {
"id": 434,
"id": 937,
"package_id": 316,
},
"js-tokens": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26725,7 +26725,7 @@ exports[`hot reloading works on the client (+ tailwind hmr) 1`] = `
"package_id": 315,
},
"jiti": {
"id": 434,
"id": 937,
"package_id": 316,
},
"js-tokens": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26725,7 +26725,7 @@ exports[`next build works: bun 1`] = `
"package_id": 315,
},
"jiti": {
"id": 434,
"id": 937,
"package_id": 316,
},
"js-tokens": {
Expand Down Expand Up @@ -54704,7 +54704,7 @@ exports[`next build works: node 1`] = `
"package_id": 315,
},
"jiti": {
"id": 434,
"id": 937,
"package_id": 316,
},
"js-tokens": {
Expand Down
Loading