[install] fix workspace override of aliased npm dependency (#3784)

2 files changed, 132 insertions, 21 deletions

diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig
index cebc7e64d..73347871a 100644
--- a/src/install/lockfile.zig
+++ b/src/install/lockfile.zig
@@ -2496,10 +2496,9 @@ pub const Package = extern struct {
         package_dependencies: []Dependency,
         dependencies_count: u32,
         in_workspace: bool,
-        tag: ?Dependency.Version.Tag,
-        workspace_path: ?String,
-        workspace_version: ?Semver.Version,
-        external_name: ExternalString,
+        comptime tag: ?Dependency.Version.Tag,
+        workspace_ver: ?Semver.Version,
+        external_alias: ExternalString,
         version: string,
         key_loc: logger.Loc,
         value_loc: logger.Loc,
@@ -2510,12 +2509,18 @@ pub const Package = extern struct {
 
         var dependency_version = Dependency.parseWithOptionalTag(
             allocator,
-            external_name.value,
+            external_alias.value,
             sliced.slice,
             tag,
             &sliced,
             log,
         ) orelse Dependency.Version{};
+        const name_hash = switch (dependency_version.tag) {
+            .npm => String.Builder.stringHash(dependency_version.value.npm.name.slice(buf)),
+            else => external_alias.hash,
+        };
+        const workspace_path = if (comptime tag == null) lockfile.workspace_paths.get(@truncate(name_hash)) else null;
+        const workspace_version = if (comptime tag == null) lockfile.workspace_versions.get(@truncate(name_hash)) else workspace_ver;
 
         switch (dependency_version.tag) {
             .folder => {
@@ -2534,11 +2539,13 @@ pub const Package = extern struct {
                     ),
                 );
             },
-            .npm => if (workspace_version) |ver| {
+            .npm => if (comptime tag != null)
+                unreachable
+            else if (workspace_version) |ver| {
                 if (dependency_version.value.npm.version.satisfies(ver)) {
                     for (package_dependencies[0..dependencies_count]) |dep| {
                         // `dependencies` & `workspaces` defined within the same `package.json`
-                        if (dep.version.tag == .workspace and dep.name_hash == external_name.hash) {
+                        if (dep.version.tag == .workspace and dep.name_hash == name_hash) {
                             return null;
                         }
                     }
@@ -2546,7 +2553,7 @@ pub const Package = extern struct {
                     const path = workspace_path.?.sliced(buf);
                     if (Dependency.parseWithTag(
                         allocator,
-                        external_name.value,
+                        external_alias.value,
                         path.slice,
                         .workspace,
                         &path,
@@ -2576,7 +2583,7 @@ pub const Package = extern struct {
                         ),
                     );
                     defer dependency_version.value.workspace = path;
-                    var workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, @truncate(external_name.hash));
+                    var workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, @truncate(name_hash));
                     if (workspace_entry.found_existing) {
                         const old_path = workspace_entry.value_ptr.*;
 
@@ -2586,7 +2593,7 @@ pub const Package = extern struct {
                         } else if (strings.eqlComptime(old_path.slice(buf), "*")) brk: {
                             workspace_entry.value_ptr.* = path;
                             for (package_dependencies[0..dependencies_count]) |*package_dep| {
-                                if (package_dep.name_hash == external_name.hash) {
+                                if (String.Builder.stringHash(package_dep.realname().slice(buf)) == name_hash) {
                                     if (package_dep.version.tag != .workspace) break :brk;
                                     package_dep.version.value.workspace = path;
                                     return null;
@@ -2597,7 +2604,7 @@ pub const Package = extern struct {
                             return null;
                         } else {
                             log.addErrorFmt(&source, logger.Loc.Empty, allocator, "Workspace name \"{s}\" already exists", .{
-                                external_name.slice(buf),
+                                external_alias.slice(buf),
                             }) catch {};
                             return error.InstallFailed;
                         }
@@ -2606,12 +2613,12 @@ pub const Package = extern struct {
                 }
 
                 if (workspace_version) |ver| {
-                    try lockfile.workspace_versions.put(allocator, @truncate(external_name.hash), ver);
+                    try lockfile.workspace_versions.put(allocator, @truncate(name_hash), ver);
 
                     for (package_dependencies[0..dependencies_count]) |*package_dep| {
                         // `dependencies` & `workspaces` defined within the same `package.json`
                         if (package_dep.version.tag == .npm and
-                            package_dep.name_hash == external_name.hash and
+                            String.Builder.stringHash(package_dep.version.value.npm.name.slice(buf)) == name_hash and
                             package_dep.version.value.npm.version.satisfies(ver))
                         {
                             package_dep.version = dependency_version;
@@ -2625,14 +2632,14 @@ pub const Package = extern struct {
 
         const this_dep = Dependency{
             .behavior = group.behavior.setWorkspace(in_workspace),
-            .name = external_name.value,
-            .name_hash = external_name.hash,
+            .name = external_alias.value,
+            .name_hash = external_alias.hash,
             .version = dependency_version,
         };
 
         // `peerDependencies` may be specified on existing dependencies
         if (comptime features.check_for_duplicate_dependencies and !group.behavior.isPeer()) {
-            var entry = lockfile.scratch.duplicate_checker_map.getOrPutAssumeCapacity(external_name.hash);
+            var entry = lockfile.scratch.duplicate_checker_map.getOrPutAssumeCapacity(external_alias.hash);
             if (entry.found_existing) {
                 // duplicate dependencies are allowed in optionalDependencies
                 if (comptime group.behavior.isOptional()) {
@@ -2647,7 +2654,7 @@ pub const Package = extern struct {
                     var notes = try allocator.alloc(logger.Data, 1);
 
                     notes[0] = .{
-                        .text = try std.fmt.allocPrint(lockfile.allocator, "\"{s}\" originally specified here", .{external_name.slice(buf)}),
+                        .text = try std.fmt.allocPrint(lockfile.allocator, "\"{s}\" originally specified here", .{external_alias.slice(buf)}),
                         .location = logger.Location.init_or_nil(&source, source.rangeOfString(entry.value_ptr.*)),
                     };
 
@@ -2657,7 +2664,7 @@ pub const Package = extern struct {
                         lockfile.allocator,
                         notes,
                         "Duplicate dependency: \"{s}\" specified in package.json",
-                        .{external_name.slice(buf)},
+                        .{external_alias.slice(buf)},
                     );
                 }
             }
@@ -3447,7 +3454,6 @@ pub const Package = extern struct {
                         total_dependencies_count,
                         in_workspace,
                         .workspace,
-                        null,
                         entry.version,
                         external_name,
                         path,
@@ -3480,8 +3486,7 @@ pub const Package = extern struct {
                                     total_dependencies_count,
                                     in_workspace,
                                     null,
-                                    lockfile.workspace_paths.get(@truncate(external_name.hash)),
-                                    lockfile.workspace_versions.get(@truncate(external_name.hash)),
+                                    null,
                                     external_name,
                                     version,
                                     key.loc,
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index 21c98efb3..b1f453f86 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -4901,6 +4901,52 @@ it("should override @scoped npm dependency by matching workspace", async () => {
   await access(join(package_dir, "bun.lockb"));
 });
 
+it("should override aliased npm dependency by matching workspace", async () => {
+  const urls: string[] = [];
+  setHandler(dummyRegistry(urls));
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      workspaces: ["*"],
+      dependencies: {
+        bar: "npm:baz@<0.0.2",
+      },
+    }),
+  );
+  await mkdir(join(package_dir, "baz"));
+  const baz_package = JSON.stringify({
+    name: "baz",
+    version: "0.0.1",
+  });
+  await writeFile(join(package_dir, "baz", "package.json"), baz_package);
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err).toContain("Saved lockfile");
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + bar@workspace:baz",
+    "",
+    " 1 packages installed",
+  ]);
+  expect(await exited).toBe(0);
+  expect(urls.sort()).toBeEmpty();
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "bar"]);
+  expect(await readlink(join(package_dir, "node_modules", "bar"))).toBe(join("..", "baz"));
+  expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(baz_package);
+  await access(join(package_dir, "bun.lockb"));
+});
+
 it("should override child npm dependency by matching workspace", async () => {
   const urls: string[] = [];
   setHandler(dummyRegistry(urls));
@@ -5082,3 +5128,63 @@ it("should override @scoped child npm dependency by matching workspace", async (
   expect(await readdirSorted(join(package_dir, "node_modules", "@moo", "baz"))).toEqual(["package.json"]);
   await access(join(package_dir, "bun.lockb"));
 });
+
+it("should override aliased child npm dependency by matching workspace", async () => {
+  const urls: string[] = [];
+  setHandler(dummyRegistry(urls));
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      workspaces: ["packages/*"],
+    }),
+  );
+  await mkdir(join(package_dir, "packages", "bar"), { recursive: true });
+  const bar_package = JSON.stringify({
+    name: "@moo/bar",
+    version: "0.0.1",
+  });
+  await writeFile(join(package_dir, "packages", "bar", "package.json"), bar_package);
+  await mkdir(join(package_dir, "packages", "baz"), { recursive: true });
+  await writeFile(
+    join(package_dir, "packages", "baz", "package.json"),
+    JSON.stringify({
+      name: "baz",
+      version: "0.1.0",
+      dependencies: {
+        bar: "npm:@moo/bar@*",
+      },
+    }),
+  );
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err).toContain("Saved lockfile");
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + @moo/bar@workspace:packages/bar",
+    " + baz@workspace:packages/baz",
+    "",
+    " 2 packages installed",
+  ]);
+  expect(await exited).toBe(0);
+  expect(urls.sort()).toBeEmpty();
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "@moo", "baz"]);
+  expect(await readdirSorted(join(package_dir, "node_modules", "@moo"))).toEqual(["bar"]);
+  expect(await readlink(join(package_dir, "node_modules", "@moo", "bar"))).toBe(join("..", "..", "packages", "bar"));
+  expect(await file(join(package_dir, "node_modules", "@moo", "bar", "package.json")).text()).toEqual(bar_package);
+  expect(await readlink(join(package_dir, "node_modules", "baz"))).toBe(join("..", "packages", "baz"));
+  expect(await readdirSorted(join(package_dir, "packages", "baz"))).toEqual(["node_modules", "package.json"]);
+  expect(await readdirSorted(join(package_dir, "packages", "baz", "node_modules"))).toEqual(["bar"]);
+  expect(await readlink(join(package_dir, "packages", "baz", "node_modules", "bar"))).toBe(join("..", "..", "bar"));
+  await access(join(package_dir, "bun.lockb"));
+});