improve cjs<>esm conversion of named exports

esinstall/package.json

@@ -54,7 +54,7 @@
     "resolve": "^1.20.0",
     "rimraf": "^3.0.0",
     "rollup": "2.37.1",
-    "rollup-plugin-polyfill-node": "^0.5.0",
+    "rollup-plugin-polyfill-node": "^0.6.2",
     "slash": "^3.0.0",
     "validate-npm-package-name": "^3.0.0",
     "vm2": "^3.9.2"

esinstall/src/index.ts

@@ -10,7 +10,6 @@ import rimraf from 'rimraf';
 import {InputOptions, OutputOptions, Plugin as RollupPlugin, rollup, RollupError} from 'rollup';
 import rollupPluginNodePolyfills from 'rollup-plugin-polyfill-node';
 import rollupPluginReplace from '@rollup/plugin-replace';
-import util from 'util';
 import {rollupPluginAlias} from './rollup-plugins/rollup-plugin-alias';
 import {rollupPluginCatchFetch} from './rollup-plugins/rollup-plugin-catch-fetch';
 import {rollupPluginCatchUnresolved} from './rollup-plugins/rollup-plugin-catch-unresolved';
@@ -53,27 +52,6 @@ type DependencyLoc = {
   loc: string;
 };
 
-// Add popular CJS packages here that use "synthetic" named imports in their documentation.
-// CJS packages should really only be imported via the default export:
-//   import React from 'react';
-// But, some large projects use named exports in their documentation:
-//   import {useState} from 'react';
-//
-// We use "/index.js here to match the official package, but not any ESM aliase packages
-// that the user may have installed instead (ex: react-esm).
-const CJS_PACKAGES_TO_AUTO_DETECT = [
-  'react/index.js',
-  'react-dom/index.js',
-  'react-dom/server.js',
-  'react-is/index.js',
-  'prop-types/index.js',
-  'scheduler/index.js',
-  'react-table',
-  'chai/index.js',
-  'events/events.js',
-  'uuid/index.js',
-];
-
 function isImportOfPackage(importUrl: string, packageName: string) {
   return packageName === importUrl || importUrl.startsWith(packageName + '/');
 }
@@ -115,7 +93,7 @@ function generateReplacements(env: Object): {[key: string]: string} {
     {
       // Other find & replacements:
       // tslib: fights with Rollup's namespace/default handling, so just remove it.
-      'return (mod && mod.__esModule) ? mod : { "default": mod };': 'return mod;',
+      'mod && mod.__esModule': 'true',
     },
   );
 }
