Skip to content
50 changes: 50 additions & 0 deletions src/install/lockfile/Package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,26 @@ pub(crate) fn value_loc_of(source: &bun_ast::Source, key_loc: bun_ast::Loc) -> b
crate::bun_json::property_value_loc(&source.contents, key_loc).unwrap_or(key_loc)
}

/// True when `folder`, a folder-dependency path relative to the workspace
/// root (either platform's separators), names the directory of the member
/// registered at `workspace_path` (posix).
Comment thread
robobun marked this conversation as resolved.
Resolved
fn folder_path_is_workspace_path(folder: &[u8], workspace_path: &[u8]) -> bool {
let mut buf = path::path_buffer_pool::get();
let normalized = resolve_path::join_string_buf::<path::platform::Auto>(&mut buf[..], &[folder]);
if normalized.len() != workspace_path.len() {
return false;
}
if cfg!(windows) {
for (&a, &b) in normalized.iter().zip(workspace_path) {
if a != b && !(a == b'\\' && b == b'/') {
return false;
}
}
return true;
}
strings::eql(normalized, workspace_path)
}

#[cold]
fn invalid_trusted_dependencies(
log: &mut bun_ast::Log,
Expand Down Expand Up @@ -1748,6 +1768,36 @@ impl Package<u64> {
// if relative is empty, we are linking the package to itself
dependency_version.value.folder = string_builder
.append::<String>(if relative.is_empty() { b"." } else { relative });

// The root package's dependencies share one node_modules
// entry per name with the workspace members, so a root folder
// dependency colliding with a member resolves to the member,
// as in npm. Both placed side by side would serialize the
// same bun.lock key twice.
Comment thread
robobun marked this conversation as resolved.
Resolved
if let Some(workspace_path) = workspace_path {
let collides = package_dependencies[0..dependencies_count as usize]
.iter()
.any(|dep| dep.name_hash == name_hash && dep.behavior.is_workspace());
if collides
&& !folder_path_is_workspace_path(relative, workspace_path.slice(buf))
{
let path = workspace_path.sliced(buf);
if let Some(mut dep) = dependency::parse_with_tag(
external_alias.value,
Some(external_alias.hash),
path.slice,
dependency::version::Tag::Workspace,
&path,
Some(&mut *log),
Some(&mut *pm),
) {
// Whole-struct move so `Drop` frees the old
// chain; keep the existing `literal`.
Comment thread
robobun marked this conversation as resolved.
Resolved
dep.literal = dependency_version.literal;
dependency_version = dep;
}
}
}
}
dependency::version::Tag::Npm => {
if let Some(workspace_version) = workspace_version {
Expand Down
199 changes: 199 additions & 0 deletions test/cli/install/bun-workspaces.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,205 @@ test("dependency on same name as workspace and dist-tag", async () => {
]);
});

describe("workspace and file: dependency sharing a name", () => {
// A root `file:` dependency and a same-named workspace member share one
// root node_modules entry. The member wins and the dependency resolves to
// it, as in npm. Both used to be placed side by side, writing duplicate
// bun.lock keys that broke --frozen-lockfile.

test.concurrent("resolves to the member and the lockfile round-trips", async () => {
using ctx = await setupTest();
const { packageDir, env } = ctx;
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "sandbox",
version: "1.0.0",
workspaces: ["packages/*"],
dependencies: { alpha: "file:./vendor/alpha" },
}),
),
write(join(packageDir, "packages", "alpha", "package.json"), JSON.stringify({ name: "alpha" })),
write(join(packageDir, "vendor", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "2.0.0" })),
]);

await runBunInstall(env, packageDir);

