Skip to content

Commit 8f298e1

Browse files
committed
fix(commonjs): avoid null access to moduleSideEffects for wrapped externals\n\n- Guard returning null (e.g., for external Node built-ins) when computing .\n- Default to treating as side-effectful when info is unavailable; preserves Rollup tri-state when available.\n- Keeps per-dependency metadata in sync when forcing wrapped CJS for Node built-ins under strictRequires.
1 parent 1935e9e commit 8f298e1

1 file changed

Lines changed: 19 additions & 6 deletions

File tree

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,22 +195,35 @@ export function getRequireResolver(extensions, detectCyclesAndConditional, curre
195195
getTypeForFullyAnalyzedModule(dependencyId));
196196
// Special-case external Node built-ins to be handled via a lazy __require
197197
// helper instead of hoisted ESM imports when strict wrapping is used.
198+
const isExternalWrapped = isWrappedId(dependencyId, EXTERNAL_SUFFIX);
198199
if (
199200
parentMeta.initialCommonJSType === IS_WRAPPED_COMMONJS &&
200201
!allowProxy &&
201-
isWrappedId(dependencyId, EXTERNAL_SUFFIX)
202+
isExternalWrapped
202203
) {
203-
const actualId = unwrapId(dependencyId, EXTERNAL_SUFFIX);
204-
const isNodeBuiltin = actualId.startsWith('node:');
205-
if (isNodeBuiltin) {
204+
const actualExternalId = unwrapId(dependencyId, EXTERNAL_SUFFIX);
205+
if (actualExternalId.startsWith('node:')) {
206206
isCommonJS = IS_WRAPPED_COMMONJS;
207+
parentMeta.isRequiredCommonJS[dependencyId] = isCommonJS;
207208
}
208209
}
209210
const isWrappedCommonJS = isCommonJS === IS_WRAPPED_COMMONJS;
210211
fullyAnalyzedModules[dependencyId] = true;
212+
const moduleInfo =
213+
isWrappedCommonJS && !isExternalWrapped
214+
? rollupContext.getModuleInfo(dependencyId)
215+
: null;
216+
// For wrapped dependencies, annotate the generated require call as pure only
217+
// when Rollup has module info and it explicitly reports no side effects.
218+
// Note: For external Node built-ins (handled via EXTERNAL_SUFFIX), the module
219+
// has not been loaded yet at this point and getModuleInfo returns null.
220+
// Default to side effects = true in that case to be safe.
221+
// Preserve Rollup's tri-state semantics (true | false | 'no-treeshake') when available.
222+
const wrappedModuleSideEffects = !isWrappedCommonJS
223+
? false
224+
: moduleInfo?.moduleSideEffects ?? true;
211225
return {
212-
wrappedModuleSideEffects:
213-
isWrappedCommonJS && rollupContext.getModuleInfo(dependencyId).moduleSideEffects,
226+
wrappedModuleSideEffects,
214227
source: sources[index].source,
215228
id: allowProxy
216229
? wrapId(dependencyId, isWrappedCommonJS ? WRAPPED_SUFFIX : PROXY_SUFFIX)

0 commit comments

Comments
 (0)