@@ -133,9 +111,10 @@ interface InstallOptions {
   polyfillNode: boolean;
   sourcemap?: boolean | 'inline';
   external: string[];
-  externalEsm: string[] | ((imp: string) => boolean);
+  externalEsm: boolean | string[] | ((imp: string) => boolean);
   packageLookupFields: string[];
   packageExportLookupFields: string[];
+  // @deprecated No longer needed, all packages now have the highest fidelity named export support possible
   namedExports: string[];
   rollup: {
     context?: string;
@@ -202,7 +181,6 @@ export async function install(
     importMap: _importMap,
     logger,
     dest: destLoc,
-    namedExports,
     external,
     externalEsm,
     sourcemap,
@@ -219,6 +197,9 @@ export async function install(
   const installTargets: InstallTarget[] = _installTargets.map((t) =>
     typeof t === 'string' ? createInstallTarget(t) : t,
   );
+
+  // TODO: warn if import from  "firebase", since that includes so many Node-specific files
+
   const allInstallSpecifiers = new Set(
     installTargets
       .filter(
@@ -237,7 +218,6 @@ export async function install(
   const importMap: ImportMap = {imports: {}};
   let dependencyStats: DependencyStatsOutput | null = null;
   const skipFailures = false;
-  const autoDetectNamedExports = [...CJS_PACKAGES_TO_AUTO_DETECT, ...namedExports];
 
   for (const installSpecifier of allInstallSpecifiers) {
     let targetName = getWebDependencyName(installSpecifier);
@@ -357,10 +337,10 @@ ${colors.dim(
         esmExternals: (id) =>
           Array.isArray(externalEsm)
             ? externalEsm.some((packageName) => isImportOfPackage(id, packageName))
-            : (externalEsm as Function)(id),
+            : externalEsm,
         requireReturnsDefault: 'auto',
       } as RollupCommonJSOptions),
-      rollupPluginWrapInstallTargets(!!isTreeshake, autoDetectNamedExports, installTargets, logger),
+      rollupPluginWrapInstallTargets(!!isTreeshake, installTargets, logger),
       stats && rollupPluginDependencyStats((info) => (dependencyStats = info)),
       rollupPluginNodeProcessPolyfill(env),
       polyfillNode && rollupPluginNodePolyfills(),
@@ -417,11 +397,9 @@ ${colors.dim(
   if (Object.keys(installEntrypoints).length > 0) {
     try {
       logger.debug(process.cwd());
-      logger.debug(`running installer with options: ${util.format(inputOptions)}`);
+      logger.debug(`running installer with options: ${JSON.stringify(inputOptions)}`);
       const packageBundle = await rollup(inputOptions);
-      logger.debug(
-        `installing npm packages:\n    ${Object.keys(installEntrypoints).join('\n    ')}`,
-      );
+      logger.debug(`installing npm packages: ${Object.keys(installEntrypoints).join(', ')}`);
       if (isFatalWarningFound) {
         throw new Error(FAILED_INSTALL_MESSAGE);
       }

esinstall/src/rollup-plugins/rollup-plugin-wrap-install-targets.ts

@@ -1,16 +1,32 @@
 import fs from 'fs';
-import * as colors from 'kleur/colors';
 import path from 'path';
 import {Plugin} from 'rollup';
+import execa from 'execa';
 import {VM as VM2} from 'vm2';
 import {AbstractLogger, InstallTarget} from '../types';
-import {getWebDependencyName, isJavaScript, isRemoteUrl, isTruthy, NATIVE_REQUIRE} from '../util';
+import {getWebDependencyName, isJavaScript, isRemoteUrl, isTruthy} from '../util';
 import isValidIdentifier from 'is-valid-identifier';
 import resolve from 'resolve';
 
 // Use CJS intentionally here! ESM interface is async but CJS is sync, and this file is sync
 const {parse} = require('cjs-module-lexer');
 
+function isValidNamedExport(name: string): boolean {
+  return name !== 'default' && name !== '__esModule' && isValidIdentifier(name);
+}
+
+// Add popular CJS/UMD packages here that use "synthetic" named imports in their documentation.
+// Our scanner can statically scan most packages without an opt-in here, but these packages
+// are built oddly, in a way that we can't statically analyze.
+const TRUSTED_CJS_PACKAGES = ['chai/index.js', 'events/events.js', 'uuid/index.js'];
+
+// These packages are written so strangely, that our CJS scanner succeeds at scanning the file
+// but fails to pick up some export. Add popular packages here to save everyone a bit of
+// headache.
+// We use "/index.js here to match the official package, but not any ESM aliase packages
+// that the user may have installed instead (ex: react-esm).
+const UNSCANNABLE_CJS_PACKAGES = ['chai/index.js'];
+
 /**
  * rollup-plugin-wrap-install-targets
  *
@@ -25,40 +41,19 @@ const {parse} = require('cjs-module-lexer');
  */
 export function rollupPluginWrapInstallTargets(
   isTreeshake: boolean,
-  autoDetectPackageExports: string[],
   installTargets: InstallTarget[],
   logger: AbstractLogger,
 ): Plugin {
   const installTargetSummaries: {[loc: string]: InstallTarget} = {};
   const cjsScannedNamedExports = new Map<string, string[]>();
-  /**
-   * Runtime analysis: High Fidelity, but not always successful.
-   * `require()` the CJS file inside of Node.js to load the package and detect it's runtime exports.
-   * TODO: Safe to remove now that cjsAutoDetectExportsUntrusted() is getting smarter?
-   */
-  function cjsAutoDetectExportsTrusted(normalizedFileLoc: string): string[] | undefined {
-    try {
-      const mod = NATIVE_REQUIRE(normalizedFileLoc);
-      // Collect and filter all properties of the object as named exports.
-      return Object.keys(mod).filter((imp) => imp !== 'default' && imp !== '__esModule');
-    } catch (err) {
-      logger.debug(
-        `✘ Runtime CJS auto-detection for ${colors.bold(
-          normalizedFileLoc,
-        )} unsuccessful. Falling back to static analysis. ${err.message}`,
-      );
-    }
-  }
 
   /**
-   * Attempt #2: Static analysis: Lower Fidelity, but safe to run on anything.
-   * 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.
+   * Attempt #1: Static analysis: Lower Fidelity, but faster.
+   * Do our best job to statically scan a file for named exports. This uses "cjs-module-lexer", the
+   * same CJS export scanner that Node.js uses internally. Very fast, but only works on some modules,
+   * depending on how they were build/written/compiled.
    */
-  function cjsAutoDetectExportsUntrusted(
-    filename: string,
-    visited = new Set(),
-  ): string[] | undefined {
+  function cjsAutoDetectExportsStatic(filename: string, visited = new Set()): string[] | undefined {
     const isMainEntrypoint = visited.size === 0;
     // Prevent infinite loops via circular dependencies.
     if (visited.has(filename)) {
@@ -77,43 +72,81 @@ export function rollupPluginWrapInstallTargets(
           [],
           reexports
             .map((e) =>
-              cjsAutoDetectExportsUntrusted(
+              cjsAutoDetectExportsStatic(
                 resolve.sync(e, {basedir: path.dirname(filename)}),
                 visited,
               ),
             )
             .filter(isTruthy),
         );
       }
-      // Attempt 2 - UMD: Run the file in a sandbox to dynamically analyze exports.
-      // This will only work on UMD and very simple CJS files (require not supported).
-      // Uses VM2 to run safely sandbox untrusted code (no access no Node.js primitives, just JS).
+      // If nothing was detected, return undefined.
       if (isMainEntrypoint && exports.length === 0 && reexports.length === 0) {
-        const vm = new VM2({wasm: false, fixAsync: false});
-        exports = Object.keys(
-          vm.run(
-            'const exports={}; const module={exports}; ' + fileContents + ';; module.exports;',
-          ),
-        );
-
-        // Verify that all of these are valid identifiers. Otherwise when we attempt to
-        // reexport it will produce invalid js like `import { a, b, 0, ) } from 'foo';
-        const allValid = exports.every((identifier) => isValidIdentifier(identifier));
-        if (!allValid) {
-          exports = [];
-        }
+        return undefined;
       }
+      // Otherwise, resolve and flatten all exports into a single array, remove invalid exports.
+      return Array.from(new Set([...exports, ...resolvedReexports])).filter(isValidNamedExport);
+    } catch (err) {
+      // Safe to ignore, this is usually due to the file not being CJS.
+      logger.debug(`cjsAutoDetectExportsStatic ${filename}: ${err.message}`);
+    }
+  }
 
-      // Resolve and flatten all exports into a single array, and remove invalid exports.
-      return Array.from(new Set([...exports, ...resolvedReexports])).filter(
-        (imp) => imp !== 'default' && imp !== '__esModule',
+  /**
+   * Attempt #2a - Runtime analysis: More powerful, but slower. (trusted code)
+   * This function spins off a Node.js process to analyze the most accurate set of named imports that this
+   * module supports. If this fails, there's not much else possible that we could do.
+   *
+   * We consider this "trusted" because it will actually run the package code in Node.js on your machine.
+   * Since this is code that you are intentionally bundling into your application, we consider this fine
+   * for most users and equivilent to the current security story of Node.js/npm. But, if you are operating
+   * a service that runs esinstall on arbitrary code, you should set `process.env.ESINSTALL_UNTRUSTED_ENVIRONMENT`
+   * so that this is skipped.
+   */
+  function cjsAutoDetectExportsRuntimeTrusted(normalizedFileName: string): string[] | undefined {
+    // Skip if set to not trust package code (besides a few popular, always-trusted packages).
+    if (
+      process.env.ESINSTALL_UNTRUSTED_ENVIRONMENT &&
+      !TRUSTED_CJS_PACKAGES.includes(normalizedFileName)
+    ) {
+      return undefined;
+    }
+    try {
+      const {stdout} = execa.sync(
+        `node`,
+        ['-p', `JSON.stringify(Object.keys(require('${normalizedFileName}')))`],
+        {
+          cwd: __dirname,
+          extendEnv: false,
+        },
       );
+      const exportsResult = JSON.parse(stdout).filter(isValidNamedExport);
+      logger.debug(`cjsAutoDetectExportsRuntime success ${normalizedFileName}: ${exportsResult}`);
+      return exportsResult;
     } catch (err) {
-      // Safe to ignore, this is usually due to the file not being CJS.
-      logger.debug(`cjsAutoDetectExportsUntrusted error: ${err.message}`);
+      logger.debug(`cjsAutoDetectExportsRuntime error ${normalizedFileName}: ${err.message}`);
     }
   }
 
+  /**
+   * Attempt #2b - Sandboxed runtime analysis: More powerful, but slower.
+   * This will only work on UMD and very simple CJS files (require not supported).
+   * Uses VM2 to run safely sandbox untrusted code (no access no Node.js primitives, just JS).
+   * If nothing was detected, return undefined.
+   */
+  function cjsAutoDetectExportsRuntimeUntrusted(filename: string): string[] | undefined {
+    try {
+      const fileContents = fs.readFileSync(filename, 'utf8');
+      const vm = new VM2({wasm: false, fixAsync: false});
+      const exportsResult = Object.keys(
+        vm.run('const exports={}; const module={exports}; ' + fileContents + ';; module.exports;'),
+      );
+      logger.debug(`cjsAutoDetectExportsRuntimeUntrusted success ${filename}: ${exportsResult}`);
+      return exports.filter((identifier) => isValidIdentifier(identifier));
+    } catch (err) {
+      logger.debug(`cjsAutoDetectExportsRuntimeUntrusted error ${filename}: ${err.message}`);
+    }
+  }
   return {
     name: 'snowpack:wrap-install-targets',
     // Mark some inputs for tree-shaking.
@@ -138,12 +171,16 @@ export function rollupPluginWrapInstallTargets(
         }, {} as any);
         installTargetSummaries[val] = installTargetSummary;
         const normalizedFileLoc = val.split(path.win32.sep).join(path.posix.sep);
-        const isExplicitAutoDetect = autoDetectPackageExports.some((p) =>
+        const knownBadPackage = UNSCANNABLE_CJS_PACKAGES.some((p) =>
           normalizedFileLoc.includes(`node_modules/${p}${p.endsWith('.js') ? '' : '/'}`),
         );
-        const cjsExports = isExplicitAutoDetect
-          ? cjsAutoDetectExportsTrusted(val)
-          : cjsAutoDetectExportsUntrusted(val);
+        const cjsExports =
+          // If we can trust the static analyzer, run that first.
+          (!knownBadPackage && cjsAutoDetectExportsStatic(val)) ||
+          // Otherwise, run our more powerful, runtime analysis.
+          // Attempted trusted first (won't run in untrusted environments).
+          cjsAutoDetectExportsRuntimeTrusted(normalizedFileLoc) ||
+          cjsAutoDetectExportsRuntimeUntrusted(normalizedFileLoc);
         if (cjsExports && cjsExports.length > 0) {
           cjsScannedNamedExports.set(normalizedFileLoc, cjsExports);
           input[key] = `snowpack-wrap:${val}`;

snowpack/src/commands/build.ts

@@ -222,7 +222,11 @@ export async function build(commandOptions: CommandOptions): Promise<SnowpackBui
     let onFileChangeCallback: OnFileChangeCallback = () => {};
     devServer.onFileChange(async ({filePath}) => {
       // First, do our own re-build logic
-      allFileUrlsToProcess.push(...getUrlsForFile(filePath, config)!);
+      const fileUrls = getUrlsForFile(filePath, config);
+      if (!fileUrls || fileUrls.length === 0) {
+        return;
+      }
+      allFileUrlsToProcess.push(fileUrls[0]);
       await flushFileQueue(false, {
         isSSR,
         isHMR,

snowpack/src/commands/dev.ts

@@ -726,7 +726,7 @@ export async function startServer(
     return http.createServer(responseHandler as http.RequestListener);
   };
 
-  let server: ReturnType<typeof createServer> | undefined;
+  let server: http.Server | http2.Http2Server | undefined;
   if (port) {
     server = createServer(async (req, res) => {
       // Attach a request logger.
@@ -829,6 +829,7 @@ export async function startServer(
   const sp = {
     port,
     hmrEngine,
+    rawServer: server,
     loadUrl,
     handleRequest,
     sendResponseFile,

snowpack/src/dev/hmr.ts

@@ -1,5 +1,5 @@
-import http from 'http';
-import http2 from 'http2';
+import type http from 'http';
+import type http2 from 'http2';
 import path from 'path';
 import onProcessExit from 'signal-exit';
 import {FileBuilder} from '../build/file-builder';
@@ -9,7 +9,7 @@ import {getCacheKey, hasExtension} from '../util';
 
 export function startHmrEngine(
   inMemoryBuildCache: Map<string, FileBuilder>,
-  server: http.Server | http2.Http2SecureServer | undefined,
+  server: http.Server | http2.Http2Server | undefined,
   serverPort: number | undefined,
   config: SnowpackConfig,
 ) {