esinstall: handle cjs auto-named-exports with deep reexports (#1194)

FredKSchott · Oct 6, 2020 · d03cb5c · d03cb5c
1 parent 63f2c1a
commit d03cb5c
Show file tree

Hide file tree

Showing 7 changed files with 43 additions and 11 deletions.
diff --git a/esinstall/src/rollup-plugins/rollup-plugin-wrap-install-targets.ts b/esinstall/src/rollup-plugins/rollup-plugin-wrap-install-targets.ts
@@ -54,14 +54,20 @@ export function rollupPluginWrapInstallTargets(
    * Get the exports that we scanned originally using static analysis. This is meant to run on
    * any file (not only CJS) but it will only return an array if CJS exports were found.
    */
-  function cjsAutoDetectExportsStatic(filename: string): string[] | undefined {
+  function cjsAutoDetectExportsStatic(filename: string, visited = new Set()): string[] | undefined {
+    // Prevent infinite loops via circular dependencies.
+    if (visited.has(filename)) {
+      return [];
+    } else {
+      visited.add(filename);
+    }
     const fileContents = fs.readFileSync(filename, 'utf-8');
     try {
-      const {exports} = parse(fileContents);
-      // TODO: Also follow & deeply parse dependency "reexports" returned by the lexer.
-      if (exports.length > 0) {
-        return exports.filter((imp) => imp !== 'default');
-      }
+      const {exports, reexports} = parse(fileContents);
+      const resolvedReexports = reexports.map((e) =>
+        cjsAutoDetectExportsStatic(require.resolve(e, {paths: [path.dirname(filename)]}), visited),
+      );
+      return Array.from(new Set([...exports, ...resolvedReexports])).filter((imp) => imp !== 'default');
     } catch (err) {
       // Safe to ignore, this is usually due to the file not being CJS.
       logger.debug(`cjsAutoDetectExportsStatic error: ${err.message}`);
@@ -92,7 +98,7 @@ export function rollupPluginWrapInstallTargets(
         const cjsExports = isExplicitAutoDetect
           ? cjsAutoDetectExportsRuntime(val)
           : cjsAutoDetectExportsStatic(val);
-        if (cjsExports) {
+        if (cjsExports && cjsExports.length > 0) {
           cjsScannedNamedExports.set(normalizedFileLoc, cjsExports);
           input[key] = `snowpack-wrap:${val}`;
         }

diff --git a/test/esinstall/__snapshots__/install.test.js.snap b/test/esinstall/__snapshots__/install.test.js.snap
@@ -3,6 +3,7 @@
 exports[`snowpack install auto-named-exports: allFiles 1`] = `
 Array [
   "cjs-named-export-pkg-02.js",
+  "cjs-named-export-pkg-03.js",
   "import-map.json",
 ]
 `;
@@ -24,10 +25,22 @@ export default entrypoint;
 export { entrypoint as __moduleExports, export1, export2, export3, export4, export5 };"
 `;
 
+exports[`snowpack install auto-named-exports: cjs-named-export-pkg-03.js 1`] = `
+"var export42 = 'foobar';
+var reexported = {
+	export42: export42
+};
+var entrypoint = reexported;
+export default entrypoint;
+var export42$1 = entrypoint.export42;
+export { entrypoint as __moduleExports, export42$1 as export42 };"
+`;
+
 exports[`snowpack install auto-named-exports: import-map.json 1`] = `
 "{
   \\"imports\\": {
-    \\"cjs-named-export-pkg-02\\": \\"./cjs-named-export-pkg-02.js\\"
+    \\"cjs-named-export-pkg-02\\": \\"./cjs-named-export-pkg-02.js\\",
+    \\"cjs-named-export-pkg-03\\": \\"./cjs-named-export-pkg-03.js\\"
   }
 }"
 `;
@@ -37,7 +50,8 @@ exports[`snowpack install auto-named-exports: output 1`] = `
 [snowpack] ✔ install complete!
 [snowpack]
   ⦿ web_modules/                   size       gzip       brotli
-    └─ cjs-named-export-pkg-02.js    XXXX KB    XXXX KB    XXXX KB"
+    ├─ cjs-named-export-pkg-02.js    XXXX KB    XXXX KB    XXXX KB
+    └─ cjs-named-export-pkg-03.js    XXXX KB    XXXX KB    XXXX KB"
 `;
 
 exports[`snowpack install config-alias: allFiles 1`] = `

diff --git a/test/esinstall/auto-named-exports/package.json b/test/esinstall/auto-named-exports/package.json
@@ -8,11 +8,13 @@
   },
   "snowpack": {
     "install": [
-      "cjs-named-export-pkg-02"
+      "cjs-named-export-pkg-02",
+      "cjs-named-export-pkg-03"
     ]
   },
   "dependencies": {
-    "cjs-named-export-pkg-02": "file:./packages/cjs-named-export-pkg-02"
+    "cjs-named-export-pkg-02": "file:./packages/cjs-named-export-pkg-02",
+    "cjs-named-export-pkg-03": "file:./packages/cjs-named-export-pkg-03"
   },
   "devDependencies": {
     "snowpack": "^2.12.1"

diff --git a/test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03/entrypoint.js b/test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03/entrypoint.js
@@ -0,0 +1 @@
+module.exports = require('./reexported');
diff --git a/test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03/package.json b/test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "cjs-named-export-pkg-03",
+  "version": "1.2.3",
+  "main":  "entrypoint.js"
+}
diff --git a/test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03/reexported.js b/test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03/reexported.js
@@ -0,0 +1 @@
+exports.export42 = 'foobar';
diff --git a/yarn.lock b/yarn.lock
@@ -4641,6 +4641,9 @@ cjs-module-lexer@^0.3.3:
 "cjs-named-export-pkg-02@file:./test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-02":
   version "1.2.3"
 
+"cjs-named-export-pkg-03@file:./test/esinstall/auto-named-exports/packages/cjs-named-export-pkg-03":
+  version "1.2.3"
+
 "cjs-named-export-pkg@file:./test/esinstall/config-named-exports/packages/cjs-named-export-pkg":
   version "1.2.3"