fix(commonjs): register dynamic modules when a different loader (i.e typescript) loads the entry file

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

@@ -34,26 +34,19 @@ export function getDynamicPackagesModule(dynamicRequireModuleDirPaths, commonDir
 }
 
 export function getDynamicPackagesEntryIntro(
-  id,
   dynamicRequireModuleDirPaths,
   dynamicRequireModuleSet
 ) {
-  try {
-    const code = readFileSync(id, { encoding: 'utf8' });
-    let dynamicImports = Array.from(
-      dynamicRequireModuleSet,
-      (dynamicId) => `require(${JSON.stringify(DYNAMIC_REGISTER_PREFIX + dynamicId)});`
-    ).join('\n');
+  let dynamicImports = Array.from(
+    dynamicRequireModuleSet,
+    (dynamicId) => `require(${JSON.stringify(DYNAMIC_REGISTER_PREFIX + dynamicId)});`
+  ).join('\n');
 
-    if (dynamicRequireModuleDirPaths.length) {
-      dynamicImports += `require(${JSON.stringify(
-        DYNAMIC_REGISTER_PREFIX + DYNAMIC_PACKAGES_ID
-      )});`;
-    }
-
-    return `${dynamicImports}\n${code}`;
-  } catch (ex) {
-    this.warn(`Failed to read file ${id}, dynamic modules might not work correctly`);
-    return null;
+  if (dynamicRequireModuleDirPaths.length) {
+    dynamicImports += `require(${JSON.stringify(
+      DYNAMIC_REGISTER_PREFIX + DYNAMIC_PACKAGES_ID
+    )});`;
   }
+
+  return dynamicImports;
 }

packages/commonjs/src/index.js

@@ -76,6 +76,13 @@ export default function commonjs(options = {}) {
   const sourceMap = options.sourceMap !== false;
 
   function transformAndCheckExports(code, id) {
+    if (isDynamicRequireModulesEnabled && this.getModuleInfo(id).isEntry) {
+      code = getDynamicPackagesEntryIntro(
+        dynamicRequireModuleDirPaths,
+        dynamicRequireModuleSet
+      ) + code;
+    }
+
     const { isEsModule, hasDefaultExport, hasNamedExports, ast } = checkEsModule(
       this.parse,
       code,
@@ -176,14 +183,6 @@ export default function commonjs(options = {}) {
         );
       }
 
-      if (isDynamicRequireModulesEnabled && this.getModuleInfo(id).isEntry) {
-        return getDynamicPackagesEntryIntro(
-          id,
-          dynamicRequireModuleDirPaths,
-          dynamicRequireModuleSet
-        );
-      }
-
       return null;
     },
 

packages/commonjs/src/transform.js

@@ -244,7 +244,7 @@ export function transformCommonjs(
       }
 
       if (isDynamicRegister || !isDynamic || sourceId.endsWith('.json')) {
-        sources.push([sourceId, !isDynamicRegister]);
+        sources.push([sourceId, isDynamicRegister]);
       }
 
       required[sourceId] = { source: sourceId, name, importsDefault: false, isDynamic };
@@ -545,36 +545,42 @@ 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;
+
+  usesCommonjsHelpers = usesCommonjsHelpers || shouldWrap;
+
   if (
     !sources.length &&
     !uses.module &&
     !uses.exports &&
     !uses.require &&
+    !usesCommonjsHelpers &&
     (ignoreGlobal || !uses.global)
   ) {
     // not a CommonJS module
     return null;
   }
 
-  // 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;
-
-  usesCommonjsHelpers = usesCommonjsHelpers || shouldWrap;
-
   const importBlock = `${(usesCommonjsHelpers
     ? [`import * as ${HELPERS_NAME} from '${HELPERS_ID}';`]
     : []
   )
     .concat(
-      sources.map(
-        ([source]) =>
-          // import the actual module before the proxy, so that we know
-          // what kind of proxy to build
-          `import '${source}';`
-      ),
+      // dynamic registers first (`commonjsRegister(,,,)`), as the may be required in the other modules
+      sources
+        .filter(([, isDynamicRegister]) => isDynamicRegister)
+        .map(([source]) => `import '${source}';`),
+
+      // now the solid modules, non-commonjsRegister
+      sources
+        .filter(([, isDynamicRegister]) => !isDynamicRegister)
+        .map(([source]) => `import '${source}';`),
+
+      // now the proxies for solid modules (non-commonjsRegister)
       sources
-        .filter(([, importProxy]) => importProxy)
+        .filter(([, isDynamicRegister]) => !isDynamicRegister)
         .map(([source]) => {
           const { name, importsDefault } = required[source];
           return `import ${importsDefault ? `${name} from ` : ``}'${

packages/commonjs/test/fixtures/function/dynamic-require-instances/package/main.js

@@ -1 +1 @@
-module.exorts = { name: 'package', value: null };
+module.exports = { name: 'package', value: null };

packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/main.js

@@ -0,0 +1 @@
+// will be actually be loaded by the custom loader

packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule1.js

@@ -0,0 +1,2 @@
+const fn = require('./submodule2');
+export default fn;

packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule2.js

@@ -0,0 +1,3 @@
+module.exports = function() {
+  return 'Hello there';
+};

packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/importer.js

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

packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/main.js

@@ -0,0 +1,5 @@
+/* eslint-disable import/no-dynamic-require, global-require */
+
+import result from './importer.js';
+
+t.is(result, 'submodule');

packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/submodule.js

@@ -0,0 +1 @@
+module.exports = 'submodule';

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

@@ -1809,7 +1809,7 @@ Generated by [AVA](https://avajs.dev).
       });␊
       ␊
       commonjsRegister("/$$rollup_base$$/fixtures/function/dynamic-require-instances/package/main.js", function (module, exports) {␊
-        module.exorts = { name: 'package', value: null };␊
+        module.exports = { name: 'package', value: null };␊
       ␊
       });␊
       ␊

packages/commonjs/test/test.js

@@ -665,3 +665,44 @@ test('imports .cjs file extension by default', async (t) => {
   const code = await getCodeFromBundle(bundle);
   t.snapshot(code);
 });
+
+test('registers dynamic requires when entry is from a different loader', async (t) => {
+  const bundle = await rollup({
+    input: 'fixtures/samples/dynamic-require-different-loader/main.js',
+    plugins: [
+      {
+        load(id) {
+          if (id === path.resolve('fixtures/samples/dynamic-require-different-loader/main.js')) {
+            return 'import submodule1 from "./submodule1"; export default submodule1();';
+          }
+          return null;
+        }
+      },
+      commonjs({
+        dynamicRequireTargets: ['fixtures/samples/dynamic-require-different-loader/submodule2.js'],
+        transformMixedEsModules: true
+      })
+    ]
+  });
+
+  t.is((await executeBundle(bundle, t)).exports, 'Hello there');
+});
+
+test('transforms the es file with a `commonjsRequire` and no `require`s', async (t) => {
+  const bundle = await rollup({
+    input: 'fixtures/samples/dynamic-require-es-mixed-helpers/main.js',
+    plugins: [
+      commonjs({
+        dynamicRequireTargets: ['fixtures/samples/dynamic-require-es-mixed-helpers/submodule.js'],
+        transformMixedEsModules: true
+      })
+    ]
+  });
+
+  const code = await getCodeFromBundle(bundle);
+
+  t.is(
+    /commonjsRequire\(["']\.\/submodule\.js/.test(code),
+    true
+  );
+});