Skip to content

Commit

Permalink
fix(wrangler): avoid path collisions between performance and Performa…
Browse files Browse the repository at this point in the history
…nce polyfills (#6025)

* fix(wrangler): avoid path collisions between performance and Performance polyfills

It turns out that ESBuild paths are case insensitive, which can result in
path collisions between polyfills for globalThis.performance and globalThis.Performance.

This change ensures that we encode all global names to lowercase and decode them appropriately.

The version of unenv used at HEAD currently doesn't suffer from this path collision,
but if not fixed the change introduced in https://github.com/unjs/unenv/pull/257/files
would result in Node.js API coverage regressions. This change prevents those regressions
from happening.

* fixup! fix(wrangler): avoid path collisions between performance and Performance polyfills

* fixup! fix(wrangler): avoid path collisions between performance and Performance polyfills
  • Loading branch information
IgorMinar committed Jun 13, 2024
1 parent 169a9fa commit 122ef06
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 12 deletions.
9 changes: 9 additions & 0 deletions .changeset/lazy-cycles-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

fix: avoid path collisions between performance and Performance Node.js polyfills

It turns out that ESBuild paths are case insensitive, which can result in path collisions between polyfills for `globalThis.performance` and `globalThis.Performance`, etc.

This change ensures that we encode all global names to lowercase and decode them appropriately.
14 changes: 14 additions & 0 deletions fixtures/nodejs-hybrid-app/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ assert(buffer2.toJSON().data[0] === 1, "global.Buffer is broken");
const buffer3 = globalThis.Buffer.of(1);
assert(buffer3.toJSON().data[0] === 1, "globalThis.Buffer is broken");

assert(performance !== undefined, "performance is missing");
assert(global.performance !== undefined, "global.performance is missing");
assert(
globalThis.performance !== undefined,
"globalThis.performance is missing"
);

assert(Performance !== undefined, "Performance is missing");
assert(global.Performance !== undefined, "global.Performance is missing");
assert(
globalThis.Performance !== undefined,
"globalThis.Performance is missing"
);

export default {
async fetch(
request: Request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function handleNodeJSGlobals(
...Object.keys(inject).map((globalName) =>
nodePath.resolve(
getBasePath(),
`_virtual_unenv_global_polyfill-${globalName}.js`
`_virtual_unenv_global_polyfill-${encodeToLowerCase(globalName)}.js`
)
),
];
Expand All @@ -104,7 +104,7 @@ function handleNodeJSGlobals(

build.onLoad({ filter: UNENV_GLOBALS_RE }, ({ path }) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const globalName = path.match(UNENV_GLOBALS_RE)![1];
const globalName = decodeFromLowerCase(path.match(UNENV_GLOBALS_RE)![1]);
const globalMapping = inject[globalName];

if (typeof globalMapping === "string") {
Expand Down Expand Up @@ -132,11 +132,6 @@ function handleNodeJSGlobals(
*/ ""
}
/* @__PURE__ */ (() => {
${
/*
// TODO: should we try to preserve globalThis.${globalName} if it exists?
*/ ""
}
return globalThis.${globalName} = globalVar;
})();
Expand Down Expand Up @@ -172,11 +167,6 @@ function handleNodeJSGlobals(
*/ ""
}
/* @__PURE__ */ (() => {
${
/*
// TODO: should we try to preserve globalThis.${globalName} if it exists?
*/ ""
}
return globalThis.${globalName} = ${exportName};
})();
Expand All @@ -189,3 +179,27 @@ function handleNodeJSGlobals(
};
});
}

/**
* Encodes a case sensitive string to lowercase string by prefixing all uppercase letters
* with $ and turning them into lowercase letters.
*
* This function exists because ESBuild requires that all resolved paths are case insensitive.
* Without this transformation, ESBuild will clobber /foo/bar.js with /foo/Bar.js
*
* This is important to support `inject` config for `performance` and `Performance` introduced
* in https://github.com/unjs/unenv/pull/257
*/
function encodeToLowerCase(str: string): string {
return str.replaceAll(/[A-Z]/g, (letter) => `$${letter.toLowerCase()}`);
}

/**
* Decodes a string lowercased using `encodeToLowerCase` to the original strings
*/
function decodeFromLowerCase(str: string): string {
return str.replaceAll(
/\$([a-z])/g,
(match, letter) => `${letter.toUpperCase()}`
);
}

0 comments on commit 122ef06

Please sign in to comment.