Skip to content

fix(build): handle rolldown external requires in nodeless builds #4136

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.

Sign up for GitHub

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

Closed
medz wants to merge 5 commits into from
Closed

fix(build): handle rolldown external requires in nodeless builds #4136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
23e14ce
fix(build): handle rolldown external requires in nodeless builds
medz Mar 20, 2026
04a0536
chore: apply automated updates
autofix-ci[bot] Mar 20, 2026
714eac9
test(vercel): update snapshots for cjs builtin route
medz Mar 20, 2026
85c8139
fix(build): always handle nodeless node builtin externals
medz Mar 20, 2026
d72a7ce
fix(build): preserve nodeless builtin alias handling
medz Mar 20, 2026
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
61 changes: 58 additions & 3 deletions src/build/rolldown/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { Nitro } from "nitro/types";
import type { OutputOptions, RolldownOptions, RolldownPlugin } from "rolldown";
import { viteAliasPlugin } from "rolldown/experimental";
import { esmExternalRequirePlugin } from "rolldown/plugins";
import { baseBuildConfig } from "../config.ts";
import { baseBuildPlugins } from "../plugins.ts";
import { builtinModules } from "node:module";
Expand All @@ -8,17 +10,36 @@ import { getChunkName, libChunkName, NODE_MODULES_RE } from "../chunks.ts";

