[wrangler] fix: stop rebuild attempts when sources are missing (#3886) (

#4877)

* [wrangler] fix: stop rebuild attempts when sources are missing (#3886)

* fixup! [wrangler] fix: stop rebuild attempts when sources are missing (#3886)

* fixup! [wrangler] fix: stop rebuild attempts when sources are missing (#3886)

* fixup! [wrangler] fix: stop rebuild attempts when sources are missing (#3886)

* Update .changeset/hip-files-count.md

Co-authored-by: Pete Bacon Darwin <[email protected]>

* !fixup: [wrangler] fix: stop rebuild attempts when sources are missing (#3886)

* !fixup: [wrangler] fix: stop rebuild attempts when sources are missing (#3886)

---------

Co-authored-by: Peter Bacon Darwin <[email protected]>
Co-authored-by: Pete Bacon Darwin <[email protected]>

.changeset/hip-files-count.md

@@ -0,0 +1,10 @@
+---
+"wrangler": minor
+---
+
+fix: Do not show unnecessary errors during watch rebuilds
+
+When Pages is used in conjunction with a full stack framework, the framework
+build will temporarily remove files that are being watched by Pages, such as
+`_worker.js` and `_routes.json`.
+Previously we would display errors for these changes, which adds confusing and excessive messages to the Pages dev output. Now builds are skipped if a watched `_worker.js` or `_routes.json` is removed.

fixtures/pages-workerjs-app/tests/index.test.ts

@@ -1,5 +1,7 @@
 import { execSync } from "node:child_process";
+import { rename } from "node:fs/promises";
 import path, { resolve } from "node:path";
+import { setTimeout } from "node:timers/promises";
 import { fetch } from "undici";
 import { describe, it } from "vitest";
 import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived";
@@ -60,4 +62,95 @@ describe("Pages _worker.js", () => {
 			await stop();
 		}
 	});
+
+	it("should not error if the worker.js file is removed while watching", async ({
+		expect,
+	}) => {
+		const basePath = resolve(__dirname, "..");
+		const { ip, port, getOutput, clearOutput, stop } =
+			await runWranglerPagesDev(resolve(__dirname, ".."), "./workerjs-test", [
+				"--port=0",
+				"--inspector-port=0",
+			]);
+		try {
+			clearOutput();
+			await tryRename(
+				basePath,
+				"workerjs-test/_worker.js",
+				"workerjs-test/XXX_worker.js"
+			);
+			await setTimeout(1000);
+			// Expect no output since the deletion of the worker should be ignored
+			expect(getOutput()).toBe("");
+			await tryRename(
+				basePath,
+				"workerjs-test/XXX_worker.js",
+				"workerjs-test/_worker.js"
+			);
+			await setTimeout(1000);
+			// Expect replacing the worker to now trigger a success build.
+			expect(getOutput()).toContain("Compiled Worker successfully");
+		} finally {
+			await stop();
+			await tryRename(
+				basePath,
+				"workerjs-test/XXX_worker.js",
+				"workerjs-test/_worker.js"
+			);
+		}
+	});
+
+	it("should not error if the _routes.json file is removed while watching", async ({
+		expect,
+	}) => {
+		const basePath = resolve(__dirname, "..");
+		const { ip, port, getOutput, clearOutput, stop } =
+			await runWranglerPagesDev(resolve(__dirname, ".."), "./workerjs-test", [
+				"--port=0",
+				"--inspector-port=0",
+			]);
+		try {
+			clearOutput();
+			await tryRename(
+				basePath,
+				"workerjs-test/_routes.json",
+				"workerjs-test/XXX_routes.json"
+			);
+			await setTimeout(1000);
+			// Expect no output since the deletion of the routes file should be ignored
+			expect(getOutput()).toBe("");
+			await tryRename(
+				basePath,
+				"workerjs-test/XXX_routes.json",
+				"workerjs-test/_routes.json"
+			);
+			await setTimeout(1000);
+			// Expect replacing the routes file to trigger a build, although
+			// the routes build does not provide any output feedback to compare against,
+			// so we just check that nothing else is being printed.
+			expect(getOutput()).toBe("");
+		} finally {
+			await stop();
+			await tryRename(
+				basePath,
+				"workerjs-test/XXX_routes.json",
+				"workerjs-test/_routes.json"
+			);
+		}
+	});
+
+	async function tryRename(
+		basePath: string,
+		from: string,
+		to: string
+	): Promise<void> {
+		try {
+			await rename(resolve(basePath, from), resolve(basePath, to));
+		} catch (e) {
+			// Do nothing if the file was not found
+			if ((e as any).code !== "ENOENT") {
+				throw e;
+			}
+		}
+	}
 });

fixtures/pages-workerjs-app/workerjs-test/_routes.json

@@ -0,0 +1,6 @@
+{
+	"version": 1,
+	"description": "",
+	"include": ["/*"],
+	"exclude": []
+}

fixtures/shared/src/run-wrangler-long-lived.ts

@@ -62,6 +62,7 @@ async function runLongLivedWrangler(command: string[], cwd: string) {
 		chunks.push(chunk);
 	});
 	const getOutput = () => Buffer.concat(chunks).toString();
+	const clearOutput = () => (chunks.length = 0);
 
 	const timeoutHandle = setTimeout(() => {
 		if (settledReadyPromise) return;
@@ -90,5 +91,5 @@ async function runLongLivedWrangler(command: string[], cwd: string) {
 	}
 
 	const { ip, port } = await ready;
-	return { ip, port, stop, getOutput };
+	return { ip, port, stop, getOutput, clearOutput };
 }

packages/wrangler/src/pages/dev.ts

@@ -371,7 +371,10 @@ export const Handler = async ({
 		watch([workerScriptPath], {
 			persistent: true,
 			ignoreInitial: true,
-		}).on("all", async () => {
+		}).on("all", async (event) => {
+			if (event === "unlink") {
+				return;
+			}
 			await runBuild();
 		});
 	} else if (usingFunctions) {
@@ -539,8 +542,11 @@ export const Handler = async ({
 			watch([routesJSONPath], {
 				persistent: true,
 				ignoreInitial: true,
-			}).on("all", async () => {
+			}).on("all", async (event) => {
 				try {
+					if (event === "unlink") {
+						return;
+					}
 					/**
 					 * Watch for _routes.json file changes and validate file each time.
 					 * If file is valid proceed to running the build.