Skip to content

Commit 6ba7148

Browse files
lukastaegertlukastaegert
authored
fix(commonjs): Only proxy detected commonjs entry points (#1180)
* fix(commonjs): Only proxy detected commonjs entry points * chore(commonjs): Make tests work on Node 18
1 parent 271eaf4 commit 6ba7148

5 files changed

Lines changed: 30 additions & 27 deletions

File tree

‎packages/commonjs/src/index.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ export default function commonjs(options = {}) {
4545
} = options;
4646
const extensions = options.extensions || ['.js'];
4747
const filter = createFilter(options.include, options.exclude);
48+
const isPossibleCjsId = (id) => {
49+
const extName = extname(id);
50+
return extName === '.cjs' || (extensions.includes(extName) && filter(id));
51+
};
52+
4853
const { strictRequiresFilter, detectCyclesAndConditional } = getStrictRequiresFilter(options);
4954

5055
const getRequireReturnsDefault =
@@ -99,7 +104,7 @@ export default function commonjs(options = {}) {
99104
};
100105
};
101106

102-
const { currentlyResolving, resolveId } = getResolveId(extensions);
107+
const { currentlyResolving, resolveId } = getResolveId(extensions, isPossibleCjsId);
103108

104109
const sourceMap = options.sourceMap !== false;
105110

@@ -295,10 +300,7 @@ export default function commonjs(options = {}) {
295300
},
296301

