Merge pull request #2655 from modernweb-dev/revert-2653-fix/storybook-builder-process-env

Revert "fix(storybook-builder): define only necessary process.env globally"

Author: bashmish

.changeset/sweet-sloths-crash.md

@@ -11,7 +11,6 @@ export async function generateIframeHtml(options: Options): Promise<string> {
     'utf-8',
   );
   const { configType, features, presets, serverChannelUrl } = options;
-  const env = await presets.apply<Record<string, string>>('env');
   const frameworkOptions = await presets.apply<Record<string, any> | null>('frameworkOptions');
   const headHtmlSnippet = await presets.apply<PreviewHtml>('previewHead');
   const bodyHtmlSnippet = await presets.apply<PreviewHtml>('previewBody');
@@ -27,14 +26,6 @@ export async function generateIframeHtml(options: Options): Promise<string> {
   }));
   return (
     iframeHtmlTemplate
-      .replace(
-        `'[PROCESS_ENV HERE]'`,
-        JSON.stringify({
-          // IMPORTANT!!!
-          // please do not include the entire "env" to prevent bundling and deploying of sensitive data
-          NODE_ENV: env.NODE_ENV,
-        }),
-      )
       .replace('[CONFIG_TYPE HERE]', configType || '')
       .replace('[LOGLEVEL HERE]', logLevel || '')
       .replace(`'[FRAMEWORK_OPTIONS HERE]'`, JSON.stringify(frameworkOptions))

packages/storybook-builder/src/generate-iframe-html.ts

@@ -60,6 +60,8 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s
   router.use('/sb-preview', express.static(previewDirOrigin, { immutable: true, maxAge: '5m' }));
   router.use(`/${PREBUNDLED_MODULES_DIR}`, express.static(resolve(`./${PREBUNDLED_MODULES_DIR}`)));
 
+  const env = await options.presets.apply<Record<string, string>>('env');
+
   const wdsStorybookConfig: DevServerConfig = {
     nodeResolve: true,
     plugins: [
@@ -72,7 +74,7 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s
           }
         },
       },
-      wdsPluginPrebundleModules(),
+      wdsPluginPrebundleModules(env),
       wdsPluginStorybookBuilder(options),
       wdsPluginExternalGlobals(globalsNameReferenceMap || globals),
     ],
@@ -128,6 +130,8 @@ export const start: WdsBuilder['start'] = async ({ startTime, options, router, s
 };
 
 export const build: WdsBuilder['build'] = async ({ startTime, options }) => {
+  const env = await options.presets.apply<Record<string, string>>('env');
+
   const rollupDefaultOutputOptions: OutputOptions = {
     dir: options.outputDir,
   };
@@ -142,7 +146,7 @@ export const build: WdsBuilder['build'] = async ({ startTime, options }) => {
         extractAssets: false,
       }),
       rollupPluginNodeResolve(),
-      rollupPluginPrebundleModules(),
+      rollupPluginPrebundleModules(env),
       rollupPluginStorybookBuilder(options),
       rollupPluginExternalGlobals(globalsNameReferenceMap || globals),
     ],

packages/storybook-builder/src/index.ts

@@ -1,11 +1,12 @@
+import { stringifyProcessEnvs } from '@storybook/core-common';
 import { build } from 'esbuild';
 import { join } from 'path';
 import type { Plugin } from 'rollup';
 import { getNodeModuleDir } from './get-node-module-dir.js';
 
 export const PREBUNDLED_MODULES_DIR = 'node_modules/.prebundled_modules';
 
-export function rollupPluginPrebundleModules(): Plugin {
+export function rollupPluginPrebundleModules(env: Record<string, string>): Plugin {
   const modulePaths: Record<string, string> = {};
 
   return {
@@ -36,6 +37,9 @@ export function rollupPluginPrebundleModules(): Plugin {
           lodash: getNodeModuleDir('lodash-es'), // more optimal, but also solves esbuild incorrectly compiling lodash/_nodeUtil
           path: require.resolve('path-browserify'),
         },
+        define: {
+          ...stringifyProcessEnvs(env),
+        },
         plugins: [esbuildCommonjsPlugin()],
       });
     },
@@ -60,7 +64,7 @@ function getModules() {
 
 // this is different to https://github.com/storybookjs/storybook/blob/v7.0.0/code/lib/builder-vite/src/optimizeDeps.ts#L7
 // builder-vite bundles different dependencies for performance reasons
-// we aim only at browserifying dependencies which are CommonJS, or sometimes ESM with issues
+// we aim only at browserifying NodeJS dependencies (CommonJS/process.env/...)
 export const CANDIDATES = [
   // @testing-library has ESM, but imports/exports are not working correctly between packages
   // specifically "@testing-library/user-event" has "dist/esm/utils/misc/getWindow.js" (see https://cdn.jsdelivr.net/npm/@testing-library/[email protected]/dist/esm/utils/misc/getWindow.js)
@@ -78,4 +82,7 @@ export const CANDIDATES = [
 
   // CommonJS module used in Storybook MJS files
   'lodash/mapValues.js',
+
+  // ESM, but uses `process.env.NODE_ENV`
+  'tiny-invariant',
 ];

packages/storybook-builder/src/rollup-plugin-prebundle-modules.ts

@@ -49,9 +49,7 @@
       // We do this so that "module && module.hot" etc. in Storybook source code
       // doesn't fail (it will simply be disabled)
       window.module = undefined;
-
       window.global = window;
-      window.process = { env: '[PROCESS_ENV HERE]' };
     </script>
     <!-- [HEAD HTML SNIPPET HERE] -->
   </head>