From f36f6501be2a82ccbfd49fe827aacb0b3628c667 Mon Sep 17 00:00:00 2001 From: Keven Arroyo Date: Tue, 10 Mar 2026 14:19:42 -0700 Subject: [PATCH] fix(externals): trace CJS require() dependencies in production builds --- src/build/plugins/externals.ts | 36 ++++++++++++------- src/presets/_all.gen.ts | 8 ++--- .../@fixture/nitro-native-mock/index.js | 2 ++ .../@fixture/nitro-native-mock/package.json | 5 +++ .../node_modules/nitro-cjs-untrace/index.js | 5 +++ .../nitro-cjs-untrace/package.json | 5 +++ test/fixture/server/routes/modules.ts | 4 +++ test/presets/node.test.ts | 11 ++++++ 8 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 test/fixture/node_modules/@fixture/nitro-native-mock/index.js create mode 100644 test/fixture/node_modules/@fixture/nitro-native-mock/package.json create mode 100644 test/fixture/node_modules/nitro-cjs-untrace/index.js create mode 100644 test/fixture/node_modules/nitro-cjs-untrace/package.json diff --git a/src/build/plugins/externals.ts b/src/build/plugins/externals.ts index e1d571d30e..d414528d33 100644 --- a/src/build/plugins/externals.ts +++ b/src/build/plugins/externals.ts @@ -71,21 +71,33 @@ export function externals(opts: ExternalsOptions): Plugin { }; } - // Skip nested rollup-node resolutions - if (rOpts.custom?.["node-resolve"]) { - return null; - } + let resolved; - // Resolve by other resolvers - let resolved = await this.resolve(id, importer, rOpts); + if (rOpts.custom?.["node-resolve"]) { + // For nested node-resolve resolutions (e.g., CJS require()), + // resolve directly to avoid recursion but still externalize + // modules that match the include filter (e.g., native deps). + // Only applies when tracing is active (production builds). + if (!opts.trace) { + return null; + } + const resolvedPath = tryResolve(id, importer); + if (!resolvedPath || !filter(resolvedPath)) { + return null; + } + resolved = { id: resolvedPath }; + } else { + // Resolve by other resolvers + resolved = await this.resolve(id, importer, rOpts); - // Skip rolldown-plugin-commonjs resolver for externals - const cjsResolved = resolved?.meta?.commonjs?.resolved; - if (cjsResolved) { - if (!filter(cjsResolved.id)) { - return resolved; // Bundled and wrapped by CJS plugin + // Skip rolldown-plugin-commonjs resolver for externals + const cjsResolved = resolved?.meta?.commonjs?.resolved; + if (cjsResolved) { + if (!filter(cjsResolved.id)) { + return resolved; // Bundled and wrapped by CJS plugin + } + resolved = cjsResolved /* non-wrapped */; } - resolved = cjsResolved /* non-wrapped */; } // Check if not resolved or explicitly marked as excluded diff --git a/src/presets/_all.gen.ts b/src/presets/_all.gen.ts index 0aabd48fde..a65f289361 100644 --- a/src/presets/_all.gen.ts +++ b/src/presets/_all.gen.ts @@ -1,7 +1,5 @@ // Auto-generated using gen-presets script -import _nitro from "./_nitro/preset.ts"; -import _static from "./_static/preset.ts"; import _alwaysdata from "./alwaysdata/preset.ts"; import _awsAmplify from "./aws-amplify/preset.ts"; import _awsLambda from "./aws-lambda/preset.ts"; @@ -27,10 +25,10 @@ import _vercel from "./vercel/preset.ts"; import _winterjs from "./winterjs/preset.ts"; import _zeabur from "./zeabur/preset.ts"; import _zerops from "./zerops/preset.ts"; +import _nitro from "./_nitro/preset.ts"; +import _static from "./_static/preset.ts"; export default [ - ..._nitro, - ..._static, ..._alwaysdata, ..._awsAmplify, ..._awsLambda, @@ -56,4 +54,6 @@ export default [ ..._winterjs, ..._zeabur, ..._zerops, + ..._nitro, + ..._static, ] as const; diff --git a/test/fixture/node_modules/@fixture/nitro-native-mock/index.js b/test/fixture/node_modules/@fixture/nitro-native-mock/index.js new file mode 100644 index 0000000000..cca3e8c96e --- /dev/null +++ b/test/fixture/node_modules/@fixture/nitro-native-mock/index.js @@ -0,0 +1,2 @@ +// Mock native dependency (simulates packages like bufferutil) +module.exports = "@fixture/nitro-native-mock"; diff --git a/test/fixture/node_modules/@fixture/nitro-native-mock/package.json b/test/fixture/node_modules/@fixture/nitro-native-mock/package.json new file mode 100644 index 0000000000..adfe91a7a6 --- /dev/null +++ b/test/fixture/node_modules/@fixture/nitro-native-mock/package.json @@ -0,0 +1,5 @@ +{ + "name": "@fixture/nitro-native-mock", + "version": "1.0.0", + "main": "./index.js" +} diff --git a/test/fixture/node_modules/nitro-cjs-untrace/index.js b/test/fixture/node_modules/nitro-cjs-untrace/index.js new file mode 100644 index 0000000000..75d1e38b95 --- /dev/null +++ b/test/fixture/node_modules/nitro-cjs-untrace/index.js @@ -0,0 +1,5 @@ +// CJS package NOT in traceDeps that require()s a package that IS in traceDeps. +// Simulates: ws (not traced) -> require('bufferutil') (in NodeNativePackages). +// Without the fix, @fixture/nitro-native-mock is bundled instead of externalized. +const nativeDep = require("@fixture/nitro-native-mock"); +module.exports = nativeDep; diff --git a/test/fixture/node_modules/nitro-cjs-untrace/package.json b/test/fixture/node_modules/nitro-cjs-untrace/package.json new file mode 100644 index 0000000000..aad68ff2b0 --- /dev/null +++ b/test/fixture/node_modules/nitro-cjs-untrace/package.json @@ -0,0 +1,5 @@ +{ + "name": "nitro-cjs-untrace", + "version": "1.0.0", + "main": "./index.js" +} diff --git a/test/fixture/server/routes/modules.ts b/test/fixture/server/routes/modules.ts index b311cefb2e..3a64f30da5 100644 --- a/test/fixture/server/routes/modules.ts +++ b/test/fixture/server/routes/modules.ts @@ -8,6 +8,9 @@ import depLib from "@fixture/nitro-lib"; import subpathLib from "@fixture/nitro-lib/subpath"; // @ts-ignore import extraUtils from "@fixture/nitro-utils/extra"; +// @ts-ignore - Untraced CJS package that require()s @fixture/nitro-native-mock (traced). +// Simulates: ws (not in traceDeps) -> require('bufferutil') (in NodeNativePackages). +import cjsUntrace from "nitro-cjs-untrace"; export default () => { return { @@ -16,5 +19,6 @@ export default () => { depLib, // expected to all be 2.0.0 subpathLib, // expected to 2.0.0 extraUtils, + cjsUntrace, }; }; diff --git a/test/presets/node.test.ts b/test/presets/node.test.ts index e6d7d68c5a..e4762e6682 100644 --- a/test/presets/node.test.ts +++ b/test/presets/node.test.ts @@ -37,4 +37,15 @@ describe("nitro:preset:node-middleware", async () => { const serverNodeModules = resolve(ctx.outDir, "server/node_modules"); expect(existsSync(resolve(serverNodeModules, "@fixture/nitro-utils/extra.mjs"))).toBe(true); }); + + it("should trace CJS require() dependencies", () => { + // Verifies that packages loaded via CJS require() inside bundled code + // are properly externalized and traced (not silently bundled as JS). + // This covers the case where @rollup/plugin-node-resolve resolves a + // require() call with custom: { "node-resolve": { isRequire: true } }. + const serverNodeModules = resolve(ctx.outDir, "server/node_modules"); + expect(existsSync(resolve(serverNodeModules, "@fixture/nitro-native-mock/index.js"))).toBe( + true + ); + }); });