fix(commonjs): handle external dependencies when using the cache (#1038)

Author: lukastaegert

packages/commonjs/README.md

@@ -13,7 +13,7 @@
 
 ## Requirements
 
-This plugin requires an [LTS](https://github.com/nodejs/Release) Node version (v8.0.0+) and Rollup v2.38.3+.
+This plugin requires an [LTS](https://github.com/nodejs/Release) Node version (v12.0.0+) and Rollup v2.68.0+. If you are using [`@rollup/plugin-node-resolve`](https://github.com/rollup/plugins/tree/master/packages/node-resolve), it should be v13.0.6+.
 
 ## Install
 

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

@@ -100,7 +100,7 @@ export function getRequireResolver(extensions, detectCyclesAndConditional) {
       resolvedSources,
       meta: { commonjs: parentMeta }
     }) {
-      // We explicitly track ES modules to handle ciruclar imports
+      // We explicitly track ES modules to handle circular imports
       if (!(parentMeta && parentMeta.isCommonJS)) knownCjsModuleTypes[parentId] = false;
       if (isWrappedId(parentId, ES_IMPORT_SUFFIX)) return false;
       const parentRequires = parentMeta && parentMeta.requires;
@@ -135,7 +135,7 @@ export function getRequireResolver(extensions, detectCyclesAndConditional) {
         await Promise.all(
           Object.keys(resolvedSources)
             .map((source) => resolvedSources[source])
-            .filter(({ id }) => !parentRequireSet.has(id))
+            .filter(({ id, external }) => !(external || parentRequireSet.has(id)))
             .map(async (resolved) => {
               if (isWrappedId(resolved.id, ES_IMPORT_SUFFIX)) {
                 return (

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

@@ -316,3 +316,22 @@ Generated by [AVA](https://avajs.dev).
     ␊
     console.log('main');␊
     `
+
+## handles external dependencies when using the cache
+
+> Snapshot 1
+
+    `'use strict';␊
+    ␊
+    var require$$0 = require('external');␊
+    ␊
+    function _interopDefaultLegacy (e) { return e && typeof e === 'object' && 'default' in e ? e : { 'default': e }; }␊
+    ␊
+    var require$$0__default = /*#__PURE__*/_interopDefaultLegacy(require$$0);␊
+    ␊
+    var second = require$$0__default["default"].second;␊
+    ␊
+    var main = require$$0.first + second;␊
+    ␊
+    module.exports = main;␊
+    `

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

@@ -1147,6 +1147,46 @@ test('handles ESM cycles when using the cache', async (t) => {
   t.snapshot(await getCodeFromBundle(bundle));
 });
 
+test('handles external dependencies when using the cache', async (t) => {
+  const modules = {};
+  const resetModules = () => {
+    modules['main.js'] =
+      "import first from 'first.js';import second from 'second.js';export default first + second;";
+    modules['first.js'] = "export {first as default} from 'external';";
+    modules['second.js'] = "module.exports = require('external').second;";
+  };
+  const options = {
+    input: 'main.js',
+    external: ['external'],
+    plugins: [commonjs(), loader(modules)],
+    onwarn
+  };
+
+  resetModules();
+  let bundle = await rollup(options);
+  t.is(
+    (
+      await executeBundle(bundle, t, {
+        context: {
+          require(id) {
+            if (id === 'external') {
+              return { first: 'first', second: 'second' };
+            }
+            throw new Error(`Unexpected require "${id}"`);
+          }
+        }
+      })
+    ).exports,
+    'firstsecond'
+  );
+  const code = await getCodeFromBundle(bundle);
+  t.snapshot(code);
+
+  options.cache = bundle.cache;
+  bundle = await rollup(options);
+  t.is(await getCodeFromBundle(bundle), code);
+});
+
 test('allows the config to be reused', async (t) => {
   const config = {
     preserveModules: true,