297302
transform(code, id) {
298-
const extName = extname(id);
299-
if (extName !== '.cjs' && (!filter(id) || !extensions.includes(extName))) {
300-
return null;
301-
}
303+
if (!isPossibleCjsId(id)) return null;
302304

303305
try {
304306
return transformAndCheckExports.call(this, code, id);

‎packages/commonjs/src/resolve-id.js‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export function resolveExtensions(importee, importer, extensions) {
4949
return undefined;
5050
}
5151

52-
export default function getResolveId(extensions) {
52+
export default function getResolveId(extensions, isPossibleCjsId) {
5353
const currentlyResolving = new Map();
5454

5555
return {
@@ -141,21 +141,27 @@ export default function getResolveId(extensions) {
141141
!resolved ||
142142
resolved.external ||
143143
resolved.id.endsWith(ENTRY_SUFFIX) ||
144-
isWrappedId(resolved.id, ES_IMPORT_SUFFIX)
144+
isWrappedId(resolved.id, ES_IMPORT_SUFFIX) ||
145+
!isPossibleCjsId(resolved.id)
145146
) {
146147
return resolved;
147148
}
148149
const moduleInfo = await this.load(resolved);
149-
if (resolveOptions.isEntry) {
150-
moduleInfo.moduleSideEffects = true;
151-
// We must not precede entry proxies with a `\0` as that will mess up relative external resolution
152-
return resolved.id + ENTRY_SUFFIX;
153-
}
154150
const {
155151
meta: { commonjs: commonjsMeta }
156152
} = moduleInfo;
157-
if (commonjsMeta && commonjsMeta.isCommonJS === IS_WRAPPED_COMMONJS) {
158-
return { id: wrapId(resolved.id, ES_IMPORT_SUFFIX), meta: { commonjs: { resolved } } };
153+
if (commonjsMeta) {
154+
const { isCommonJS } = commonjsMeta;
155+
if (isCommonJS) {
156+
if (resolveOptions.isEntry) {
157+
moduleInfo.moduleSideEffects = true;
158+
// We must not precede entry proxies with a `\0` as that will mess up relative external resolution
159+
return resolved.id + ENTRY_SUFFIX;
160+
}
161+
if (isCommonJS === IS_WRAPPED_COMMONJS) {
162+
return { id: wrapId(resolved.id, ES_IMPORT_SUFFIX), meta: { commonjs: { resolved } } };
163+
}
164+
}
159165
}
160166
return resolved;
161167
}

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5537,7 +5537,7 @@ Generated by [AVA](https://avajs.dev).
55375537
{
55385538
'main.js': `'use strict';␊
55395539
␊
5540-
require('./polyfill.js');␊
5540+
global.entryDetected = true;␊
55415541
␊
55425542
var commonjsGlobal = typeof globalThis !== 'undefined' ? globalThis : typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {};␊
55435543
␊
@@ -5563,16 +5563,10 @@ Generated by [AVA](https://avajs.dev).
55635563
␊
55645564
Object.defineProperty(exports, '__esModule', { value: true });␊
55655565
␊
5566-
require('./polyfill.js');␊
5567-
␊
55685566
const other = true;␊
55695567
␊
55705568
exports.other = other;␊
55715569
`,
5572-
'polyfill.js': `'use strict';␊
5573-
␊
5574-
global.entryDetected = true;␊
5575-
`,
55765570
}
55775571

55785572
## preserve-modules

‎packages/commonjs/test/snapshots/function.js.snap‎

-33 Bytes
Binary file not shown.

‎packages/commonjs/test/test.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import test from 'ava';
99
import { getLocator } from 'locate-character';
1010

1111
import { rollup } from 'rollup';
12-
import { SourceMapConsumer } from 'source-map';
1312
import { install } from 'source-map-support';
1413

1514
import { testBundle } from '../../../util/test';
@@ -63,6 +62,9 @@ test('generates a sourcemap', async (t) => {
6362
sourcemapFile: path.resolve('bundle.js')
6463
});
6564

65+
// Hack to make it work on Node 18
66+
delete global.fetch;
67+
const { SourceMapConsumer } = await import('source-map');
6668
const smc = await new SourceMapConsumer(map);
6769
const locator = getLocator(code, { offsetLine: 1 });
6870

@@ -804,7 +806,7 @@ test('handles when an imported dependency of an ES module changes type', async (
804806
let bundle = await rollup(options);
805807
t.is(meta.isCommonJS, false);
806808
t.deepEqual((await executeBundle(bundle, t)).exports, 'esm');
807-
t.deepEqual(trackedTransforms, ['main.js', 'dep.js', 'main.js?commonjs-entry']);
809+
t.deepEqual(trackedTransforms, ['main.js', 'dep.js']);
808810
trackedTransforms.length = 0;
809811
const esCode = await getCodeFromBundle(bundle);
810812
t.snapshot(esCode);
@@ -889,7 +891,7 @@ test('handles when a dynamically imported dependency of an ES module changes typ
889891
let bundle = await rollup(options);
890892
t.is(meta.isCommonJS, false);
891893
t.deepEqual(await (await executeBundle(bundle, t)).exports, 'esm');
892-
t.deepEqual(trackedTransforms, ['main.js', 'main.js?commonjs-entry', 'dep.js']);
894+
t.deepEqual(trackedTransforms, ['main.js', 'dep.js']);
893895
trackedTransforms.length = 0;
894896

895897
modules['dep.js'] = "exports.dep = 'cjs';";
@@ -1057,7 +1059,6 @@ test('handles when a required dependency of a mixed ES module changes type', asy
10571059
t.deepEqual(trackedTransforms, [
10581060
'dep.js',
10591061
'main.js',
1060-
'main.js?commonjs-entry',
10611062
'\0commonjsHelpers.js',
10621063
'\0dep.js?commonjs-proxy'
10631064
]);
@@ -1201,11 +1202,11 @@ test('allows the config to be reused', async (t) => {
12011202
let bundle = await rollup({ input: 'foo.js', ...config });
12021203
t.deepEqual(
12031204
bundle.cache.modules.map(({ id }) => id),
1204-
['foo.js', 'foo.js?commonjs-entry']
1205+
['foo.js']
12051206
);
12061207
bundle = await rollup({ input: 'bar.js', ...config });
12071208
t.deepEqual(
12081209
bundle.cache.modules.map(({ id }) => id),
1209-
['bar.js', 'bar.js?commonjs-entry']
1210+
['bar.js']
12101211
);
12111212
});

0 commit comments

Comments
 (0)