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
rollup · Jun 24, 2022 · 6ba7148 · 6ba7148
1 parent 271eaf4
commit 6ba7148
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 27 deletions.
diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js
@@ -45,6 +45,11 @@ export default function commonjs(options = {}) {
   } = options;
   const extensions = options.extensions || ['.js'];
   const filter = createFilter(options.include, options.exclude);
+  const isPossibleCjsId = (id) => {
+    const extName = extname(id);
+    return extName === '.cjs' || (extensions.includes(extName) && filter(id));
+  };
+
   const { strictRequiresFilter, detectCyclesAndConditional } = getStrictRequiresFilter(options);
 
   const getRequireReturnsDefault =
@@ -99,7 +104,7 @@ export default function commonjs(options = {}) {
     };
   };
 
-  const { currentlyResolving, resolveId } = getResolveId(extensions);
+  const { currentlyResolving, resolveId } = getResolveId(extensions, isPossibleCjsId);
 
   const sourceMap = options.sourceMap !== false;
 
@@ -295,10 +300,7 @@ export default function commonjs(options = {}) {
     },
 
     transform(code, id) {
-      const extName = extname(id);
-      if (extName !== '.cjs' && (!filter(id) || !extensions.includes(extName))) {
-        return null;
-      }
+      if (!isPossibleCjsId(id)) return null;
 
       try {
         return transformAndCheckExports.call(this, code, id);

diff --git a/packages/commonjs/src/resolve-id.js b/packages/commonjs/src/resolve-id.js
@@ -49,7 +49,7 @@ export function resolveExtensions(importee, importer, extensions) {
   return undefined;
 }
 
-export default function getResolveId(extensions) {
+export default function getResolveId(extensions, isPossibleCjsId) {
   const currentlyResolving = new Map();
 
   return {
@@ -141,21 +141,27 @@ export default function getResolveId(extensions) {
         !resolved ||
         resolved.external ||
         resolved.id.endsWith(ENTRY_SUFFIX) ||
-        isWrappedId(resolved.id, ES_IMPORT_SUFFIX)
+        isWrappedId(resolved.id, ES_IMPORT_SUFFIX) ||
+        !isPossibleCjsId(resolved.id)
       ) {
         return resolved;
       }
       const moduleInfo = await this.load(resolved);
-      if (resolveOptions.isEntry) {
-        moduleInfo.moduleSideEffects = true;
-        // We must not precede entry proxies with a `\0` as that will mess up relative external resolution
-        return resolved.id + ENTRY_SUFFIX;
-      }
       const {
         meta: { commonjs: commonjsMeta }
       } = moduleInfo;
-      if (commonjsMeta && commonjsMeta.isCommonJS === IS_WRAPPED_COMMONJS) {
-        return { id: wrapId(resolved.id, ES_IMPORT_SUFFIX), meta: { commonjs: { resolved } } };
+      if (commonjsMeta) {
+        const { isCommonJS } = commonjsMeta;
+        if (isCommonJS) {
+          if (resolveOptions.isEntry) {
+            moduleInfo.moduleSideEffects = true;
+            // We must not precede entry proxies with a `\0` as that will mess up relative external resolution
+            return resolved.id + ENTRY_SUFFIX;
+          }
+          if (isCommonJS === IS_WRAPPED_COMMONJS) {
+            return { id: wrapId(resolved.id, ES_IMPORT_SUFFIX), meta: { commonjs: { resolved } } };
+          }
+        }
       }
       return resolved;
     }

diff --git a/packages/commonjs/test/snapshots/function.js.md b/packages/commonjs/test/snapshots/function.js.md
@@ -5537,7 +5537,7 @@ Generated by [AVA](https://avajs.dev).
     {
       'main.js': `'use strict';␊
       ␊
-      require('./polyfill.js');␊
+      global.entryDetected = true;␊
       ␊
       var commonjsGlobal = typeof globalThis !== 'undefined' ? globalThis : typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {};␊
       ␊
@@ -5563,16 +5563,10 @@ Generated by [AVA](https://avajs.dev).
       ␊
       Object.defineProperty(exports, '__esModule', { value: true });␊
       ␊
-      require('./polyfill.js');␊
-      ␊
       const other = true;␊
       ␊
       exports.other = other;␊
       `,
-      'polyfill.js': `'use strict';␊
-      ␊
-      global.entryDetected = true;␊
-      `,
     }
 
 ## preserve-modules

diff --git a/packages/commonjs/test/snapshots/function.js.snap b/packages/commonjs/test/snapshots/function.js.snap
diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js
@@ -9,7 +9,6 @@ import test from 'ava';
 import { getLocator } from 'locate-character';
 
 import { rollup } from 'rollup';
-import { SourceMapConsumer } from 'source-map';
 import { install } from 'source-map-support';
 
 import { testBundle } from '../../../util/test';
@@ -63,6 +62,9 @@ test('generates a sourcemap', async (t) => {
     sourcemapFile: path.resolve('bundle.js')
   });
 
+  // Hack to make it work on Node 18
+  delete global.fetch;
+  const { SourceMapConsumer } = await import('source-map');
   const smc = await new SourceMapConsumer(map);
   const locator = getLocator(code, { offsetLine: 1 });
 
@@ -804,7 +806,7 @@ test('handles when an imported dependency of an ES module changes type', async (
   let bundle = await rollup(options);
   t.is(meta.isCommonJS, false);
   t.deepEqual((await executeBundle(bundle, t)).exports, 'esm');
-  t.deepEqual(trackedTransforms, ['main.js', 'dep.js', 'main.js?commonjs-entry']);
+  t.deepEqual(trackedTransforms, ['main.js', 'dep.js']);
   trackedTransforms.length = 0;
   const esCode = await getCodeFromBundle(bundle);
   t.snapshot(esCode);
@@ -889,7 +891,7 @@ test('handles when a dynamically imported dependency of an ES module changes typ
   let bundle = await rollup(options);
   t.is(meta.isCommonJS, false);
   t.deepEqual(await (await executeBundle(bundle, t)).exports, 'esm');
-  t.deepEqual(trackedTransforms, ['main.js', 'main.js?commonjs-entry', 'dep.js']);
+  t.deepEqual(trackedTransforms, ['main.js', 'dep.js']);
   trackedTransforms.length = 0;
 
   modules['dep.js'] = "exports.dep = 'cjs';";
@@ -1057,7 +1059,6 @@ test('handles when a required dependency of a mixed ES module changes type', asy
   t.deepEqual(trackedTransforms, [
     'dep.js',
     'main.js',
-    'main.js?commonjs-entry',
     '\0commonjsHelpers.js',
     '\0dep.js?commonjs-proxy'
   ]);
@@ -1201,11 +1202,11 @@ test('allows the config to be reused', async (t) => {
   let bundle = await rollup({ input: 'foo.js', ...config });
   t.deepEqual(
     bundle.cache.modules.map(({ id }) => id),
-    ['foo.js', 'foo.js?commonjs-entry']
+    ['foo.js']
   );
   bundle = await rollup({ input: 'bar.js', ...config });
   t.deepEqual(
     bundle.cache.modules.map(({ id }) => id),
-    ['bar.js', 'bar.js?commonjs-entry']
+    ['bar.js']
   );
 });