Merge pull request #30930 from storybookjs/shilman/cli-new-users

CLI: Add skip onboarding, recommended/minimal config
(cherry picked from commit dde78a0)

Author: shilman

.vscode/settings.json

@@ -8,7 +8,7 @@
     "editor.formatOnSave": true
   },
   "[typescript]": {
-    "editor.defaultFormatter": "dbaeumer.vscode-eslint",
+    "editor.defaultFormatter": "esbenp.prettier-vscode",
     "editor.formatOnSave": true
   },
   "[typescriptreact]": {

code/core/package.json

@@ -417,7 +417,8 @@
     "typescript": "^5.7.3",
     "unique-string": "^3.0.0",
     "use-resize-observer": "^9.1.0",
-    "watchpack": "^2.2.0"
+    "watchpack": "^2.2.0",
+    "zod": "^3.24.1"
   },
   "peerDependencies": {
     "prettier": "^2 || ^3"

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

@@ -12,6 +12,7 @@ import invariant from 'tiny-invariant';
 import { version } from '../../../package.json';
 import { build } from '../build';
 import { dev } from '../dev';
+import { globalSettings } from '../globalSettings';
 
 addToGlobalContext('cliVersion', versions.storybook);
 
@@ -27,7 +28,14 @@ const command = (name: string) =>
       process.env.STORYBOOK_DISABLE_TELEMETRY && process.env.STORYBOOK_DISABLE_TELEMETRY !== 'false'
     )
     .option('--debug', 'Get more logs in debug mode', false)
-    .option('--enable-crash-reports', 'Enable sending crash reports to telemetry data');
+    .option('--enable-crash-reports', 'Enable sending crash reports to telemetry data')
+    .hook('preAction', async () => {
+      try {
+        await globalSettings();
+      } catch (e) {
+        consoleLogger.error('Error loading global settings', e);
+      }
+    });
 
 command('dev')
   .option('-p, --port <number>', 'Port to run Storybook', (str) => parseInt(str, 10))

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

@@ -0,0 +1,100 @@
+import fs from 'node:fs/promises';
+import { dirname } from 'node:path';
+import { afterEach } from 'node:test';
+
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { type Settings, _clearGlobalSettings, globalSettings } from './globalSettings';
+
+vi.mock('node:fs');
+vi.mock('node:fs/promises');
+
+const userSince = new Date();
+const baseSettings = { version: 1, userSince: +userSince };
+const baseSettingsJson = JSON.stringify(baseSettings, null, 2);
+
+const TEST_SETTINGS_FILE = '/test/settings.json';
+
+beforeEach(() => {
+  _clearGlobalSettings();
+
+  vi.useFakeTimers();
+  vi.setSystemTime(userSince);
+
+  vi.resetAllMocks();
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+});
+
+describe('globalSettings', () => {
+  it('loads settings when called for the first time', async () => {
+    vi.mocked(fs.readFile).mockResolvedValue(baseSettingsJson);
+
+    const settings = await globalSettings(TEST_SETTINGS_FILE);
+
+    expect(settings.value.userSince).toBe(+userSince);
+  });
+
+  it('does nothing if settings are already loaded', async () => {
+    vi.mocked(fs.readFile).mockResolvedValue(baseSettingsJson);
+    await globalSettings(TEST_SETTINGS_FILE);
+
+    vi.mocked(fs.readFile).mockClear();
+    await globalSettings(TEST_SETTINGS_FILE);
+    expect(fs.readFile).not.toHaveBeenCalled();
+  });
+
+  it('does not save settings if they exist', async () => {
+    vi.mocked(fs.readFile).mockResolvedValue(baseSettingsJson);
+
+    await globalSettings(TEST_SETTINGS_FILE);
+
+    expect(fs.writeFile).not.toHaveBeenCalled();
+  });
+
+  it('saves settings and creates directory if they do not exist', async () => {
+    const error = new Error() as Error & { code: string };
+    error.code = 'ENOENT';
+    vi.mocked(fs.readFile).mockRejectedValue(error);
+
+    await globalSettings(TEST_SETTINGS_FILE);
+
+    expect(fs.mkdir).toHaveBeenCalledWith(dirname(TEST_SETTINGS_FILE), { recursive: true });
+    expect(fs.writeFile).toHaveBeenCalledWith(TEST_SETTINGS_FILE, baseSettingsJson);
+  });
+});
+
+describe('Settings', () => {
+  let settings: Settings;
+  beforeEach(async () => {
+    vi.mocked(fs.readFile).mockResolvedValue(baseSettingsJson);
+
+    settings = await globalSettings(TEST_SETTINGS_FILE);
+  });
+
+  describe('save', () => {
+    it('overwrites existing settings', async () => {
+      settings.value.init = { skipOnboarding: true };
+      await settings.save();
+
+      expect(fs.writeFile).toHaveBeenCalledWith(
+        TEST_SETTINGS_FILE,
+        JSON.stringify({ ...baseSettings, init: { skipOnboarding: true } }, null, 2)
+      );
+    });
+
+    it('throws error if write fails', async () => {
+      vi.mocked(fs.writeFile).mockRejectedValue(new Error('Write error'));
+
+      await expect(settings.save()).rejects.toThrow('Unable to save global settings');
+    });
+
+    it('throws error 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');
+    });
+  });
+});

