Some refactor suggestions

src/install/PackageManager.zig

@@ -135,6 +135,8 @@ active_lifecycle_scripts: LifecycleScriptSubprocess.List,
 last_reported_slow_lifecycle_script_at: u64 = 0,
 cached_tick_for_slow_lifecycle_script_logging: u64 = 0,
 
+nohoist_patterns: []const []const u8 = &.{},
+
 /// Corresponds to possible commands from the CLI.
 pub const Subcommand = enum {
     install,

src/install/lockfile.zig

@@ -951,15 +951,25 @@ pub fn hoist(
         Tree.root_dep_id,
         Tree.invalid_id,
         method,
+        if (comptime method == .filter) .init() else {},
         &builder,
     );
 
     // This goes breadth-first
     while (builder.queue.readItem()) |item| {
+        var subpath = item.subpath;
+
+        defer {
+            if (comptime method == .filter) {
+                subpath.deinit();
+            }
+        }
+
         try builder.list.items(.tree)[item.tree_id].processSubtree(
             item.dependency_id,
             item.hoist_root_id,
             method,
+            if (comptime method == .filter) subpath else {},
             &builder,
         );
     }

src/install/lockfile/Package.zig

@@ -1578,6 +1578,28 @@ pub fn Package(comptime SemverIntType: type) type {
 
                 if (json.get("workspaces")) |workspaces_expr| {
                     lockfile.catalogs.parseCount(lockfile, workspaces_expr, &string_builder);
+
+                    if (workspaces_expr.get("nohoist")) |nohoist_expr| {
+                        if (!nohoist_expr.isArray()) {
+                            try log.addError(source, nohoist_expr.loc, "Expected an array of strings");
+                            return error.InvalidPackageJSON;
+                        }
+
+                        const nohoist_arr = nohoist_expr.data.e_array;
+
+                        var nohoist_patterns: bun.collections.ArrayListDefault([]const u8) = try .initCapacity(nohoist_arr.items.len);
+
+                        for (nohoist_arr.items.slice()) |nohoist_item| {
+                            if (!nohoist_item.isString()) {
+                                try log.addError(source, nohoist_item.loc, "Expected a string pattern");
+                                return error.InvalidPackageJSON;
+                            }
+
+                            nohoist_patterns.appendAssumeCapacity(try nohoist_item.data.e_string.stringCloned(bun.default_allocator));
+                        }
+
+                        pm.nohoist_patterns = nohoist_patterns.items();
+                    }
                 }
 
                 // Count catalog strings in top-level package.json as well, since parseAppend

src/install/lockfile/Tree.zig

@@ -256,6 +256,19 @@ pub fn Builder(comptime method: BuilderMethod) type {
         install_root_dependencies: if (method == .filter) bool else void,
         packages_to_install: if (method == .filter) ?[]const PackageID else void,
 
+        pub const FillItem = struct {
+            tree_id: Tree.Id,
+            dependency_id: DependencyID,
+
+            /// If valid, dependencies will not hoist
+            /// beyond this tree if they're in a subtree
+            hoist_root_id: Tree.Id,
+
+            subpath: if (method == .filter) bun.collections.ArrayListDefault(u8) else void,
+        };
+
+        pub const TreeFiller = bun.LinearFifo(FillItem, .Dynamic);
+
         pub fn maybeReportError(this: *@This(), comptime fmt: string, args: anytype) void {
             this.log.addErrorFmt(null, logger.Loc.Empty, this.allocator, fmt, args) catch {};
         }
@@ -282,6 +295,103 @@ pub fn Builder(comptime method: BuilderMethod) type {
             dep_ids: std.ArrayListUnmanaged(DependencyID),
         };
 
+        pub fn tryNohoist(
+            self: *const @This(),
+            subpath: if (method == .filter) []const u8 else void,
+            dependency_id: DependencyID,
+        ) error{ DependencyLoop, OutOfMemory }!?struct {
+            result: HoistDependencyResult,
+            subpath: bun.collections.ArrayListDefault(u8),
+        } {
+            if (comptime method != .filter) {
+                return null;
+            }
+
+            const dependency = self.dependencies[dependency_id];
+            const pkg_id = self.resolutions[dependency_id];
+            const list_slice = self.list.slice();
+            const trees = list_slice.items(.tree);
+            const next: *const Tree = &trees[self.list.len - 1];
+
+            var dep_subpath: bun.collections.ArrayListDefault(u8) = .init();
+            errdefer dep_subpath.deinit();
+
+            if (self.manager.nohoist_patterns.len == 0) {
+                return null;
+            }
+
+            const string_buf = self.lockfile.buffers.string_bytes.items;
+
+            try dep_subpath.ensureTotalCapacity(subpath.len + @intFromBool(subpath.len != 0) + dependency.name.len());
+            dep_subpath.appendSliceAssumeCapacity(subpath);
+            if (subpath.len != 0) {
+                dep_subpath.appendAssumeCapacity('/');
+            }
+            dep_subpath.appendSliceAssumeCapacity(dependency.name.slice(string_buf));
+
+            if (dependency.version.tag == .workspace) {
+                return null;
+            }
+
+            if (try self.shouldNoHoist(dep_subpath.items(), pkg_id, trees, next)) {
+                return .{
+                    .result = .{ .placement = .{ .id = next.id } },
+                    .subpath = dep_subpath,
+                };
+            }
+
+            return null;
+        }
+
+        fn pkgHasCircularReference(
+            self: *const @This(),
+            pkg_id: PackageID,
+            forest: []const Tree,
+            tree: *const Tree,
+        ) bool {
+            var curr = forest[tree.id].parent;
+            while (curr != invalid_id) {
+                const curr_dep_id = forest[curr].dependency_id;
+                const curr_pkg_id = switch (curr_dep_id) {
+                    root_dep_id => 0,
+                    else => self.resolutions[curr_dep_id],
+                };
+                var curr_resolutions = self.resolution_lists[curr_pkg_id];
+                for (curr_resolutions.begin()..curr_resolutions.end()) |tree_dep_id| {
+                    const res_id = self.resolutions[tree_dep_id];
+                    if (res_id == pkg_id) {
+                        return true;
+                    }
+                }
+
+                curr = forest[curr].parent;
+            }
+
+            return false;
+        }
+
+        fn shouldNoHoist(
+            self: *const @This(),
+            dep_path: []const u8,
+            pkg_id: PackageID,
+            forest: []const Tree,
+            next_tree: *const Tree,
+        ) error{DependencyLoop}!bool {
+            for (self.manager.nohoist_patterns) |nohoist_pattern| {
+                if (!glob.match(nohoist_pattern, dep_path).matches()) {
+                    continue;
+                }
+
+                if (self.pkgHasCircularReference(pkg_id, forest, next_tree)) {
+                    return error.DependencyLoop;
+                }
+
+                return true;
+            }
+
+            return false;
+        }
+
         /// Flatten the multi-dimensional ArrayList of package IDs into a single easily serializable array
         pub fn clean(this: *@This()) OOM!CleanResult {
             var total: u32 = 0;
@@ -429,7 +539,7 @@ pub fn isFilteredDependencyOrWorkspace(
             },
         };
 
-        switch (bun.glob.match(pattern, name_or_path)) {
+        switch (glob.match(pattern, name_or_path)) {
             .match, .negate_match => workspace_matched = true,
 
             .negate_no_match => {
@@ -452,6 +562,7 @@ pub fn processSubtree(
     dependency_id: DependencyID,
     hoist_root_id: Tree.Id,
     comptime method: BuilderMethod,
+    subpath: if (method == .filter) bun.collections.ArrayListDefault(u8) else void,
     builder: *Builder(method),
 ) SubtreeError!void {
     const parent_pkg_id = switch (dependency_id) {
@@ -496,6 +607,7 @@ pub fn processSubtree(
     );
 
     for (builder.sort_buf.items) |dep_id| {
+        const dependency = builder.dependencies[dep_id];
         const pkg_id = builder.resolutions[dep_id];
 
         // filter out disabled dependencies
@@ -534,9 +646,12 @@ pub fn processSubtree(
             }
         }
 
-        const dependency = builder.dependencies[dep_id];
-
+        var dep_subpath: bun.collections.ArrayListDefault(u8) = .init();
         const hoisted: HoistDependencyResult = hoisted: {
+            if (try builder.tryNohoist(if (method == .filter) subpath.items() else {}, dep_id)) |r| {
+                dep_subpath = r.subpath;
+                break :hoisted r.result;
+            }
 
             // don't hoist if it's a folder dependency or a bundled dependency.
             if (dependency.behavior.isBundled()) {
@@ -617,6 +732,7 @@ pub fn processSubtree(
                         .tree_id = replace.id,
                         .dependency_id = dep_id,
                         .hoist_root_id = hoist_root_id,
+                        .subpath = if (comptime method == .filter) dep_subpath else {},
                     });
                 }
             },
@@ -640,6 +756,7 @@ pub fn processSubtree(
 
                         // if it's bundled, start a new hoist root
                         .hoist_root_id = if (dest.bundled) dest.id else hoist_root_id,
+                        .subpath = if (comptime method == .filter) dep_subpath else {},
                     });
                 }
             },
@@ -763,17 +880,6 @@ fn hoistDependency(
     return .{ .placement = .{ .id = this.id } }; // 2
 }
 
-pub const FillItem = struct {
-    tree_id: Tree.Id,
-    dependency_id: DependencyID,
-
-    /// If valid, dependencies will not hoist
-    /// beyond this tree if they're in a subtree
-    hoist_root_id: Tree.Id,
-};
-
-pub const TreeFiller = bun.LinearFifo(FillItem, .Dynamic);
-
 const string = []const u8;
 const stringZ = [:0]const u8;
 
@@ -786,6 +892,7 @@ const OOM = bun.OOM;
 const Output = bun.Output;
 const Path = bun.path;
 const assert = bun.assert;
+const glob = bun.glob;
 const logger = bun.logger;
 const Bitset = bun.bit_set.DynamicBitSetUnmanaged;
 const String = bun.Semver.String;

test/cli/install/nohoist.test.ts

@@ -0,0 +1,90 @@
+import { afterAll, beforeAll, describe, expect, test } from "bun:test";
+import { exists } from "fs/promises";
+import { VerdaccioRegistry, bunEnv, readdirSorted, runBunInstall } from "harness";
+import { join } from "path";
+
+var registry = new VerdaccioRegistry();
+
+beforeAll(async () => {
+  await registry.start();
+});
+
+afterAll(() => {
+  registry.stop();
+});
+
+describe("workspaces.nohoist", () => {
+  test("basic", async () => {
+    const { packageDir } = await registry.createTestDir({
+      files: {
+        "package.json": JSON.stringify({
+          name: "basic-nohoist",
+          workspaces: {
+            nohoist: ["one-dep/no-deps"],
+          },
+          dependencies: {
+            "one-dep": "1.0.0",
+          },
+        }),
+      },
+    });
+
+    await runBunInstall(bunEnv, packageDir);
+
+    expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["one-dep"]);
+  });
+
+  test("can keep package in workspace node_modules", async () => {
+    const { packageDir } = await registry.createTestDir({
+      files: {
+        "package.json": JSON.stringify({
+          name: "workspace-nohoist",
+          workspaces: {
+            packages: ["packages/*"],
+            nohoist: ["**/one-dep"],
+          },
+          dependencies: {
+            "a-dep": "1.0.1",
+          },
+        }),
+        "packages/pkg1/package.json": JSON.stringify({
+          name: "pkg1",
+          dependencies: {
+            "one-dep": "1.0.0",
+          },
+        }),
+      },
+    });
+
+    await runBunInstall(bunEnv, packageDir, { linker: "hoisted" });
+
+    expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["a-dep", "no-deps", "pkg1"]);
+    expect(await readdirSorted(join(packageDir, "packages/pkg1/node_modules"))).toEqual(["one-dep"]);
+  });
+
+  test("handles cycles", async () => {
+    const { packageDir } = await registry.createTestDir({
+      files: {
+        "package.json": JSON.stringify({
+          name: "cycles",
+          workspaces: {
+            nohoist: ["**"],
+          },
+          dependencies: {
+            "a-dep-b": "1.0.0",
+          },
+        }),
+      },
+    });
+
+    await runBunInstall(bunEnv, packageDir, { linker: "hoisted" });
+
+    expect(
+      await Promise.all([
+        readdirSorted(join(packageDir, "node_modules")),
+        readdirSorted(join(packageDir, "node_modules/a-dep-b/node_modules")),
+        exists(join(packageDir, "node_modules/a-dep-b/node_modules/b-dep-a/node_modules")),
+      ]),
+    ).toEqual([["a-dep-b"], ["b-dep-a"], false]);
+  });
+});

test/harness.ts

@@ -1225,6 +1225,7 @@ export async function runBunInstall(
     saveTextLockfile?: boolean;
     packages?: string[];
     verbose?: boolean;
+    linker?: "hoisted" | "isolated";
   } = {},
 ) {
   const production = options?.production ?? false;
@@ -1235,6 +1236,9 @@ export async function runBunInstall(
   if (production) {
     args.push("--production");
   }
+  if (options?.linker) {
+    args.push(`--linker=${options?.linker}`);
+  }
   if (options?.frozenLockfile) {
     args.push("--frozen-lockfile");
   }