fix assets + custom build = infinite loop (#6881)

.changeset/happy-balloons-work.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: custom builds outputting files in assets watched directory no longer cause the custom build to run again in an infinite loop

packages/wrangler/e2e/dev.test.ts

@@ -877,6 +877,66 @@ describe("custom builds", () => {
 		text = await fetchText(url);
 		expect(text).toMatchInlineSnapshot(`"Hello, World!"`);
 	});
+
+	it("does not infinite-loop custom build with assets", async () => {
+		const helper = new WranglerE2ETestHelper();
+		await helper.seed({
+			"wrangler.toml": dedent`
+                    name = "${workerName}"
+                    compatibility_date = "2023-01-01"
+                    main = "src/index.ts"
+                    build.command = "echo 'hello' > ./public/index.html"
+
+                    [assets]
+                    directory = "./public"
+            `,
+			"src/index.ts": dedent`
+                export default {
+                    async fetch(request) {
+                        return new Response("Hello, World!")
+                    }
+                }
+            `,
+			"public/other.html": "ensure ./public exists",
+		});
+		const worker = helper.runLongLived("wrangler dev");
+
+		// first build on startup
+		await worker.readUntil(/Running custom build/, 5_000);
+		// second build for first watcher notification (can be optimised away, leaving as-is for now)
+		await worker.readUntil(/Running custom build/, 5_000);
+
+		// Need to get the url in this order because waitForReady calls readUntil
+		// which keeps track of where it's read up to so far,
+		// so the expect(waitUntil).reject assertion below
+		// will eat up the "Ready on http://localhost:8787" message if called before.
+		// This could cause a flake if eg the 2nd custom build starts after ready.
+		const { url } = await worker.waitForReady();
+
+		// assert no more custom builds happen
+		// regression: https://github.com/cloudflare/workers-sdk/issues/6876
+		await expect(
+			worker.readUntil(/Running custom build:/, 5_000)
+		).rejects.toThrowError();
+
+		// now check assets are still fetchable, even after updates
+
+		const res = await fetch(url);
+		await expect(res.text()).resolves.toBe("hello\n");
+
+		await helper.seed({
+			"public/index.html": "world",
+		});
+
+		const resText = await retry(
+			(text) => text === "hello\n",
+			async () => {
+				const res2 = await fetch(url);
+				return res2.text();
+			}
+		);
+		await expect(resText).toBe("world");
+	});
 });
 
 describe("watch mode", () => {

packages/wrangler/src/api/startDevWorker/BundlerController.ts

@@ -10,6 +10,7 @@ import {
 	getWrangler1xLegacyModuleReferences,
 } from "../../deployment-bundle/module-collection";
 import { runCustomBuild } from "../../deployment-bundle/run-custom-build";
+import { getAssetChangeMessage } from "../../dev";
 import { runBuild } from "../../dev/use-esbuild";
 import { logger } from "../../logger";
 import { isNavigatorDefined } from "../../navigator-user-agent";
@@ -259,6 +260,24 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
 		);
 	}
 
+	#assetsWatcher?: ReturnType<typeof watch>;
+	async #ensureWatchingAssets(config: StartDevWorkerOptions) {
+		await this.#assetsWatcher?.close();
+
+		if (config.assets?.directory) {
+			this.#assetsWatcher = watch(config.assets.directory, {
+				persistent: true,
+				ignoreInitial: true,
+			}).on("all", async (eventName, filePath) => {
+				const message = getAssetChangeMessage(eventName, filePath);
+				logger.debug(`🌀 ${message}...`);
+				if (this.#currentBundle) {
+					this.emitBundleCompleteEvent(config, this.#currentBundle);
+				}
+			});
+		}
+	}
+
 	#tmpDir?: EphemeralDirectory;
 
 	onConfigUpdate(event: ConfigUpdateEvent) {
@@ -280,6 +299,7 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
 
 		void this.#startCustomBuild(event.config);
 		void this.#startBundle(event.config);
+		void this.#ensureWatchingAssets(event.config);
 	}
 
 	async teardown() {
@@ -289,6 +309,7 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
 		await Promise.all([
 			this.#bundlerCleanup?.(),
 			this.#customBuildWatcher?.close(),
+			this.#assetsWatcher?.close(),
 		]);
 		logger.debug("BundlerController teardown complete");
 	}

packages/wrangler/src/api/startDevWorker/ConfigController.ts

@@ -13,7 +13,6 @@ import { processAssetsArg, validateAssetsArgsAndConfig } from "../../assets";
 import { printBindings, readConfig } from "../../config";
 import { getEntry } from "../../deployment-bundle/entry";
 import {
-	getAssetChangeMessage,
 	getBindings,
 	getHostAndRoutes,
 	getInferredHost,
@@ -358,7 +357,6 @@ export class ConfigController extends Controller<ConfigControllerEventMap> {
 	latestConfig?: StartDevWorkerOptions;
 
 	#configWatcher?: ReturnType<typeof watch>;
-	#assetsWatcher?: ReturnType<typeof watch>;
 	#abortController?: AbortController;
 
 	async #ensureWatchingConfig(configPath: string | undefined) {
@@ -378,24 +376,6 @@ export class ConfigController extends Controller<ConfigControllerEventMap> {
 		}
 	}
 
-	async #ensureWatchingAssets(assetsPath: string | undefined) {
-		await this.#assetsWatcher?.close();
-
-		if (assetsPath) {
-			this.#assetsWatcher = watch(assetsPath, {
-				persistent: true,
-				ignoreInitial: true,
-			}).on("all", async (eventName, filePath) => {
-				const message = getAssetChangeMessage(eventName, filePath);
-				logger.debug(`🌀 ${message}...`);
-				assert(
-					this.latestInput,
-					"Cannot be watching config without having first set an input"
-				);
-				void this.#updateConfig(this.latestInput);
-			});
-		}
-	}
 	public set(input: StartDevWorkerInput, throwErrors = false) {
 		return this.#updateConfig(input, throwErrors);
 	}
@@ -442,11 +422,6 @@ export class ConfigController extends Controller<ConfigControllerEventMap> {
 				void this.#ensureWatchingConfig(fileConfig.configPath);
 			}
 
-			const assets = processAssetsArg({ assets: input?.assets }, fileConfig);
-			if (assets && typeof vitest === "undefined") {
-				void this.#ensureWatchingAssets(assets.directory);
-			}
-
 			const resolvedConfig = await resolveConfig(fileConfig, input);
 			if (signal.aborted) {
 				return;
@@ -476,10 +451,7 @@ export class ConfigController extends Controller<ConfigControllerEventMap> {
 
 	async teardown() {
 		logger.debug("ConfigController teardown beginning...");
-		await Promise.allSettled([
-			this.#configWatcher?.close(),
-			this.#assetsWatcher?.close(),
-		]);
+		await this.#configWatcher?.close();
 		logger.debug("ConfigController teardown complete");
 	}