fix: consolidate getEntry() logic (#567)

This consolidates some logic into `getEntry()`, namely including `guessWorkerFormat()` and custom builds. This simplifies the code for both `dev` and `publish`.

- Previously, the implementation of custom builds inside `dev` assumed it could be a long running process; however it's not (else consider that `publish` would never work).
- By running custom builds inside `getEntry()`, we can be certain that the entry point exists as we validate it and before we enter `dev`/`publish`, simplifying their internals
- We don't have to do periodic checks inside `wrangler dev` because it's now a one shot build (and always should have been)
- This expands test coverage a little for both `dev` and `publish`.
- The 'format' of a worker is intrinsic to its contents, so it makes sense to establish its value inside `getEntry()`
- This also means less async logic inside `<Dev/>`, which is always a good thing

.changeset/clean-trainers-clap.md

@@ -0,0 +1,14 @@
+---
+"wrangler": patch
+---
+
+fix: consolidate `getEntry()` logic
+
+This consolidates some logic into `getEntry()`, namely including `guessWorkerFormat()` and custom builds. This simplifies the code for both `dev` and `publish`.
+
+- Previously, the implementation of custom builds inside `dev` assumed it could be a long running process; however it's not (else consider that `publish` would never work).
+- By running custom builds inside `getEntry()`, we can be certain that the entry point exists as we validate it and before we enter `dev`/`publish`, simplifying their internals
+- We don't have to do periodic checks inside `wrangler dev` because it's now a one shot build (and always should have been)
+- This expands test coverage a little for both `dev` and `publish`.
+- The 'format' of a worker is intrinsic to its contents, so it makes sense to establish its value inside `getEntry()`
+- This also means less async logic inside `<Dev/>`, which is always a good thing

packages/wrangler/src/__tests__/dev.test.tsx

@@ -1,4 +1,5 @@
 import * as fs from "node:fs";
+import { readFileSync } from "node:fs";
 import patchConsole from "patch-console";
 import Dev from "../dev";
 import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
@@ -9,7 +10,6 @@ import { runWrangler } from "./helpers/run-wrangler";
 import writeWranglerToml from "./helpers/write-wrangler-toml";
 
 describe("Dev component", () => {
-  beforeEach(() => {});
   mockAccountId();
   mockApiToken();
   runInTempDir();
@@ -231,6 +231,82 @@ describe("Dev component", () => {
       );
     });
   });
