fix: disallow imports in _worker.js (cloudflare#1629)

* fix: disallow imports in _worker.js

Fixes: cloudflare#1214

This does not actually prevent the user from importing things (could be modified to do so)

It currently just logs an error to console.

* chore: prettier

* draft: start to add tests

* apply changes

* Update packages/wrangler/src/pages/dev.tsx

Co-authored-by: Greg Brimble <[email protected]>

* mark test as todo

Co-authored-by: Greg Brimble <[email protected]>

.changeset/rotten-timers-do.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: disallow imports in \_worker.js (https://github.com/cloudflare/wrangler2/issues/1214)

fixtures/pages-workerjs-app/package.json

@@ -0,0 +1,33 @@
+{
+	"name": "pages-workerjs-app",
+	"version": "0.0.0",
+	"private": true,
+	"sideEffects": false,
+	"scripts": {
+		"dev": "npx wrangler pages dev ./workerjs-test --port 8792",
+		"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.5.1"
+	},
+	"engines": {
+		"node": ">=14"
+	}
+}

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

@@ -0,0 +1,69 @@
+import { spawn } from "child_process";
+import * as path from "path";
+import patchConsole from "patch-console";
+import { fetch } from "undici";
+// import { mockConsoleMethods } from "../../../packages/wrangler/src/__tests__/helpers/mock-console";
+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 _worker.js", () => {
+	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:8792/");
+		const text = await response.text();
+		expect(text).toContain("test");
+	});
+
+	it.todo("shows an error for the import in _worker.js");
+	// it("shows an error for the import in _worker.js", async () => {
+	// 	const _ = await waitUntilReady("http://127.0.0.1:8792/");
+	// 	expect(std.err).toMatchInlineSnapshot(`""`);
+	// });
+});

fixtures/pages-workerjs-app/tsconfig.json

@@ -0,0 +1,10 @@
+{
+	"compilerOptions": {
+		"target": "ES2020",
+		"esModuleInterop": true,
+		"module": "CommonJS",
+		"lib": ["ES2020"],
+		"types": ["jest"],
+		"moduleResolution": "node"
+	}
+}

fixtures/pages-workerjs-app/workerjs-test/_worker.js

@@ -0,0 +1,7 @@
+import text from "./other-script";
+
+export default {
+	async fetch() {
+		return new Response(text);
+	},
+};

fixtures/pages-workerjs-app/workerjs-test/other-script.js

@@ -0,0 +1 @@
+export default "test";

packages/wrangler/src/pages/dev.tsx

@@ -3,13 +3,15 @@ import { existsSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join, resolve } from "node:path";
 import { watch } from "chokidar";
+import { build as workerJsBuild } from "esbuild";
 import { unstable_dev } from "../api";
 import { FatalError } from "../errors";
 import { logger } from "../logger";
 import * as metrics from "../metrics";
 import { buildFunctions } from "./build";
 import { SECONDS_TO_WAIT_FOR_PROXY } from "./constants";
 import { CLEANUP, CLEANUP_CALLBACKS, pagesBetaWarning } from "./utils";
+import type { Plugin } from "esbuild";
 import type { ArgumentsCamelCase, Argv } from "yargs";
 
 type PagesDevArgs = {
@@ -228,6 +230,23 @@ export const Handler = async ({
 		if (!existsSync(scriptPath)) {
 			logger.log("No functions. Shimming...");
 			scriptPath = resolve(__dirname, "../templates/pages-shim.ts");
+		} else {
+			const runBuild = async () => {
+				try {
+					await workerJsBuild({
+						entryPoints: [scriptPath],
+						write: false,
+						plugins: [blockWorkerJsImports],
+					});
+				} catch {}
+			};
+			await runBuild();
+			watch([scriptPath], {
+				persistent: true,
+				ignoreInitial: true,
+			}).on("all", async () => {
+				await runBuild();
+			});
 		}
 	}
 
@@ -431,3 +450,15 @@ async function spawnProxyProcess({
 
 	return port;
 }
+
+const blockWorkerJsImports: Plugin = {
+	name: "block-worker-js-imports",
+	setup(build) {
+		build.onResolve({ filter: /.*/g }, (_args) => {
+			logger.error(
+				`_worker.js is importing from another file. This will throw an error if deployed.\nYou should bundle your Worker or remove the import if it is unused.`
+			);
+			return null;
+		});
+	},
+};