From 1c16a2b25f764fa38142ad8c6b75ccce61f4a446 Mon Sep 17 00:00:00 2001
From: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
Date: Thu, 24 Feb 2022 13:39:29 +0100
Subject: [PATCH] fix(commonjs): handle external dependencies when using the
 cache (#1038)

---
 packages/commonjs/README.md                   |   2 +-
 .../commonjs/src/resolve-require-sources.js   |   4 +-
 packages/commonjs/test/snapshots/test.js.md   |  19 +++++++++
 packages/commonjs/test/snapshots/test.js.snap | Bin 1298 -> 1442 bytes
 packages/commonjs/test/test.js                |  40 ++++++++++++++++++
 5 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/packages/commonjs/README.md b/packages/commonjs/README.md
index c126e4ac2..44a7652db 100644
--- a/packages/commonjs/README.md
+++ b/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
 
diff --git a/packages/commonjs/src/resolve-require-sources.js b/packages/commonjs/src/resolve-require-sources.js
index 11647e010..f96ace50e 100644
--- a/packages/commonjs/src/resolve-require-sources.js
+++ b/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 (
diff --git a/packages/commonjs/test/snapshots/test.js.md b/packages/commonjs/test/snapshots/test.js.md
index 684f9bc65..63fc433c8 100644
--- a/packages/commonjs/test/snapshots/test.js.md
+++ b/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;␊
+    `
diff --git a/packages/commonjs/test/snapshots/test.js.snap b/packages/commonjs/test/snapshots/test.js.snap
index 46745cca1e118eadba1549a887865b282670e4a0..dbca1b612c8afce4e7b746e26997480a0f0409e6 100644
GIT binary patch
literal 1442
zcmV;T1zq|<RzV<BZeeh9Xm4~Nb}|YA0C}xANb61{qNKr~4v#69bRUZd00000000B+
zR$FKsRT!SlHjTR~T73|-mfyB!X1AN!M#RgmF&2_mFg2mA1=%j+*_|`HlT2pTGc&mu
z_CX2}C5lk23JTtkLZBrrcm*q7un$G6zA8l@>gC<bgNXk*GqbmBYAAh7;OxwN=lr+t
z|Ihy?7n7u<w6%C=-^C{m-}~~p)0g^oyz)AN__q(&p`&|;mW)No`PO*j;_a_}7Zd2e
zc)+@E^D}oJ{qp2nZ;suZ+V<7CxIjmHg7reUK8#Be`{2wb^+bGT-^9LOPCdMc+Burp
zBuUF>k3Dey)4x|L%O{r(FD3tq3H0q=Nm@B|WbTU_p1yGA@#k*UFTB?u6KHue?)?1w
zXXZ%pwvX}$rnVk)evJwAJg)bz`{R>L-|*Rgj{SH`e(xg};{rXN2o{DZR`0cv^!3O=
z%{-nRJ3TyPZF|RjDJIZwu9u`Aep>$glK%Yn&nmyYIR5V9J#m4)dSkHSTY`1XP56}h
z1WU1AX-n*W*>@?p9y80HoKGgpj_o=YRV}BY$U3dbsZMa3m^MdS)%2W*b?6x{P13Tb
z>eMi8I>DS8W!}6(=V}h~+ziN?M&136?psvOzyiz=%cnW;m`|b3P0kPo+!74ILLI-e
zX`rrU+TMU}x};=LNN1Bt!?(+x>Db^_eZ#Qm9>&PLLileA7Lovl!$7g92PP^KxN1?m
z;#FY)2J?tM2)pqyFc2IE;GudjiKq7yuc{iB<1j_MGGgn_w331}C?@QJ!4&$*MG8W1
z!7OG13x&yI5#<ZfM+BS2A{VG9>p^&}K5K(0!Q)k1QN&t5k%o}K!geH<_?EZ57|P|N
zt;;FU2HVrtHTxc5+EGxZ?li}yMV8c1J#OH3hnjqc8Xg^<5<$u3Np~$Zo&NM}?JT2E
z)6T)D4meOH?g9FwZ!)Tna9^5bzycUiP<TcYrU6R(@f05Nd}aq~^T|Z}HrnB!*i=Iz
zL<G=;b`7-{Qc|iQB+;(6-tc|25pk<fGq!+=@Km5{J)G07sdGxaba^S09LOa(gKs5#
zE6d4x$F0I0RaH?y?5wgF6jfVxs@<{^2DNkN?q-CFOy@kxY+{M_(MXSm_m^TX7zv7Z
zj8;f_9u$hHin2nXzaPB$8g&eO9~wgX9nnf;z9ENYFN4XC4&%z9Yy-|AP|riX(=|;C
zPoR(4^bSp%c<jKarlqr8JyM#2QJ!3i{OZjJTXnWvMm0Fs&U8~mHB5#{+69ec>!E!`
z;>rxi5x3y3!A7zXLO^UF#!EdSE>w31r_Z;8CalXrkMpX`nFX89!YE_d+~h-5$}p={
z$G3DSQLr7TSWbyp4LukJv0?Ne1n%;r)}vm8^m9Lm3lYWjj!G^3U)41DTZ^}K67_LU
zhowg0Rj9YeuS`=+w*DxYrf!XtsbU9&2zF7Ut%iQ&gT4`xiCgGmq-wipD+w+`IQfx<
zpxq>*E?}snR<q_6uWGuy|5to&+(qINi8}uwYK=Lb!)<Ui%`Y(`fLa&`37Z~3_i0L2
z&C!-I%BN@|l$)aSE=H^3G#n-6s?zj&su>Qi?(jU)H9l)cc1EaMW@gR9TCp=QC1P09
zp{We$TpsVH&hIAYgxVNA<3l@eEyF=PlUq$+#GhT5DsnrCIwy>q6hGlE5%t$?ShdcJ
zwu>G*Y%ofnq6*s5G(owh<-qnWD<gV-jteIn|A4erX*T^(Q(gCh5|&eI-u^Xu`|3)L
z9qB#>qalc|ZL_J(V7R%?JM*V>c~e()-d<+fu9~&$nzW5M+fH`N;f=<z(Kxy{4iPe+
wKH?>sJ=jOCpml`wiN}?<5462z^Vq25jY{4afg2<6DvrSa06tc_3PKYA0M4P&;Q#;t

literal 1298
zcmV+t1?~DlRzV<BZeeh9Xm4~Nb}|YA0GQ^gag7f@&TC#iYk-Y>s2__600000000B+
zRa=M~RT!RZTGt7MDn1Ch*5B>wOw#P65%H3+u7yokuyvPhD@q#1lgyc9Hj|k+m))*Q
z9;C2RsYNMX9u!nW3WB9Acm)M7*e9v_sx10YFZilYBL3&hOm3T9meR)r&Sd60=f8dD
zKi`3vAc(?P>F&|>r=NJ>waXW;j_!T^O$6~bi{t3YgHxyVHNpN~e{21YH-3n5^gmo+
zK6u0P_nrLagLmFObW3v2x0mA_EsO-?OgKJ?3xf0Ur5(z-_|okB>~H5ET|@5-P45td
z3+IoY_<HwqS1vvE!fo1>5ATU_^e;B^>z`j5Go{-<%`eo)&f34nIC^ZSAbhmz@6Xet
z(_j31_UBvk2OnFHbM(#|gK;VxXGB5xZsv$;yqY<5aeB(!^RDqqjH6HP5`>?AdH<`c
z+KWG)lK*&V?!C49;~X8|9gNfAc=;wlhzp~fi`Y)#=Ga4$?^19*$EbKxK9Q){mTQ|-
zG3}}>X|yRN`@se=EQWTw895j0^D|doq!mxmsBTzv-m#n1@m6GdqG>yxn+8c$se9Pg
ze3MFPScN5G`ZNcg<5Os{kW0h?773<cwT0iA6j0YREN@aXTv9eEq%w(w?pqbluq|+F
zzOI|}fa8!AnXqRPRucfa?SO1j4-9m~amA!o)vLiIWb=qV4EynKaxz#Bz)JOC5mz53
zUQN+W+jeCB>JUq_8*&m-ARDj`vPsO7nH0I-qESi*5sHhY651D|j|etOC1%h{w1V=w
zb5^rx!Rl4IQN+4Gk%o}K;y4mZeA62*g?9O9YAXk7ws+eG?!E`O?Pw@bx52Om-z6n<
zj}tiE=O*9hhE<2Fcu`Vi(cR2Ve?B8){cRL_n#e|dz<?TY7w9v-;ZSXc<<e;bCx8(J
zneC`(=pgqlPvRoacdWo|J|Xr_V;lzZNhKtLR{$Mo-&9v1IjL|$#9p_pcJ76C#ob1o
z+5#%VRe`VVa83tqon2<F%UYS_Kq^aVd@JKyMM|{#P8H55ih>4wW<$iFsd{$jg23!`
z+veMGZ?+v3Aq4mYVvKWdrAciaB6oL9A&tHjsDUO0hs~=w_A*#>848Yrr%XCpqYjo-
zvwc&8G6l<qs%e*r+4h5(6AKDQAV`<VP%AnVs3I&6;zD>fpw&lNaX@p0donSxA1i2|
zs$Qx(Yqm+$XCr-)+6`Kz-T}Ybpq59q$4P^_O;Vw<6%fMNMUR#e=8+E?I3yFNFvZB#
zIPY(f*$_s46d~w)F=|I$A<d?_B6~H%WkOW-S#TGL6U7#L5T)tZp3Pz~H=Qr8RX|NF
zgoLLLV6d!{4T9dK=S<GU(5_BbT&z|>SuT)DP3~k}@06oqv~WGrHMeX<ab~Dnag3&i
zy<(-I&TH5-p?VrLW)IWoFc)w_Ev(*Qw?~j#nt`|`w~@YxKdV?Tu{dIj5ynFDM>tD(
z{|!>jZsE$L_Nwaqv4w)FrZQo|x<l!+RK{4U$|+aX99X_-rn&g!m~o<g6ZUjzCiO_i
z-Qb4ek&~Ns@n#~kv6ExR2kUX9#`xMJl05;#$!(sQe}t_=U031XLiRU^{<b8)E%?1;
zcP$<Y$51#13kNTm-9G#!x_eMZu0c9N`uN|q%Lm5ZR6K^AJnZD*6*#;Cuj3W?Kll)&
IPq7gI0F9b{a{vGU

diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js
index 138ee2e06..845bd8d48 100644
--- a/packages/commonjs/test/test.js
+++ b/packages/commonjs/test/test.js
@@ -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,