Fix to retain a newline after removing a package (#3231)

2 files changed, 289 insertions, 0 deletions

diff --git a/src/install/install.zig b/src/install/install.zig
index 209f2eb96..6ff76d421 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -6220,6 +6220,7 @@ pub const PackageManager = struct {
 
         var buffer_writer = try JSPrinter.BufferWriter.init(ctx.allocator);
         try buffer_writer.buffer.list.ensureTotalCapacity(ctx.allocator, current_package_json_buf.len + 1);
+        buffer_writer.append_newline = preserve_trailing_newline_at_eof_for_package_json;
         var package_json_writer = JSPrinter.BufferPrinter.init(buffer_writer);
 
         var written = JSPrinter.printJSON(@TypeOf(&package_json_writer), &package_json_writer, current_package_json, &package_json_source) catch |err| {
diff --git a/test/cli/install/bun-remove.test.ts b/test/cli/install/bun-remove.test.ts
new file mode 100644
index 000000000..5a6f612c4
--- /dev/null
+++ b/test/cli/install/bun-remove.test.ts
@@ -0,0 +1,288 @@
+import { bunExe, bunEnv as env } from "harness";
+import { access, mkdir, mkdtemp, readlink, realpath, rm, writeFile } from "fs/promises";
+import { join, relative } from "path";
+import { tmpdir } from "os";
+import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "bun:test";
+import {
+  dummyAfterAll,
+  dummyAfterEach,
+  dummyBeforeAll,
+  dummyBeforeEach,
+  dummyRegistry,
+  package_dir,
+  readdirSorted,
+  requested,
+  root_url,
+  setHandler,
+} from "./dummy.registry";
+import { spawn, write } from "bun";
+import { file } from "bun";
+
+beforeAll(dummyBeforeAll);
+afterAll(dummyAfterAll);
+
+let remove_dir: string;
+
+beforeEach(async () => {
+  remove_dir = await mkdtemp(join(await realpath(tmpdir()), "bun-remove.test"));
+  await dummyBeforeEach();
+});
+
+afterEach(async () => {
+  await rm(remove_dir, { force: true, recursive: true });
+  await dummyAfterEach();
+});
+
+it("should remove existing package", async () => {
+  const pkg1_dir = join(remove_dir, "pkg1");
+  const pkg1_path = relative(package_dir, pkg1_dir);
+  await mkdir(pkg1_dir);
+  const pkg2_dir = join(remove_dir, "pkg2");
+  const pkg2_path = relative(package_dir, pkg2_dir);
+  await mkdir(pkg2_dir);
+
+  await writeFile(
+    join(pkg1_dir, "package.json"),
+    JSON.stringify({
+      name: "pkg1",
+      version: "0.0.1",
+    }),
+  );
+  await writeFile(
+    join(pkg2_dir, "package.json"),
+    JSON.stringify({
+      name: "pkg2",
+      version: "0.0.1",
+    }),
+  );
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.2",
+    }),
+  );
+  const { exited: exited1 } = spawn({
+    cmd: [bunExe(), "add", `file:${pkg1_path}`],
+    cwd: package_dir,
+    env,
+  });
+  expect(await exited1).toBe(0);
+  const { exited: exited2 } = spawn({
+    cmd: [bunExe(), "add", `file:${pkg2_path}`],
+    cwd: package_dir,
+    env,
+  });
+  expect(await exited2).toBe(0);
+  expect(await file(join(package_dir, "package.json")).text()).toEqual(
+    JSON.stringify(
+      {
+        name: "foo",
+        version: "0.0.2",
+        dependencies: {
+          pkg1: `file:${pkg1_path}`,
+          pkg2: `file:${pkg2_path}`,
+        },
+      },
+      null,
+      2,
+    ),
+  );
+
+  const {
+    exited: removeExited1,
+    stdout: stdout1,
+    stderr: stderr1,
+  } = spawn({
+    cmd: [bunExe(), "remove", "pkg1"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(await removeExited1).toBe(0);
+  expect(stdout1).toBeDefined();
+  const out1 = await new Response(stdout1).text();
+  expect(out1.replace(/\s*\[[0-9\.]+m?s\]/, "").split(/\r?\n/)).toEqual([
+    ` + pkg2@${pkg2_path}`,
+    "",
+    " 1 packages installed",
+    "  Removed: 1",
+    "",
+  ]);
+  expect(stderr1).toBeDefined();
+  const err1 = await new Response(stderr1).text();
+  expect(err1.replace(/^(.*?) v[^\n]+/, "$1").split(/\r?\n/)).toEqual(["bun remove", " Saved lockfile", ""]);
+  expect(await file(join(package_dir, "package.json")).text()).toEqual(
+    JSON.stringify(
+      {
+        name: "foo",
+        version: "0.0.2",
+        dependencies: {
+          pkg2: `file:${pkg2_path}`,
+        },
+      },
+      null,
+      2,
+    ),
+  );
+
+  const {
+    exited: removeExited2,
+    stdout: stdout2,
+    stderr: stderr2,
+  } = spawn({
+    cmd: [bunExe(), "remove", "pkg2"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(await removeExited2).toBe(0);
+  expect(stdout2).toBeDefined();
+  const out2 = await new Response(stdout2).text();
+  expect(out2.replace(/\s*\[[0-9\.]+m?s\]/, "").split(/\r?\n/)).toEqual([" done", ""]);
+  expect(stderr2).toBeDefined();
+  const err2 = await new Response(stderr2).text();
+  expect(err2.replace(/^(.*?) v[^\n]+/, "$1").split(/\r?\n/)).toEqual([
+    "bun remove",
+    "No packages! Deleted empty lockfile",
+    "",
+  ]);
+  expect(await file(join(package_dir, "package.json")).text()).toEqual(
+    JSON.stringify(
+      {
+        name: "foo",
+        version: "0.0.2",
+      },
+      null,
+      2,
+    ),
+  );
+});
+
+it("should reject missing package", async () => {
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+    }),
+  );
+  await writeFile(
+    join(remove_dir, "package.json"),
+    JSON.stringify({
+      name: "pkg1",
+      version: "0.0.2",
+    }),
+  );
+  const pkg_path = relative(package_dir, remove_dir);
+  const { exited: addExited } = spawn({
+    cmd: [bunExe(), "add", `file:${pkg_path}`],
+    cwd: package_dir,
+    env,
+  });
+  expect(await addExited).toBe(0);
+
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "remove", "pkg2"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(await exited).toBe(1);
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out).toEqual("");
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err.replace(/^(.*?) v[^\n]+/, "$1").split(/\r?\n/)).toEqual([
+    "bun remove",
+    "",
+    `error: "pkg2" is not in a package.json file`,
+    "",
+  ]);
+});
+
+it("should not affect if package is not installed", async () => {
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+    }),
+  );
+
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "remove", "pkg"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(await exited).toBe(0);
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out).toEqual("");
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err.replace(/^(.*?) v[^\n]+/, "$1").split(/\r?\n/)).toEqual([
+    "bun remove",
+    "package.json doesn't have dependencies, there's nothing to remove!",
+    "",
+  ]);
+});
+
+it("should retain a new line in the end of package.json", async () => {
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+    }),
+  );
+  await writeFile(
+    join(remove_dir, "package.json"),
+    JSON.stringify({
+      name: "pkg",
+      version: "0.0.2",
+    }),
+  );
+  const pkg_path = relative(package_dir, remove_dir);
+  const { exited: addExited } = spawn({
+    cmd: [bunExe(), "add", `file:${pkg_path}`],
+    cwd: package_dir,
+    env,
+  });
+  expect(await addExited).toBe(0);
+  const content_before_remove = await file(join(package_dir, "package.json")).text();
+  expect(content_before_remove.endsWith("}")).toBe(true);
+  await writeFile(join(package_dir, "package.json"), content_before_remove + "\n");
+
+  const { exited } = spawn({
+    cmd: [bunExe(), "remove", "pkg"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(await exited).toBe(0);
+  const content_after_remove = await file(join(package_dir, "package.json")).text();
+  expect(content_after_remove.endsWith("}\n")).toBe(true);
+  expect(content_after_remove).toEqual(
+    JSON.stringify(
+      {
+        name: "foo",
+        version: "0.0.1",
+      },
+      null,
+      2,
+    ) + "\n",
+  );
+});