fix(commonjs): avoid wrapping commonjsRegister call in `createCommo…

…njsModule(...)` (#602)

This happens when the code has an strong indicator for wrapping, like /__esModule/.test(code)

packages/commonjs/src/dynamic-packages-manager.js

@@ -50,3 +50,8 @@ export function getDynamicPackagesEntryIntro(
 
   return dynamicImports;
 }
+
+export function isModuleRegistrationProxy(id, dynamicRequireModuleSet) {
+  const normalizedPath = normalizePathSlashes(id);
+  return dynamicRequireModuleSet.has(normalizedPath) && !normalizedPath.endsWith('.json');
+}

packages/commonjs/src/index.js

@@ -5,7 +5,11 @@ import getCommonDir from 'commondir';
 
 import { peerDependencies } from '../package.json';
 
-import { getDynamicPackagesEntryIntro, getDynamicPackagesModule } from './dynamic-packages-manager';
+import {
+  getDynamicPackagesEntryIntro,
+  getDynamicPackagesModule,
+  isModuleRegistrationProxy
+} from './dynamic-packages-manager';
 import getDynamicRequirePaths from './dynamic-require-paths';
 import {
   DYNAMIC_JSON_PREFIX,
@@ -103,6 +107,9 @@ export default function commonjs(options = {}) {
       return null;
     }
 
+    // avoid wrapping in createCommonjsModule, as this is a commonjsRegister call
+    const disableWrap = isModuleRegistrationProxy(id, dynamicRequireModuleSet);
+
     const transformed = transformCommonjs(
       this.parse,
       code,
@@ -113,6 +120,7 @@ export default function commonjs(options = {}) {
       sourceMap,
       isDynamicRequireModulesEnabled,
       dynamicRequireModuleSet,
+      disableWrap,
       commonDir,
       ast
     );
@@ -168,9 +176,8 @@ export default function commonjs(options = {}) {
         return getDynamicJsonProxy(id, commonDir);
       }
 
-      const normalizedPath = normalizePathSlashes(id);
-      if (dynamicRequireModuleSet.has(normalizedPath) && !normalizedPath.endsWith('.json')) {
-        return getDynamicRequireProxy(normalizedPath, commonDir);
+      if (isModuleRegistrationProxy(id, dynamicRequireModuleSet)) {
+        return getDynamicRequireProxy(normalizePathSlashes(id), commonDir);
       }
 
       if (id.endsWith(PROXY_SUFFIX)) {

packages/commonjs/src/transform.js

@@ -127,6 +127,7 @@ export function transformCommonjs(
   sourceMap,
   isDynamicRequireModulesEnabled,
   dynamicRequireModuleSet,
+  disableWrap,
   commonDir,
   astCache
 ) {
@@ -547,7 +548,7 @@ export function transformCommonjs(
 
   // If `isEsModule` is on, it means it has ES6 import/export statements,
   //   which just can't be wrapped in a function.
-  if (isEsModule) shouldWrap = false;
+  shouldWrap = shouldWrap && !disableWrap && !isEsModule;
 
   usesCommonjsHelpers = usesCommonjsHelpers || shouldWrap;
 

packages/commonjs/test/fixtures/samples/dynamic-require-double-wrap/main.js

@@ -0,0 +1 @@
+require('./submodule');

packages/commonjs/test/fixtures/samples/dynamic-require-double-wrap/submodule.js

@@ -0,0 +1,3 @@
+Object.defineProperty(exports, '__esModule', { value: true });
+
+module.exports = 'submodule';

packages/commonjs/test/test.js

@@ -706,3 +706,19 @@ test('transforms the es file with a `commonjsRequire` and no `require`s', async
     true
   );
 });
+
+test('does not wrap commonjsRegister calls in createCommonjsModule', async (t) => {
+  const bundle = await rollup({
+    input: 'fixtures/samples/dynamic-require-double-wrap/main.js',
+    plugins: [
+      commonjs({
+        sourceMap: true,
+        dynamicRequireTargets: ['fixtures/samples/dynamic-require-double-wrap/submodule.js']
+      })
+    ]
+  });
+
+  const code = await getCodeFromBundle(bundle, { exports: 'named' });
+
+  t.not(/createCommonjsModule\(function/.test(code), true);
+});