fix life-cycle script execution (#1799)

- change current working directory for workspaces
- add `node_modules/.bin` to `PATH` before running

4 files changed, 173 insertions, 113 deletions

diff --git a/src/cli/run_command.zig b/src/cli/run_command.zig
index c2d5f407a..312776c79 100644
--- a/src/cli/run_command.zig
+++ b/src/cli/run_command.zig
@@ -533,82 +533,68 @@ pub const RunCommand = struct {
         var needs_to_force_bun = force_using_bun or !found_node;
         var optional_bun_self_path: string = "";
 
-        if (bin_dirs.len > 0 or package_json_dir.len > 0) {
-            var new_path_len: usize = PATH.len + 2;
-            for (bin_dirs) |bin| {
-                new_path_len += bin.len + 1;
-            }
+        var new_path_len: usize = PATH.len + 2;
+        for (bin_dirs) |bin| {
+            new_path_len += bin.len + 1;
+        }
 
-            if (package_json_dir.len > 0) {
-                new_path_len += package_json_dir.len + 1;
-            }
+        if (package_json_dir.len > 0) {
+            new_path_len += package_json_dir.len + 1;
+        }
 
-            new_path_len += if (needs_to_force_bun) bun_node_dir.len + 1 else 0;
+        new_path_len += root_dir_info.abs_path.len + "node_modules/.bin".len + 1;
 
-            var new_path = try std.ArrayList(u8).initCapacity(ctx.allocator, new_path_len);
+        if (needs_to_force_bun) {
+            new_path_len += bun_node_dir.len + 1;
+        }
 
-            if (needs_to_force_bun) {
-                createFakeTemporaryNodeExecutable(&new_path, &optional_bun_self_path) catch unreachable;
-                if (!force_using_bun) {
-                    this_bundler.env.map.put("NODE", bun_node_dir ++ "/node") catch unreachable;
-                    this_bundler.env.map.put("npm_node_execpath", bun_node_dir ++ "/node") catch unreachable;
-                    this_bundler.env.map.put("npm_execpath", optional_bun_self_path) catch unreachable;
-                }
+        var new_path = try std.ArrayList(u8).initCapacity(ctx.allocator, new_path_len);
 
-                needs_to_force_bun = false;
+        if (needs_to_force_bun) {
+            createFakeTemporaryNodeExecutable(&new_path, &optional_bun_self_path) catch unreachable;
+            if (!force_using_bun) {
+                this_bundler.env.map.put("NODE", bun_node_dir ++ "/node") catch unreachable;
+                this_bundler.env.map.put("npm_node_execpath", bun_node_dir ++ "/node") catch unreachable;
+                this_bundler.env.map.put("npm_execpath", optional_bun_self_path) catch unreachable;
             }
 
-            {
-                var needs_colon = false;
-                if (package_json_dir.len > 0) {
-                    defer needs_colon = true;
-                    if (needs_colon) {
-                        try new_path.append(':');
-                    }
-                    try new_path.appendSlice(package_json_dir);
-                }
+            needs_to_force_bun = false;
+        }
 
-                var bin_dir_i: i32 = @intCast(i32, bin_dirs.len) - 1;
-                // Iterate in reverse order
-                // Directories are added to bin_dirs in top-down order
-                // That means the parent-most node_modules/.bin will be first
-                while (bin_dir_i >= 0) : (bin_dir_i -= 1) {
-                    defer needs_colon = true;
-                    if (needs_colon) {
-                        try new_path.append(':');
-                    }
-                    try new_path.appendSlice(bin_dirs[@intCast(usize, bin_dir_i)]);
+        {
+            var needs_colon = false;
+            if (package_json_dir.len > 0) {
+                defer needs_colon = true;
+                if (needs_colon) {
+                    try new_path.append(':');
                 }
+                try new_path.appendSlice(package_json_dir);
+            }
 
+            var bin_dir_i: i32 = @intCast(i32, bin_dirs.len) - 1;
+            // Iterate in reverse order
+            // Directories are added to bin_dirs in top-down order
+            // That means the parent-most node_modules/.bin will be first
+            while (bin_dir_i >= 0) : (bin_dir_i -= 1) {
+                defer needs_colon = true;
                 if (needs_colon) {
                     try new_path.append(':');
                 }
-                try new_path.appendSlice(PATH);
+                try new_path.appendSlice(bin_dirs[@intCast(usize, bin_dir_i)]);
             }
 
-            this_bundler.env.map.put("PATH", new_path.items) catch unreachable;
-            PATH = new_path.items;
-        }
-
-        if (needs_to_force_bun) {
-            needs_to_force_bun = false;
-
-            var new_path = try std.ArrayList(u8).initCapacity(ctx.allocator, PATH.len);
-            createFakeTemporaryNodeExecutable(&new_path, &optional_bun_self_path) catch unreachable;
-            if (new_path.items.len > 0)
+            if (needs_colon) {
                 try new_path.append(':');
-            try new_path.appendSlice(PATH);
-
-            this_bundler.env.map.put("PATH", new_path.items) catch unreachable;
-
-            if (!force_using_bun) {
-                this_bundler.env.map.put("NODE", bun_node_dir ++ "/node") catch unreachable;
-                this_bundler.env.map.put("npm_node_execpath", bun_node_dir ++ "/node") catch unreachable;
-                this_bundler.env.map.put("npm_execpath", optional_bun_self_path) catch unreachable;
             }
-            PATH = new_path.items;
+            try new_path.appendSlice(root_dir_info.abs_path);
+            try new_path.appendSlice("node_modules/.bin");
+            try new_path.append(':');
+            try new_path.appendSlice(PATH);
         }
 
+        this_bundler.env.map.put("PATH", new_path.items) catch unreachable;
+        PATH = new_path.items;
+
         this_bundler.env.map.putDefault("npm_config_local_prefix", this_bundler.fs.top_level_dir) catch unreachable;
 
         // we have no way of knowing what version they're expecting without running the node executable
diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig
index 4e008ac1e..8fc900649 100644
--- a/src/install/lockfile.zig
+++ b/src/install/lockfile.zig
@@ -112,7 +112,11 @@ const Stream = std.io.FixedBufferStream([]u8);
 pub const default_filename = "bun.lockb";
 
 pub const Scripts = struct {
-    const StringArrayList = std.ArrayListUnmanaged(string);
+    const Entry = struct {
+        cwd: string,
+        script: string,
+    };
+    const StringArrayList = std.ArrayListUnmanaged(Entry);
     const RunCommand = @import("../cli/run_command.zig").RunCommand;
 
     preinstall: StringArrayList = .{},
@@ -122,7 +126,7 @@ pub const Scripts = struct {
     prepare: StringArrayList = .{},
     postprepare: StringArrayList = .{},
 
-    pub fn hasAny(this: Scripts) bool {
+    pub fn hasAny(this: *Scripts) bool {
         return (this.preinstall.items.len +
             this.install.items.len +
             this.postinstall.items.len +
@@ -131,13 +135,28 @@ pub const Scripts = struct {
             this.postprepare.items.len) > 0;
     }
 
-    pub fn run(this: Scripts, allocator: std.mem.Allocator, env: *DotEnv.Loader, silent: bool, comptime hook: []const u8) !void {
-        for (@field(this, hook).items) |script| {
+    pub fn run(this: *Scripts, allocator: std.mem.Allocator, env: *DotEnv.Loader, silent: bool, comptime hook: []const u8) !void {
+        for (@field(this, hook).items) |entry| {
             std.debug.assert(Fs.FileSystem.instance_loaded);
-            const cwd = Fs.FileSystem.instance.top_level_dir;
-            _ = try RunCommand.runPackageScript(allocator, script, hook, cwd, env, &.{}, silent);
+            const cwd = Path.joinAbsString(
+                FileSystem.instance.top_level_dir,
+                &[_]string{
+                    entry.cwd,
+                },
+                .posix,
+            );
+            _ = try RunCommand.runPackageScript(allocator, entry.script, hook, cwd, env, &.{}, silent);
         }
     }
+
+    pub fn deinit(this: *Scripts, allocator: std.mem.Allocator) void {
+        this.preinstall.deinit(allocator);
+        this.install.deinit(allocator);
+        this.postinstall.deinit(allocator);
+        this.preprepare.deinit(allocator);
+        this.prepare.deinit(allocator);
+        this.postprepare.deinit(allocator);
+    }
 };
 
 pub fn isEmpty(this: *const Lockfile) bool {
@@ -2593,20 +2612,18 @@ pub const Package = extern struct {
                         "prepare",
                         "preprepare",
                     };
+                    var cwd: string = "";
 
                     inline for (scripts) |script_name| {
                         if (scripts_prop.expr.get(script_name)) |script| {
                             if (script.asString(allocator)) |input| {
-                                var list = @field(lockfile.scripts, script_name);
-                                if (list.capacity == 0) {
-                                    list.capacity = 1;
-                                    list.items = try allocator.alloc(string, 1);
-                                    list.items[0] = input;
-                                } else {
-                                    try list.append(allocator, input);
+                                if (cwd.len == 0 and source.path.name.dir.len > 0) {
+                                    cwd = try allocator.dupe(u8, source.path.name.dir);
                                 }
-
-                                @field(lockfile.scripts, script_name) = list;
+                                try @field(lockfile.scripts, script_name).append(allocator, .{
+                                    .cwd = cwd,
+                                    .script = input,
+                                });
                             }
                         }
                     }
@@ -3100,6 +3117,7 @@ pub fn deinit(this: *Lockfile) void {
     this.packages.deinit(this.allocator);
     this.unique_packages.deinit(this.allocator);
     this.string_pool.deinit();
+    this.scripts.deinit(this.allocator);
     this.workspace_paths.deinit(this.allocator);
 }
 
diff --git a/src/js_ast.zig b/src/js_ast.zig
index f82eadf74..82c9de809 100644
--- a/src/js_ast.zig
+++ b/src/js_ast.zig
@@ -2403,10 +2403,7 @@ pub const Expr = struct {
 
     pub inline fn asString(expr: *const Expr, allocator: std.mem.Allocator) ?string {
         if (std.meta.activeTag(expr.data) != .e_string) return null;
-
-        const key_str = expr.data.e_string;
-
-        return if (key_str.isUTF8()) key_str.data else key_str.string(allocator) catch null;
+        return expr.data.e_string.string(allocator) catch null;
     }
 
     pub fn asBool(
@@ -6841,11 +6838,9 @@ pub const Macro = struct {
                     var p = self.p;
 
                     const node_type: JSNode.Tag = JSNode.Tag.names.get(str.data) orelse {
-                        if (!str.isUTF8()) {
-                            self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{strings.toUTF8Alloc(self.p.allocator, str.slice16()) catch unreachable}) catch unreachable;
-                        } else {
-                            self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{str.data}) catch unreachable;
-                        }
+                        self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{
+                            str.string(self.p.allocator) catch unreachable,
+                        }) catch unreachable;
                         return false;
                     };
 
@@ -6863,13 +6858,9 @@ pub const Macro = struct {
                     var p = self.p;
 
                     const node_type: JSNode.Tag = JSNode.Tag.names.get(str.data) orelse {
-                        if (!str.isUTF8()) {
-                            self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{
-                                strings.toUTF8Alloc(self.p.allocator, str.slice16()) catch unreachable,
-                            }) catch unreachable;
-                        } else {
-                            self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{str.data}) catch unreachable;
-                        }
+                        self.log.addErrorFmt(p.source, tag_expr.loc, p.allocator, "Tag \"{s}\" is invalid", .{
+                            str.string(self.p.allocator) catch unreachable,
+                        }) catch unreachable;
                         return false;
                     };
 
diff --git a/test/bun.js/install/bun-install.test.ts b/test/bun.js/install/bun-install.test.ts
index 78b1c12b8..a9b62e9e4 100644
--- a/test/bun.js/install/bun-install.test.ts
+++ b/test/bun.js/install/bun-install.test.ts
@@ -57,7 +57,7 @@ it("should handle missing package", async () => {
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err.split(/\r?\n/)).toContain('error: package "foo" not found localhost/foo 404');
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBe("");
@@ -96,7 +96,7 @@ it("should handle @scoped authentication", async () => {
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err.split(/\r?\n/)).toContain(`GET ${url} - 555`);
   expect(stdout).toBeDefined();
   expect(await new Response(stdout).text()).toBe("");
@@ -131,12 +131,15 @@ it("should handle workspaces", async () => {
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err).toContain("Saved lockfile");
   expect(stdout).toBeDefined();
-  var out = await new Response(stdout).text();
-  expect(out).toContain("+ Bar@workspace://bar");
-  expect(out).toContain("1 packages installed");
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + Bar@workspace://bar",
+    "",
+    " 1 packages installed",
+  ]);
   expect(await exited).toBe(0);
   expect(requested).toBe(0);
   expect(await readdir(join(package_dir, "node_modules"))).toEqual(["Bar"]);
@@ -180,13 +183,16 @@ it("should handle inter-dependency between workspaces", async () => {
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err).toContain("Saved lockfile");
   expect(stdout).toBeDefined();
-  var out = await new Response(stdout).text();
-  expect(out).toContain("+ Bar@workspace://bar");
-  expect(out).toContain("+ Baz@workspace://packages/baz");
-  expect(out).toContain("2 packages installed");
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + Bar@workspace://bar",
+    " + Baz@workspace://packages/baz",
+    "",
+    " 2 packages installed",
+  ]);
   expect(await exited).toBe(0);
   expect(requested).toBe(0);
   expect(await readdir(join(package_dir, "node_modules"))).toEqual(["Bar", "Baz"]);
@@ -231,13 +237,16 @@ it("should handle inter-dependency between workspaces (devDependencies)", async
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err).toContain("Saved lockfile");
   expect(stdout).toBeDefined();
-  var out = await new Response(stdout).text();
-  expect(out).toContain("+ Bar@workspace://bar");
-  expect(out).toContain("+ Baz@workspace://packages/baz");
-  expect(out).toContain("2 packages installed");
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + Bar@workspace://bar",
+    " + Baz@workspace://packages/baz",
+    "",
+    " 2 packages installed",
+  ]);
   expect(await exited).toBe(0);
   expect(requested).toBe(0);
   expect(await readdir(join(package_dir, "node_modules"))).toEqual(["Bar", "Baz"]);
@@ -282,13 +291,16 @@ it("should handle inter-dependency between workspaces (optionalDependencies)", a
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err).toContain("Saved lockfile");
   expect(stdout).toBeDefined();
-  var out = await new Response(stdout).text();
-  expect(out).toContain("+ Bar@workspace://bar");
-  expect(out).toContain("+ Baz@workspace://packages/baz");
-  expect(out).toContain("2 packages installed");
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + Bar@workspace://bar",
+    " + Baz@workspace://packages/baz",
+    "",
+    " 2 packages installed",
+  ]);
   expect(await exited).toBe(0);
   expect(requested).toBe(0);
   expect(await readdir(join(package_dir, "node_modules"))).toEqual(["Bar", "Baz"]);
@@ -327,14 +339,67 @@ it("should ignore peerDependencies within workspaces", async () => {
     },
   });
   expect(stderr).toBeDefined();
-  var err = await new Response(stderr).text();
+  const err = await new Response(stderr).text();
   expect(err).toContain("Saved lockfile");
   expect(stdout).toBeDefined();
-  var out = await new Response(stdout).text();
-  expect(out).toContain("+ Baz@workspace://packages/baz");
-  expect(out).toContain("1 packages installed");
+  const out = await new Response(stdout).text();
+  expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + Baz@workspace://packages/baz",
+    "",
+    " 1 packages installed",
+  ]);
   expect(await exited).toBe(0);
   expect(requested).toBe(0);
   expect(await readdir(join(package_dir, "node_modules"))).toEqual(["Baz"]);
   expect(await readlink(join(package_dir, "node_modules", "Baz"))).toBe(join("..", "packages", "baz"));
 });
+
+it("should handle life-cycle scripts within workspaces", async () => {
+  await writeFile(join(package_dir, "package.json"), JSON.stringify({
+    name: "Foo",
+    version: "0.0.1",
+    scripts: {
+      install: [bunExe(), "index.js"].join(" "),
+    },
+    workspaces: [
+      "bar",
+    ],
+  }));
+  await writeFile(join(package_dir, "index.js"), 'console.log("[scripts:run] Foo");');
+  await mkdir(join(package_dir, "bar"));
+  await writeFile(join(package_dir, "bar", "package.json"), JSON.stringify({
+    name: "Bar",
+    version: "0.0.2",
+    scripts: {
+      preinstall: [bunExe(), "index.js"].join(" "),
+    },
+  }));
+  await writeFile(join(package_dir, "bar", "index.js"), 'console.log("[scripts:run] Bar");');
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "install", "--config", import.meta.dir + "/basic.toml"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env: {
+      ...process.env,
+      BUN_DEBUG_QUIET_LOGS: "1",
+    },
+  });
+  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\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([
+    "[scripts:run] Bar",
+    " + Bar@workspace://bar",
+    "[scripts:run] Foo",
+    "",
+    " 1 packages installed",
+  ]);
+  expect(await exited).toBe(0);
+  expect(requested).toBe(0);
+  expect(await readdir(join(package_dir, "node_modules"))).toEqual(["Bar"]);
+  expect(await readlink(join(package_dir, "node_modules", "Bar"))).toBe(join("..", "bar"));
+});