fix(pages): wrangler pages dev should prioritize _worker.js (#1950)

When using a _worker.js file, the entire /functions directory should be ignored – this includes its routing and middleware characteristics. Currently `wrangler pages dev` does the reverse, by prioritizing `/functions` over `_worker.js`. This commit fixes that. Co-authored-by: Carmen Popoviciu <[email protected]>
cloudflare · Oct 4, 2022 · daf73fb · daf73fb
1 parent c172217
commit daf73fb
Show file tree

Hide file tree

Showing 10 changed files with 243 additions and 34 deletions.
diff --git a/.changeset/silent-swans-relax.md b/.changeset/silent-swans-relax.md
@@ -0,0 +1,9 @@
+---
+"pages-workerjs-and-functions-app": patch
+"wrangler": patch
+---
+
+`wrangler pages dev` should prioritize `_worker.js`
+
+When using a `_worker.js` file, the entire `/functions` directory should be ignored – this includes its routing and middleware characteristics. Currently `wrangler pages dev` does the reverse, by prioritizing
+`/functions` over `_worker.js`. These changes fix the current behaviour.
diff --git a/fixtures/pages-workerjs-and-functions-app/README.md b/fixtures/pages-workerjs-and-functions-app/README.md
@@ -0,0 +1,32 @@
+# ⚡️ pages-workerjs-and-functions-app
+
+`pages-workerjs-and-functions-app` is a test fixture that sets up an [Advanced Mode](https://developers.cloudflare.com/pages/platform/functions/#advanced-mode) ⚡️Pages project that also includes a `/functions` directory. This fixture is meant to test that for such projects, the single worker script provided by users will
+always take precedence over the `functions` directory.
+
+## Publish
+
+> Please note that in order to deploy this project to `.pages.dev` you need to have a [Cloudflare account](https://dash.cloudflare.com/login)
+
+```bash
+# cd into the test fixture folder
+cd fixtures/pages-workerjs-and-functions-app
+
+# Deploy the directory of static assets as a Pages deployment
+npx wrangler pages publish public
+```
+
+If deployment was successful, follow the URL refrenced in the success message in your terminal
+
+```
+✨ Deployment complete! Take a peek over at https:/<hash>.<PROJECT_NAME>.pages.dev
+```
+
+## Run tests
+
+```bash
+# cd into the test fixture folder
+cd fixtures/pages-workerjs-and-functions-app
+
+# Run tests
+npm run test
+```
diff --git a/fixtures/pages-workerjs-and-functions-app/functions/date.ts b/fixtures/pages-workerjs-and-functions-app/functions/date.ts
@@ -0,0 +1,3 @@
+export async function onRequest() {
+	return new Response(new Date().toISOString());
+}
diff --git a/fixtures/pages-workerjs-and-functions-app/functions/greeting/hello.ts b/fixtures/pages-workerjs-and-functions-app/functions/greeting/hello.ts
@@ -0,0 +1,3 @@
+export async function onRequest() {
+	return new Response("Hello world!");
+}
diff --git a/fixtures/pages-workerjs-and-functions-app/package.json b/fixtures/pages-workerjs-and-functions-app/package.json
@@ -0,0 +1,33 @@
+{
+	"name": "pages-workerjs-and-functions-app",
+	"version": "0.0.0",
+	"private": true,
+	"sideEffects": false,
+	"scripts": {
+		"dev": "npx wrangler pages dev public --port 8955",
+		"test": "npx jest --forceExit --verbose",
+		"test:ci": "npx jest --forceExit"
+	},
+	"jest": {
+		"restoreMocks": true,
+		"testRegex": ".*.(test|spec)\\.[jt]sx?$",
+		"testTimeout": 30000,
+		"transform": {
+			"^.+\\.c?(t|j)sx?$": [
+				"esbuild-jest",
+				{
+					"sourcemap": true
+				}
+			]
+		},
+		"transformIgnorePatterns": [
+			"node_modules/(?!find-up|locate-path|p-locate|p-limit|yocto-queue|path-exists|execa|strip-final-newline|npm-run-path|path-key|onetime|mimic-fn|human-signals|is-stream)"
+		]
+	},
+	"devDependencies": {
+		"undici": "^5.9.1"
+	},
+	"engines": {
+		"node": ">=16.13"
+	}
+}
diff --git a/fixtures/pages-workerjs-and-functions-app/public/_worker.js b/fixtures/pages-workerjs-and-functions-app/public/_worker.js
@@ -0,0 +1,24 @@
+export default {
+	async fetch(request, env) {
+		const url = new URL(request.url);
+		if (url.pathname.startsWith("/greeting/hello")) {
+			return new Response("Bonjour le monde!");
+		}
+
+		if (url.pathname.startsWith("/greeting/goodbye")) {
+			return new Response("A plus tard alligator 👋");
+		}
+
+		if (url.pathname.startsWith("/party")) {
+			return new Response("Oops! Tous les alligators sont allés à la fête 🎉");
+		}
+
+		if (url.pathname.startsWith("/date")) {
+			return new Response(
+				"Yesterday is history, tomorrow is a mystery, but today is a gift. That’s why it is called the present."
+			);
+		}
+
+		return env.ASSETS.fetch(request);
+	},
+};
diff --git a/fixtures/pages-workerjs-and-functions-app/public/index.html b/fixtures/pages-workerjs-and-functions-app/public/index.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+	<body>
+		<h1>
+			Bienvenue sur notre projet &#10024; pages-workerjs-and-functions-app!
+		</h1>
+	</body>
+</html>
diff --git a/fixtures/pages-workerjs-and-functions-app/tests/index.test.ts b/fixtures/pages-workerjs-and-functions-app/tests/index.test.ts
@@ -0,0 +1,84 @@
+import { spawn } from "child_process";
+import * as path from "path";
+import patchConsole from "patch-console";
+import { fetch } from "undici";
+import type { ChildProcess } from "child_process";
+import type { Response } from "undici";
+
+const waitUntilReady = async (url: string): Promise<Response> => {
+	let response: Response | undefined = undefined;
+
+	while (response === undefined) {
+		await new Promise((resolvePromise) => setTimeout(resolvePromise, 500));
+
+		try {
+			response = await fetch(url);
+		} catch (err) {}
+	}
+
+	return response as Response;
+};
+
+const isWindows = process.platform === "win32";
+
+describe("Pages project with `_worker.js` and `/functions` directory", () => {
+	let wranglerProcess: ChildProcess;
+
+	// const std = mockConsoleMethods();
+	beforeEach(() => {
+		wranglerProcess = spawn("npm", ["run", "dev"], {
+			shell: isWindows,
+			cwd: path.resolve(__dirname, "../"),
+			env: { BROWSER: "none", ...process.env },
+		});
+		wranglerProcess.stdout?.on("data", (chunk) => {
+			console.log(chunk.toString());
+		});
+		wranglerProcess.stderr?.on("data", (chunk) => {
+			console.log(chunk.toString());
+		});
+	});
+
+	afterEach(async () => {
+		patchConsole(() => {});
+
+		await new Promise((resolve, reject) => {
+			wranglerProcess.once("exit", (code) => {
+				if (!code) {
+					resolve(code);
+				} else {
+					reject(code);
+				}
+			});
+			wranglerProcess.kill("SIGTERM");
+		});
+	});
+
+	it("renders static pages", async () => {
+		const response = await waitUntilReady("http://127.0.0.1:8955/");
+		const text = await response.text();
+		expect(text).toContain(
+			"Bienvenue sur notre projet &#10024; pages-workerjs-and-functions-app!"
+		);
+	});
+
+	it("runs our _worker.js and ignores the functions directory", async () => {
+		let response = await waitUntilReady("http://127.0.0.1:8955/greeting/hello");
+		let text = await response.text();
+		expect(text).toEqual("Bonjour le monde!");
+
+		response = await waitUntilReady("http://127.0.0.1:8955/greeting/goodbye");
+		text = await response.text();
+		expect(text).toEqual("A plus tard alligator 👋");
+
+		response = await waitUntilReady("http://127.0.0.1:8955/date");
+		text = await response.text();
+		expect(text).toEqual(
+			"Yesterday is history, tomorrow is a mystery, but today is a gift. That’s why it is called the present."
+		);
+
+		response = await waitUntilReady("http://127.0.0.1:8955/party");
+		text = await response.text();
+		expect(text).toEqual("Oops! Tous les alligators sont allés à la fête 🎉");
+	});
+});
diff --git a/fixtures/pages-workerjs-and-functions-app/tsconfig.json b/fixtures/pages-workerjs-and-functions-app/tsconfig.json
@@ -0,0 +1,10 @@
+{
+	"compilerOptions": {
+		"target": "ES2020",
+		"esModuleInterop": true,
+		"module": "CommonJS",
+		"lib": ["ES2020"],
+		"types": ["jest"],
+		"moduleResolution": "node"
+	}
+}
diff --git a/packages/wrangler/src/pages/dev.tsx b/packages/wrangler/src/pages/dev.tsx
@@ -200,10 +200,6 @@ export const Handler = async ({
 		throw new FatalError("Pages does not support wrangler.toml", 1);
 	}
 
-	const functionsDirectory = "./functions";
-	let usingFunctions = existsSync(functionsDirectory);
-	let usingWorkerScript = false;
-
 	const command = remaining;
 
 	let proxyPort: number | undefined;
@@ -236,10 +232,42 @@ export const Handler = async ({
 		(promiseResolve) => (scriptReadyResolve = promiseResolve)
 	);
 
+	const workerScriptPath =
+		directory !== undefined
+			? join(directory, singleWorkerScriptPath)
+			: singleWorkerScriptPath;
+	const usingWorkerScript = existsSync(workerScriptPath);
+
+	const functionsDirectory = "./functions";
+	let usingFunctions = !usingWorkerScript && existsSync(functionsDirectory);
+
 	let scriptPath = "";
 
-	// Try to use Functions
-	if (usingFunctions) {
+	if (usingWorkerScript) {
+		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+		scriptReadyResolve!();
+
+		scriptPath = workerScriptPath;
+
+		const runBuild = async () => {
+			try {
+				await esbuild.build({
+					entryPoints: [scriptPath],
+					write: false,
+					plugins: [blockWorkerJsImports],
+				});
+			} catch {}
+		};
+
+		await runBuild();
+		watch([scriptPath], {
+			persistent: true,
+			ignoreInitial: true,
+		}).on("all", async () => {
+			await runBuild();
+		});
+	} else if (usingFunctions) {
+		// Try to use Functions
 		const outfile = join(tmpdir(), `./functionsWorker-${Math.random()}.js`);
 		scriptPath = outfile;
 
@@ -307,37 +335,12 @@ export const Handler = async ({
 
 	// Depending on the result of building Functions, we may not actually be using
 	// Functions even if the directory exists.
-	if (!usingFunctions) {
+	if (!usingFunctions && !usingWorkerScript) {
 		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 		scriptReadyResolve!();
 
-		scriptPath =
-			directory !== undefined
-				? join(directory, singleWorkerScriptPath)
-				: singleWorkerScriptPath;
-		usingWorkerScript = existsSync(scriptPath);
-
-		if (!usingWorkerScript) {
-			logger.log("No functions. Shimming...");
-			scriptPath = resolve(getBasePath(), "templates/pages-shim.ts");
-		} else {
-			const runBuild = async () => {
-				try {
-					await esbuild.build({
-						entryPoints: [scriptPath],
-						write: false,
-						plugins: [blockWorkerJsImports],
-					});
-				} catch {}
-			};
-			await runBuild();
-			watch([scriptPath], {
-				persistent: true,
-				ignoreInitial: true,
-			}).on("all", async () => {
-				await runBuild();
-			});
-		}
+		logger.log("No functions. Shimming...");
+		scriptPath = resolve(getBasePath(), "templates/pages-shim.ts");
 	}
 
 	await scriptReadyPromise;