Merge pull request #32175 from storybookjs/version-patch-from-9.1.0

Release: Patch 9.1.1

Author: yannbf

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 9.1.1
+
+- CLI: Fix throwing in readonly environments - [#31785](https://github.com/storybookjs/storybook/pull/31785), thanks @JReinhold!
+- Onboarding: Tweak referral wording in survey - [#32185](https://github.com/storybookjs/storybook/pull/32185), thanks @shilman!
+- Telemetry: Send index stats on dev exit - [#32168](https://github.com/storybookjs/storybook/pull/32168), thanks @shilman!
+
 ## 9.1.0
 
 Storybook 9.1 is packed with new features and improvements to enhance accessibility, streamline testing, and make your development workflow even smoother!

code/addons/onboarding/src/features/IntentSurvey/IntentSurvey.tsx

@@ -111,7 +111,7 @@ export const IntentSurvey = ({
       },
     },
     referrer: {
-      label: 'How did you learn about Storybook?',
+      label: 'How did you discover Storybook?',
       type: 'select',
       required: true,
       options: shuffleObject({

code/addons/vitest/src/postinstall-logger.ts

@@ -1,7 +1,7 @@
+import { isCI } from 'storybook/internal/common';
 import { colors, logger } from 'storybook/internal/node-logger';
 
-const fancy =
-  process.platform !== 'win32' || process.env.CI || process.env.TERM === 'xterm-256color';
+const fancy = process.platform !== 'win32' || isCI() || process.env.TERM === 'xterm-256color';
 
 export const step = colors.gray('›');
 export const info = colors.blue(fancy ? 'ℹ' : 'i');

code/addons/vitest/src/postinstall.ts

@@ -8,6 +8,7 @@ import {
   extractProperFrameworkName,
   formatFileContent,
   getProjectRoot,
+  isCI,
   loadAllPresets,
   loadMainConfig,
   scanAndTransformFiles,
@@ -79,7 +80,7 @@ export default async function postInstall(options: PostinstallOptions) {
 
   const hasCustomWebpackConfig = !!config.getFieldNode(['webpackFinal']);
 
-  const isInteractive = process.stdout.isTTY && !process.env.CI;
+  const isInteractive = process.stdout.isTTY && !isCI();
 
   if (info.frameworkPackageName === '@storybook/nextjs' && !hasCustomWebpackConfig) {
     const out =

code/addons/vitest/src/vitest-plugin/index.ts

@@ -8,6 +8,7 @@ import {
   DEFAULT_FILES_PATTERN,
   getInterpretedFile,
   normalizeStories,
+  optionalEnvToBoolean,
   resolvePathInStorybookCache,
   validateConfigurationFiles,
 } from 'storybook/internal/common';
@@ -123,14 +124,18 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
     },
   } as InternalOptions;
 
-  if (process.env.DEBUG) {
+  if (optionalEnvToBoolean(process.env.DEBUG)) {
     finalOptions.debug = true;
   }
 
   // To be accessed by the global setup file
   process.env.__STORYBOOK_URL__ = finalOptions.storybookUrl;
   process.env.__STORYBOOK_SCRIPT__ = finalOptions.storybookScript;
 
+  // We signal the test runner that we are not running it via Storybook
+  // We are overriding the environment variable to 'true' if vitest runs via @storybook/addon-vitest's backend
+  const isVitestStorybook = optionalEnvToBoolean(process.env.VITEST_STORYBOOK);
+
   const directories = {
     configDir: finalOptions.configDir,
     workingDir: WORKING_DIR,
@@ -212,10 +217,6 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
       //   plugin.name?.startsWith('vitest:browser')
       // )
 
-      // We signal the test runner that we are not running it via Storybook
-      // We are overriding the environment variable to 'true' if vitest runs via @storybook/addon-vitest's backend
-      const vitestStorybook = process.env.VITEST_STORYBOOK ?? 'false';
-
       const testConfig = nonMutableInputConfig.test;
       finalOptions.vitestRoot =
         testConfig?.dir || testConfig?.root || nonMutableInputConfig.root || process.cwd();
@@ -260,7 +261,7 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
             // To be accessed by the setup file
             __STORYBOOK_URL__: finalOptions.storybookUrl,
 
-            VITEST_STORYBOOK: vitestStorybook,
+            VITEST_STORYBOOK: isVitestStorybook ? 'true' : 'false',
             __VITEST_INCLUDE_TAGS__: finalOptions.tags.include.join(','),
             __VITEST_EXCLUDE_TAGS__: finalOptions.tags.exclude.join(','),
             __VITEST_SKIP_TAGS__: finalOptions.tags.skip.join(','),
@@ -288,9 +289,7 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
               getInitialGlobals: () => {
                 const envConfig = JSON.parse(process.env.VITEST_STORYBOOK_CONFIG ?? '{}');
 
-                const shouldRunA11yTests = process.env.VITEST_STORYBOOK
-                  ? (envConfig.a11y ?? false)
-                  : true;
+                const shouldRunA11yTests = isVitestStorybook ? (envConfig.a11y ?? false) : true;
 
                 return {
                   a11y: {
@@ -373,10 +372,10 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
     configureVitest(context) {
       context.vitest.config.coverage.exclude.push('storybook-static');
 
-      const disableTelemetryVar =
-        process.env.STORYBOOK_DISABLE_TELEMETRY &&
-        process.env.STORYBOOK_DISABLE_TELEMETRY !== 'false';
-      if (!core?.disableTelemetry && !disableTelemetryVar) {
+      if (
+        !core?.disableTelemetry &&
+        !optionalEnvToBoolean(process.env.STORYBOOK_DISABLE_TELEMETRY)
+      ) {
         // NOTE: we start telemetry immediately but do not wait on it. Typically it should complete
         // before the tests do. If not we may miss the event, we are OK with that.
         telemetry(
@@ -410,7 +409,7 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
       }
     },
     async transform(code, id) {
-      if (process.env.VITEST !== 'true') {
+      if (!optionalEnvToBoolean(process.env.VITEST)) {
         return code;
       }
 
@@ -434,7 +433,7 @@ export const storybookTest = async (options?: UserOptions): Promise<Plugin[]> =>
   // When running tests via the Storybook UI, we need
   // to find the right project to run, thus we override
   // with a unique identifier using the path to the config dir
-  if (process.env.VITEST_STORYBOOK) {
+  if (isVitestStorybook) {
     const projectName = `storybook:${normalize(finalOptions.configDir)}`;
     plugins.push({
       name: 'storybook:workspace-name-override',

code/core/src/cli/bin/index.ts

@@ -1,4 +1,4 @@
-import { getEnvConfig, parseList } from 'storybook/internal/common';
+import { getEnvConfig, optionalEnvToBoolean, parseList } from 'storybook/internal/common';
 import { logTracker, logger } from 'storybook/internal/node-logger';
 import { addToGlobalContext } from 'storybook/internal/telemetry';
 
@@ -22,8 +22,7 @@ const command = (name: string) =>
     .option(
       '--disable-telemetry',
       'Disable sending telemetry data',
-      // default value is false, but if the user sets STORYBOOK_DISABLE_TELEMETRY, it can be true
-      process.env.STORYBOOK_DISABLE_TELEMETRY && process.env.STORYBOOK_DISABLE_TELEMETRY !== 'false'
+      optionalEnvToBoolean(process.env.STORYBOOK_DISABLE_TELEMETRY)
     )
     .option('--debug', 'Get more logs in debug mode', false)
     .option('--enable-crash-reports', 'Enable sending crash reports to telemetry data')
@@ -151,7 +150,7 @@ command('build')
     await build({
       ...options,
       packageJson: pkg,
-      test: !!options.test || process.env.SB_TESTBUILD === 'true',
+      test: !!options.test || optionalEnvToBoolean(process.env.SB_TESTBUILD),
     }).catch(() => process.exit(1));
   });
 

code/core/src/cli/globalSettings.test.ts

@@ -85,16 +85,22 @@ describe('Settings', () => {
       );
     });
 
-    it('throws error if write fails', async () => {
+    it('logs warning if write fails', async () => {
       vi.mocked(fs.writeFile).mockRejectedValue(new Error('Write error'));
 
-      await expect(settings.save()).rejects.toThrow('Unable to save global settings');
+      await expect(settings.save()).resolves.toBeUndefined();
+      expect(console.warn).toHaveBeenCalledWith(
+        'Unable to save global settings file to /test/settings.json\nReason: Write error'
+      );
     });
 
-    it('throws error if directory creation fails', async () => {
+    it('logs warning if directory creation fails', async () => {
       vi.mocked(fs.mkdir).mockRejectedValue(new Error('Directory creation error'));
 
-      await expect(settings.save()).rejects.toThrow('Unable to save global settings');
+      await expect(settings.save()).resolves.toBeUndefined();
+      expect(console.warn).toHaveBeenCalledWith(
+        'Unable to save global settings file to /test/settings.json\nReason: Directory creation error'
+      );
     });
   });
 });

code/core/src/cli/globalSettings.ts

@@ -2,10 +2,9 @@ import fs from 'node:fs/promises';
 import { homedir } from 'node:os';
 import { dirname, join } from 'node:path';
 
+import { dedent } from 'ts-dedent';
 import { z } from 'zod';
 
-import { SavingGlobalSettingsFileError } from '../server-errors';
-
 const DEFAULT_SETTINGS_PATH = join(homedir(), '.storybook', 'settings.json');
 
 const VERSION = 1;
@@ -71,10 +70,9 @@ export class Settings {
       await fs.mkdir(dirname(this.filePath), { recursive: true });
       await fs.writeFile(this.filePath, JSON.stringify(this.value, null, 2));
     } catch (err) {
-      throw new SavingGlobalSettingsFileError({
-        filePath: this.filePath,
-        error: err,
-      });
+      console.warn(dedent`
+        Unable to save global settings file to ${this.filePath}
+        ${err && `Reason: ${(err as Error).message ?? err}`}`);
     }
   }
 }

code/core/src/common/utils/envs.ts

@@ -1,15 +1,14 @@
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore - Needed for Angular sandbox running without --no-link option. Do NOT convert to @ts-expect-error!
-import { getEnvironment } from 'lazy-universal-dotenv';
-
 import { nodePathsToArray } from './paths';
 
 // Load environment variables starts with STORYBOOK_ to the client side.
 
-export function loadEnvs(options: { production?: boolean } = {}): {
+export async function loadEnvs(options: { production?: boolean } = {}): Promise<{
   stringified: Record<string, string>;
   raw: Record<string, string>;
-} {
+}> {
+  const { getEnvironment } = await import('lazy-universal-dotenv');
   const defaultNodeEnv = options.production ? 'production' : 'development';
 
   const env: Record<string, string | undefined> = {
@@ -67,3 +66,26 @@ export const stringifyProcessEnvs = (raw: Record<string, string>): Record<string
   // envs['process.env'] = JSON.stringify(raw);
   return envs;
 };
+
+export const optionalEnvToBoolean = (input: string | undefined): boolean | undefined => {
+  if (input === undefined) {
+    return undefined;
+  }
+  if (input.toUpperCase() === 'FALSE' || input === '0') {
+    return false;
+  }
+  if (input.toUpperCase() === 'TRUE' || input === '1') {
+    return true;
+  }
+  return Boolean(input);
+};
+
+/**
+ * Consistently determine if we are in a CI environment
+ *
+ * Doing Boolean(process.env.CI) or !process.env.CI is not enough, because users might set CI=false
+ * or CI=0, which would be truthy, and thus return true in those cases.
+ */
+export function isCI(): boolean | undefined {
+  return optionalEnvToBoolean(process.env.CI);
+}

code/core/src/core-server/dev-server.ts

@@ -7,6 +7,7 @@ import compression from '@polka/compression';
 import polka from 'polka';
 import invariant from 'tiny-invariant';
 
+import { telemetry } from '../telemetry';
 import type { StoryIndexGenerator } from './utils/StoryIndexGenerator';
 import { doTelemetry } from './utils/doTelemetry';
 import { getManagerBuilder, getPreviewBuilder } from './utils/get-builders';
@@ -19,6 +20,7 @@ import { openInBrowser } from './utils/open-in-browser';
 import { getServerAddresses } from './utils/server-address';
 import { getServer } from './utils/server-init';
 import { useStatics } from './utils/server-statics';
+import { summarizeIndex } from './utils/summarizeIndex';
 
 export async function storybookDevServer(options: Options) {
   const [server, core] = await Promise.all([getServer(options), options.presets.apply('core')]);
@@ -130,5 +132,26 @@ export async function storybookDevServer(options: Options) {
   // Now the preview has successfully started, we can count this as a 'dev' event.
   doTelemetry(app, core, initializedStoryIndexGenerator, options);
 
+  async function cancelTelemetry() {
+    const payload = { eventType: 'dev' };
+    try {
+      const generator = await initializedStoryIndexGenerator;
+      const indexAndStats = await generator?.getIndexAndStats();
+      // compute stats so we can get more accurate story counts
+      if (indexAndStats) {
+        Object.assign(payload, {
+          storyIndex: summarizeIndex(indexAndStats.storyIndex),
+          storyStats: indexAndStats.stats,
+        });
+      }
+    } catch (err) {}
+    await telemetry('canceled', payload, { immediate: true });
+    process.exit(0);
+  }
+
+  if (!core?.disableTelemetry) {
+    process.on('SIGINT', cancelTelemetry);
+  }
+
   return { previewResult, managerResult, address, networkAddress };
 }