Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/large-coins-open.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"@cloudflare/workers-utils": minor
"wrangler": minor
---

Add `bundling_external` configuration option for marking modules as external during bundling

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the bike-shedding, but I wonder if external_imports might be a bit clearer here? It may make it more obvious what the array contains. 😄

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mh... I would really keep bundling in the name though... no? just to make it extra clear that this is related to bundling 🤔

I was also thinking... what if we added a bundling config object and in it the only field (for now at least) would be external_imports?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. In that case, I wonder if bundle.external would be a better fit, since it would read as a more explicit counterpart to no_bundle.

Then, if this area grows over time, we could potentially add things like bundle.minify and gradually move some of the current top-level config there and then deprecate the old names . Definitely not something this PR needs to solve, just thinking out loud about how this config might evolve.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that sounds like a potentially good plan to me 🙂

shall I just then turn bundling_external to bundle.external? 🙂


You can now configure modules to be excluded from bundling using the `bundling_external` option in your Wrangler configuration:

```json
{
"bundling_external": ["external-module", "another-external-module"]
}
```

This corresponds to esbuild's `external` option and is useful when you have modules that should be resolved at runtime rather than bundled. The option is inheritable, so it can be set at the top level or per-environment.

Additionally, when a module cannot be resolved during bundling, Wrangler now suggests using `bundling_external` or `alias` to fix the issue.
1 change: 1 addition & 0 deletions packages/workers-utils/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ export const defaultWranglerConfig: Config = {
no_bundle: undefined,
minify: undefined,
keep_names: undefined,
bundling_external: undefined,
dispatch_namespaces: [],
first_party_worker: undefined,
logfwdr: { bindings: [] },
Expand Down
8 changes: 8 additions & 0 deletions packages/workers-utils/src/config/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,14 @@ interface EnvironmentInheritable {
*/
no_bundle: boolean | undefined;

/**
* A list of modules to be considered external during bundling (not applicable when bundling is disabled).
* Corresponds with esbuild's `external` config (https://esbuild.github.io/api/#external).
*
* @inheritable
*/
bundling_external: string[] | undefined;

/**
* Minify the script before uploading.
* @inheritable
Expand Down
8 changes: 8 additions & 0 deletions packages/workers-utils/src/config/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1916,6 +1916,14 @@ function normalizeAndValidateEnvironment(
isBoolean,
undefined
),
bundling_external: inheritable(
diagnostics,
topLevelEnv,
rawEnv,
"bundling_external",
isStringArray,
undefined
),
first_party_worker: inheritable(
diagnostics,
topLevelEnv,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ describe("normalizeAndValidateConfig()", () => {
base_dir: undefined,
limits: undefined,
keep_names: undefined,
bundling_external: undefined,
assets: undefined,
observability: undefined,
cache: undefined,
Expand Down Expand Up @@ -1027,6 +1028,7 @@ describe("normalizeAndValidateConfig()", () => {
},
no_bundle: true,
minify: true,
bundling_external: ["external-a", "external-b"],
first_party_worker: true,
logpush: true,
upload_source_maps: true,
Expand Down Expand Up @@ -1127,6 +1129,7 @@ describe("normalizeAndValidateConfig()", () => {
},
no_bundle: "INVALID",
minify: "INVALID",
bundling_external: "INVALID",
first_party_worker: "INVALID",
logpush: "INVALID",
upload_source_maps: "INVALID",
Expand Down Expand Up @@ -1217,6 +1220,7 @@ describe("normalizeAndValidateConfig()", () => {
- The field "define.DEF1" should be a string but got 1777.
- Expected "no_bundle" to be of type boolean but got "INVALID".
- Expected "minify" to be of type boolean but got "INVALID".
- Expected "bundling_external" to be of type string array but got "INVALID".
- Expected "first_party_worker" to be of type boolean but got "INVALID".
- Expected "logpush" to be of type boolean but got "INVALID".
- Expected "upload_source_maps" to be of type boolean but got "INVALID".
Expand Down Expand Up @@ -5351,6 +5355,7 @@ describe("normalizeAndValidateConfig()", () => {
},
no_bundle: true,
minify: true,
bundling_external: ["top-level-external"],
first_party_worker: true,
logpush: true,
upload_source_maps: true,
Expand Down Expand Up @@ -5403,6 +5408,7 @@ describe("normalizeAndValidateConfig()", () => {
},
no_bundle: false,
minify: false,
bundling_external: ["env-external"],
first_party_worker: false,
logpush: false,
upload_source_maps: false,
Expand All @@ -5429,6 +5435,7 @@ describe("normalizeAndValidateConfig()", () => {
},
no_bundle: true,
minify: true,
bundling_external: ["top-level-external"],
first_party_worker: true,
logpush: true,
upload_source_maps: true,
Expand Down Expand Up @@ -5790,6 +5797,7 @@ describe("normalizeAndValidateConfig()", () => {
},
no_bundle: "INVALID",
minify: "INVALID",
bundling_external: "INVALID",
first_party_worker: "INVALID",
logpush: "INVALID",
upload_source_maps: "INVALID",
Expand Down Expand Up @@ -5828,6 +5836,7 @@ describe("normalizeAndValidateConfig()", () => {
- Expected "main" to be of type string but got 1333.
- Expected "no_bundle" to be of type boolean but got "INVALID".
- Expected "minify" to be of type boolean but got "INVALID".
- Expected "bundling_external" to be of type string array but got "INVALID".
- Expected "first_party_worker" to be of type boolean but got "INVALID".
- Expected "logpush" to be of type boolean but got "INVALID".
- Expected "upload_source_maps" to be of type boolean but got "INVALID"."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,32 @@ describe("BundleController", { retry: 5, timeout: 10_000 }, () => {
`);
});

test("external modules via build.external", async () => {
await seed({
"src/index.ts": dedent/* javascript */ `
import foo from 'external-module';
export default {
fetch(request, env, ctx) {
return new Response(foo)
}
} satisfies ExportedHandler
`,
});
const config = configDefaults({
entrypoint: path.resolve("src/index.ts"),
projectRoot: path.resolve("src"),
build: {
external: ["external-module"],
},
});
const ev = bus.waitFor("bundleComplete");
controller.onConfigUpdate({ type: "configUpdate", config });
const bundleSource = (await ev).bundle.entrypointSource;

