Skip to content

Commit 968c3d9

Browse files
fix(hybrid-nodejs-compat): inject modules only once (#8191)
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
1 parent 71eea4f commit 968c3d9

File tree

3 files changed

+49
-96
lines changed

3 files changed

+49
-96
lines changed

.changeset/beige-buses-poke.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
Optimize global injection in node compat mode

packages/wrangler/src/__tests__/hybrid-nodejs-compat.test.ts

-47
This file was deleted.

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

+44-49
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import assert from "node:assert";
12
import { builtinModules } from "node:module";
23
import nodePath from "node:path";
34
import dedent from "ts-dedent";
@@ -169,71 +170,65 @@ function handleNodeJSGlobals(
169170
build: PluginBuild,
170171
inject: Record<string, string | string[]>
171172
) {
172-
const UNENV_GLOBALS_RE = /_virtual_unenv_global_polyfill-([^.]+)\.js$/;
173+
const UNENV_GLOBALS_RE = /_virtual_unenv_global_polyfill-(.+)$/;
173174
const prefix = nodePath.resolve(
174175
getBasePath(),
175176
"_virtual_unenv_global_polyfill-"
176177
);
177178

179+
/**
180+
* Map of module identifiers to
181+
* - `injectedName`: the name injected on `globalThis`
182+
* - `exportName`: the export name from the module
183+
* - `importName`: the imported name
184+
*/
185+
const injectsByModule = new Map<
186+
string,
187+
{ injectedName: string; exportName: string; importName: string }[]
188+
>();
189+
190+
// Module specifier (i.e. `/unenv/runtime/node/...`) keyed by path (i.e. `/prefix/_virtual_unenv_global_polyfill-...`)
191+
const virtualModulePathToSpecifier = new Map<string, string>();
192+
193+
for (const [injectedName, moduleSpecifier] of Object.entries(inject)) {
194+
const [module, exportName, importName] = Array.isArray(moduleSpecifier)
195+
? [moduleSpecifier[0], moduleSpecifier[1], moduleSpecifier[1]]
196+
: [moduleSpecifier, "default", "defaultExport"];
197+
198+
if (!injectsByModule.has(module)) {
199+
injectsByModule.set(module, []);
200+
virtualModulePathToSpecifier.set(
201+
prefix + module.replaceAll("/", "-"),
202+
module
203+
);
204+
}
205+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
206+
injectsByModule.get(module)!.push({ injectedName, exportName, importName });
207+
}
208+
209+
// Inject the virtual modules
178210
build.initialOptions.inject = [
179211
...(build.initialOptions.inject ?? []),
180-
//convert unenv's inject keys to absolute specifiers of custom virtual modules that will be provided via a custom onLoad
181-
...Object.keys(inject).map(
182-
(globalName) => `${prefix}${encodeToLowerCase(globalName)}.js`
183-
),
212+
...virtualModulePathToSpecifier.keys(),
184213
];
185214

186215
build.onResolve({ filter: UNENV_GLOBALS_RE }, ({ path }) => ({ path }));
187216

188217
build.onLoad({ filter: UNENV_GLOBALS_RE }, ({ path }) => {
189-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
190-
const globalName = decodeFromLowerCase(path.match(UNENV_GLOBALS_RE)![1]);
191-
const { importStatement, exportName } = getGlobalInject(inject[globalName]);
218+
const module = virtualModulePathToSpecifier.get(path);
219+
assert(module, `Expected ${path} to be mapped to a module specifier`);
220+
const injects = injectsByModule.get(module);
221+
assert(injects, `Expected ${module} to inject values`);
222+
223+
const imports = injects.map(({ exportName, importName }) =>
224+
importName === exportName ? exportName : `${exportName} as ${importName}`
225+
);
192226

193227
return {
194228
contents: dedent`
195-
${importStatement}
196-
globalThis.${globalName} = ${exportName};
229+
import { ${imports.join(", ")} } from "${module}";
230+
${injects.map(({ injectedName, importName }) => `globalThis.${injectedName} = ${importName};`).join("\n")}
197231
`,
198232
};
199233
});
200234
}
201-
202-
/**
203-
* Get the import statement and export name to be used for the given global inject setting.
204-
*/
205-
function getGlobalInject(globalInject: string | string[]) {
206-
if (typeof globalInject === "string") {
207-
// the mapping is a simple string, indicating a default export, so the string is just the module specifier.
208-
return {
209-
importStatement: `import globalVar from "${globalInject}";`,
210-
exportName: "globalVar",
211-
};
212-
}
213-
// the mapping is a 2 item tuple, indicating a named export, made up of a module specifier and an export name.
214-
const [moduleSpecifier, exportName] = globalInject;
215-
return {
216-
importStatement: `import { ${exportName} } from "${moduleSpecifier}";`,
217-
exportName,
218-
};
219-
}
220-
221-
/**
222-
* Encodes a case sensitive string to lowercase string.
223-
*
224-
* - Escape $ with another $ ("$" -> "$$")
225-
* - Escape uppercase letters with $ and turn them into lowercase letters ("L" -> "$L")
226-
*
227-
* This function exists because ESBuild requires that all resolved paths are case insensitive.
228-
* Without this transformation, ESBuild will clobber /foo/bar.js with /foo/Bar.js
229-
*/
230-
export function encodeToLowerCase(str: string): string {
231-
return str.replace(/[A-Z$]/g, (escape) => `$${escape.toLowerCase()}`);
232-
}
233-
234-
/**
235-
* Decodes a string lowercased using `encodeToLowerCase` to the original strings
236-
*/
237-
export function decodeFromLowerCase(str: string): string {
238-
return str.replace(/\$[a-z$]/g, (escaped) => escaped[1].toUpperCase());
239-
}

0 commit comments

Comments
 (0)