sort and workspace only

This commit is contained in:
Dylan Conway
2025-07-10 00:57:04 -07:00
parent a1ef8f00ac
commit 37bd79682e
4 changed files with 353 additions and 12 deletions

View File

@@ -1413,15 +1413,15 @@ pub const Behavior = packed struct(u8) {
.gt;
}
if (lhs.isProd() != rhs.isProd()) {
return if (lhs.isProd())
if (lhs.isPeer() != rhs.isPeer()) {
return if (lhs.isPeer())
.gt
else
.lt;
}
if (lhs.isDev() != rhs.isDev()) {
return if (lhs.isDev())
if (lhs.isProd() != rhs.isProd()) {
return if (lhs.isProd())
.gt
else
.lt;
@@ -1434,8 +1434,8 @@ pub const Behavior = packed struct(u8) {
.lt;
}
if (lhs.isPeer() != rhs.isPeer()) {
return if (lhs.isPeer())
if (lhs.isDev() != rhs.isDev()) {
return if (lhs.isDev())
.gt
else
.lt;

View File

@@ -1112,7 +1112,7 @@ pub const Package = extern struct {
} else {
// It doesn't satisfy, but a workspace shares the same name. Override the workspace with the other dependency
for (package_dependencies[0..dependencies_count]) |*dep| {
if (dep.name_hash == name_hash and dep.version.tag == .workspace) {
if (dep.name_hash == name_hash and dep.behavior.isWorkspaceOnly()) {
dep.* = .{
.behavior = if (in_workspace) group.behavior.add(.workspace) else group.behavior,
.name = external_alias.value,
@@ -2141,6 +2141,8 @@ pub const Package = extern struct {
};
};
// @sortImports
const TrustedDependenciesSet = Lockfile.TrustedDependenciesSet;
const Aligner = install.Aligner;
const Allocator = std.mem.Allocator;

View File

@@ -1904,6 +1904,9 @@ pub fn parseIntoBinaryLockfile(
lockfile.buffers.resolutions.expandToCapacity();
@memset(lockfile.buffers.resolutions.items, invalid_package_id);
var seen_deps: bun.StringHashMap(void) = .init(allocator);
defer seen_deps.deinit();
const pkgs = lockfile.packages.slice();
const pkg_deps = pkgs.items(.dependencies);
const pkg_names = pkgs.items(.name);
@@ -1919,6 +1922,8 @@ pub fn parseIntoBinaryLockfile(
const dep_id: DependencyID = @intCast(_dep_id);
const dep = &lockfile.buffers.dependencies.items[dep_id];
const is_duplicate_dep = (try seen_deps.getOrPut(dep.name.slice(lockfile.buffers.string_bytes.items))).found_existing;
const res_id = pkg_map.get(dep.name.slice(lockfile.buffers.string_bytes.items)) orelse {
if (dep.behavior.optional) {
continue;
@@ -1927,7 +1932,7 @@ pub fn parseIntoBinaryLockfile(
return error.InvalidPackageInfo;
};
mapDepToPkg(dep, dep_id, res_id, lockfile, pkg_resolutions);
mapDepToPkg(dep, dep_id, res_id, lockfile, pkg_resolutions, is_duplicate_dep);
}
}
@@ -1939,12 +1944,16 @@ pub fn parseIntoBinaryLockfile(
const pkg_id: PackageID = @intCast(_pkg_id);
const workspace_name = pkg_names[pkg_id].slice(lockfile.buffers.string_bytes.items);
seen_deps.clearRetainingCapacity();
const deps = pkg_deps[pkg_id];
for (deps.begin()..deps.end()) |_dep_id| {
const dep_id: DependencyID = @intCast(_dep_id);
const dep = &lockfile.buffers.dependencies.items[dep_id];
const dep_name = dep.name.slice(lockfile.buffers.string_bytes.items);
const is_duplicate_dep = (try seen_deps.getOrPut(dep_name)).found_existing;
const workspace_node_modules = std.fmt.bufPrint(&path_buf, "{s}/{s}", .{ workspace_name, dep_name }) catch {
try log.addErrorFmt(source, root_pkg_exr.loc, allocator, "Workspace and dependency name too long: '{s}/{s}'", .{ workspace_name, dep_name });
return error.InvalidPackageInfo;
@@ -1958,7 +1967,7 @@ pub fn parseIntoBinaryLockfile(
return error.InvalidPackageInfo;
};
mapDepToPkg(dep, dep_id, res_id, lockfile, pkg_resolutions);
mapDepToPkg(dep, dep_id, res_id, lockfile, pkg_resolutions, is_duplicate_dep);
}
}
}
@@ -1973,12 +1982,16 @@ pub fn parseIntoBinaryLockfile(
return error.InvalidPackagesObject;
};
seen_deps.clearRetainingCapacity();
// find resolutions. iterate up to root through the pkg path.
const deps = pkg_deps[pkg_id];
deps: for (deps.begin()..deps.end()) |_dep_id| {
const dep_id: DependencyID = @intCast(_dep_id);
const dep = &lockfile.buffers.dependencies.items[dep_id];
const is_duplicate_dep = (try seen_deps.getOrPut(dep.name.slice(lockfile.buffers.string_bytes.items))).found_existing;
const res_id = pkg_map.findResolution(pkg_path, dep, lockfile.buffers.string_bytes.items, &path_buf) catch |err| switch (err) {
error.InvalidPackageKey => {
try log.addError(source, key.loc, "Invalid package path");
@@ -1993,7 +2006,7 @@ pub fn parseIntoBinaryLockfile(
},
};
mapDepToPkg(dep, dep_id, res_id, lockfile, pkg_resolutions);
mapDepToPkg(dep, dep_id, res_id, lockfile, pkg_resolutions, is_duplicate_dep);
}
}
@@ -2008,12 +2021,15 @@ pub fn parseIntoBinaryLockfile(
}
}
fn mapDepToPkg(dep: *Dependency, dep_id: DependencyID, pkg_id: PackageID, lockfile: *BinaryLockfile, pkg_resolutions: []const Resolution) void {
fn mapDepToPkg(dep: *Dependency, dep_id: DependencyID, pkg_id: PackageID, lockfile: *BinaryLockfile, pkg_resolutions: []const Resolution, is_duplicate_dep: bool) void {
lockfile.buffers.resolutions.items[dep_id] = pkg_id;
if (lockfile.text_lockfile_version != .v0) {
const res = &pkg_resolutions[pkg_id];
if (res.tag == .workspace) {
// when a package has duplicate dependencies and one is a workspace
// we don't want to override the other because it might not be a workspace.
if (res.tag == .workspace and !is_duplicate_dep) {
dep.version.tag = .workspace;
dep.version.value = .{ .workspace = res.value.workspace };

View File

@@ -0,0 +1,323 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDirWithFiles } from "harness";
import { join } from "path";
test("workspace devDependencies should take priority over peerDependencies for resolution", async () => {
const dir = tempDirWithFiles("dev-peer-priority", {
"package.json": JSON.stringify({
name: "test-monorepo",
version: "1.0.0",
workspaces: {
packages: ["packages/*"],
nodeLinker: "isolated",
},
}),
"packages/lib/package.json": JSON.stringify({
name: "lib",
version: "1.0.0",
dependencies: {},
devDependencies: {
"jquery": "workspace:*", // Use workspace protocol for dev
},
peerDependencies: {
"jquery": "3.7.0", // Range wants 3.7.0
},
}),
"packages/lib/test.js": `const dep = require("jquery"); console.log(dep.version);`,
// Only provide workspace package with version 2.0.0
"packages/my-dep/package.json": JSON.stringify({
name: "jquery",
version: "2.0.0",
main: "index.js",
}),
"packages/my-dep/index.js": `module.exports = { version: "2.0.0" };`,
});
// Run initial install
let { stdout, stderr, exitCode } = await new Promise<{ stdout: string; stderr: string; exitCode: number }>(
resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "install", "--no-progress", "--no-summary"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
proc.exited.then(exitCode => {
Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]).then(([stdout, stderr]) => {
resolve({ stdout, stderr, exitCode });
});
});
},
);
if (exitCode !== 0) {
console.error("Install failed with exit code:", exitCode);
console.error("stdout:", stdout);
console.error("stderr:", stderr);
}
expect(exitCode).toBe(0);
// Now run bun install with a dead registry to ensure no network requests
({ stdout, stderr, exitCode } = await new Promise<{ stdout: string; stderr: string; exitCode: number }>(resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "install", "--no-progress", "--no-summary"],
cwd: dir,
env: {
...bunEnv,
NPM_CONFIG_REGISTRY: "http://localhost:9999/", // Dead URL - will fail if used
},
stdout: "pipe",
stderr: "pipe",
});
proc.exited.then(exitCode => {
Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]).then(([stdout, stderr]) => {
resolve({ stdout, stderr, exitCode });
});
});
}));
if (exitCode !== 0) {
console.error("Install failed with exit code:", exitCode);
console.error("stdout:", stdout);
console.error("stderr:", stderr);
}
expect(exitCode).toBe(0);
// Check that no network requests were made for packages that should be resolved locally
expect(stderr).not.toContain("GET");
expect(stderr).not.toContain("http");
// Check that the lockfile was created correctly
const lockfilePath = join(dir, "bun.lock");
expect(await Bun.file(lockfilePath).exists()).toBe(true);
// Verify that version 2.0.0 (devDependency) was linked
// If peerDependency range ^1.0.0 was used, it would try to fetch from npm and fail
const testResult = await new Promise<string>(resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "packages/lib/test.js"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
});
new Response(proc.stdout).text().then(resolve);
});
expect(testResult.trim()).toBe("2.0.0");
});
test("devDependencies and peerDependencies with different versions should coexist", async () => {
const dir = tempDirWithFiles("dev-peer-different-versions", {
"package.json": JSON.stringify({
name: "test-monorepo",
version: "1.0.0",
workspaces: {
packages: ["packages/*"],
nodeLinker: "isolated",
},
}),
"packages/lib/package.json": JSON.stringify({
name: "lib",
version: "1.0.0",
dependencies: {},
devDependencies: {
"utils": "1.0.0",
},
peerDependencies: {
"utils": "^1.0.0",
},
}),
"packages/lib/index.js": `console.log("lib");`,
"packages/utils/package.json": JSON.stringify({
name: "utils",
version: "1.0.0",
main: "index.js",
}),
"packages/utils/index.js": `console.log("utils");`,
});
// Run bun install in the monorepo
const { stdout, stderr, exitCode } = await new Promise<{ stdout: string; stderr: string; exitCode: number }>(
resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "install", "--no-progress", "--no-summary"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
proc.exited.then(exitCode => {
Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]).then(([stdout, stderr]) => {
resolve({ stdout, stderr, exitCode });
});
});
},
);
if (exitCode !== 0) {
console.error("Install failed with exit code:", exitCode);
console.error("stdout:", stdout);
console.error("stderr:", stderr);
}
expect(exitCode).toBe(0);
// Check that the lockfile was created correctly
const lockfilePath = join(dir, "bun.lock");
expect(await Bun.file(lockfilePath).exists()).toBe(true);
});
test("dependency behavior comparison prioritizes devDependencies", async () => {
const dir = tempDirWithFiles("behavior-comparison", {
"package.json": JSON.stringify({
name: "test-app",
version: "1.0.0",
dependencies: {},
devDependencies: {
"typescript": "^5.0.0",
},
peerDependencies: {
"typescript": "^4.0.0 || ^5.0.0",
},
}),
"index.js": `console.log("app");`,
});
// Run bun install
const { stdout, stderr, exitCode } = await new Promise<{ stdout: string; stderr: string; exitCode: number }>(
resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "install", "--no-progress", "--no-summary"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
proc.exited.then(exitCode => {
Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]).then(([stdout, stderr]) => {
resolve({ stdout, stderr, exitCode });
});
});
},
);
if (exitCode !== 0) {
console.error("Install failed with exit code:", exitCode);
console.error("stdout:", stdout);
console.error("stderr:", stderr);
}
expect(exitCode).toBe(0);
// Check that the lockfile was created correctly
const lockfilePath = join(dir, "bun.lock");
expect(await Bun.file(lockfilePath).exists()).toBe(true);
});
test("Next.js monorepo scenario should not make unnecessary network requests", async () => {
const dir = tempDirWithFiles("nextjs-monorepo", {
"package.json": JSON.stringify({
name: "nextjs-monorepo",
version: "1.0.0",
workspaces: {
packages: ["packages/*"],
nodeLinker: "isolated",
},
}),
"packages/web/package.json": JSON.stringify({
name: "web",
version: "1.0.0",
dependencies: {},
devDependencies: {
"next": "15.0.0-canary.119", // Specific canary version for dev
},
peerDependencies: {
"next": "^14.0.0 || ^15.0.0", // Range that would accept 14.x or 15.x stable
},
}),
"packages/web/test.js": `const next = require("next/package.json"); console.log(next.version);`,
// Only provide the canary version that matches devDependencies
"packages/next/package.json": JSON.stringify({
name: "next",
version: "15.0.0-canary.119",
main: "index.js",
}),
"packages/next/index.js": `console.log("next workspace");`,
});
// Run initial install
let { stdout, stderr, exitCode } = await new Promise<{ stdout: string; stderr: string; exitCode: number }>(
resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "install", "--no-progress", "--no-summary"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
proc.exited.then(exitCode => {
Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]).then(([stdout, stderr]) => {
resolve({ stdout, stderr, exitCode });
});
});
},
);
if (exitCode !== 0) {
console.error("Install failed with exit code:", exitCode);
console.error("stdout:", stdout);
console.error("stderr:", stderr);
}
expect(exitCode).toBe(0);
// Run bun install with dead registry
({ stdout, stderr, exitCode } = await new Promise<{ stdout: string; stderr: string; exitCode: number }>(resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "install", "--no-progress", "--no-summary"],
cwd: dir,
env: {
...bunEnv,
NPM_CONFIG_REGISTRY: "http://localhost:9999/", // Dead URL
},
stdout: "pipe",
stderr: "pipe",
});
proc.exited.then(exitCode => {
Promise.all([new Response(proc.stdout).text(), new Response(proc.stderr).text()]).then(([stdout, stderr]) => {
resolve({ stdout, stderr, exitCode });
});
});
}));
expect(exitCode).toBe(0);
// The key test: should not make network requests for packages that exist in workspace
// When devDependencies are prioritized over peerDependencies, the workspace version should be used
expect(stderr).not.toContain("GET");
expect(stderr).not.toContain("404");
expect(stderr).not.toContain("http");
// Check that the lockfile was created correctly
const lockfilePath = join(dir, "bun.lock");
expect(await Bun.file(lockfilePath).exists()).toBe(true);
// Verify that version 15.0.0-canary.119 (devDependency) was used
// If peer range was used, it would try to fetch a stable version from npm and fail
const testResult = await new Promise<string>(resolve => {
const proc = Bun.spawn({
cmd: [bunExe(), "packages/web/test.js"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
});
new Response(proc.stdout).text().then(resolve);
});
expect(testResult.trim()).toBe("15.0.0-canary.119");
});