mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-10 10:58:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix bun add with multiple packages (#20987)

Browse Source 
Co-authored-by: Meghan Denny <meghan@bun.sh>
This commit is contained in:
taylor.fish
2025-07-12 02:10:45 -07:00
committed by GitHub
parent 7827df5745
commit 0baf5b94e2
3 changed files with 94 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
src/install/PackageManager/PackageJSONEditor.zig
View File

@@ -407,10 +407,7 @@ pub fn edit(
inline for ([_]string{ "dependencies", "devDependencies", "optionalDependencies", "peerDependencies" }) |list| {
if (current_package_json.asProperty(list)) |query| {
if (query.expr.data == .e_object) {
const name = if (request.is_aliased)
request.name
else
request.version.literal.slice(request.version_buf);
const name = request.getName();
if (query.expr.asProperty(name)) |value| {
if (value.expr.data == .e_string) {
@@ -537,69 +534,37 @@ pub fn edit(
break :brk deps;
};
outer: for (updates.*) |*request| {
for (updates.*) |*request| {
if (request.e_string != null) continue;
defer if (comptime Environment.allow_assert) bun.assert(request.e_string != null);
var k: usize = 0;
while (k < new_dependencies.len) : (k += 1) {
if (new_dependencies[k].key) |key| {
if (!request.is_aliased and request.package_id != invalid_package_id and key.data.e_string.eql(
string,
manager.lockfile.packages.items(.name)[request.package_id].slice(request.version_buf),
)) {
// This actually is a duplicate which we did not
// pick up before dependency resolution.
// For this case, we'll just swap remove it.
if (new_dependencies.len > 1) {
new_dependencies[k] = new_dependencies[new_dependencies.len - 1];
new_dependencies = new_dependencies[0 .. new_dependencies.len - 1];
} else {
new_dependencies = &[_]G.Property{};
}
continue;
}
if (key.data.e_string.eql(
string,
if (request.is_aliased)
request.name
else
request.version.literal.slice(request.version_buf),
)) {
if (request.package_id == invalid_package_id) {
// This actually is a duplicate like "react"
// appearing in both "dependencies" and "optionalDependencies".
// For this case, we'll just swap remove it
if (new_dependencies.len > 1) {
new_dependencies[k] = new_dependencies[new_dependencies.len - 1];
new_dependencies = new_dependencies[0 .. new_dependencies.len - 1];
} else {
new_dependencies = &[_]G.Property{};
}
continue;
}
new_dependencies[k].key = null;
const name = request.getName();
if (!key.data.e_string.eql(string, name)) continue;
if (request.package_id == invalid_package_id) {
// Duplicate dependency (e.g., "react" in both "dependencies" and
// "optionalDependencies"). Remove the old dependency.
new_dependencies[k] = .{};
new_dependencies = new_dependencies[0 .. new_dependencies.len - 1];
}
}
if (new_dependencies[k].key == null) {
new_dependencies[k].key = JSAst.Expr.allocate(
allocator,
JSAst.E.String,
.{ .data = try allocator.dupe(u8, request.getResolvedName(manager.lockfile)) },
logger.Loc.Empty,
);
new_dependencies[k].key = JSAst.Expr.allocate(
allocator,
JSAst.E.String,
.{ .data = try allocator.dupe(u8, request.getResolvedName(manager.lockfile)) },
logger.Loc.Empty,
);
new_dependencies[k].value = JSAst.Expr.allocate(allocator, JSAst.E.String, .{
// we set it later
.data = "",
}, logger.Loc.Empty);
new_dependencies[k].value = JSAst.Expr.allocate(allocator, JSAst.E.String, .{
// we set it later
.data = "",
}, logger.Loc.Empty);
request.e_string = new_dependencies[k].value.?.data.e_string;
if (request.is_aliased) continue :outer;
}
request.e_string = new_dependencies[k].value.?.data.e_string;
break;
}
}

21
src/install/PackageManager/UpdateRequest.zig
View File

@@ -17,6 +17,21 @@ pub inline fn matches(this: PackageManager.UpdateRequest, dependency: Dependency
dependency.name_hash;
}
pub fn getName(this: *const UpdateRequest) string {
return if (this.is_aliased)
this.name
else
this.version.literal.slice(this.version_buf);
}
/// If `this.package_id` is not `invalid_package_id`, it must be less than `lockfile.packages.len`.
pub fn getNameInLockfile(this: *const UpdateRequest, lockfile: *const Lockfile) ?string {
return if (this.package_id == invalid_package_id)
null
else
lockfile.packages.items(.name)[this.package_id].slice(this.version_buf);
}
/// It is incorrect to call this function before Lockfile.cleanWithLogger() because
/// resolved_name should be populated if possible.
///
@@ -25,10 +40,10 @@ pub inline fn matches(this: PackageManager.UpdateRequest, dependency: Dependency
pub fn getResolvedName(this: *const UpdateRequest, lockfile: *const Lockfile) string {
return if (this.is_aliased)
this.name
else if (this.package_id == invalid_package_id)
this.version.literal.slice(this.version_buf)
else if (this.getNameInLockfile(lockfile)) |name|
name
else
lockfile.packages.items(.name)[this.package_id].slice(this.version_buf);
this.version.literal.slice(this.version_buf);
}
pub fn fromJS(globalThis: *JSC.JSGlobalObject, input: JSC.JSValue) bun.JSError!JSC.JSValue {

56
test/cli/install/bun-add.test.ts
View File

@@ -187,6 +187,7 @@ it("bun add --only-missing should not install existing package", async () => {
stdout: "pipe",
stdin: "pipe",
stderr: "pipe",
env,
});
const out = await new Response(stdout).text();
expect(out).not.toContain("Saved lockfile");
@@ -260,6 +261,7 @@ it("bun add --analyze should scan dependencies", async () => {
stdout: "pipe",
stdin: "pipe",
stderr: "pipe",
env,
});
const out = await new Response(stdout).text();
expect(out).not.toContain("Saved lockfile");
@@ -601,7 +603,7 @@ it("should add to devDependencies with --dev", async () => {
);
await access(join(package_dir, "bun.lockb"));
});
it.only("should add to optionalDependencies with --optional", async () => {
it("should add to optionalDependencies with --optional", async () => {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
await writeFile(
@@ -818,6 +820,7 @@ it("should add exact version with -E", async () => {
});
it("should add dependency with package.json in it and http tarball", async () => {
const old_check_npm_auth_type = check_npm_auth_type.check;
check_npm_auth_type.check = false;
using server = Bun.serve({
port: 0,
@@ -902,6 +905,8 @@ it("should add dependency with package.json in it and http tarball", async () =>
},
});
await access(join(package_dir, "bun.lockb"));
// Reset to old value for other tests
check_npm_auth_type.check = old_check_npm_auth_type;
});
it("should add dependency with specified semver", async () => {
@@ -2269,3 +2274,52 @@ it("should add local tarball dependency", async () => {
(expect(await file(join(package_dir, "package.json")).text()).toInclude('"baz-0.0.3.tgz"'),
await access(join(package_dir, "bun.lockb")));
});
it("should add multiple dependencies specified on command line", async () => {
expect(check_npm_auth_type.check).toBe(true);
using server = Bun.serve({
port: 0,
fetch(req) {
return new Response(Bun.file(join(__dirname, "baz-0.0.3.tgz")));
},
});
const server_url = server.url.href.replace(/\/+$/, "");
const urls: string[] = [];
setHandler(dummyRegistry(urls));
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "add", `${server_url}/baz-0.0.3.tgz`, "bar"],
cwd: package_dir,
stdout: "pipe",
stdin: "pipe",
stderr: "pipe",
env,
});
await writeFile(
join(add_dir, "package.json"),
JSON.stringify({
name: "foo",
version: "0.0.1",
}),
);
const err = await new Response(stderr).text();
expect(err).not.toContain("error:");
expect(err).toContain("Saved lockfile");
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
expect.stringMatching(/^bun add v1./),
"",
expect.stringMatching(/^installed baz@http:\/\/.* with binaries:$/),
" - baz-run",
"installed bar@0.0.2",
"",
"2 packages installed",
]);
expect(await exited).toBe(0);
expect(await file(join(package_dir, "package.json")).json()).toStrictEqual({
dependencies: {
bar: "^0.0.2",
baz: `${server_url}/baz-0.0.3.tgz`,
},
});
await access(join(package_dir, "bun.lockb"));
});