Merge branch 'main' into vitest-fixtures

cloudflare · Jan 4, 2023 · dfc06f6 · dfc06f6
2 parents 928f041 + 4c0e230
commit dfc06f6
Show file tree

Hide file tree

Showing 16 changed files with 200 additions and 7 deletions.
diff --git a/.changeset/popular-pumas-move.md b/.changeset/popular-pumas-move.md
@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: Pages Plugin routing when mounted at the root of a project
+
+Previously, there was a bug which meant that Plugins mounted at the root of a Pages project were not correctly matching incoming requests. This change fixes that bug so Plugins mounted at the root should now correctly work.
diff --git a/fixtures/pages-functions-app/functions/mounted-with-param/[p]/plugin/_middleware.ts b/fixtures/pages-functions-app/functions/mounted-with-param/[p]/plugin/_middleware.ts
@@ -0,0 +1,3 @@
+import examplePlugin from "../../../../../pages-plugin-example";
+
+export const onRequest = examplePlugin({ footerText: "Set from a Plugin!" });
diff --git a/fixtures/pages-functions-app/tests/index.test.ts b/fixtures/pages-functions-app/tests/index.test.ts
@@ -131,6 +131,39 @@ describe("Pages Functions", () => {
 			expect(text).toContain("<footer>Set from a Plugin!</footer>");
 		});
 
