Skip to content
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

fix: allow requireing alias modules #6932

Merged
merged 7 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions .changeset/nasty-melons-camp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"wrangler": minor
---

fix: allow `require`ing unenv aliased packages

Before this PR `require`ing packages aliased in unenv would fail.
That's because `require` would load the mjs file.

This PR adds wraps the mjs file in a virtual ES module to allow `require`ing it.
14 changes: 13 additions & 1 deletion fixtures/nodejs-hybrid-app/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,30 @@ export default {
return testPostgresLibrary(env, ctx);
case "/test-x509-certificate":
return testX509Certificate();
case "/test-require-alias":
return testRequireUenvAliasedPackages();
}

return new Response(
'<a href="query">Postgres query</a> | ' +
'<a href="test-process">Test process global</a> | ' +
'<a href="test-random">Test getRandomValues()</a> | ' +
'<a href="test-x509-certificate">Test X509Certificate</a>',
'<a href="test-x509-certificate">Test X509Certificate</a>' +
'<a href="test-require-alias">Test require unenv aliased packages</a>',
{ headers: { "Content-Type": "text/html; charset=utf-8" } }
);
},
};

function testRequireUenvAliasedPackages() {
const fetch = require("cross-fetch");
const supportsDefaultExports = typeof fetch === "function";
const supportsNamedExports = typeof fetch.Headers === "function";
return new Response(
supportsDefaultExports && supportsNamedExports ? `"OK!"` : `"KO!"`
);
}

function testX509Certificate() {
try {
new nodeCrypto.X509Certificate(`-----BEGIN CERTIFICATE-----
Expand Down
13 changes: 13 additions & 0 deletions fixtures/nodejs-hybrid-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,17 @@ describe("nodejs compat", () => {
await stop();
}
});

test("import unenv aliased packages", async ({ expect }) => {
const { ip, port, stop } = await runWranglerDev(
resolve(__dirname, "../src"),
["--port=0", "--inspector-port=0"]
);
try {
const response = await fetch(`http://${ip}:${port}/test-require-alias`);
await expect(response.text()).resolves.toBe(`"OK!"`);
} finally {
await stop();
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getBasePath } from "../../paths";
import type { Plugin, PluginBuild } from "esbuild";

const REQUIRED_NODE_BUILT_IN_NAMESPACE = "node-built-in-modules";
const REQUIRED_UNENV_ALIAS_NAMESPACE = "required-unenv-alias";

export const nodejsHybridPlugin: () => Plugin = () => {
const { alias, inject, external } = env(nodeless, cloudflare);
Expand All @@ -14,7 +15,7 @@ export const nodejsHybridPlugin: () => Plugin = () => {
setup(build) {
errorOnServiceWorkerFormat(build);
handleRequireCallsToNodeJSBuiltins(build);
handleAliasedNodeJSPackages(build, alias, external);
handleUnenvAliasedPackages(build, alias, external);
handleNodeJSGlobals(build, inject);
},
};
Expand Down Expand Up @@ -55,7 +56,7 @@ function errorOnServiceWorkerFormat(build: PluginBuild) {
}

/**
* We must convert `require()` calls for Node.js to a virtual ES Module that can be imported avoiding the require calls.
* We must convert `require()` calls for Node.js modules to a virtual ES Module that can be imported avoiding the require calls.
* We do this by creating a special virtual ES module that re-exports the library in an onLoad handler.
* The onLoad handler is triggered by matching the "namespace" added to the resolve.
*/
Expand All @@ -81,38 +82,76 @@ function handleRequireCallsToNodeJSBuiltins(build: PluginBuild) {
);
}

function handleAliasedNodeJSPackages(
function handleUnenvAliasedPackages(
build: PluginBuild,
alias: Record<string, string>,
external: string[]
) {
// esbuild expects alias paths to be absolute
const aliasAbsolute = Object.fromEntries(
Object.entries(alias)
.map(([key, value]) => {
let resolvedAliasPath;
try {
resolvedAliasPath = require.resolve(value);
} catch (e) {
// this is an alias for package that is not installed in the current app => ignore
resolvedAliasPath = "";
}
const aliasAbsolute: Record<string, string> = {};
for (const [module, unresolvedAlias] of Object.entries(alias)) {
try {
aliasAbsolute[module] = require
.resolve(unresolvedAlias)
.replace(/\.cjs$/, ".mjs");
} catch (e) {
// this is an alias for package that is not installed in the current app => ignore
}
}

return [key, resolvedAliasPath.replace(/\.cjs$/, ".mjs")];
})
.filter((entry) => entry[1] !== "")
);
const UNENV_ALIAS_RE = new RegExp(
`^(${Object.keys(aliasAbsolute).join("|")})$`
);

build.onResolve({ filter: UNENV_ALIAS_RE }, (args) => {
const unresolvedAlias = alias[args.path];
// Convert `require()` calls for NPM packages to a virtual ES Module that can be imported avoiding the require calls.
// Note: Does not apply to Node.js packages that are handled in `handleRequireCallsToNodeJSBuiltins`
if (
args.kind === "require-call" &&
(unresolvedAlias.startsWith("unenv/runtime/npm/") ||
unresolvedAlias.startsWith("unenv/runtime/mock/"))
) {
return {
path: args.path,
namespace: REQUIRED_UNENV_ALIAS_NAMESPACE,
};
}
// Resolve the alias to its absolute path and potentially mark it as external
return {
path: aliasAbsolute[args.path],
external: external.includes(alias[args.path]),
external: external.includes(unresolvedAlias),
};
});

build.initialOptions.banner = { js: "", ...build.initialOptions.banner };
build.initialOptions.banner.js += dedent`
function __cf_cjs(esm) {
const cjs = 'default' in esm ? esm.default : {};
vicb marked this conversation as resolved.
Show resolved Hide resolved
for (const [k, v] of Object.entries(esm)) {
if (k !== 'default') {
Object.defineProperty(cjs, k, {
vicb marked this conversation as resolved.
Show resolved Hide resolved
enumerable: true,
value: v,
});
}
}
return cjs;
vicb marked this conversation as resolved.
Show resolved Hide resolved
}
`;

build.onLoad(
{ filter: /.*/, namespace: REQUIRED_UNENV_ALIAS_NAMESPACE },
({ path }) => {
return {
contents: dedent`
import * as esm from '${path}';
module.exports = __cf_cjs(esm);
`,
loader: "js",
};
}
);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading