-
Notifications
You must be signed in to change notification settings - Fork 1k
Add find_additional_modules option to support partial bundling with externals
#3726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
8a7e769
typo
petebacondarwin f73331c
move stuff
petebacondarwin 3660aed
remove unnecessary typing
petebacondarwin 662fabc
remove unnecessary `active` property from middleware loader config
petebacondarwin 1738368
more refactorings
petebacondarwin bc2c1ff
rename traverseModuleGraph as this is not what it does
petebacondarwin e686026
refactor to simplify findAdditionalModules()
petebacondarwin a297503
Rename the functions build helpers to make it more clear what their r…
petebacondarwin 6cd5eee
invert module collector config - no longer need to return it as it is…
petebacondarwin cb56381
implement module finding in normal bundling if the find_additional_mo…
petebacondarwin 6bfb60c
allow `__STATIC_CONTENT_MANIFEST` to be imported from any module
mrbbot 42fa7e6
test: improve no-bundle-import tests
petebacondarwin 52f1373
refactor: convert additional file finding to a generator function
petebacondarwin 2025314
fix: ensure that additional modules appear in the out-dir
petebacondarwin 4c13313
pnpm fixups
petebacondarwin 407eaa6
test: ignore failure to remove tmp dir on Windows
petebacondarwin ee6e162
test: do not show output in d1 time-travel tests
petebacondarwin bc11492
refactor: consolidate writing additional modules
petebacondarwin 1deec5e
test: add CommonJS lazy import to additional-modules fixture
petebacondarwin 83372be
test: rename spec to test
petebacondarwin a593ca0
Add debug logging when writing additional modules
petebacondarwin 57eacce
Test additional module failure case and fix message typo
petebacondarwin cce3d79
Display build warnings before updating the bundle
petebacondarwin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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:*" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| module.exports = "common"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default "bundled"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default "dynamic"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 }); | ||
| }, | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default { hello: "hello" }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default { hello: "bonjour" }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| declare module "*.txt" { | ||
| const value: string; | ||
| export default value; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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." | ||
| ); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "extends": "@cloudflare/workers-tsconfig/tsconfig.json", | ||
| "compilerOptions": { | ||
| "types": ["node"] | ||
| }, | ||
| "include": ["**/*.ts", "../../../node-types.d.ts"] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"]}, | ||
petebacondarwin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { type = "ESModule", globs = ["**/*.js"]}, | ||
| ] | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.