Skip to content

Commit

Permalink
[install] handle bun add of existing peerDependencies correctly (#…
Browse files Browse the repository at this point in the history 
…4028)
  • Loading branch information
@alexlamsl
alexlamsl authored Aug 7, 2023
1 parent a9b3d58 commit b93f304
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 22 deletions.
24 changes: 8 additions & 16 deletions src/install/install.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4881,7 +4881,7 @@ pub const PackageManager = struct {
ast_modifier: {
// Try to use the existing spot in the dependencies list if possible
for (updates) |*update| {
for (dependency_lists_to_check) |list| {
inline for ([_]string{ "dependencies", "devDependencies", "optionalDependencies" }) |list| {
if (current_package_json.asProperty(list)) |query| {
if (query.expr.data == .e_object) {
if (query.expr.asProperty(
Expand Down Expand Up @@ -6111,13 +6111,6 @@ pub const PackageManager = struct {
}
}

const dependency_lists_to_check = [_]string{
"dependencies",
"devDependencies",
"optionalDependencies",
"peerDependencies",
};

fn updatePackageJSONAndInstallWithManager(
ctx: Command.Context,
manager: *PackageManager,
Expand Down Expand Up @@ -6299,21 +6292,20 @@ pub const PackageManager = struct {
}
}

const dependency_list = if (manager.options.update.development)
"devDependencies"
else if (manager.options.update.optional)
"optionalDependencies"
else
"dependencies";
var any_changes = false;

var dependency_list: string = "dependencies";
if (manager.options.update.development) {
dependency_list = "devDependencies";
} else if (manager.options.update.optional) {
dependency_list = "optionalDependencies";
}

switch (op) {
.remove => {
// if we're removing, they don't have to specify where it is installed in the dependencies list
// they can even put it multiple times and we will just remove all of them
for (updates) |update| {
inline for (dependency_lists_to_check) |list| {
inline for ([_]string{ "dependencies", "devDependencies", "optionalDependencies", "peerDependencies" }) |list| {
if (current_package_json.asProperty(list)) |query| {
if (query.expr.data == .e_object) {
var dependencies = query.expr.data.e_object.properties.slice();
Expand Down
66 changes: 60 additions & 6 deletions test/cli/install/bun-add.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ it("should add dependency (GitHub)", 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"]);
Expand Down Expand Up @@ -680,7 +680,7 @@ it("should add aliased dependency (GitHub)", 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"]);
Expand Down Expand Up @@ -993,7 +993,7 @@ it("should handle Git URL in dependencies (SCP-style)", async () => {
" 1 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", ".cache", "uglify-js"]);
expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["uglifyjs"]);
Expand Down Expand Up @@ -1053,7 +1053,7 @@ it("should handle Git URL in dependencies (SCP-style)", 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(0);
}, 20000);

Expand Down Expand Up @@ -1255,7 +1255,7 @@ it("should add dependency without duplication", async () => {
const out2 = await new Response(stdout2).text();
expect(out2.replace(/\s*\[[0-9\.]+m?s\] done\s*$/, "").split(/\r?\n/)).toEqual(["", " installed bar@0.0.2"]);
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([".cache", "bar"]);
expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
Expand Down Expand Up @@ -1480,7 +1480,7 @@ it("should redirect 'install X --save' to 'add'", async () => {
await installRedirectsToAdd(false);
});

async function installRedirectsToAdd(saveFlagFirst) {
async function installRedirectsToAdd(saveFlagFirst: boolean) {
await writeFile(
join(add_dir, "package.json"),
JSON.stringify({
Expand Down Expand Up @@ -1516,3 +1516,57 @@ async function installRedirectsToAdd(saveFlagFirst) {
expect(await exited).toBe(0);
expect((await file(join(package_dir, "package.json")).text()).includes("bun-add.test"));
}

it("should add dependency alongside peerDependencies", async () => {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
peerDependencies: {
bar: "~0.0.1",
},
}),
);
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "add", "bar"],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});
expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).not.toContain("error:");
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([
"",
" installed bar@0.0.2",
"",
"",
" 1 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"]);
expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
expect(await file(join(package_dir, "node_modules", "bar", "package.json")).json()).toEqual({
name: "bar",
version: "0.0.2",
});
expect(await file(join(package_dir, "package.json")).json()).toEqual({
name: "foo",
dependencies: {
bar: "^0.0.2",
},
peerDependencies: {
bar: "~0.0.1",
},
});
await access(join(package_dir, "bun.lockb"));
});
30 changes: 30 additions & 0 deletions test/cli/install/bun-remove.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,3 +287,33 @@ it("should retain a new line in the end of package.json", async () => {
) + "\n",
);
});

it("should remove peerDependencies", async () => {
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
peerDependencies: {
bar: "~0.0.1",
},
}),
);
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "remove", "bar"],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});
expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).not.toContain("error:");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]/, "").split(/\r?\n/)).toEqual([" done", ""]);
expect(await exited).toBe(0);
expect(await file(join(package_dir, "package.json")).json()).toEqual({
name: "foo",
});
});

0 comments on commit b93f304

Please sign in to comment.