Merge pull request #2692 from modernweb-dev/fix/storybook-builder-named-exports-improvements

fix(storybook-builder): simplify and speed up the CommonJS to ESM transformation

Author: bashmish

.changeset/warm-poems-watch.md

@@ -0,0 +1,6 @@
+---
+'@web/storybook-builder': patch
+---
+
+simplify and speed up the CommonJS to ESM transformation
+make React conditional reexports work in production

package-lock.json

@@ -47,7 +47,6 @@
     "esm"
   ],
   "dependencies": {
-    "@chialab/esbuild-plugin-commonjs": "^0.17.2",
     "@rollup/plugin-node-resolve": "^15.1.0",
     "@rollup/pluginutils": "^5.0.2",
     "@storybook/core-common": "^7.0.0",
@@ -58,6 +57,7 @@
     "@web/dev-server-rollup": "^0.6.1",
     "@web/rollup-plugin-html": "^2.3.0",
     "browser-assert": "^1.2.1",
+    "cjs-module-lexer": "^1.2.3",
     "es-module-lexer": "^1.2.1",
     "esbuild": "^0.19.5",
     "express": "^4.18.2",

packages/storybook-builder/package.json

@@ -1,35 +1,86 @@
 import type { Plugin } from 'esbuild';
+import { readFile } from 'fs-extra';
+import { dirname } from 'path';
 
-export function esbuildPluginCommonjsNamedExports(module: string, namedExports: string[]): Plugin {
+export function esbuildPluginCommonjsNamedExports(modules: string[]): Plugin {
   return {
     name: 'commonjs-named-exports',
-    setup(build) {
-      build.onResolve({ filter: new RegExp(`^${module}$`) }, args => {
+    async setup(build) {
+      const { init, parse } = await import('cjs-module-lexer');
+      await init();
+
+      build.onResolve({ filter: new RegExp(`^(${modules.join('|')})$`) }, async args => {
+        if (args.pluginData?.preventInfiniteRecursion) return;
+
+        const { path, ...rest } = args;
+        rest.pluginData = { preventInfiniteRecursion: true };
+        const resolveResult = await build.resolve(path, rest);
+        const resolvedPath = resolveResult.path;
+
+        // skip if resolved to an ESM file
+        if (resolvedPath.endsWith('.mjs')) return;
+
+        const namedExports = await getNamedExports(resolvedPath);
+
+        // skip if nothing is exported
+        // (or was an ESM file with .js extension or just failed)
+        if (namedExports.length === 0) return;
+
         return {
           path: args.path,
-          namespace: `commonjs-named-exports-${module}`,
+          namespace: 'commonjs-named-exports',
           pluginData: {
             resolveDir: args.resolveDir,
+            resolvedPath,
+            namedExports,
           },
         };
       });
-      build.onLoad({ filter: /.*/, namespace: `commonjs-named-exports-${module}` }, async args => {
+
+      build.onLoad({ filter: /.*/, namespace: `commonjs-named-exports` }, async args => {
+        const { resolveDir, resolvedPath, namedExports } = args.pluginData;
+
+        const filteredNamedExports = namedExports.filter((name: string) => {
+          return (
+            // interop for "default" export heavily relies on the esbuild work done automatically
+            // we just always reexport it
+            // but we need to filter it out here to prevent double reexport if "default" was identified by the lexer
+            name !== 'default' &&
+            // we don't need "__esModule" flag in this wrapper
+            // because it outputs native ESM which will be consumed by other native ESM in the browser
+            name !== '__esModule'
+          );
+        });
+
+        const finalExports = ['default', ...filteredNamedExports];
+
         return {
-          resolveDir: args.pluginData.resolveDir,
-          contents: `
-            import { default as commonjsExports } from '${module}?force-original';
-            ${namedExports
-              .map(name => {
-                if (name === 'default') {
-                  return `export default commonjsExports;`;
-                } else {
-                  return `export const ${name} = commonjsExports.${name};`;
-                }
-              })
-              .join('\n')}
-          `,
+          resolveDir,
+          contents: `export { ${finalExports.join(',')} } from '${resolvedPath}';`,
         };
       });
+
+      async function getNamedExports(path: string): Promise<string[]> {
+        const source = await readFile(path, 'utf8');
+
+        let exports: string[] = [];
+        let reexports: string[] = [];
+        try {
+          ({ exports, reexports } = parse(source));
+        } catch (e) {
+          // good place to start debugging if imports are not working
+        }
+
+        for (const reexport of reexports) {
+          const reexportPath = require.resolve(reexport, { paths: [dirname(path)] });
+          const deepExports = await getNamedExports(reexportPath);
+          for (const deepExport of deepExports) {
+            exports.push(deepExport);
+          }
+        }
+
+        return exports;
+      }
     },
   };
 }

packages/storybook-builder/src/esbuild-plugin-commonjs-named-exports.ts

@@ -14,21 +14,20 @@ export function rollupPluginPrebundleModules(env: Record<string, string>): Plugi
     name: 'rollup-plugin-prebundle-modules',
 
     async buildStart() {
-      const esbuildPluginCommonjs = (await import('@chialab/esbuild-plugin-commonjs')).default; // for CJS compatibility
-
       const modules = CANDIDATES.filter(moduleExists);
 
       for (const module of modules) {
         modulePaths[module] = join(
           process.cwd(),
           PREBUNDLED_MODULES_DIR,
-          module.endsWith('.js') ? module : `${module}.js`,
+          module.endsWith('.js') ? module.replace(/\.js$/, '.mjs') : `${module}.mjs`,
         );
       }
 
       await build({
         entryPoints: modules,
         outdir: PREBUNDLED_MODULES_DIR,
+        outExtension: { '.js': '.mjs' },
         bundle: true,
         format: 'esm',
         splitting: true,
@@ -47,12 +46,15 @@ export function rollupPluginPrebundleModules(env: Record<string, string>): Plugi
           ...stringifyProcessEnvs(env),
         },
         plugins: [
-          /* for @storybook/addon-docs */
-          // tocbot can't be automatically transformed by @chialab/esbuild-plugin-commonjs
-          // so we need a manual wrapper
-          esbuildPluginCommonjsNamedExports('tocbot', ['default', 'init', 'destroy']),
-
-          esbuildPluginCommonjs(),
+          esbuildPluginCommonjsNamedExports(
+            modules.filter(
+              module =>
+                // lodash is solved by the lodash-es alias
+                !module.startsWith('lodash/') &&
+                // @storybook/react-dom-shim is just an alias to an ESM module
+                module !== '@storybook/react-dom-shim',
+            ),
+          ),
         ],
       });
     },