feat: enhance wrangler init

This PR adds some enhancements/fixes to the `wrangler init` command.

- doesn't overwrite `wrangler.toml` if it already exists
- installs `wrangler` when creating `package.json`
- offers to install `wrangler` into `package.json` even if `package.json` already exists
- offers to install `@cloudflare/workers-types` even if `tsconfig.json` already exists
- pipes stdio back to the terminal so there's feedback when it's installing npm packages

This does have the side effect of making our tests a _little_ slower. I added `--prefer-offline` to the `npm install` calls to make this a shade quicker, but I can't figure out a good way of mocking these. I'll think about it some more later. We should work on making the installs themselves quicker (re: #66)

This PR also fixes an issue with our tests: `runWrangler()` would catch thrown errors, and if we didn't manually verify the error, tests would pass. Instead, it now throws correctly, and I modified all the tests to assert on thrown errors. It seems like a lot, but it was just mechanical rewriting.

Author: threepointone

.changeset/sour-toys-heal.md

@@ -0,0 +1,17 @@
+---
+"wrangler": patch
+---
+
+feat: enhance `wrangler init`
+
+This PR adds some enhancements/fixes to the `wrangler init` command.
+
+- doesn't overwrite `wrangler.toml` if it already exists
+- installs `wrangler` when creating `package.json`
+- offers to install `wrangler` into `package.json` even if `package.json` already exists
+- offers to install `@cloudflare/workers-types` even if `tsconfig.json` already exists
+- pipes stdio back to the terminal so there's feedback when it's installing npm packages
+
+This does have the side effect of making out tests slower. I added `--prefer-offline` to the `npm install` calls to make this a shade quicker, but I can't figure out a good way of mocking these. I'll think about it some more later. We should work on making the installs themselves quicker (re: https://github.com/cloudflare/wrangler2/issues/66)
+
+This PR also fixes a bug with our tests - `runWrangler` would catch thrown errors, and if we didn't manually verify the error, tests would pass. Instead, it now throws correctly, and I modified all the tests to assert on thrown errors. It seems like a lot, but it was just mechanical rewriting.

packages/wrangler/package.json

@@ -107,6 +107,7 @@
   },
   "jest": {
     "restoreMocks": true,
+    "testTimeout": 30000,
     "testRegex": ".*.(test|spec)\\.[jt]sx?$",
     "transformIgnorePatterns": [
       "node_modules/(?!node-fetch|fetch-blob|find-up|locate-path|p-locate|p-limit|yocto-queue|path-exists|data-uri-to-buffer|formdata-polyfill|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream)"

packages/wrangler/src/__tests__/index.test.ts

@@ -3,16 +3,19 @@ import * as TOML from "@iarna/toml";
 import { mockConfirm } from "./mock-dialogs";
 import { runWrangler } from "./run-wrangler";
 import { runInTempDir } from "./run-in-tmp";
+import { mockConsoleMethods } from "./mock-console";
 import * as fs from "node:fs";
 
 describe("wrangler", () => {
   runInTempDir();
 
+  const std = mockConsoleMethods();
+
   describe("no command", () => {
     it("should display a list of available commands", async () => {
-      const { stdout, stderr } = await runWrangler();
+      await runWrangler();
 
-      expect(stdout).toMatchInlineSnapshot(`
+      expect(std.out).toMatchInlineSnapshot(`
         "wrangler
 
         Commands:
@@ -36,16 +39,23 @@ describe("wrangler", () => {
           -l, --local  Run on my machine  [boolean] [default: false]"
       `);
 
-      expect(stderr).toMatchInlineSnapshot(`""`);
+      expect(std.err).toMatchInlineSnapshot(`""`);
     });
   });
 
   describe("invalid command", () => {
     it("should display an error", async () => {
-      const { error, stdout, stderr } = await runWrangler("invalid-command");
-
-      expect(stdout).toMatchInlineSnapshot(`""`);
-      expect(stderr).toMatchInlineSnapshot(`
+      let err: Error | undefined;
+      try {
+        await runWrangler("invalid-command");
+      } catch (e) {
+        err = e;
+      } finally {
+        expect(err?.message).toBe(`Unknown command: invalid-command.`);
+      }
+
+      expect(std.out).toMatchInlineSnapshot(`""`);
+      expect(std.err).toMatchInlineSnapshot(`
         "wrangler
 
         Commands:
@@ -70,9 +80,6 @@ describe("wrangler", () => {
 
         Unknown command: invalid-command."
       `);
-      expect(error).toMatchInlineSnapshot(
-        `[Error: Unknown command: invalid-command.]`
-      );
     });
   });
 
@@ -90,15 +97,19 @@ describe("wrangler", () => {
     });
 
     it("should display warning when wrangler.toml already exists, and exit if user does not want to carry on", async () => {
-      fs.writeFileSync("./wrangler.toml", "", "utf-8");
+      fs.writeFileSync(
+        "./wrangler.toml",
+        'compatibility_date="something-else"', // use a fake value to make sure the file is not overwritten
+        "utf-8"
+      );
       mockConfirm({
         text: "Do you want to continue initializing this project?",
         result: false,
       });
-      const { warnings } = await runWrangler("init");
-      expect(warnings).toContain("wrangler.toml file already exists!");
+      await runWrangler("init");
+      expect(std.warn).toContain("wrangler.toml file already exists!");
       const parsed = TOML.parse(await fsp.readFile("./wrangler.toml", "utf-8"));
-      expect(typeof parsed.compatibility_date).toBe("undefined");
+      expect(parsed.compatibility_date).toBe("something-else");
     });
 
     it("should display warning when wrangler.toml already exists, but continue if user does want to carry on", async () => {
@@ -113,10 +124,8 @@ describe("wrangler", () => {
           result: false,
         }
       );
-      const { warnings } = await runWrangler("init");
-      expect(warnings).toContain("wrangler.toml file already exists!");
-      const parsed = TOML.parse(await fsp.readFile("./wrangler.toml", "utf-8"));
-      expect(typeof parsed.compatibility_date).toBe("string");
+      await runWrangler("init");
+      expect(std.warn).toContain("wrangler.toml file already exists!");
     });
 
     it("should create a package.json if none is found and user confirms", async () => {
@@ -137,14 +146,23 @@ describe("wrangler", () => {
       );
       expect(packageJson.name).toEqual("worker"); // TODO: should we infer the name from the directory?
       expect(packageJson.version).toEqual("0.0.1");
+      expect(packageJson.devDependencies).toEqual({
+        wrangler: expect.any(String),
+      });
       expect(fs.existsSync("./tsconfig.json")).toBe(false);
     });
 
     it("should not touch an existing package.json in the same directory", async () => {
-      mockConfirm({
-        text: "Would you like to use typescript?",
-        result: false,
-      });
+      mockConfirm(
+        {
+          text: "Would you like to install wrangler into your package.json?",
+          result: false,
+        },
+        {
+          text: "Would you like to use typescript?",
+          result: false,
+        }
+      );
 
       fs.writeFileSync(
         "./package.json",
@@ -160,11 +178,46 @@ describe("wrangler", () => {
       expect(packageJson.version).toEqual("1.0.0");
     });
 
-    it("should not touch an existing package.json in an ancestor directory", async () => {
-      mockConfirm({
-        text: "Would you like to use typescript?",
-        result: false,
+    it("should offer to install wrangler into an existing package.json", async () => {
+      mockConfirm(
+        {
+          text: "Would you like to install wrangler into your package.json?",
+          result: true,
+        },
+        {
+          text: "Would you like to use typescript?",
+          result: false,
+        }
+      );
+
+      fs.writeFileSync(
+        "./package.json",
+        JSON.stringify({ name: "test", version: "1.0.0" }),
+        "utf-8"
+      );
+
+      await runWrangler("init");
+      const packageJson = JSON.parse(
+        fs.readFileSync("./package.json", "utf-8")
+      );
+      expect(packageJson.name).toEqual("test");
+      expect(packageJson.version).toEqual("1.0.0");
+      expect(packageJson.devDependencies).toEqual({
+        wrangler: expect.any(String),
       });
+    });
+
+    it("should not touch an existing package.json in an ancestor directory", async () => {
+      mockConfirm(
+        {
+          text: "Would you like to install wrangler into your package.json?",
+          result: false,
+        },
+        {
+          text: "Would you like to use typescript?",
+          result: false,
+        }
+      );
 
       fs.writeFileSync(
         "./package.json",
@@ -182,8 +235,12 @@ describe("wrangler", () => {
       const packageJson = JSON.parse(
         fs.readFileSync("../../package.json", "utf-8")
       );
-      expect(packageJson.name).toEqual("test");
-      expect(packageJson.version).toEqual("1.0.0");
+      expect(packageJson).toMatchInlineSnapshot(`
+        Object {
+          "name": "test",
+          "version": "1.0.0",
+        }
+      `);
     });
 
     it("should create a tsconfig.json and install `workers-types` if none is found and user confirms", async () => {
@@ -210,13 +267,21 @@ describe("wrangler", () => {
       );
       expect(packageJson.devDependencies).toEqual({
         "@cloudflare/workers-types": expect.any(String),
+        wrangler: expect.any(String),
       });
     });
 
     it("should not touch an existing tsconfig.json in the same directory", async () => {
       fs.writeFileSync(
         "./package.json",
-        JSON.stringify({ name: "test", version: "1.0.0" }),
+        JSON.stringify({
+          name: "test",
+          version: "1.0.0",
+          devDependencies: {
+            wrangler: "0.0.0",
+            "@cloudflare/workers-types": "0.0.0",
+          },
+        }),
         "utf-8"
       );
       fs.writeFileSync(
@@ -232,10 +297,56 @@ describe("wrangler", () => {
       expect(tsconfigJson.compilerOptions).toEqual({});
     });
 
-    it("should not touch an existing package.json in an ancestor directory", async () => {
+    it("should offer to install type definitions in an existing typescript project", async () => {
+      mockConfirm(
+        {
+          text: "Would you like to install wrangler into your package.json?",
+          result: false,
+        },
+        {
+          text: "Would you like to install the type definitions for Workers into your package.json?",
+          result: true,
+        }
+      );
       fs.writeFileSync(
         "./package.json",
-        JSON.stringify({ name: "test", version: "1.0.0" }),
+        JSON.stringify({
+          name: "test",
+          version: "1.0.0",
+        }),
+        "utf-8"
+      );
+      fs.writeFileSync(
+        "./tsconfig.json",
+        JSON.stringify({ compilerOptions: {} }),
+        "utf-8"
+      );
+
+      await runWrangler("init");
+      const tsconfigJson = JSON.parse(
+        fs.readFileSync("./tsconfig.json", "utf-8")
+      );
+      // unchanged tsconfig
+      expect(tsconfigJson.compilerOptions).toEqual({});
+      const packageJson = JSON.parse(
+        fs.readFileSync("./package.json", "utf-8")
+      );
+      expect(packageJson.devDependencies).toEqual({
+        "@cloudflare/workers-types": expect.any(String),
+      });
+    });
+
+    it("should not touch an existing tsconfig.json in an ancestor directory", async () => {
+      fs.writeFileSync(
+        "./package.json",
+        JSON.stringify({
+          name: "test",
+          version: "1.0.0",
+          devDependencies: {
+            wrangler: "0.0.0",
+            "@cloudflare/workers-types": "0.0.0",
+          },
+        }),
         "utf-8"
       );
       fs.writeFileSync(
@@ -258,32 +369,50 @@ describe("wrangler", () => {
     });
 
     it("should error if `--type` is used", async () => {
-      const { error } = await runWrangler("init --type");
-      expect(error).toMatchInlineSnapshot(
-        `[Error: The --type option is no longer supported.]`
-      );
+      let err: undefined | Error;
+      try {
+        await runWrangler("init --type");
+      } catch (e) {
+        err = e;
+      } finally {
+        expect(err?.message).toBe(`The --type option is no longer supported.`);
+      }
     });
 
     it("should error if `--type javascript` is used", async () => {
-      const { error } = await runWrangler("init --type javascript");
-      expect(error).toMatchInlineSnapshot(
-        `[Error: The --type option is no longer supported.]`
-      );
+      let err: undefined | Error;
+      try {
+        await runWrangler("init --type javascript");
+      } catch (e) {
+        err = e;
+      } finally {
+        expect(err?.message).toBe(`The --type option is no longer supported.`);
+      }
     });
 
     it("should error if `--type rust` is used", async () => {
-      const { error } = await runWrangler("init --type rust");
-      expect(error).toMatchInlineSnapshot(
-        `[Error: The --type option is no longer supported.]`
-      );
+      let err: undefined | Error;
+      try {
+        await runWrangler("init --type rust");
+      } catch (e) {
+        err = e;
+      } finally {
+        expect(err?.message).toBe(`The --type option is no longer supported.`);
+      }
     });
 
     it("should error if `--type webpack` is used", async () => {
-      const { error } = await runWrangler("init --type webpack");
-      expect(error).toMatchInlineSnapshot(`
-        [Error: The --type option is no longer supported.
-        If you wish to use webpack then you will need to create a custom build.]
-      `);
+      let err: undefined | Error;
+      try {
+        await runWrangler("init --type webpack");
+      } catch (e) {
+        err = e;
+      } finally {
+        expect(err?.message).toBe(
+          `The --type option is no longer supported.
+If you wish to use webpack then you will need to create a custom build.`
+        );
+      }
     });
   });
 });