// External import should be preserved, not bundled
expect(bundleSource).toMatch(/from\s*["']external-module["']/);
});

test("custom build", async () => {
await seed({
"custom_build_dir/index.ts": dedent/* javascript */ `
Expand Down
189 changes: 186 additions & 3 deletions packages/wrangler/src/__tests__/deploy/build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from "@cloudflare/workers-utils/test-helpers";
import * as esbuild from "esbuild";
import { http, HttpResponse } from "msw";
import dedent from "ts-dedent";
import { afterEach, beforeEach, describe, expect, it, test, vi } from "vitest";
import { getInstalledPackageVersion } from "../../autoconfig/frameworks/utils/packages";
import { printBundleSize } from "../../deployment-bundle/bundle-reporter";
Expand Down Expand Up @@ -512,6 +513,152 @@ describe("deploy", () => {
await runWrangler("deploy -e testEnv index.js");
});
});
describe("bundling_external", () => {
it("should mark modules as external when `bundling_external` is configured", async () => {
writeWranglerConfig({
main: "./index.js",
bundling_external: ["external-module"],
});
fs.writeFileSync(
"./index.js",
`
import foo from 'external-module';
export default {
fetch() {
return new Response(foo);
}
}
`
);

mockUploadWorkerRequest({
expectedType: "esm",
expectedEntry: (str) => {
// External import should be preserved as an import statement
expect(str).toMatch(/from\s*["']external-module["']/);
},
});

mockSubDomainRequest();
await runWrangler("deploy index.js");
expect(std.out).toMatchInlineSnapshot(`
"
⛅️ wrangler x.x.x
──────────────────
Total Upload: xx KiB / gzip: xx KiB
Worker Startup Time: 100 ms
Uploaded test-name (TIMINGS)
Deployed test-name triggers (TIMINGS)
https://test-name.test-sub-domain.workers.dev
Current Version ID: Galaxy-Class"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});

it("should mark modules as external when `bundling_external` is configured in environment", async () => {
writeWranglerConfig({
main: "./index.js",
legacy_env: false,
env: {
testEnv: {
bundling_external: ["env-external-module"],
},
},
});
fs.writeFileSync(
"./index.js",
`
import bar from 'env-external-module';
export default {
fetch() {
return new Response(bar);
}
}`
);

mockUploadWorkerRequest({
env: "testEnv",
expectedType: "esm",
useServiceEnvironments: true,
expectedEntry: (str) => {
// External import should be preserved
expect(str).toMatch(/from\s*["']env-external-module["']/);
},
});

mockSubDomainRequest();
await runWrangler("deploy -e testEnv index.js");
expect(std.out).toMatchInlineSnapshot(`
"
⛅️ wrangler x.x.x
──────────────────
Total Upload: xx KiB / gzip: xx KiB
Worker Startup Time: 100 ms
Uploaded test-name (testEnv) (TIMINGS)
Deployed test-name (testEnv) triggers (TIMINGS)
https://testEnv.test-name.test-sub-domain.workers.dev
Current Version ID: undefined"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});

it("should recommend bundling_external when a non-Node module cannot be resolved", async () => {
writeWranglerConfig();
fs.writeFileSync(
"index.js",
dedent/* javascript */ `
import foo from 'some-nonexistent-module';
export default { fetch() { return new Response(foo); } }
`
);

await expect(
runWrangler("deploy index.js --dry-run").catch((e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
)
)
).resolves.toMatchInlineSnapshot(`
"X [ERROR] Could not resolve "some-nonexistent-module"

index.js:1:16:
1 │ import foo from 'some-nonexistent-module';
╵ ~~~~~~~~~~~~~~~~~~~~~~~~~

To fix this, you can:
- Add "some-nonexistent-module" to "bundling_external" in your Wrangler configuration to exclude it from the bundle
- Add an entry to "alias" in your Wrangler configuration to map it to a different module (see https://developers.cloudflare.com/workers/wrangler/configuration/#module-aliasing)"
`);
});

it("should NOT recommend bundling_external for Node built-in modules", async () => {
writeWranglerConfig();
fs.writeFileSync("index.js", "import fs from 'fs';");

await expect(
runWrangler("deploy index.js --dry-run").catch((e) =>
normalizeString(
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
)
)
).resolves.toMatchInlineSnapshot(`
"X [ERROR] Could not resolve "fs"

index.js:1:15:
1 │ import fs from 'fs';
╵ ~~~~

The package "fs" wasn't found on the file system but is built into node.
- Add the "nodejs_compat" compatibility flag to your project."
`);
});
});
describe("--node-compat", () => {
it("should error when using node compatibility mode", async () => {
writeWranglerConfig();
Expand Down Expand Up @@ -1140,10 +1287,46 @@ describe("deploy", () => {
fs.writeFileSync("index.js", scriptContent);
await runWrangler("deploy index.js --dry-run --outdir dist");
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] \`--minify\` and \`--no-bundle\` can't be used together. If you want to minify your Worker and disable Wrangler's bundling, please minify as part of your own bundling process.
"▲ [WARNING] \`--minify\` and \`--no-bundle\` can't be used together. If you want to minify your Worker and disable Wrangler's bundling, please minify as part of your own bundling process.

"
`);
"
`);
});
});
describe("--no-bundle bundling_external", () => {
it("should warn that no-bundle and bundling_external can't be used together (CLI flag)", async () => {
writeWranglerConfig({
bundling_external: ["external-module"],
});
const scriptContent = `
import foo from 'external-module';
export default { fetch() { return new Response(foo); } }
`;
fs.writeFileSync("index.js", scriptContent);
await runWrangler("deploy index.js --no-bundle --dry-run --outdir dist");
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] \`bundling_external\` and \`--no-bundle\` can't be used together. If you want to exclude modules from your bundle and disable Wrangler's bundling, please handle external modules as part of your own bundling process.

"
`);
});

it("should warn that no-bundle and bundling_external can't be used together (config)", async () => {
writeWranglerConfig({
no_bundle: true,
bundling_external: ["external-module"],
});
const scriptContent = `
import foo from 'external-module';
export default { fetch() { return new Response(foo); } }
`;
fs.writeFileSync("index.js", scriptContent);
await runWrangler("deploy index.js --dry-run --outdir dist");
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] \`bundling_external\` and \`--no-bundle\` can't be used together. If you want to exclude modules from your bundle and disable Wrangler's bundling, please handle external modules as part of your own bundling process.

"
`);
});
});
describe("source maps", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class BundlerController extends Controller {
entryName: undefined,
inject: undefined,
isOutfile: undefined,
external: undefined,
external: config.build.external,

// We don't use esbuild watching for custom builds
watch: undefined,
Expand Down Expand Up @@ -284,6 +284,7 @@ export class BundlerController extends Controller {
config.compatibilityFlags
),
pythonModulesExcludes: config.pythonModules?.exclude ?? [],
external: config.build.external,
},
(cb) => {
const newBundle = cb(this.#currentBundle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ async function resolveConfig(

minify: input.build?.minify ?? config.minify,
keepNames: input.build?.keepNames ?? config.keep_names,
external: input.build?.external ?? config.bundling_external,
define: { ...config.define, ...input.build?.define },
custom: {
command: input.build?.custom?.command ?? config.build?.command,
Expand Down
Loading
Loading