export const getRolldownConfig = async (nitro: Nitro): Promise<RolldownOptions> => {
const base = baseBuildConfig(nitro);
const isNodeless = nitro.options.node === false;
const builtinExternal = isNodeless
? []
: [...builtinModules, ...builtinModules.map((m) => `node:${m}`)];
const nodeBuiltinAliases = getNodeBuiltinAliases(base.aliases);
const external = isNodeless
? base.env.external.filter((id) => !id.startsWith("node:"))
: [...base.env.external, ...builtinExternal];
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const tsc = nitro.options.typescript.tsConfig?.compilerOptions;

let config: RolldownOptions = {
platform: nitro.options.node ? "node" : "neutral",
cwd: nitro.options.rootDir,
input: nitro.options.entry,
external: [...base.env.external, ...builtinModules, ...builtinModules.map((m) => `node:${m}`)],
plugins: [...((await baseBuildPlugins(nitro, base)) as RolldownPlugin[])],
external,
plugins: [
...(isNodeless
? [
...(nodeBuiltinAliases.length > 0
? [viteAliasPlugin({ entries: nodeBuiltinAliases })]
: []),
nodeBuiltinImportExternalPlugin(),
esmExternalRequirePlugin({ external: [/^node:/] }),
]
: []),
...((await baseBuildPlugins(nitro, base)) as RolldownPlugin[]),
],
resolve: {
alias: base.aliases,
alias: isNodeless ? omitAliases(base.aliases, nodeBuiltinAliases) : base.aliases,

@coderabbitai coderabbitai Bot Mar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the file structure and read the relevant sections
head -150 src/build/rolldown/config.ts | tail -n +1 | cat -n

Repository: nitrojs/nitro

Length of output: 5621

🏁 Script executed:

# Get the file size first
wc -l src/build/rolldown/config.ts

Repository: nitrojs/nitro

Length of output: 89

🏁 Script executed:

# Read the key sections: around line 42 and 88-92
sed -n '1,110p' src/build/rolldown/config.ts | cat -n

Repository: nitrojs/nitro

Length of output: 4744

🏁 Script executed:

# Search for helper functions like omitAliases and defu imports
rg -n "omitAliases|defu|import.*defu" src/build/rolldown/config.ts -A 2 -B 2

Repository: nitrojs/nitro

Length of output: 792

🏁 Script executed:

# Check where defu is imported from
rg -n "import.*defu" src/build/ -A 1 -B 1

Repository: nitrojs/nitro

Length of output: 1304

Add post-merge sanitization for resolve.alias in nodeless mode.

Line 42 sanitizes base.aliases to remove node:* mappings in nodeless mode. However, lines 88–92 merge user-provided rolldownConfig/rollupConfig via defu(), which can reintroduce bare-to-node:* alias entries (e.g., { "events": "node:events" }). Unlike external (which is scrubbed post-merge on lines 94–96), resolve.alias is not cleaned, leaving nodeless builds vulnerable to node:* specifiers on the resolve.alias path. Add a post-merge filter for config.resolve.alias to remove entries targeting node:* modules when isNodeless is true.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/build/rolldown/config.ts` at line 42, After merging user configs with
defu(rolldownConfig, rollupConfig) into config, add a post-merge sanitization
step that, when isNodeless is true, iterates config.resolve.alias and removes
any alias entries whose target starts with "node:" (mirroring how external is
scrubbed on lines 94–96); this ensures that even if user-provided aliases (e.g.,
{ "events": "node:events" }) are reintroduced, they are filtered out. Locate the
merge/defu usage and then apply the filter to config.resolve.alias (using the
same omitAliases-style logic or a simple predicate) so the final resolve.alias
contains no node:* mappings. Ensure you reference isNodeless,
config.resolve.alias, base.aliases, omitAliases, and defu in the change.

extensions: base.extensions,
conditionNames: nitro.options.exportConditions,
},
Expand Down Expand Up @@ -70,6 +91,10 @@ export const getRolldownConfig = async (nitro: Nitro): Promise<RolldownOptions>
config
);

if (isNodeless && Array.isArray(config.external)) {
config.external = config.external.filter((id) => !String(id).startsWith("node:"));
Comment on lines +94 to +95

@coderabbitai coderabbitai Bot Mar 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/build/rolldown/config.ts | head -150

Repository: nitrojs/nitro

Length of output: 5621

🏁 Script executed:

# Search for references to config.external and rolldownConfig/rollupConfig to understand override patterns
rg "rolldownConfig|rollupConfig" src/ -A 3 -B 3 --type ts

Repository: nitrojs/nitro

Length of output: 13573

🏁 Script executed:

# Check Rolldown/Rollup types for external property to see what types it accepts
fd "node_modules" -type d -prune -o -type f \( -name "*.d.ts" -o -name "package.json" \) | head -20
# Let's check what types can be assigned to external in rolldown config
rg "external\s*:" src/build/rolldown/config.ts -B 5 -A 5

Repository: nitrojs/nitro

Length of output: 898

🏁 Script executed:

# Search for where external is actually used in the rolldown plugins or build process
rg "config\.external|external\s*[=:]" src/build/ --type ts -B 2 -A 2

Repository: nitrojs/nitro

Length of output: 2931

🏁 Script executed:

cat -n src/presets/deno/preset.ts | grep -A 5 -B 5 "external:"

Repository: nitrojs/nitro

Length of output: 888

🏁 Script executed:

# Check the esmExternalRequirePlugin behavior - does it handle function-based external?
rg "esmExternalRequirePlugin" src/ -B 5 -A 10

Repository: nitrojs/nitro

Length of output: 2208

🏁 Script executed:

# Look for any test files or documentation about external handling in nodeless mode
fd "test" -type f | xargs rg "external|nodeless" 2>/dev/null | head -30

Repository: nitrojs/nitro

Length of output: 3125

🏁 Script executed:

# Check if Deno preset sets node: false (nodeless)
rg "node.*false" src/presets/deno/preset.ts

Repository: nitrojs/nitro

Length of output: 73

🏁 Script executed:

# Check the base-worker preset since it explicitly sets node: false
cat -n src/presets/_nitro/base-worker.ts | head -40

Repository: nitrojs/nitro

Length of output: 716

🏁 Script executed:

# Search for any preset that combines node: false with a function-based external
rg -B 10 "external\s*:\s*\(" src/presets/ | grep -A 10 "node.*false"

Repository: nitrojs/nitro

Length of output: 651

🏁 Script executed:

# Check how defu merges - does it override or merge external configs?
rg "defu" src/build/rolldown/config.ts -B 2 -A 5

Repository: nitrojs/nitro

Length of output: 587

🏁 Script executed:

# Let's check if the nodeBuiltinImportExternalPlugin provides a safeguard
cat -n src/build/rolldown/config.ts | sed -n '125,135p'

Repository: nitrojs/nitro

Length of output: 480

🏁 Script executed:

# Check if there are any other safeguards in the rolldown/rollup plugins
rg "node:" src/build/plugins/ -B 2 -A 2

Repository: nitrojs/nitro

Length of output: 1474

🏁 Script executed:

# Verify the Deno preset setup one more time - checking both node: false and external function
cat -n src/presets/deno/preset.ts | head -50

Repository: nitrojs/nitro

Length of output: 1928

The post-merge external scrub only applies to arrays and can be bypassed by non-array overrides.

When a preset provides external as a function via rollupConfig, the defu merge at lines 88-92 replaces the initial filtered external array with that function. The check at lines 94-95 then skips filtering because it only handles Array.isArray(config.external). This allows a non-array external function that doesn't explicitly filter node:* to override the nodeless mode's intent to strip Node.js modules. The nodeBuiltinImportExternalPlugin safeguard explicitly skips require-call kinds (line 129), so if a user-provided function doesn't handle node:*, require calls to Node builtins can be bundled instead of externalized, risking __require(...) in ESM output.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/build/rolldown/config.ts` around lines 94 - 95, The merged
`config.external` can be a function and bypass the nodeless scrub; ensure when
isNodeless you normalize/wrap `config.external` into a function that always
filters out ids that start with "node:"—if `config.external` is an array,
replace it with a function returning the filtered array, and if it's already a
function (from defu), wrap it with a new function that calls the original and
then removes "node:" ids from its result while preserving the original behavior
for require-call kinds (so compatibility with nodeBuiltinImportExternalPlugin
and its require-call skip). Update the logic around isNodeless/config.external
to perform this normalization instead of only handling Array.isArray.

}

const outputConfig = config.output as OutputOptions;
if (outputConfig.inlineDynamicImports || outputConfig.format === "iife") {
delete outputConfig.inlineDynamicImports;
Expand All @@ -78,3 +103,33 @@ export const getRolldownConfig = async (nitro: Nitro): Promise<RolldownOptions>

return config as RolldownOptions;
};

function getNodeBuiltinAliases(aliases: Record<string, string>) {
return Object.entries(aliases)
.filter(([find, replacement]) => !find.startsWith("node:") && replacement.startsWith("node:"))
.map(([find, replacement]) => ({
key: find,
find,
replacement,
}));
}

function omitAliases(
aliases: Record<string, string>,
omittedAliases: { key: string; find: string; replacement: string }[]
) {
const omitted = new Set(omittedAliases.map((entry) => entry.key));
return Object.fromEntries(Object.entries(aliases).filter(([key]) => !omitted.has(key)));
}

function nodeBuiltinImportExternalPlugin(): RolldownPlugin {
return {
name: "nitro:rolldown-node-builtin-import-external",
resolveId(source, _importer, extraOptions) {
if (!source.startsWith("node:") || extraOptions.kind === "require-call") {
return null;
}
return { id: source, external: true };
},
};
}
8 changes: 8 additions & 0 deletions test/fixture/server/lib/cjs-builtin-require.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const { EventEmitter } = require("events");

class Client extends EventEmitter {}

module.exports = {
Client,
EventEmitter,
};
5 changes: 5 additions & 0 deletions test/fixture/server/lib/cjs-builtin-require.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import client from "./cjs-builtin-require.cjs";

export const Client = client.Client;
export const EventEmitter = client.EventEmitter;
export default client;
10 changes: 10 additions & 0 deletions test/fixture/server/routes/api/cjs-builtin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { defineEventHandler } from "h3";
import builtin from "../../lib/cjs-builtin-require.mjs";

export default defineEventHandler(() => {
return {
ok: true,
clientName: builtin.Client.name,
eventEmitterType: typeof builtin.EventEmitter,
};
});
45 changes: 27 additions & 18 deletions test/presets/cloudflare-module.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,25 @@ import { setupTest, testNitro } from "../tests.ts";

describe("nitro:preset:cloudflare-module", async () => {
const ctx = await setupTest("cloudflare-module");

testNitro(ctx, () => {
const mf = new Miniflare({
modules: true,
compatibilityDate: "2025-04-01",
scriptPath: resolve(ctx.outDir, "server/index.mjs"),
modulesRules: [{ type: "CompiledWasm", include: ["**/*.wasm"] }],
assets: {
directory: resolve(ctx.outDir, "public"),
routerConfig: { has_user_worker: true },
assetConfig: {
// https://developers.cloudflare.com/workers/static-assets/routing/#routing-configuration
html_handling: "auto-trailing-slash" /* default */,
not_found_handling: "none" /* default */,
},
const mf = new Miniflare({
modules: true,
compatibilityDate: "2025-04-01",
scriptPath: resolve(ctx.outDir, "server/index.mjs"),
modulesRules: [{ type: "CompiledWasm", include: ["**/*.wasm"] }],
assets: {
directory: resolve(ctx.outDir, "public"),
routerConfig: { has_user_worker: true },
assetConfig: {
// https://developers.cloudflare.com/workers/static-assets/routing/#routing-configuration
html_handling: "auto-trailing-slash" /* default */,
not_found_handling: "none" /* default */,
},
compatibilityFlags: ["nodejs_compat", "no_nodejs_compat_v2"],
bindings: { ...ctx.env },
});
},
compatibilityFlags: ["nodejs_compat", "no_nodejs_compat_v2"],
bindings: { ...ctx.env },
});

testNitro(ctx, () => {
return async ({ url, headers, method, body }) => {
const res = await mf.dispatchFetch("http://localhost" + url, {
headers: headers || {},
Expand All @@ -52,4 +51,14 @@ describe("nitro:preset:cloudflare-module", async () => {
crons: ["* * * * *"],
});
});

it("should handle cjs builtin requires imported through esm wrappers", async () => {
const res = await mf.dispatchFetch("http://localhost/api/cjs-builtin");
expect(res.status).toBe(200);
await expect(res.json()).resolves.toMatchObject({
ok: true,
clientName: "Client",
eventEmitterType: "function",
});
});
});
5 changes: 5 additions & 0 deletions test/presets/vercel.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ describe("nitro:preset:vercel:web", async () => {
"dest": "/api/db",
"src": "/api/db",
},
{
"dest": "/api/cjs-builtin",
"src": "/api/cjs-builtin",
},
{
"dest": "/api/cached",
"src": "/api/cached",
Expand Down Expand Up @@ -419,6 +423,7 @@ describe("nitro:preset:vercel:web", async () => {
"functions/_swagger.func (symlink)",
"functions/_vercel",
"functions/api/cached.func (symlink)",
"functions/api/cjs-builtin.func (symlink)",
"functions/api/db.func (symlink)",
"functions/api/echo.func (symlink)",
"functions/api/headers.func (symlink)",
Expand Down
Loading