const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1);
expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`);
expect(lockfile).not.toContain("alpha@file:");
expect(await file(join(packageDir, "node_modules", "alpha", "package.json")).json()).toEqual({
name: "alpha",
});

const second = await runBunInstall(env, packageDir, { savesLockfile: false });
expect(second.err).not.toContain("Saved lockfile");
expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);

await runBunInstall(env, packageDir, { frozenLockfile: true });
});

test.concurrent("member with its own dependencies still resolves them", async () => {
using ctx = await setupTest();
const { packageDir, env } = ctx;
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "sandbox",
version: "1.0.0",
workspaces: ["packages/*"],
dependencies: { alpha: "file:./vendor/alpha" },
}),
),
write(
join(packageDir, "packages", "alpha", "package.json"),
JSON.stringify({
name: "alpha",
version: "1.0.0",
dependencies: { "alpha-dep": "file:../../vendor/alpha-dep" },
}),
),
write(
join(packageDir, "packages", "beta", "package.json"),
JSON.stringify({ name: "beta", version: "1.0.0", dependencies: { alpha: "workspace:*" } }),
),
write(join(packageDir, "vendor", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "2.0.0" })),
write(
join(packageDir, "vendor", "alpha-dep", "package.json"),
JSON.stringify({ name: "alpha-dep", version: "3.0.0" }),
),
]);

await runBunInstall(env, packageDir);

const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1);
expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`);
expect(lockfile).not.toContain("alpha@file:");
expect(await file(join(packageDir, "node_modules", "alpha", "package.json")).json()).toEqual({
name: "alpha",
version: "1.0.0",
dependencies: { "alpha-dep": "file:../../vendor/alpha-dep" },
});
expect(
await file(join(packageDir, "node_modules", "alpha", "node_modules", "alpha-dep", "package.json")).json(),
).toEqual({
name: "alpha-dep",
version: "3.0.0",
});

const second = await runBunInstall(env, packageDir, { savesLockfile: false });
expect(second.err).not.toContain("Saved lockfile");
expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);

await runBunInstall(env, packageDir, { frozenLockfile: true });
});

test.concurrent("file: dependency folder depending back on the member", async () => {
using ctx = await setupTest();
const { packageDir, env } = ctx;
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "sandbox",
version: "1.0.0",
workspaces: ["packages/*"],
dependencies: { alpha: "file:./vendor/alpha" },
}),
),
write(join(packageDir, "packages", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "1.0.0" })),
write(
join(packageDir, "vendor", "alpha", "package.json"),
JSON.stringify({ name: "alpha", version: "2.0.0", dependencies: { alpha: "workspace:*" } }),
),
]);

await runBunInstall(env, packageDir);

const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1);
expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`);
expect(lockfile).not.toContain("alpha@file:");
expect(lockfile).not.toContain("alpha/alpha");

const second = await runBunInstall(env, packageDir, { savesLockfile: false });
expect(second.err).not.toContain("Saved lockfile");
expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);

await runBunInstall(env, packageDir, { frozenLockfile: true });
});

test.concurrent.each(["relative", "absolute"])(
"%s file: dependency naming the member's directory still links the member",
async kind => {
using ctx = await setupTest();
const { packageDir, env } = ctx;
const literal = kind === "absolute" ? `file:${join(packageDir, "packages", "alpha")}` : "file:./packages/alpha";
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "sandbox",
version: "1.0.0",
workspaces: ["packages/*"],
dependencies: { alpha: literal },
}),
),
write(
join(packageDir, "packages", "alpha", "package.json"),
JSON.stringify({ name: "alpha", version: "1.0.0" }),
),
]);

await runBunInstall(env, packageDir);

const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile.match(/"alpha": \[/g)).toHaveLength(1);
expect(lockfile).toContain(`"alpha": ["alpha@workspace:packages/alpha"]`);
expect(lockfile).not.toContain("alpha@file:");
expect(await file(join(packageDir, "node_modules", "alpha", "package.json")).json()).toEqual({
name: "alpha",
version: "1.0.0",
});
},
);

test.concurrent("aliased file: dependency colliding with another member's name", async () => {
using ctx = await setupTest();
const { packageDir, env } = ctx;
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "sandbox",
version: "1.0.0",
workspaces: ["packages/*"],
dependencies: { beta: "file:./vendor/alpha" },
}),
),
write(join(packageDir, "packages", "beta", "package.json"), JSON.stringify({ name: "beta", version: "1.0.0" })),
write(join(packageDir, "vendor", "alpha", "package.json"), JSON.stringify({ name: "alpha", version: "2.0.0" })),
]);

await runBunInstall(env, packageDir);

const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile.match(/"beta": \[/g)).toHaveLength(1);
expect(lockfile).toContain(`"beta": ["beta@workspace:packages/beta"]`);
expect(lockfile).not.toContain("@file:");
expect(await file(join(packageDir, "node_modules", "beta", "package.json")).json()).toEqual({
name: "beta",
version: "1.0.0",
});

await runBunInstall(env, packageDir, { frozenLockfile: true });
});
});

test.concurrent("successfully installs workspace when path already exists in node_modules", async () => {
using ctx = await setupTest();
const { packageDir, env } = ctx;
Expand Down
Loading