Skip to content

Commit ff8f5ae

Browse files
vicbpetebacondarwin
vicb
and
petebacondarwin
authored
refactor(wrangler): Explicitely pick node compat plugins for each mode (#7387)
* refactor: cleanup & simplify * refactor(wrangler): Explicitely pick node compat plugins for each mode * Update packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts Co-authored-by: Pete Bacon Darwin <[email protected]> * fixup! renamed vars (review feedback) * fixup! format --------- Co-authored-by: Pete Bacon Darwin <[email protected]>
1 parent 4aed2d6 commit ff8f5ae

File tree

4 files changed

+66
-67
lines changed

4 files changed

+66
-67
lines changed

‎packages/wrangler/src/deployment-bundle/bundle.ts‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import * as fs from "node:fs";
22
import * as path from "node:path";
3-
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";
4-
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
53
import chalk from "chalk";
64
import * as esbuild from "esbuild";
75
import {
@@ -18,12 +16,9 @@ import {
1816
} from "./build-failures";
1917
import { dedupeModulesByName } from "./dedupe-modules";
2018
import { getEntryPointFromMetafile } from "./entry-point-from-metafile";
21-
import { asyncLocalStoragePlugin } from "./esbuild-plugins/als-external";
2219
import { cloudflareInternalPlugin } from "./esbuild-plugins/cloudflare-internal";
2320
import { configProviderPlugin } from "./esbuild-plugins/config-provider";
24-
import { nodejsHybridPlugin } from "./esbuild-plugins/hybrid-nodejs-compat";
25-
import { nodejsCompatPlugin } from "./esbuild-plugins/nodejs-compat";
26-
import { standardURLPlugin } from "./esbuild-plugins/standard-url";
21+
import { getNodeJSCompatPlugins } from "./esbuild-plugins/nodejs-plugins";
2722
import { writeAdditionalModules } from "./find-additional-modules";
2823
import { noopModuleCollector } from "./module-collection";
2924
import type { Config } from "../config";
@@ -440,20 +435,7 @@ export async function bundleWorker(
440435
plugins: [
441436
aliasPlugin,
442437
moduleCollector.plugin,
443-
...(nodejsCompatMode === "als" ? [asyncLocalStoragePlugin] : []),
444-
...(nodejsCompatMode === "legacy"
445-
? [
446-
NodeGlobalsPolyfills({ buffer: true }),
447-
standardURLPlugin(),
448-
NodeModulesPolyfills(),
449-
]
450-
: []),
451-
// Runtime Node.js compatibility (will warn if not using nodejs compat flag and are trying to import from a Node.js builtin).
452-
...(nodejsCompatMode === "v1" || nodejsCompatMode !== "v2"
453-
? [nodejsCompatPlugin(nodejsCompatMode === "v1")]
454-
: []),
455-
// Hybrid Node.js compatibility
456-
...(nodejsCompatMode === "v2" ? [nodejsHybridPlugin()] : []),
438+
...getNodeJSCompatPlugins(nodejsCompatMode ?? null),
457439
cloudflareInternalPlugin,
458440
buildResultPlugin,
459441
...(plugins || []),

‎packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts‎

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ function handleRequireCallsToNodeJSBuiltins(build: PluginBuild) {
7979
({ path }) => {
8080
return {
8181
contents: dedent`
82-
import libDefault from '${path}';
83-
module.exports = libDefault;`,
82+
import libDefault from '${path}';
83+
module.exports = libDefault;`,
8484
loader: "js",
8585
};
8686
}
@@ -150,8 +150,8 @@ function handleUnenvAliasedPackages(
150150
({ path }) => {
151151
return {
152152
contents: dedent`
153-
import * as esm from '${path}';
154-
module.exports = __cf_cjs(esm);
153+
import * as esm from '${path}';
154+
module.exports = __cf_cjs(esm);
155155
`,
156156
loader: "js",
157157
};
@@ -188,7 +188,10 @@ function handleNodeJSGlobals(
188188
const { importStatement, exportName } = getGlobalInject(inject[globalName]);
189189

190190
return {
191-
contents: `${importStatement}\nglobalThis.${globalName} = ${exportName};`,
191+
contents: dedent`
192+
${importStatement}
193+
globalThis.${globalName} = ${exportName};
194+
`,
192195
};
193196
});
194197
}
@@ -213,38 +216,21 @@ function getGlobalInject(globalInject: string | string[]) {
213216
}
214217

215218
/**
216-
* Encodes a case sensitive string to lowercase string by prefixing all uppercase letters
217-
* with $ and turning them into lowercase letters.
219+
* Encodes a case sensitive string to lowercase string.
220+
*
221+
* - Escape $ with another $ ("$" -> "$$")
222+
* - Escape uppercase letters with $ and turn them into lowercase letters ("L" -> "$L")
218223
*
219224
* This function exists because ESBuild requires that all resolved paths are case insensitive.
220225
* Without this transformation, ESBuild will clobber /foo/bar.js with /foo/Bar.js
221-
*
222-
* This is important to support `inject` config for `performance` and `Performance` introduced
223-
* in https://github.com/unjs/unenv/pull/257
224226
*/
225227
export function encodeToLowerCase(str: string): string {
226-
return str
227-
.replaceAll(/\$/g, () => "$$")
228-
.replaceAll(/[A-Z]/g, (letter) => `$${letter.toLowerCase()}`);
228+
return str.replace(/[A-Z$]/g, (escape) => `$${escape.toLowerCase()}`);
229229
}
230230

231231
/**
232232
* Decodes a string lowercased using `encodeToLowerCase` to the original strings
233233
*/
234234
export function decodeFromLowerCase(str: string): string {
235-
let out = "";
236-
let i = 0;
237-
while (i < str.length - 1) {
238-
if (str[i] === "$") {
239-
i++;
240-
out += str[i].toUpperCase();
241-
} else {
242-
out += str[i];
243-
}
244-
i++;
245-
}
246-
if (i < str.length) {
247-
out += str[i];
248-
}
249-
return out;
235+
return str.replace(/\$[a-z$]/g, (escaped) => escaped[1].toUpperCase());
250236
}

‎packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,32 @@ import chalk from "chalk";
33
import { logger } from "../../logger";
44
import { dedent } from "../../utils/dedent";
55
import type { Plugin } from "esbuild";
6+
import type { NodeJSCompatMode } from "miniflare";
67

78
/**
8-
* An esbuild plugin that will mark any `node:...` imports as external.
9+
* An esbuild plugin that will:
10+
* - mark any `node:...` imports as external
11+
* - warn if there are node imports (if not in v1 mode)
12+
*
13+
* Applies to: null, als, legacy and v1 modes.
914
*/
10-
export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
11-
silenceWarnings
12-
) => ({
15+
export const nodejsCompatPlugin = (mode: NodeJSCompatMode): Plugin => ({
1316
name: "nodejs_compat-imports",
1417
setup(pluginBuild) {
1518
// Infinite loop detection
1619
const seen = new Set<string>();
1720

1821
// Prevent multiple warnings per package
19-
const warnedPackaged = new Map<string, string[]>();
22+
const warnedPackages = new Map<string, string[]>();
2023

2124
pluginBuild.onStart(() => {
2225
seen.clear();
23-
warnedPackaged.clear();
26+
warnedPackages.clear();
2427
});
2528
pluginBuild.onResolve(
2629
{ filter: /node:.*/ },
27-
async ({ path, kind, resolveDir, ...opts }) => {
28-
const specifier = `${path}:${kind}:${resolveDir}:${opts.importer}`;
30+
async ({ path, kind, resolveDir, importer }) => {
31+
const specifier = `${path}:${kind}:${resolveDir}:${importer}`;
2932
if (seen.has(specifier)) {
3033
return;
3134
}
@@ -35,18 +38,15 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
3538
const result = await pluginBuild.resolve(path, {
3639
kind,
3740
resolveDir,
38-
importer: opts.importer,
41+
importer,
3942
});
4043

4144
if (result.errors.length > 0) {
4245
// esbuild couldn't resolve the package
4346
// We should warn the user, but not fail the build
44-
45-
let pathWarnedPackaged = warnedPackaged.get(path);
46-
if (pathWarnedPackaged === undefined) {
47-
warnedPackaged.set(path, (pathWarnedPackaged = []));
48-
}
49-
pathWarnedPackaged.push(opts.importer);
47+
const pathWarnedPackages = warnedPackages.get(path) ?? [];
48+
pathWarnedPackages.push(importer);
49+
warnedPackages.set(path, pathWarnedPackages);
5050

5151
return { external: true };
5252
}
@@ -64,10 +64,10 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
6464
pluginBuild.onEnd(() => {
6565
if (
6666
pluginBuild.initialOptions.format === "iife" &&
67-
warnedPackaged.size > 0
67+
warnedPackages.size > 0
6868
) {
6969
const paths = new Intl.ListFormat("en-US").format(
70-
Array.from(warnedPackaged.keys())
70+
Array.from(warnedPackages.keys())
7171
.map((p) => `"${p}"`)
7272
.sort()
7373
);
@@ -90,8 +90,8 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
9090
// Wait until the build finishes to log warnings, so that all files which import a package
9191
// can be collated
9292
pluginBuild.onEnd(() => {
93-
if (!silenceWarnings) {
94-
warnedPackaged.forEach((importers: string[], path: string) => {
93+
if (mode !== "v1") {
94+
warnedPackages.forEach((importers: string[], path: string) => {
9595
logger.warn(
9696
dedent`
9797
The package "${path}" wasn't found on the file system but is built into node.

‎packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-plugins.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";
2+
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
3+
import { asyncLocalStoragePlugin } from "./als-external";
4+
import { nodejsHybridPlugin } from "./hybrid-nodejs-compat";
5+
import { nodejsCompatPlugin } from "./nodejs-compat";
6+
import { standardURLPlugin } from "./standard-url";
7+
import type { Plugin } from "esbuild";
8+
import type { NodeJSCompatMode } from "miniflare";
9+
10+
/**
11+
* Returns the list of ESBuild plugins to use for a given compat mode.
12+
*/
13+
export function getNodeJSCompatPlugins(mode: NodeJSCompatMode): Plugin[] {
14+
switch (mode) {
15+
case "als":
16+
return [asyncLocalStoragePlugin, nodejsCompatPlugin(mode)];
17+
case "legacy":
18+
return [
19+
NodeGlobalsPolyfills({ buffer: true }),
20+
standardURLPlugin(),
21+
NodeModulesPolyfills(),
22+
nodejsCompatPlugin(mode),
23+
];
24+
case "v1":
25+
return [nodejsCompatPlugin(mode)];
26+
case "v2":
27+
return [nodejsHybridPlugin()];
28+
case null:
29+
return [nodejsCompatPlugin(mode)];
30+
}
31+
}

0 commit comments

Comments
 (0)