Add find_additional_modules option to support partial bundling with…

… externals (#3726)

* typo

* move stuff

* remove unnecessary typing

* remove unnecessary `active` property from middleware loader config

* more refactorings

* rename traverseModuleGraph as this is not what it does

* refactor to simplify findAdditionalModules()

* Rename the functions build helpers to make it more clear what their role is.

* invert module collector config - no longer need to return it as it is passed in

* implement module finding in normal bundling if the find_additional_modules flag is on

* allow `__STATIC_CONTENT_MANIFEST` to be imported from any module

* test: improve no-bundle-import tests

The no-bundle-import tests would actually pass if Wrangler actually bundled the code,
since it would identify and inline all the dynamic
imports that were being tested.

This change adds a test that would not pass if we are not capturing the additional modules.

It also moves the declaration of no-bundle to the wrangler.toml to ensure that normal use of `wrangler dev` will get this behavior.

* refactor: convert additional file finding to a generator function

This has a few of benefits:
- we don't risk creating too many file reads in a single go (using up all the available file handles); previously file reads were all done in parallel
- we don't read the files over and over for each rule
- the code is a bit easier to follow as we don't have to create `Promise.all()` objects

* fix: ensure that additional modules appear in the out-dir

When using `find_additional_modules` (or `no_bundle`) we find files that
will be uploaded to be deployed alongside the Worker.

Previously, if an `outDir` was specified, only the Worker code was output
to this directory. Now all additional modules are also output there too.

* pnpm fixups

* test: ignore failure to remove tmp dir on Windows

* test: do not show output in d1 time-travel tests

* refactor: consolidate writing additional modules

* test: add CommonJS lazy import to additional-modules fixture

* test: rename spec to test

* Add debug logging when writing additional modules

* Test additional module failure case and fix message typo

* Display build warnings before updating the bundle

---------

Co-authored-by: bcoll <[email protected]>

.changeset/blue-donkeys-pretend.md

@@ -0,0 +1,11 @@
+---
+"wrangler": patch
+---
+
+fix: ensure that additional modules appear in the out-dir
+
+When using `find_additional_modules` (or `no_bundle`) we find files that
+will be uploaded to be deployed alongside the Worker.
+
+Previously, if an `outDir` was specified, only the Worker code was output
+to this directory. Now all additional modules are also output there too.

.changeset/clever-turkeys-leave.md

@@ -0,0 +1,20 @@
+---
+"wrangler": minor
+---
+
+feat: support partial bundling with configurable external modules
+
+Setting `find_additional_modules` to `true` in your configuration file will now instruct Wrangler to look for files in
+your `base_dir` that match your configured `rules`, and deploy them as unbundled, external modules with your Worker.
+`base_dir` defaults to the directory containing your `main` entrypoint.
+
+Wrangler can operate in two modes: the default bundling mode and `--no-bundle` mode. In bundling mode, dynamic imports
+(e.g. `await import("./large-dep.mjs")`) would be bundled into your entrypoint, making lazy loading less effective.
+Additionally, variable dynamic imports (e.g. `` await import(`./lang/${language}.mjs`) ``) would always fail at runtime,
+as Wrangler would have no way of knowing which modules to upload. The `--no-bundle` mode sought to address these issues
+by disabling Wrangler's bundling entirely, and just deploying code as is. Unfortunately, this also disabled Wrangler's
+code transformations (e.g. TypeScript compilation, `--assets`, `--test-scheduled`, etc).
+
+With this change, we now additionally support _partial bundling_. Files are bundled into a single Worker entry-point file
+unless `find_additional_modules` is `true`, and the file matches one of the configured `rules`. See
+https://developers.cloudflare.com/workers/wrangler/bundling/ for more details and examples.

.changeset/hot-deers-return.md

@@ -0,0 +1,8 @@
+---
+"wrangler": patch
+---
+
+fix: allow `__STATIC_CONTENT_MANIFEST` module to be imported anywhere
+
+`__STATIC_CONTENT_MANIFEST` can now be imported in subdirectories when
+`--no-bundle` or `find_additional_modules` are enabled.

.prettierignore

@@ -7,6 +7,9 @@ packages/wrangler/CHANGELOG.md
 packages/jest-environment-wrangler/CHANGELOG.md
 packages/wranglerjs-compat-webpack-plugin/lib
 packages/wrangler-devtools/built-devtools
+packages/wrangler-devtools/.cipd
+packages/wrangler-devtools/depot
+packages/wrangler-devtools/devtools-frontend
 packages/edge-preview-authenticated-proxy/package.json
 packages/format-errors/package.json
 packages/**/dist/**

fixtures/additional-modules/package.json

@@ -0,0 +1,21 @@
+{
+	"name": "additional-modules",
+	"version": "0.0.1",
+	"private": true,
+	"scripts": {
+		"build": "wrangler deploy --dry-run --outdir=dist",
+		"check:type": "tsc",
+		"deploy": "wrangler deploy",
+		"start": "wrangler dev",
+		"test": "vitest run",
+		"test:ci": "vitest run",
+		"test:watch": "vitest",
+		"type:tests": "tsc -p ./test/tsconfig.json"
+	},
+	"devDependencies": {
+		"@cloudflare/workers-tsconfig": "workspace:*",
+		"@cloudflare/workers-types": "^4.20230724.0",
+		"undici": "^5.9.1",
+		"wrangler": "workspace:*"
+	}
+}

fixtures/additional-modules/src/common.cjs

@@ -0,0 +1 @@
+module.exports = "common";

fixtures/additional-modules/src/dep.ts

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

fixtures/additional-modules/src/dynamic.js

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

fixtures/additional-modules/src/index.ts

@@ -0,0 +1,28 @@
+import dep from "./dep";
+import text from "./text.txt";
+import common from "./common.cjs";
+
+export default <ExportedHandler>{
+	async fetch(request) {
+		const url = new URL(request.url);
+		if (url.pathname === "/dep") {
+			return new Response(dep);
+		}
+		if (url.pathname === "/text") {
+			return new Response(text);
+		}
+		if (url.pathname === "/common") {
+			return new Response(common);
+		}
+		if (url.pathname === "/dynamic") {
+			return new Response((await import("./dynamic.js")).default);
+		}
+		if (url.pathname.startsWith("/lang/")) {
+			// Build the path dynamically to ensure esbuild doesn't inline the import.
+			const language =
+				"./lang/" + url.pathname.substring("/lang/".length) + ".js";
+			return new Response((await import(language)).default.hello);
+		}
+		return new Response("Not Found", { status: 404 });
+	},
+};

fixtures/additional-modules/src/lang/en.js

@@ -0,0 +1 @@
+export default { hello: "hello" };

fixtures/additional-modules/src/lang/fr.js

@@ -0,0 +1 @@
+export default { hello: "bonjour" };

fixtures/additional-modules/src/text.d.ts

@@ -0,0 +1,4 @@
+declare module "*.txt" {
+	const value: string;
+	export default value;
+}

fixtures/additional-modules/src/text.txt

@@ -0,0 +1 @@
+test

fixtures/additional-modules/test/index.test.ts

@@ -0,0 +1,239 @@
+import assert from "node:assert";
+import childProcess from "node:child_process";
+import { existsSync } from "node:fs";
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import {
+	runWranglerDev,
+	wranglerEntryPath,
+} from "../../shared/src/run-wrangler-long-lived";
+import { describe, beforeAll, afterAll, expect, test } from "vitest";
+import { setTimeout } from "node:timers/promises";
+import { fetch } from "undici";
+
+async function getTmpDir() {
+	return fs.mkdtemp(path.join(os.tmpdir(), "wrangler-modules-"));
+}
+
+type WranglerDev = Awaited<ReturnType<typeof runWranglerDev>>;
+function get(worker: WranglerDev, pathname: string) {
+	const url = `http://${worker.ip}:${worker.port}${pathname}`;
+	// Setting the `MF-Original-URL` header will make Miniflare think this is
+	// coming from a `dispatchFetch()` request, meaning it won't return the pretty
+	// error page, and we'll be able to parse errors as JSON.
+	return fetch(url, { headers: { "MF-Original-URL": url } });
+}
+
+async function retry<T>(closure: () => Promise<T>, max = 30): Promise<T> {
+	for (let attempt = 1; attempt <= max; attempt++) {
+		try {
+			return await closure();
+		} catch (e) {
+			if (attempt === max) throw e;
+		}
+		await setTimeout(1_000);
+	}
+	assert.fail("Unreachable");
+}
+
+describe("find_additional_modules dev", () => {
+	let tmpDir: string;
+	let worker: WranglerDev;
+
+	beforeAll(async () => {
+		// Copy over files to a temporary directory as we'll be modifying them
+		tmpDir = await getTmpDir();
+		await fs.cp(
+			path.resolve(__dirname, "..", "src"),
+			path.join(tmpDir, "src"),
+			{ recursive: true }
+		);
+		await fs.cp(
+			path.resolve(__dirname, "..", "wrangler.toml"),
+			path.join(tmpDir, "wrangler.toml")
+		);
+
+		worker = await runWranglerDev(tmpDir, ["--port=0"]);
+	});
+	afterAll(async () => {
+		await worker.stop();
+		try {
+			await fs.rm(tmpDir, { recursive: true, force: true });
+		} catch (e) {
+			// It seems that Windows doesn't let us delete this, with errors like:
+			//
+			// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
+			// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
+			// Serialized Error: {
+			// 	"code": "EBUSY",
+			// 	"errno": -4082,
+			// 	"path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
+			// 	"syscall": "rmdir",
+			// }
+			console.error(e);
+		}
+	});
+
+	test("supports bundled modules", async () => {
+		const res = await get(worker, "/dep");
+		expect(await res.text()).toBe("bundled");
+	});
+	test("supports text modules", async () => {
+		const res = await get(worker, "/text");
+		expect(await res.text()).toBe("test\n");
+	});
+	test("supports dynamic imports", async () => {
+		const res = await get(worker, "/dynamic");
+		expect(await res.text()).toBe("dynamic");
+	});
+	test("supports commonjs lazy imports", async () => {
+		const res = await get(worker, "/common");
+		expect(await res.text()).toBe("common");
+	});
+	test("supports variable dynamic imports", async () => {
+		const res = await get(worker, "/lang/en");
+		expect(await res.text()).toBe("hello");
+	});
+
+	test("watches additional modules", async () => {
+		const srcDir = path.join(tmpDir, "src");
+
+		// Update dynamically imported file
+		await fs.writeFile(
+			path.join(srcDir, "dynamic.js"),
+			'export default "new dynamic";'
+		);
+		await retry(async () => {
+			const res = await get(worker, "/dynamic");
+			assert.strictEqual(await res.text(), "new dynamic");
+		});
+
+		// Delete dynamically imported file
+		await fs.rm(path.join(srcDir, "lang", "en.js"));
+		const res = await retry(async () => {
+			const res = await get(worker, "/lang/en");
+			assert.strictEqual(res.status, 500);
+			return res;
+		});
+		const error = (await res.json()) as { message?: string };
+		expect(error.message).toBe('No such module "lang/en.js".');
+
+		// Create new dynamically imported file in new directory
+		await fs.mkdir(path.join(srcDir, "lang", "en"));
+		await fs.writeFile(
+			path.join(srcDir, "lang", "en", "us.js"),
+			'export default { hello: "hey" };'
+		);
+		await retry(async () => {
+			const res = await get(worker, "/lang/en/us");
+			assert.strictEqual(await res.text(), "hey");
+		});
+
+		// Update newly created file
+		await fs.writeFile(
+			path.join(srcDir, "lang", "en", "us.js"),
+			'export default { hello: "bye" };'
+		);
+		await retry(async () => {
+			const res = await get(worker, "/lang/en/us");
+			assert.strictEqual(await res.text(), "bye");
+		});
+	});
+});
+
+function build(cwd: string, outDir: string) {
+	return childProcess.spawnSync(
+		process.execPath,
+		[wranglerEntryPath, "deploy", "--dry-run", `--outdir=${outDir}`],
+		{ cwd }
+	);
+}
+
+describe("find_additional_modules deploy", () => {
+	let tmpDir: string;
+	beforeAll(async () => {
+		tmpDir = await getTmpDir();
+	});
+	afterAll(async () => {
+		await fs.rm(tmpDir, { recursive: true, force: true });
+	});
+
+	test("doesn't bundle additional modules", async () => {
+		const outDir = path.join(tmpDir, "out");
+		const result = await build(path.resolve(__dirname, ".."), outDir);
+		expect(result.status).toBe(0);
+
+		// Check additional modules marked external, but other dependencies bundled
+		const bundledEntryPath = path.join(outDir, "index.js");
+		const bundledEntry = await fs.readFile(bundledEntryPath, "utf8");
+		expect(bundledEntry).toMatchInlineSnapshot(`
+			"// src/dep.ts
+			var dep_default = \\"bundled\\";
+
+			// src/index.ts
+			import text from \\"./text.txt\\";
+			import common from \\"./common.cjs\\";
+			var src_default = {
+			  async fetch(request) {
+			    const url = new URL(request.url);
+			    if (url.pathname === \\"/dep\\") {
+			      return new Response(dep_default);
+			    }
+			    if (url.pathname === \\"/text\\") {
+			      return new Response(text);
+			    }
+			    if (url.pathname === \\"/common\\") {
+			      return new Response(common);
+			    }
+			    if (url.pathname === \\"/dynamic\\") {
+			      return new Response((await import(\\"./dynamic.js\\")).default);
+			    }
+			    if (url.pathname.startsWith(\\"/lang/\\")) {
+			      const language = \\"./lang/\\" + url.pathname.substring(\\"/lang/\\".length) + \\".js\\";
+			      return new Response((await import(language)).default.hello);
+			    }
+			    return new Response(\\"Not Found\\", { status: 404 });
+			  }
+			};
+			export {
+			  src_default as default
+			};
+			//# sourceMappingURL=index.js.map
+			"
+		`);
+
+		// Check additional modules included in output
+		expect(existsSync(path.join(outDir, "text.txt"))).toBe(true);
+		expect(existsSync(path.join(outDir, "dynamic.js"))).toBe(true);
+		expect(existsSync(path.join(outDir, "lang", "en.js"))).toBe(true);
+		expect(existsSync(path.join(outDir, "lang", "fr.js"))).toBe(true);
+	});
+
+	test("fails with service worker entrypoint", async () => {
+		// Write basic service worker with `find_additional_modules` enabled
+		const serviceWorkerDir = path.join(tmpDir, "service-worker");
+		await fs.mkdir(serviceWorkerDir, { recursive: true });
+		await fs.writeFile(
+			path.join(serviceWorkerDir, "index.js"),
+			"addEventListener('fetch', (e) => e.respondWith(new Response()))"
+		);
+		await fs.writeFile(
+			path.join(serviceWorkerDir, "wrangler.toml"),
+			[
+				'name="service-worker-test"',
+				'main = "index.js"',
+				'compatibility_date = "2023-08-01"',
+				"find_additional_modules = true",
+			].join("\n")
+		);
+
+		// Try build, and check fails
+		const serviceWorkerOutDir = path.join(tmpDir, "service-worker-out");
+		const result = await build(serviceWorkerDir, serviceWorkerOutDir);
+		expect(result.status).toBe(1);
+		expect(result.stderr.toString()).toContain(
+			"`find_additional_modules` can only be used with an ES module entrypoint."
+		);
+	});
+});

fixtures/additional-modules/test/tsconfig.json

@@ -0,0 +1,7 @@
+{
+	"extends": "@cloudflare/workers-tsconfig/tsconfig.json",
+	"compilerOptions": {
+		"types": ["node"]
+	},
+	"include": ["**/*.ts", "../../../node-types.d.ts"]
+}

fixtures/additional-modules/tsconfig.json

@@ -0,0 +1,14 @@
+{
+	"compilerOptions": {
+		"module": "esnext",
+		"target": "esnext",
+		"lib": ["esnext"],
+		"strict": true,
+		"isolatedModules": true,
+		"noEmit": true,
+		"types": ["@cloudflare/workers-types/experimental"],
+		"allowJs": true,
+		"allowSyntheticDefaultImports": true
+	},
+	"include": ["src"]
+}

fixtures/additional-modules/wrangler.toml

@@ -0,0 +1,9 @@
+name = "additional-modules"
+main = "src/index.ts"
+compatibility_date = "2023-08-01"
+
+find_additional_modules = true
+rules = [
+  { type = "CommonJS", globs = ["**/*.cjs"]},
+  { type = "ESModule", globs = ["**/*.js"]},
+]