+
+  describe("custom builds", () => {
+    it("should run a custom build before starting `dev`", async () => {
+      writeWranglerToml({
+        build: {
+          command: `node -e "console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')"`,
+        },
+      });
+
+      await runWrangler("dev index.js");
+
+      expect(readFileSync("index.js", "utf-8")).toMatchInlineSnapshot(
+        `"export default { fetch(){ return new Response(123) } }"`
+      );
+
+      // and the command would pass through
+      expect((Dev as jest.Mock).mock.calls[0][0].buildCommand).toEqual({
+        command:
+          "node -e \"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\"",
+        cwd: undefined,
+        watch_dir: undefined,
+      });
+      expect(std.out).toMatchInlineSnapshot(
+        `"running: node -e \\"console.log('custom build'); require('fs').writeFileSync('index.js', 'export default { fetch(){ return new Response(123) } }')\\""`
+      );
+      expect(std.err).toMatchInlineSnapshot(`""`);
+      expect(std.warn).toMatchInlineSnapshot(`""`);
+    });
+
+    if (process.platform !== "win32") {
+      it("should run a custom build of multiple steps combined by && before starting `dev`", async () => {
+        writeWranglerToml({
+          build: {
+            command: `echo "custom build" && echo "export default { fetch(){ return new Response(123) } }" > index.js`,
+          },
+        });
+
+        await runWrangler("dev index.js");
+
+        expect(readFileSync("index.js", "utf-8")).toMatchInlineSnapshot(`
+          "export default { fetch(){ return new Response(123) } }
+          "
+        `);
+
+        expect(std.out).toMatchInlineSnapshot(
+          `"running: echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123) } }\\" > index.js"`
+        );
+        expect(std.err).toMatchInlineSnapshot(`""`);
+        expect(std.warn).toMatchInlineSnapshot(`""`);
+      });
+    }
+
+    it("should throw an error if the entry doesn't exist after the build finishes", async () => {
+      writeWranglerToml({
+        main: "index.js",
+        build: {
+          command: `node -e "console.log('custom build');"`,
+        },
+      });
+
+      await expect(
+        runWrangler("dev")
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\""`
+      );
+      expect(std.out).toMatchInlineSnapshot(
+        `"running: node -e \\"console.log('custom build');\\""`
+      );
+      expect(std.err).toMatchInlineSnapshot(`
+        "Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\"
+
+        %s If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
+      `);
+      expect(std.warn).toMatchInlineSnapshot(`""`);
+    });
+  });
 });
 
 function mockGetZones(domain: string, zones: { id: string }[] = []) {

packages/wrangler/src/__tests__/guess-worker-format.test.ts

@@ -1,6 +1,6 @@
 import { writeFile } from "fs/promises";
 import path from "path";
-import guessWorkerFormat from "../guess-worker-format";
+import guessWorkerFormat from "../entry";
 import { runInTempDir } from "./helpers/run-in-tmp";
 
 describe("guess worker format", () => {
@@ -10,10 +10,8 @@ describe("guess worker format", () => {
     // Note that this isn't actually a valid worker, because it's missing
     // a fetch handler. Regardless, our heuristic is simply to check for exports.
     const guess = await guessWorkerFormat(
-      {
-        file: path.join(process.cwd(), "./index.ts"),
-        directory: process.cwd(),
-      },
+      path.join(process.cwd(), "./index.ts"),
+      process.cwd(),
       undefined
     );
     expect(guess).toBe("modules");
@@ -25,10 +23,8 @@ describe("guess worker format", () => {
     // a fetch listener. Regardless, our heuristic is simply to check for
     // the lack of exports.
     const guess = await guessWorkerFormat(
-      {
-        file: path.join(process.cwd(), "./index.ts"),
-        directory: process.cwd(),
-      },
+      path.join(process.cwd(), "./index.ts"),
+      process.cwd(),
       undefined
     );
     expect(guess).toBe("service-worker");
@@ -38,10 +34,8 @@ describe("guess worker format", () => {
     await writeFile("./index.ts", "export default {};");
     await expect(
       guessWorkerFormat(
-        {
-          file: path.join(process.cwd(), "./index.ts"),
-          directory: process.cwd(),
-        },
+        path.join(process.cwd(), "./index.ts"),
+        process.cwd(),
         "service-worker"
       )
     ).rejects.toThrow(
@@ -53,10 +47,8 @@ describe("guess worker format", () => {
     await writeFile("./index.ts", "");
     await expect(
       guessWorkerFormat(
-        {
-          file: path.join(process.cwd(), "./index.ts"),
-          directory: process.cwd(),
-        },
+        path.join(process.cwd(), "./index.ts"),
+        process.cwd(),
         "modules"
       )
     ).rejects.toThrow(

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

@@ -1388,6 +1388,30 @@ export default{
         expect(std.warn).toMatchInlineSnapshot(`""`);
       });
     }
+
+    it("should throw an error if the entry doesn't exist after the build finishes", async () => {
+      writeWranglerToml({
+        main: "index.js",
+        build: {
+          command: `node -e "console.log('custom build');"`,
+        },
+      });
+
+      await expect(
+        runWrangler("publish index.js")
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\""`
+      );
+      expect(std.out).toMatchInlineSnapshot(
+        `"running: node -e \\"console.log('custom build');\\""`
+      );
+      expect(std.err).toMatchInlineSnapshot(`
+        "Could not resolve \\"index.js\\" after running custom build: node -e \\"console.log('custom build');\\"
+
+        %s If you think this is a bug then please create an issue at https://github.com/cloudflare/wrangler2/issues/new."
+      `);
+      expect(std.warn).toMatchInlineSnapshot(`""`);
+    });
   });
 
   describe("[wasm_modules]", () => {

packages/wrangler/src/bundle.ts

@@ -5,7 +5,7 @@ import * as esbuild from "esbuild";
 import makeModuleCollector from "./module-collection";
 import type { Config } from "./config";
 import type { Entry } from "./entry";
-import type { CfModule, CfScriptFormat } from "./worker";
+import type { CfModule } from "./worker";
 
 type BundleResult = {
   modules: CfModule[];
@@ -24,20 +24,13 @@ export async function bundleWorker(
     serveAssetsFromWorker: boolean;
     jsxFactory: string | undefined;
     jsxFragment: string | undefined;
-    format: CfScriptFormat;
     rules: Config["rules"];
     watch?: esbuild.WatchMode;
   }
 ): Promise<BundleResult> {
-  const {
-    serveAssetsFromWorker,
-    jsxFactory,
-    jsxFragment,
-    format,
-    rules,
-    watch,
-  } = options;
-  const moduleCollector = makeModuleCollector({ format, rules });
+  const { serveAssetsFromWorker, jsxFactory, jsxFragment, rules, watch } =
+    options;
+  const moduleCollector = makeModuleCollector({ format: entry.format, rules });
   const result = await esbuild.build({
     ...getEntryPoint(entry.file, serveAssetsFromWorker),
     bundle: true,

packages/wrangler/src/config/config.ts

@@ -209,7 +209,10 @@ export interface ConfigFields<Env extends RawEnvironment> {
     cwd?: string;
     /** The directory to watch for changes while using wrangler dev, defaults to the current working directory */
     watch_dir?: string;
-    /** Deprecated field previously used to configure the build and upload of the script. */
+    /**
+     * Deprecated field previously used to configure the build and upload of the script.
+     * @deprecated
+     */
     upload?: DeprecatedUpload;
   };
 }