code/core/src/cli/globalSettings.ts

@@ -0,0 +1,80 @@
+import fs from 'node:fs/promises';
+import { homedir } from 'node:os';
+import { dirname, join } from 'node:path';
+
+import { z } from 'zod';
+
+import { SavingGlobalSettingsFileError } from '../server-errors';
+
+const DEFAULT_SETTINGS_PATH = join(homedir(), '.storybook', 'settings.json');
+
+const VERSION = 1;
+
+const userSettingSchema = z.object({
+  version: z.number(),
+  // NOTE: every key (and subkey) below must be optional, for forwards compatibility reasons
+  // (we can remove keys once they are deprecated)
+  userSince: z.number().optional(),
+  init: z.object({ skipOnboarding: z.boolean().optional() }).optional(),
+});
+
+let settings: Settings | undefined;
+export async function globalSettings(filePath = DEFAULT_SETTINGS_PATH) {
+  if (settings) {
+    return settings;
+  }
+
+  try {
+    const content = await fs.readFile(filePath, 'utf8');
+    const settingsValue = userSettingSchema.parse(JSON.parse(content));
+    settings = new Settings(filePath, settingsValue);
+  } catch (err: any) {
+    // We don't currently log the issue we have loading the setting file here, but if it doesn't
+    // yet exist we'll get err.code = 'ENOENT'
+
+    // There is no existing settings file or it has a problem;
+    settings = new Settings(filePath, { version: VERSION, userSince: Date.now() });
+    await settings.save();
+  }
+
+  return settings;
+}
+
+// For testing
+export function _clearGlobalSettings() {
+  settings = undefined;
+}
+
+/**
+ * A class for reading and writing settings from a JSON file. Supports nested settings with dot
+ * notation.
+ */
+export class Settings {
+  private filePath: string;
+
+  public value: z.infer<typeof userSettingSchema>;
+
+  /**
+   * Create a new Settings instance
+   *
+   * @param filePath Path to the JSON settings file
+   * @param value Loaded value of settings
+   */
+  constructor(filePath: string, value: z.infer<typeof userSettingSchema>) {
+    this.filePath = filePath;
+    this.value = value;
+  }
+
+  /** Save settings to the file */
+  async save(): Promise<void> {
+    try {
+      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,
+      });
+    }
+  }
+}

code/core/src/cli/index.ts

@@ -5,3 +5,4 @@ export * from './dirs';
 export * from './project_types';
 export * from './NpmOptions';
 export * from './eslintPlugin';
+export * from './globalSettings';

code/core/src/common/utils/notify-telemetry.ts

@@ -25,7 +25,7 @@ export const notifyTelemetry = async () => {
   );
   logger.log(`This information is used to shape Storybook's roadmap and prioritize features.`);
   logger.log(
-    `You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:`
+    `You can learn more, including how to opt-out of this anonymous program, by visiting:`
   );
   logger.log(picocolors.cyan('https://storybook.js.org/telemetry'));
   logger.log();

code/core/src/manager/globals/exports.ts

@@ -268,6 +268,7 @@ export default {
     'SubtractIcon',
     'SunIcon',
     'SupportIcon',
+    'SweepIcon',
     'SwitchAltIcon',
     'SyncIcon',
     'TabletIcon',

code/core/src/server-errors.ts

@@ -538,3 +538,14 @@ export class FindPackageVersionsError extends StorybookError {
     });
   }
 }
+export class SavingGlobalSettingsFileError extends StorybookError {
+  constructor(public data: { filePath: string; error: Error | unknown }) {
+    super({
+      category: Category.CORE_SERVER,
+      code: 1,
+      message: dedent`
+        Unable to save global settings file to ${data.filePath}
+        ${data.error && `Reason: ${data.error}`}`,
+    });
+  }
+}

code/core/src/telemetry/storybook-metadata.test.ts

@@ -420,5 +420,15 @@ describe('storybook-metadata', () => {
         });
       }
     );
+
+    it('should detect userSince info', async () => {
+      const res = await computeStorybookMetadata({
+        packageJson: packageJsonMock,
+        packageJsonPath,
+        mainConfig: mainJsMock,
+      });
+
+      expect(res.userSince).toBeDefined();
+    });
   });
 });