Skip to content

Commit 7e38149

Browse files
committed
fix(commonjs): avoid hoisting dynamically required node: builtins under strictRequires
- Treat external node: builtins as wrapped requires only when the parent module is wrapped (strictRequires or dynamic require), so requires are left lazy via a __require helper and resolved with createRequire instead of top-level ESM imports. - Add getExternalBuiltinRequireProxy to emit a proxy that exports a lazy __require for builtins. - Keep non-strict behavior unchanged to preserve existing unresolved-import warnings (e.g., require("path")). - Add test: strict-requires-external-node-builtin.
1 parent 764910a commit 7e38149

File tree

7 files changed

+75
-2
lines changed

7 files changed

+75
-2
lines changed

packages/commonjs/src/index.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import {
2525
getEntryProxy,
2626
getEsImportProxy,
2727
getStaticRequireProxy,
28-
getUnknownRequireProxy
28+
getUnknownRequireProxy,
29+
getExternalBuiltinRequireProxy
2930
} from './proxies';
3031
import getResolveId from './resolve-id';
3132
import { getRequireResolver } from './resolve-require-sources';
@@ -262,6 +263,10 @@ export default function commonjs(options = {}) {
262263

263264
if (isWrappedId(id, EXTERNAL_SUFFIX)) {
264265
const actualId = unwrapId(id, EXTERNAL_SUFFIX);
266+
const isNodeBuiltin = actualId.startsWith('node:');
267+
if (isNodeBuiltin) {
268+
return getExternalBuiltinRequireProxy(actualId);
269+
}
265270
return getUnknownRequireProxy(
266271
actualId,
267272
isEsmExternal(actualId) ? getRequireReturnsDefault(actualId) : true

packages/commonjs/src/proxies.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,16 @@ export function getEsImportProxy(id, defaultIsModuleExports, moduleSideEffects)
8585
syntheticNamedExports: '__moduleExports'
8686
};
8787
}
88+
89+
// For external Node built-ins required from wrapped CommonJS modules, we must not
90+
// hoist an ESM import of the built-in (which would eagerly load it). Instead,
91+
// expose a lazy `__require()` that resolves the built-in at runtime via
92+
// `createRequire(import.meta.url)`.
93+
export function getExternalBuiltinRequireProxy(id) {
94+
const stringifiedId = JSON.stringify(id);
95+
return (
96+
`import { createRequire } from 'node:module';\n` +
97+
`const require = createRequire(import.meta.url);\n` +
98+
`export function __require() { return require(${stringifiedId}); }`
99+
);
100+
}

packages/commonjs/src/resolve-require-sources.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
isWrappedId,
66
PROXY_SUFFIX,
77
wrapId,
8+
unwrapId,
89
WRAPPED_SUFFIX
910
} from './helpers';
1011
import { resolveExtensions } from './resolve-id';
@@ -190,8 +191,21 @@ export function getRequireResolver(extensions, detectCyclesAndConditional, curre
190191
fullyAnalyzedModules[parentId] = true;
191192
return requireTargets.map(({ id: dependencyId, allowProxy }, index) => {
192193
// eslint-disable-next-line no-multi-assign
193-
const isCommonJS = (parentMeta.isRequiredCommonJS[dependencyId] =
194+
let isCommonJS = (parentMeta.isRequiredCommonJS[dependencyId] =
194195
getTypeForFullyAnalyzedModule(dependencyId));
196+
// Special-case external Node built-ins to be handled via a lazy __require
197+
// helper instead of hoisted ESM imports when strict wrapping is used.
198+
if (
199+
parentMeta.initialCommonJSType === IS_WRAPPED_COMMONJS &&
200+
!allowProxy &&
201+
isWrappedId(dependencyId, EXTERNAL_SUFFIX)
202+
) {
203+
const actualId = unwrapId(dependencyId, EXTERNAL_SUFFIX);
204+
const isNodeBuiltin = actualId.startsWith('node:');
205+
if (isNodeBuiltin) {
206+
isCommonJS = IS_WRAPPED_COMMONJS;
207+
}
208+
}
195209
const isWrappedCommonJS = isCommonJS === IS_WRAPPED_COMMONJS;
196210
fullyAnalyzedModules[dependencyId] = true;
197211
return {
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
module.exports = {
2+
description: "does not hoist external node built-in requires when strictRequires is true",
3+
pluginOptions: {
4+
strictRequires: true
5+
},
6+
exports: (exports, t) => {
7+
t.is(exports, 42);
8+
}
9+
};
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
if (false) {
2+
require('node:sqlite');
3+
}
4+
module.exports = 42;

packages/commonjs/test/snapshots/function.js.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9194,6 +9194,34 @@ Generated by [AVA](https://avajs.dev).
91949194
`,
91959195
}
91969196

9197+
## strict-requires-external-node-builtin
9198+
9199+
> Snapshot 1
9200+
9201+
{
9202+
'main.js': `'use strict';␊
9203+
9204+
function getDefaultExportFromCjs (x) {␊
9205+
return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;␊
9206+
}␊
9207+
9208+
var main$1;␊
9209+
var hasRequiredMain;␊
9210+
9211+
function requireMain () {␊
9212+
if (hasRequiredMain) return main$1;␊
9213+
hasRequiredMain = 1;␊
9214+
main$1 = 42;␊
9215+
return main$1;␊
9216+
}␊
9217+
9218+
var mainExports = requireMain();␊
9219+
var main = /*@__PURE__*/getDefaultExportFromCjs(mainExports);␊
9220+
9221+
module.exports = main;␊
9222+
`,
9223+
}
9224+
91979225
## strict-requires-file-without-module-type
91989226

91999227
> Snapshot 1
48 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)