Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[install] handle bun add of existing peerDependencies correctly #4028

Merged
merged 1 commit into from
Aug 7, 2023
Merged

[install] handle bun add of existing peerDependencies correctly #4028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions src/install/install.zig
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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",
});
});