Skip to content
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
19 changes: 12 additions & 7 deletions packages/commonjs/src/resolve-require-sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,16 +196,21 @@ export function getRequireResolver(extensions, detectCyclesAndConditional, curre
// Special-case external Node built-ins to be handled via a lazy __require
// helper instead of hoisted ESM imports when strict wrapping is used.
const isExternalWrapped = isWrappedId(dependencyId, EXTERNAL_SUFFIX);
if (
parentMeta.initialCommonJSType === IS_WRAPPED_COMMONJS &&
!allowProxy &&
isExternalWrapped
) {
let resolvedDependencyId = dependencyId;
if (parentMeta.isCommonJS === IS_WRAPPED_COMMONJS && !allowProxy && isExternalWrapped) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Previously, we checked parentMeta.initialCommonJSType === IS_WRAPPED_COMMONJS instead of parentMeta.isCommonJS === IS_WRAPPED_COMMONJS. With strictRequires: 'auto', I believe the initial type (before cycle detection) can differ from the final type.

const actualExternalId = unwrapId(dependencyId, EXTERNAL_SUFFIX);
if (actualExternalId.startsWith('node:')) {
isCommonJS = IS_WRAPPED_COMMONJS;
parentMeta.isRequiredCommonJS[dependencyId] = isCommonJS;
}
} else if (isExternalWrapped && !allowProxy) {
// If the parent is not wrapped but the dependency is a node: builtin external,
// unwrap the EXTERNAL_SUFFIX so it's treated as a normal external.
// This avoids trying to load the lazy __require proxy for non-wrapped contexts.
const actualExternalId = unwrapId(dependencyId, EXTERNAL_SUFFIX);
if (actualExternalId.startsWith('node:')) {
resolvedDependencyId = actualExternalId;
}
Comment on lines +206 to +213
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. node: builtins with the EXTERNAL_SUFFIX were always loaded with getExternalBuiltinRequireProxy() which only exports __require(). However, when the parent module is not wrapped (no cycles detected with strictRequires: 'auto'), the generated code tried to import a default export that didn't exist.

(so in this case we can remove the suffix)

}
const isWrappedCommonJS = isCommonJS === IS_WRAPPED_COMMONJS;
fullyAnalyzedModules[dependencyId] = true;
Expand All @@ -226,8 +231,8 @@ export function getRequireResolver(extensions, detectCyclesAndConditional, curre
wrappedModuleSideEffects,
source: sources[index].source,
id: allowProxy
? wrapId(dependencyId, isWrappedCommonJS ? WRAPPED_SUFFIX : PROXY_SUFFIX)
: dependencyId,
? wrapId(resolvedDependencyId, isWrappedCommonJS ? WRAPPED_SUFFIX : PROXY_SUFFIX)
: resolvedDependencyId,
isCommonJS
};
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module.exports = {
description: 'handles node: builtins correctly with strictRequires: auto',
pluginOptions: {
strictRequires: 'auto'
},
exports: (exports, t) => {
// Should be able to access properties of node:stream
t.truthy(exports.Readable);
t.is(typeof exports.Readable, 'function');
// Should be able to instantiate
t.truthy(exports.readable);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const stream = require('node:stream');
const readable = new stream.Readable({});

module.exports = { Readable: stream.Readable, readable };
28 changes: 28 additions & 0 deletions packages/commonjs/test/snapshots/function.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -8715,6 +8715,34 @@ Generated by [AVA](https://avajs.dev).
`,
}

## strict-requires-auto-external-node-builtin

> Snapshot 1

{
'main.js': `'use strict';␊
var require$$0 = require('node:stream');␊
function _interopDefaultCompat (e) { return e && typeof e === 'object' && 'default' in e ? e : { default: e }; }␊
var require$$0__default = /*#__PURE__*/_interopDefaultCompat(require$$0);␊
function getDefaultExportFromCjs (x) {␊
return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;␊
}␊
const stream = require$$0__default.default;␊
const readable = new stream.Readable({});␊
var main = { Readable: stream.Readable, readable };␊
var main$1 = /*@__PURE__*/getDefaultExportFromCjs(main);␊
module.exports = main$1;␊
`,
}

## strict-requires-circular

> Snapshot 1
Expand Down
Binary file modified packages/commonjs/test/snapshots/function.js.snap
Binary file not shown.
Loading