+		it.concurrent("should return a status code", async ({ expect }) => {
+			const response = await fetch(
+				`http://${ip}:${port}/mounted-plugin/status`
+			);
+			const text = await response.text();
+			expect(text).toMatchInlineSnapshot(
+				`"This should return a 502 status code"`
+			);
+			expect(response.status).toBe(502);
+		});
+
+		it.concurrent(
+			"should mount a Plugin even if in a parameterized route",
+			async ({ expect }) => {
+				const response = await fetch(
+					`http://${ip}:${port}/mounted-with-param/p123/plugin/status`
+				);
+				const text = await response.text();
+				expect(text).toMatchInlineSnapshot(
+					`"This should return a 502 status code"`
+				);
+				expect(response.status).toBe(502);
+			}
+		);
+
+		it.concurrent("should work for nested folders", async ({ expect }) => {
+			const response = await fetch(
+				`http://${ip}:${port}/mounted-plugin/api/v1/instance`
+			);
+			const text = await response.text();
+			expect(text).toMatchInlineSnapshot(`"Response from a nested folder"`);
+		});
+
 		it.concurrent("should mount Fixed page", async ({ expect }) => {
 			const response = await fetch(`http://${ip}:${port}/mounted-plugin/fixed`);
 			const text = await response.text();

diff --git a/fixtures/pages-plugin-example/functions/api/v1/instance.ts b/fixtures/pages-plugin-example/functions/api/v1/instance.ts
@@ -0,0 +1 @@
+export const onRequest = () => new Response("Response from a nested folder");
diff --git a/fixtures/pages-plugin-example/functions/status.ts b/fixtures/pages-plugin-example/functions/status.ts
@@ -0,0 +1,3 @@
+export const onRequest = () => {
+	return new Response("This should return a 502 status code", { status: 502 });
+};
diff --git a/fixtures/pages-plugin-mounted-on-root-app/.gitignore b/fixtures/pages-plugin-mounted-on-root-app/.gitignore
@@ -0,0 +1 @@
+cdn-cgi/
diff --git a/fixtures/pages-plugin-mounted-on-root-app/functions/_middleware.ts b/fixtures/pages-plugin-mounted-on-root-app/functions/_middleware.ts
@@ -0,0 +1,3 @@
+import examplePlugin from "../../pages-plugin-example";
+
+export const onRequest = examplePlugin({ footerText: "Set from a Plugin!" });
diff --git a/fixtures/pages-plugin-mounted-on-root-app/index.d.ts b/fixtures/pages-plugin-mounted-on-root-app/index.d.ts
@@ -0,0 +1,4 @@
+declare module "*.html" {
+	const content: string;
+	export default content;
+}
diff --git a/fixtures/pages-plugin-mounted-on-root-app/package.json b/fixtures/pages-plugin-mounted-on-root-app/package.json
@@ -0,0 +1,36 @@
+{
+	"name": "pages-plugin-mounted-on-root-app",
+	"version": "0.0.0",
+	"private": true,
+	"sideEffects": false,
+	"main": "dist/worker.js",
+	"scripts": {
+		"check:type": "tsc && tsc -p tests/tsconfig.json",
+		"dev": "npx wrangler pages dev public --port 8793",
+		"test": "npx jest --forceExit",
+		"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": {
+		"@cloudflare/workers-types": "^4.20221111.1",
+		"undici": "^5.9.1"
+	},
+	"engines": {
+		"node": ">=16.13"
+	}
+}
diff --git a/fixtures/pages-plugin-mounted-on-root-app/public/index.html b/fixtures/pages-plugin-mounted-on-root-app/public/index.html
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<html>
+	<body>
+		<h1>Hello, world!</h1>
+	</body>
+</html>
diff --git a/fixtures/pages-plugin-mounted-on-root-app/tests/index.test.ts b/fixtures/pages-plugin-mounted-on-root-app/tests/index.test.ts
@@ -0,0 +1,51 @@
+import { fork } from "child_process";
+import * as path from "path";
+import { fetch } from "undici";
+import type { ChildProcess } from "child_process";
+
+describe("Pages Functions", () => {
+	let wranglerProcess: ChildProcess;
+	let ip: string;
+	let port: number;
+	let resolveReadyPromise: (value: unknown) => void;
+	const readyPromise = new Promise((resolve) => {
+		resolveReadyPromise = resolve;
+	});
+
+	beforeAll(async () => {
+		wranglerProcess = fork(
+			path.join("..", "..", "packages", "wrangler", "bin", "wrangler.js"),
+			["pages", "dev", "public", "--port=0"],
+			{
+				stdio: ["inherit", "inherit", "inherit", "ipc"],
+				cwd: path.resolve(__dirname, ".."),
+			}
+		).on("message", (message) => {
+			const parsedMessage = JSON.parse(message.toString());
+			ip = parsedMessage.ip;
+			port = parsedMessage.port;
+			resolveReadyPromise(undefined);
+		});
+	});
+
+	afterAll(async () => {
+		await readyPromise;
+		await new Promise((resolve, reject) => {
+			wranglerProcess.once("exit", (code) => {
+				if (!code) {
+					resolve(code);
+				} else {
+					reject(code);
+				}
+			});
+			wranglerProcess.kill("SIGTERM");
+		});
+	});
+
+	it.concurrent("mounts a plugin correctly at root", async () => {
+		await readyPromise;
+		const response = await fetch(`http://${ip}:${port}/api/v1/instance`);
+		const text = await response.text();
+		expect(text).toMatchInlineSnapshot(`"Response from a nested folder"`);
+	});
+});
diff --git a/fixtures/pages-plugin-mounted-on-root-app/tests/tsconfig.json b/fixtures/pages-plugin-mounted-on-root-app/tests/tsconfig.json
@@ -0,0 +1,8 @@
+{
+	"extends": "../tsconfig.json",
+	"compilerOptions": {
+		"types": ["node", "jest"]
+	},
+	"include": ["**/*.ts"],
+	"exclude": []
+}
diff --git a/fixtures/pages-plugin-mounted-on-root-app/tsconfig.json b/fixtures/pages-plugin-mounted-on-root-app/tsconfig.json
@@ -0,0 +1,11 @@
+{
+	"include": ["index.d.ts", "functions"],
+	"compilerOptions": {
+		"target": "ES2020",
+		"module": "CommonJS",
+		"lib": ["ES2020"],
+		"types": ["@cloudflare/workers-types"],
+		"moduleResolution": "node",
+		"noEmit": true
+	}
+}
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts
@@ -141,7 +141,7 @@ export function buildWorker({
 										// TODO: Watch args.path for changes and re-copy when updated
 										contents: `export const onRequest = ({ request, env, functionPath }) => {
                     const url = new URL(request.url)
-                    const relativePathname = \`/\${url.pathname.split(functionPath)[1] || ''}\`.replace(/^\\/\\//, '/');
+                    const relativePathname = \`/\${url.pathname.replace(functionPath, "") || ""}\`.replace(/^\\/\\//, '/');
                     url.pathname = '/cdn-cgi/pages-plugins/${identifier}' + relativePathname
                     request = new Request(url.toString(), request)
                     return env.ASSETS.fetch(request)

diff --git a/packages/wrangler/templates/pages-template-plugin.ts b/packages/wrangler/templates/pages-template-plugin.ts
@@ -125,8 +125,9 @@ export default function (pluginArgs: unknown) {
 		const { env, next, data } = workerContext;
 
 		const url = new URL(request.url);
+		// TODO: Replace this with something actually legible.
 		const relativePathname = `/${
-			url.pathname.split(workerContext.functionPath)[1] || ""
+			url.pathname.replace(workerContext.functionPath, "") || ""
 		}`.replace(/^\/\//, "/");
 
 		const handlerIterator = executeRequest(request, relativePathname);
@@ -154,11 +155,7 @@ export default function (pluginArgs: unknown) {
 
 				const response = await handler(context);
 
-				// https://fetch.spec.whatwg.org/#null-body-status
-				return new Response(
-					[101, 204, 205, 304].includes(response.status) ? null : response.body,
-					{ ...response, headers: new Headers(response.headers) }
-				);
+				return cloneResponse(response);
 			} else {
 				return next();
 			}
@@ -169,3 +166,11 @@ export default function (pluginArgs: unknown) {
 
 	return onRequest;
 }
+
+// This makes a Response mutable
+const cloneResponse = (response: Response) =>
+	// https://fetch.spec.whatwg.org/#null-body-status
+	new Response(
+		[101, 204, 205, 304].includes(response.status) ? null : response.body,
+		response
+	);
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		import examplePlugin from "../../../../../pages-plugin-example";

		export const onRequest = examplePlugin({ footerText: "Set from a Plugin!" });
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export const onRequest = () => new Response("Response from a nested folder");
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		import examplePlugin from "../../pages-plugin-example";

		export const onRequest = examplePlugin({ footerText: "Set from a Plugin!" });