From 07e08b086ae7bb78feffe6b0d325dcecb1765ad9 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Sun, 23 Jul 2023 06:05:24 +0300 Subject: [install] improve workspace substitution of npm dependencies (#3754) - respect semver ranges --- src/bun.js/test/expect.zig | 2 +- src/install/install.zig | 2 +- src/install/lockfile.zig | 231 ++++++++++++++++++------ test/cli/install/bun-install.test.ts | 271 +++++++++++++++++++++++++--- test/js/node/readline/readline.node.test.ts | 2 +- 5 files changed, 415 insertions(+), 93 deletions(-) diff --git a/src/bun.js/test/expect.zig b/src/bun.js/test/expect.zig index e7209e683..d6f8ebb12 100644 --- a/src/bun.js/test/expect.zig +++ b/src/bun.js/test/expect.zig @@ -513,7 +513,7 @@ pub const Expect = struct { const value_fmt = value.toFmt(globalObject, &formatter); const expected_fmt = expected.toFmt(globalObject, &formatter); if (not) { - const expected_line = "Expected to contain: not {any}\n"; + const expected_line = "Expected to not contain: {any}\n"; const fmt = comptime getSignature("toContain", "expected", true) ++ "\n\n" ++ expected_line; if (Output.enable_ansi_colors) { globalObject.throw(Output.prettyFmt(fmt, true), .{expected_fmt}); diff --git a/src/install/install.zig b/src/install/install.zig index 7b8d71bad..74904b276 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -5213,7 +5213,7 @@ pub const PackageManager = struct { initializeStore(); const json = try json_parser.ParseJSONUTF8(&json_source, ctx.log, ctx.allocator); if (json.asProperty("workspaces")) |prop| { - var workspace_names = bun.StringMap.init(ctx.allocator, true); + var workspace_names = Package.WorkspaceMap.init(ctx.allocator); defer workspace_names.deinit(); const json_array = switch (prop.expr.data) { .e_array => |arr| arr, diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index cf0ee8267..cebc7e64d 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -86,6 +86,7 @@ const MetaHash = [std.crypto.hash.sha2.Sha512256.digest_length]u8; const zero_hash = std.mem.zeroes(MetaHash); const NameHashMap = std.ArrayHashMapUnmanaged(u32, String, ArrayIdentityContext, false); const NameHashSet = std.ArrayHashMapUnmanaged(u32, void, ArrayIdentityContext, false); +const VersionHashMap = std.ArrayHashMapUnmanaged(u32, Semver.Version, ArrayIdentityContext, false); // Serialized data /// The version of the lockfile format, intended to prevent data corruption for format changes. @@ -106,6 +107,7 @@ scratch: Scratch = .{}, scripts: Scripts = .{}, trusted_dependencies: NameHashSet = .{}, workspace_paths: NameHashMap = .{}, +workspace_versions: VersionHashMap = .{}, const Stream = std.io.FixedBufferStream([]u8); pub const default_filename = "bun.lockb"; @@ -199,6 +201,7 @@ pub fn loadFromBytes(this: *Lockfile, buf: []u8, allocator: Allocator, log: *log this.scripts = .{}; this.trusted_dependencies = .{}; this.workspace_paths = .{}; + this.workspace_versions = .{}; Lockfile.Serializer.load(this, &stream, allocator, log) catch |err| { return LoadFromDiskResult{ .err = .{ .step = .parse_file, .value = err } }; @@ -1516,6 +1519,7 @@ pub fn initEmpty(this: *Lockfile, allocator: Allocator) !void { .scripts = .{}, .trusted_dependencies = .{}, .workspace_paths = .{}, + .workspace_versions = .{}, }; } @@ -2494,6 +2498,7 @@ pub const Package = extern struct { in_workspace: bool, tag: ?Dependency.Version.Tag, workspace_path: ?String, + workspace_version: ?Semver.Version, external_name: ExternalString, version: string, key_loc: logger.Loc, @@ -2529,52 +2534,91 @@ pub const Package = extern struct { ), ); }, + .npm => 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) { + return null; + } + } + + const path = workspace_path.?.sliced(buf); + if (Dependency.parseWithTag( + allocator, + external_name.value, + path.slice, + .workspace, + &path, + log, + )) |dep| { + dependency_version = dep; + } + } + }, .workspace => if (workspace_path) |path| { dependency_version.value.workspace = path; } else { - const workspace = dependency_version.value.workspace.slice(buf); - var path = string_builder.append( - String, - if (strings.eqlComptime(workspace, "*")) "*" else Path.relative( - FileSystem.instance.top_level_dir, - Path.joinAbsString( + { + const workspace = dependency_version.value.workspace.slice(buf); + var path = string_builder.append( + String, + if (strings.eqlComptime(workspace, "*")) "*" else Path.relative( FileSystem.instance.top_level_dir, - &[_]string{ - source.path.name.dir, - workspace, - }, - .posix, + Path.joinAbsString( + FileSystem.instance.top_level_dir, + &[_]string{ + source.path.name.dir, + workspace, + }, + .posix, + ), ), - ), - ); - defer dependency_version.value.workspace = path; - var workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, @as(u32, @truncate(external_name.hash))); - if (workspace_entry.found_existing) { - const old_path = workspace_entry.value_ptr.*; - - if (strings.eqlComptime(workspace, "*")) { - path = old_path; - return null; - } 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 (package_dep.version.tag != .workspace) break :brk; - package_dep.version.value.workspace = path; - return null; + ); + defer dependency_version.value.workspace = path; + var workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, @truncate(external_name.hash)); + if (workspace_entry.found_existing) { + const old_path = workspace_entry.value_ptr.*; + + if (strings.eqlComptime(workspace, "*")) { + path = old_path; + return null; + } 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 (package_dep.version.tag != .workspace) break :brk; + package_dep.version.value.workspace = path; + return null; + } } + return error.InstallFailed; + } else if (strings.eql(old_path.slice(buf), path.slice(buf))) { + return null; + } else { + log.addErrorFmt(&source, logger.Loc.Empty, allocator, "Workspace name \"{s}\" already exists", .{ + external_name.slice(buf), + }) catch {}; + return error.InstallFailed; + } + } + workspace_entry.value_ptr.* = path; + } + + if (workspace_version) |ver| { + try lockfile.workspace_versions.put(allocator, @truncate(external_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 + package_dep.version.value.npm.version.satisfies(ver)) + { + package_dep.version = dependency_version; + return null; } - return error.InstallFailed; - } else if (strings.eql(old_path.slice(buf), path.slice(buf))) { - return null; - } else { - log.addErrorFmt(&source, logger.Loc.Empty, allocator, "Workspace name \"{s}\" already exists", .{ - external_name.slice(buf), - }) catch {}; - return error.InstallFailed; } } - workspace_entry.value_ptr.* = path; }, else => {}, } @@ -2586,7 +2630,7 @@ pub const Package = extern struct { .version = dependency_version, }; - // peerDependencies may be specified on on existing dependencies + // `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); if (entry.found_existing) { @@ -2624,11 +2668,68 @@ pub const Package = extern struct { return this_dep; } - const WorkspaceIterator = struct { + pub const WorkspaceMap = struct { + map: Map, + + const Map = bun.StringArrayHashMap(Entry); pub const Entry = struct { - path: []const u8 = "", - name: []const u8 = "", + name: string, + version: ?Semver.Version, }; + + pub fn init(allocator: std.mem.Allocator) WorkspaceMap { + return .{ + .map = Map.init(allocator), + }; + } + + pub fn keys(self: WorkspaceMap) []const string { + return self.map.keys(); + } + + pub fn values(self: WorkspaceMap) []const Entry { + return self.map.values(); + } + + pub fn count(self: WorkspaceMap) usize { + return self.map.count(); + } + + pub fn insert(self: *WorkspaceMap, key: string, value: Entry) !void { + var entry = try self.map.getOrPut(key); + if (!entry.found_existing) { + entry.key_ptr.* = try self.map.allocator.dupe(u8, key); + } else { + self.map.allocator.free(entry.value_ptr.name); + } + + entry.value_ptr.* = .{ + .name = try self.map.allocator.dupe(u8, value.name), + .version = value.version, + }; + } + + pub fn sort(self: *WorkspaceMap, sort_ctx: anytype) void { + self.map.sort(sort_ctx); + } + + pub fn deinit(self: *WorkspaceMap) void { + for (self.map.values()) |value| { + self.map.allocator.free(value.name); + } + + for (self.map.keys()) |key| { + self.map.allocator.free(key); + } + + self.map.deinit(); + } + }; + + const WorkspaceEntry = struct { + path: []const u8 = "", + name: []const u8 = "", + version: ?Semver.Version = null, }; fn processWorkspaceName( @@ -2639,7 +2740,7 @@ pub const Package = extern struct { path_buf: *[bun.MAX_PATH_BYTES]u8, name_to_copy: *[1024]u8, log: *logger.Log, - ) !WorkspaceIterator.Entry { + ) !WorkspaceEntry { const path_to_use = if (path.len == 0) "package.json" else brk: { const paths = [_]string{ path, "package.json" }; break :brk bun.path.joinStringBuf(path_buf, &paths, .auto); @@ -2658,14 +2759,22 @@ pub const Package = extern struct { return error.MissingPackageName; } bun.copy(u8, name_to_copy[0..], workspace_json.found_name); - return WorkspaceIterator.Entry{ + var entry = WorkspaceEntry{ .name = name_to_copy[0..workspace_json.found_name.len], .path = path_to_use, }; + if (workspace_json.has_found_version) { + const version = SlicedString.init(workspace_json.found_version, workspace_json.found_version); + const result = Semver.Version.parse(version, allocator); + if (result.valid and result.wildcard == .none) { + entry.version = result.version.fill(); + } + } + return entry; } pub fn processWorkspaceNamesArray( - workspace_names: *bun.StringMap, + workspace_names: *WorkspaceMap, allocator: Allocator, log: *logger.Log, arr: *JSAst.E.Array, @@ -2776,7 +2885,10 @@ pub const Package = extern struct { builder.cap += bun.MAX_PATH_BYTES; } - try workspace_names.insert(input_path, workspace_entry.name); + try workspace_names.insert(input_path, .{ + .name = workspace_entry.name, + .version = workspace_entry.version, + }); } if (asterisked_workspace_paths.items.len > 0) { @@ -2910,7 +3022,10 @@ pub const Package = extern struct { break :brk relative; } else bun.span(entry_path); - try workspace_names.insert(workspace_path, workspace_entry.name); + try workspace_names.insert(workspace_path, .{ + .name = workspace_entry.name, + .version = workspace_entry.version, + }); } } } @@ -2919,13 +3034,13 @@ pub const Package = extern struct { // Sort the names for determinism workspace_names.sort(struct { - values: []const string, + values: []const WorkspaceMap.Entry, pub fn lessThan( self: @This(), a: usize, b: usize, ) bool { - return std.mem.order(u8, self.values[a], self.values[b]) == .lt; + return std.mem.order(u8, self.values[a].name, self.values[b].name) == .lt; } }{ .values = workspace_names.values(), @@ -3039,7 +3154,7 @@ pub const Package = extern struct { break :brk out_groups; }; - var workspace_names = bun.StringMap.init(allocator, true); + var workspace_names = WorkspaceMap.init(allocator); defer workspace_names.deinit(); inline for (dependency_groups) |group| { @@ -3317,8 +3432,8 @@ pub const Package = extern struct { inline for (dependency_groups) |group| { if (group.behavior.isWorkspace()) { - for (workspace_names.values(), workspace_names.keys()) |name, path| { - const external_name = string_builder.append(ExternalString, name); + for (workspace_names.values(), workspace_names.keys()) |entry, path| { + const external_name = string_builder.append(ExternalString, entry.name); if (try parseDependency( lockfile, @@ -3333,6 +3448,7 @@ pub const Package = extern struct { in_workspace, .workspace, null, + entry.version, external_name, path, logger.Loc.Empty, @@ -3351,13 +3467,6 @@ pub const Package = extern struct { const value = item.value.?; const external_name = string_builder.append(ExternalString, key.asString(allocator).?); const version = value.asString(allocator) orelse ""; - var tag: ?Dependency.Version.Tag = null; - var workspace_path: ?String = null; - - if (lockfile.workspace_paths.get(@as(u32, @truncate(external_name.hash)))) |path| { - tag = .workspace; - workspace_path = path; - } if (try parseDependency( lockfile, @@ -3370,8 +3479,9 @@ pub const Package = extern struct { package_dependencies, total_dependencies_count, in_workspace, - tag, - workspace_path, + null, + lockfile.workspace_paths.get(@truncate(external_name.hash)), + lockfile.workspace_versions.get(@truncate(external_name.hash)), external_name, version, key.loc, @@ -3587,6 +3697,7 @@ pub fn deinit(this: *Lockfile) void { this.scripts.deinit(this.allocator); this.trusted_dependencies.deinit(this.allocator); this.workspace_paths.deinit(this.allocator); + this.workspace_versions.deinit(this.allocator); } const Buffers = struct { diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index f1d8e0d17..e0c6715c9 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -64,7 +64,7 @@ registry = "http://localhost:${server.port}/" const err = await new Response(stderr).text(); expect(err.split(/\r?\n/)).toContain("error: ConnectionClosed downloading package manifest bar"); expect(stdout).toBeDefined(); - expect(await new Response(stdout).text()).toBe(""); + expect(await new Response(stdout).text()).toBeEmpty(); expect(await exited).toBe(1); try { await access(join(package_dir, "bun.lockb")); @@ -82,7 +82,7 @@ it("should handle missing package", async () => { "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*", ); expect(request.headers.get("npm-auth-type")).toBe(null); - expect(await request.text()).toBe(""); + expect(await request.text()).toBeEmpty(); urls.push(request.url); return new Response("bar", { status: 404 }); }); @@ -98,7 +98,7 @@ it("should handle missing package", async () => { 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(""); + expect(await new Response(stdout).text()).toBeEmpty(); expect(await exited).toBe(1); expect(urls.sort()).toEqual([`${root_url}/foo`]); expect(requested).toBe(1); @@ -126,7 +126,7 @@ it("should handle @scoped authentication", async () => { } else { expect(request.headers.get("npm-auth-type")).toBe(null); } - expect(await request.text()).toBe(""); + expect(await request.text()).toBeEmpty(); urls.push(request.url); return new Response("Feeling lucky?", { status: 555 }); }); @@ -150,7 +150,7 @@ foo = { token = "bar" } 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(""); + expect(await new Response(stdout).text()).toBeEmpty(); expect(await exited).toBe(1); expect(urls.sort()).toEqual([url]); expect(seen_token).toBe(true); @@ -852,7 +852,7 @@ it("should handle ^1 in dependencies", async () => { const err = await new Response(stderr).text(); expect(err).toContain('error: No version matching "^1" found for specifier "bar" (but package exists)'); expect(stdout).toBeDefined(); - expect(await new Response(stdout).text()).toBe(""); + expect(await new Response(stdout).text()).toBeEmpty(); expect(await exited).toBe(1); expect(urls.sort()).toEqual([`${root_url}/bar`]); expect(requested).toBe(1); @@ -932,7 +932,7 @@ it("should handle ^0.1 in dependencies", async () => { const err = await new Response(stderr).text(); expect(err).toContain('error: No version matching "^0.1" found for specifier "bar" (but package exists)'); expect(stdout).toBeDefined(); - expect(await new Response(stdout).text()).toBe(""); + expect(await new Response(stdout).text()).toBeEmpty(); expect(await exited).toBe(1); expect(urls.sort()).toEqual([`${root_url}/bar`]); expect(requested).toBe(1); @@ -969,7 +969,7 @@ it("should handle ^0.0.0 in dependencies", async () => { const err = await new Response(stderr).text(); expect(err).toContain('error: No version matching "^0.0.0" found for specifier "bar" (but package exists)'); expect(stdout).toBeDefined(); - expect(await new Response(stdout).text()).toBe(""); + expect(await new Response(stdout).text()).toBeEmpty(); expect(await exited).toBe(1); expect(urls.sort()).toEqual([`${root_url}/bar`]); expect(requested).toBe(1); @@ -1422,7 +1422,7 @@ it("should not reinstall aliased dependencies", async () => { "Checked 1 installs across 2 packages (no changes)", ]); expect(await exited2).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(2); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "Bar"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["baz-run"]); @@ -1889,7 +1889,7 @@ it("should handle GitHub URL in dependencies (user/repo)", async () => { out = out.replace(/(github:[^#]+)#[a-f0-9]+/, "$1"); expect(out.split(/\r?\n/)).toEqual([" + uglify@github:mishoo/UglifyJS", "", " 1 packages installed"]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -1944,7 +1944,7 @@ it("should handle GitHub URL in dependencies (user/repo#commit-id)", async () => " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2010,7 +2010,7 @@ it("should handle GitHub URL in dependencies (user/repo#tag)", async () => { " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2076,7 +2076,7 @@ it("should handle GitHub URL in dependencies (github:user/repo#tag)", async () = " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2143,7 +2143,7 @@ it("should handle GitHub URL in dependencies (https://github.com/user/repo.git)" out = out.replace(/(github:[^#]+)#[a-f0-9]+/, "$1"); expect(out.split(/\r?\n/)).toEqual([" + uglify@github:mishoo/UglifyJS", "", " 1 packages installed"]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2198,7 +2198,7 @@ it("should handle GitHub URL in dependencies (git://github.com/user/repo.git#com " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2265,7 +2265,7 @@ it("should handle GitHub URL in dependencies (git+https://github.com/user/repo.g out = out.replace(/(github:[^#]+)#[a-f0-9]+/, "$1"); expect(out.split(/\r?\n/)).toEqual([" + uglify@github:mishoo/UglifyJS", "", " 1 packages installed"]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2331,7 +2331,7 @@ cache = false " 12 packages installed", ]); expect(await exited1).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ ".bin", @@ -2384,7 +2384,7 @@ cache = false " 12 packages installed", ]); expect(await exited2).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ ".bin", @@ -2791,7 +2791,7 @@ it("should handle Git URL in dependencies", async () => { " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify-js"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2852,7 +2852,7 @@ it("should handle Git URL in dependencies (SCP-style)", async () => { " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2911,7 +2911,7 @@ it("should handle Git URL with committish in dependencies", async () => { " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "uglify"]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]); @@ -2968,9 +2968,9 @@ it("should fail on invalid Git URL", async () => { expect(err.split(/\r?\n/)).toContain('error: "git clone" for "uglify" failed'); expect(stdout).toBeDefined(); const out = await new Response(stdout).text(); - expect(out).toBe(""); + expect(out).toBeEmpty(); expect(await exited).toBe(1); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); try { await access(join(package_dir, "bun.lockb")); @@ -3008,9 +3008,9 @@ it("should fail on Git URL with invalid committish", async () => { ); expect(stdout).toBeDefined(); const out = await new Response(stdout).text(); - expect(out).toBe(""); + expect(out).toBeEmpty(); expect(await exited).toBe(1); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); try { await access(join(package_dir, "bun.lockb")); @@ -3054,7 +3054,7 @@ it("should de-duplicate committish in Git URLs", async () => { " 1 packages installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ ".bin", @@ -3150,7 +3150,7 @@ cache = false " 12 packages installed", ]); expect(await exited1).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ ".bin", @@ -3203,7 +3203,7 @@ cache = false " 12 packages installed", ]); expect(await exited2).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ ".bin", @@ -3273,7 +3273,7 @@ cache = false " 12 packages installed", ]); expect(await exited3).toBe(0); - expect(urls.sort()).toEqual([]); + expect(urls.sort()).toBeEmpty(); expect(requested).toBe(0); expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ ".bin", @@ -4515,7 +4515,7 @@ cache = false expect(await exited2).toBe(0); expect(await readdirSorted(package_dir)).toEqual(["bun.lockb", "bunfig.toml", "node_modules", "package.json"]); expect(await file(join(package_dir, "package.json")).text()).toEqual(foo_package); - expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([]); + expect(await readdirSorted(join(package_dir, "node_modules"))).toBeEmpty(); }, 20000); it("should handle trustedDependencies", async () => { @@ -4761,3 +4761,214 @@ it("should handle `workspaces:bar` and `workspace:bar` gracefully", async () => expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(bar_package); await access(join(package_dir, "bun.lockb")); }); + +it("should override 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: "*", + }, + }), + ); + await mkdir(join(package_dir, "bar")); + const bar_package = JSON.stringify({ + name: "bar", + version: "0.0.1", + }); + await writeFile(join(package_dir, "bar", "package.json"), bar_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:bar", + "", + " 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("..", "bar")); + expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(bar_package); + await access(join(package_dir, "bun.lockb")); +}); + +it("should not override npm dependency by workspace with mismatched version", async () => { + const urls: string[] = []; + setHandler(dummyRegistry(urls)); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "foo", + workspaces: ["*"], + dependencies: { + bar: "^0.0.2", + }, + }), + ); + await mkdir(join(package_dir, "bar")); + const bar_package = JSON.stringify({ + name: "bar", + version: "0.0.1", + }); + await writeFile(join(package_dir, "bar", "package.json"), bar_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).not.toContain("Saved lockfile"); + expect(err).toContain('error: Duplicate dependency: "bar" specified in package.json'); + expect(stdout).toBeDefined(); + expect(await new Response(stdout).text()).toBeEmpty(); + expect(await exited).toBe(1); + expect(urls.sort()).toEqual([`${root_url}/bar`, `${root_url}/bar-0.0.2.tgz`]); + expect(requested).toBe(2); + try { + await access(join(package_dir, "bun.lockb")); + expect(() => {}).toThrow(); + } catch (err: any) { + expect(err.code).toBe("ENOENT"); + } +}); + +it("should override 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: ["*"], + }), + ); + await mkdir(join(package_dir, "bar")); + const bar_package = JSON.stringify({ + name: "bar", + version: "0.0.1", + }); + await writeFile(join(package_dir, "bar", "package.json"), bar_package); + await mkdir(join(package_dir, "baz")); + await writeFile( + join(package_dir, "baz", "package.json"), + JSON.stringify({ + name: "baz", + version: "0.1.0", + dependencies: { + 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([ + " + bar@workspace:bar", + " + baz@workspace: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", "bar", "baz"]); + expect(await readlink(join(package_dir, "node_modules", "bar"))).toBe(join("..", "bar")); + expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(bar_package); + expect(await readlink(join(package_dir, "node_modules", "baz"))).toBe(join("..", "baz")); + expect(await readdirSorted(join(package_dir, "node_modules", "baz"))).toEqual(["package.json"]); + await access(join(package_dir, "bun.lockb")); +}); + +it("should not override child npm dependency by workspace with mismatched version", async () => { + const urls: string[] = []; + setHandler(dummyRegistry(urls)); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "foo", + workspaces: ["*"], + }), + ); + await mkdir(join(package_dir, "bar")); + const bar_package = JSON.stringify({ + name: "bar", + version: "0.0.1", + }); + await writeFile(join(package_dir, "bar", "package.json"), bar_package); + await mkdir(join(package_dir, "baz")); + await writeFile( + join(package_dir, "baz", "package.json"), + JSON.stringify({ + name: "baz", + version: "0.1.0", + dependencies: { + bar: "^0.0.2", + }, + }), + ); + 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:bar", + " + baz@workspace:baz", + "", + " 3 packages installed", + ]); + expect(await exited).toBe(0); + expect(urls.sort()).toEqual([`${root_url}/bar`, `${root_url}/bar-0.0.2.tgz`]); + expect(requested).toBe(2); + expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "bar", "baz"]); + expect(await readlink(join(package_dir, "node_modules", "bar"))).toBe(join("..", "bar")); + expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(bar_package); + expect(await readlink(join(package_dir, "node_modules", "baz"))).toBe(join("..", "baz")); + expect(await readdirSorted(join(package_dir, "node_modules", "baz", "node_modules"))).toEqual(["bar"]); + expect(await readdirSorted(join(package_dir, "node_modules", "baz", "node_modules", "bar"))).toEqual([ + "package.json", + ]); + expect(await file(join(package_dir, "node_modules", "baz", "node_modules", "bar", "package.json")).json()).toEqual({ + name: "bar", + version: "0.0.2", + }); + await access(join(package_dir, "bun.lockb")); +}); diff --git a/test/js/node/readline/readline.node.test.ts b/test/js/node/readline/readline.node.test.ts index 0ad442eb4..a30afcc82 100644 --- a/test/js/node/readline/readline.node.test.ts +++ b/test/js/node/readline/readline.node.test.ts @@ -143,7 +143,7 @@ describe("readline.clearLine()", () => { }, /ERR_INVALID_ARG_TYPE/); }); - it("shouldn't throw on on null or undefined stream", done => { + it("should not throw on null or undefined stream", done => { const { mustCall } = createCallCheckCtx(done); // Verify that clearLine() does not throw on null or undefined stream. assert.strictEqual(readline.clearLine(null, 0), true); -- cgit v1.2.3