CLI: Implement design feedback

code/addons/themes/src/postinstall.ts

@@ -1,4 +1,4 @@
-import { spawn } from 'child_process';
+import { spawnSync } from 'child_process';
 
 const PACKAGE_MANAGER_TO_COMMAND = {
   npm: 'npx',
@@ -12,11 +12,11 @@ const selectPackageManagerCommand = (packageManager: string) =>
   PACKAGE_MANAGER_TO_COMMAND[packageManager as keyof typeof PACKAGE_MANAGER_TO_COMMAND];
 
 export default async function postinstall({ packageManager = 'npm' }) {
-  const command = selectPackageManagerCommand(packageManager);
+  const commandString = selectPackageManagerCommand(packageManager);
+  const [command, ...commandArgs] = commandString.split(' ');
 
-  await spawn(`${command} @storybook/auto-config themes`, {
+  spawnSync(command, [...commandArgs, '@storybook/auto-config', 'themes'], {
     stdio: 'inherit',
     cwd: process.cwd(),
-    shell: true,
   });
 }

code/addons/vitest/src/postinstall.ts

@@ -46,6 +46,7 @@ export default async function postInstall(options: PostinstallOptions) {
     );
 
   const vitestVersionSpecifier = await packageManager.getInstalledVersion('vitest');
+  logger.debug(`Vitest version specifier: ${vitestVersionSpecifier}`);
   const isVitest3_2To4 = vitestVersionSpecifier
     ? satisfies(vitestVersionSpecifier, '>=3.2.0 <4.0.0')
     : false;
@@ -328,21 +329,22 @@ export default async function postInstall(options: PostinstallOptions) {
         'storybook',
         'automigrate',
         'addon-a11y-addon-test',
-        '--loglevel=silent',
+        '--loglevel',
+        'silent',
         '--yes',
         '--skip-doctor',
       ];
 
       if (options.packageManager) {
-        command.push(`--package-manager=${options.packageManager}`);
+        command.push('--package-manager', options.packageManager);
       }
 
       if (options.skipInstall) {
         command.push('--skip-install');
       }
 
       if (options.configDir !== '.storybook') {
-        command.push(`--config-dir="${options.configDir}"`);
+        command.push('--config-dir', options.configDir);
       }
 
       await prompt.executeTask(

code/addons/vitest/src/vitest-plugin/global-setup.ts

@@ -47,7 +47,6 @@ const startStorybookIfNotRunning = async () => {
     storybookProcess = spawn(storybookScript, [], {
       stdio: process.env.DEBUG === 'storybook' ? 'pipe' : 'ignore',
       cwd: process.cwd(),
-      shell: true,
     });
 
     storybookProcess.on('error', (error) => {

code/core/package.json

@@ -265,7 +265,6 @@
     "bundle-require": "^5.1.0",
     "camelcase": "^8.0.0",
     "chai": "^5.1.1",
-    "cli-table3": "^0.6.1",
     "commander": "^14.0.1",
     "comment-parser": "^1.4.1",
     "copy-to-clipboard": "^3.3.1",

code/core/src/bin/dispatcher.ts

@@ -61,7 +61,7 @@ async function run() {
     if (targetCliPackageJson.version === versions[targetCli.pkg]) {
       command = [
         'node',
-        `"${join(resolvePackageDir(targetCli.pkg), 'dist/bin/index.js')}"`,
+        join(resolvePackageDir(targetCli.pkg), 'dist/bin/index.js'),
         ...targetCli.args,
       ];
     }
@@ -70,7 +70,7 @@ async function run() {
   }
   command ??= ['npx', '--yes', `${targetCli.pkg}@${versions[targetCli.pkg]}`, ...targetCli.args];
 
-  const child = spawn(command[0], command.slice(1), { stdio: 'inherit', shell: true });
+  const child = spawn(command[0], command.slice(1), { stdio: 'inherit' });
   child.on('exit', (code) => {
     process.exit(code);
   });

code/core/src/builder-manager/index.ts

@@ -141,7 +141,7 @@ const starter: StarterFunction = async function* starterGeneratorFn({
   router,
 }) {
   if (!options.quiet) {
-    logger.info('Starting manager..');
+    logger.info('Starting...');
   }
 
   const {

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

@@ -390,7 +390,7 @@
       });
       expect(prompt.executeTaskWithSpinner).toHaveBeenCalledWith(expect.any(Function), {
         id: 'playwright-installation',
-        intro: 'Installing Playwright browser binaries',
+        intro: 'Installing Playwright browser binaries (Press "c" to abort)',
         error: expect.stringContaining('An error occurred'),
         success: 'Playwright browser binaries installed successfully',
         abortable: true,
@@ -411,7 +411,7 @@
 
       await service.installPlaywright(mockPackageManager);
 
       expect(mockPackageManager.runPackageCommand).toHaveBeenCalledWith({
         args: ['playwright', 'install', 'chromium', '--with-deps'],
         signal: undefined,
         stdio: 'ignore',

code/core/src/cli/AddonVitestService.ts

@@ -126,7 +126,7 @@ export class AddonVitestService {
         : await (async () => {
             logger.log(dedent`
             Playwright browser binaries are necessary for @storybook/addon-vitest. The download can take some time. If you don't want to wait, you can skip the installation and run the following command manually later:
-            ${CLI_COLORS.cta(playwrightCommand.join(' '))}
+            ${CLI_COLORS.cta(`npx ${playwrightCommand.join(' ')}`)}
             `);
             return prompt.confirm({
               message: 'Do you want to install Playwright with Chromium now?',
@@ -139,12 +139,12 @@ export class AddonVitestService {
           (signal) =>
             packageManager.runPackageCommand({
               args: playwrightCommand,
-              stdio: 'ignore',
+              stdio: ['inherit', 'pipe', 'pipe'],
               signal,
             }),
           {
             id: 'playwright-installation',
-            intro: 'Installing Playwright browser binaries',
+            intro: 'Installing Playwright browser binaries (Press "c" to abort)',
             error: `An error occurred while installing Playwright browser binaries. Please run the following command later: npx ${playwrightCommand.join(' ')}`,
             success: 'Playwright browser binaries installed successfully',
             abortable: true,

code/core/src/common/js-package-manager/JsPackageManager.test.ts

@@ -2,22 +2,23 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { JsPackageManager } from './JsPackageManager';
 
+const mockVersions = vi.hoisted(() => ({
+  '@storybook/react': '8.3.0',
+}));
+
 vi.mock('../versions', () => ({
-  default: {
-    '@storybook/react': '8.3.0',
-  },
+  default: mockVersions,
 }));
 
 describe('JsPackageManager', () => {
   let jsPackageManager: JsPackageManager;
-  let mockLatestVersion: ReturnType<typeof vi.fn>;
+  let mockLatestVersion: ReturnType<typeof vi.spyOn>;
 
   beforeEach(() => {
-    mockLatestVersion = vi.fn();
-
     // @ts-expect-error Ignore abstract class error
     jsPackageManager = new JsPackageManager();
-    jsPackageManager.latestVersion = mockLatestVersion;
+    // @ts-expect-error latestVersion is a method that exists on the instance
+    mockLatestVersion = vi.spyOn(jsPackageManager, 'latestVersion');
 
     vi.clearAllMocks();
   });

code/core/src/common/js-package-manager/JsPackageManager.ts

@@ -9,7 +9,7 @@ import { type ExecaChildProcess } from 'execa';
 // eslint-disable-next-line depend/ban-dependencies
 import { globSync } from 'glob';
 import picocolors from 'picocolors';
-import { gt, satisfies } from 'semver';
+import { coerce, gt, satisfies } from 'semver';
 import invariant from 'tiny-invariant';
 
 import { HandledError } from '../utils/HandledError';
@@ -635,10 +635,13 @@ export abstract class JsPackageManager {
 
       const version = Object.entries(installations.dependencies)[0]?.[1]?.[0].version || null;
 
+      const coercedVersion = coerce(version, { includePrerelease: true })?.toString() ?? version;
+
+      logger.debug(`Installed version for ${packageName}: ${coercedVersion}`);
       // Cache the result
-      JsPackageManager.installedVersionCache.set(cacheKey, version);
+      JsPackageManager.installedVersionCache.set(cacheKey, coercedVersion);
 
-      return version;
+      return coercedVersion;
     } catch (e) {
       JsPackageManager.installedVersionCache.set(cacheKey, null);
       return null;

code/core/src/common/js-package-manager/JsPackageManagerFactory.ts

@@ -1,8 +1,8 @@
 import { basename, parse, relative } from 'node:path';
 
-import { sync as spawnSync } from 'cross-spawn';
 import * as find from 'empathic/find';
 
+import { executeCommandSync } from '../utils/command';
 import { getProjectRoot } from '../utils/paths';
 import { BUNProxy } from './BUNProxy';
 import type { JsPackageManager, PackageManagerName } from './JsPackageManager';
@@ -195,56 +195,70 @@ export class JsPackageManagerFactory {
 }
 
 function hasNPM(cwd?: string) {
-  const npmVersionCommand = spawnSync('npm --version', {
-    cwd,
-    shell: true,
-    env: {
-      ...process.env,
-      ...COMMON_ENV_VARS,
-    },
-  });
-  return npmVersionCommand.status === 0;
+  try {
+    executeCommandSync({
+      command: 'npm',
+      args: ['--version'],
+      cwd,
+      env: {
+        ...process.env,
+        ...COMMON_ENV_VARS,
+      },
+    });
+    return true;
+  } catch (err) {
+    return false;
+  }
 }
 
 function hasBun(cwd?: string) {
-  const pnpmVersionCommand = spawnSync('bun --version', {
-    cwd,
-    shell: true,
-    env: {
-      ...process.env,
-      ...COMMON_ENV_VARS,
-    },
-  });
-  return pnpmVersionCommand.status === 0;
+  try {
+    executeCommandSync({
+      command: 'bun',
+      args: ['--version'],
+      cwd,
+      env: {
+        ...process.env,
+        ...COMMON_ENV_VARS,
+      },
+    });
+    return true;
+  } catch (err) {
+    return false;
+  }
 }
 
 function hasPNPM(cwd?: string) {
-  const pnpmVersionCommand = spawnSync('pnpm --version', {
-    cwd,
-    shell: true,
-    env: {
-      ...process.env,
-      ...COMMON_ENV_VARS,
-    },
-  });
-  return pnpmVersionCommand.status === 0;
+  try {
+    executeCommandSync({
+      command: 'pnpm',
+      args: ['--version'],
+      cwd,
+      env: {
+        ...process.env,
+        ...COMMON_ENV_VARS,
+      },
+    });
+
+    return true;
+  } catch (err) {
+    return false;
+  }
 }
 
 function getYarnVersion(cwd?: string): 1 | 2 | undefined {
-  const yarnVersionCommand = spawnSync('yarn --version', {
-    cwd,
-    shell: true,
-    env: {
-      ...process.env,
-      ...COMMON_ENV_VARS,
-    },
-  });
-
-  if (yarnVersionCommand.status !== 0) {
+  try {
+    const yarnVersion = executeCommandSync({
+      command: 'yarn',
+      args: ['--version'],
+      cwd,
+      env: {
+        ...process.env,
+        ...COMMON_ENV_VARS,
+      },
+    });
+    return /^1\.+/.test(yarnVersion.trim()) ? 1 : 2;
+  } catch (err) {
     return undefined;
   }
-
-  const yarnVersion = yarnVersionCommand.output.toString().replace(/,/g, '').replace(/"/g, '');
-
-  return /^1\.+/.test(yarnVersion) ? 1 : 2;
 }

code/core/src/common/js-package-manager/Yarn2Proxy.ts

@@ -11,6 +11,7 @@ import * as find from 'empathic/find';
 // eslint-disable-next-line depend/ban-dependencies
 import type { ExecaChildProcess } from 'execa';
 
+import { logger } from '../../node-logger';
 import type { ExecuteCommandOptions } from '../utils/command';
 import { executeCommand } from '../utils/command';
 import { getProjectRoot } from '../utils/paths';
@@ -136,6 +137,8 @@ export class Yarn2Proxy extends JsPackageManager {
       });
       const commandResult = childProcess.stdout ?? '';
 
+      logger.debug(`Installation found for ${pattern.join(', ')}: ${commandResult}`);
+
       return this.mapDependencies(commandResult, pattern);
     } catch (e) {
       return undefined;
@@ -280,6 +283,7 @@ export class Yarn2Proxy extends JsPackageManager {
     const duplicatedDependencies: Record<string, string[]> = {};
 
     lines.forEach((packageName) => {
+      logger.debug(`Processing package ${packageName}`);
       if (
         !packageName ||
         !pattern.some((p) => new RegExp(`${p.replace(/\*/g, '.*')}`).test(packageName))
@@ -288,6 +292,7 @@ export class Yarn2Proxy extends JsPackageManager {
       }
 
       const { name, value } = parsePackageData(packageName.replaceAll(`"`, ''));
+      logger.debug(`Package ${name} found with version ${value.version}`);
       if (!existingVersions[name]?.includes(value.version)) {
         if (acc[name]) {
           acc[name].push(value);

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

@@ -1,7 +1,7 @@
 import { logger, prompt } from 'storybook/internal/node-logger';
 
 // eslint-disable-next-line depend/ban-dependencies
-import { type CommonOptions, type ExecaChildProcess, execa } from 'execa';
+import { type CommonOptions, type ExecaChildProcess, execa, execaCommandSync } from 'execa';
 
 const COMMON_ENV_VARS = {
   COREPACK_ENABLE_STRICT: '0',
@@ -22,7 +22,6 @@ function getExecaOptions({ stdio, cwd, env, ...execaOptions }: ExecuteCommandOpt
     cwd,
     stdio: stdio ?? prompt.getPreferredStdio(),
     encoding: 'utf8' as const,
-    shell: true,
     cleanup: true,
     env: {
       ...COMMON_ENV_VARS,
@@ -45,3 +44,16 @@ export function executeCommand(options: ExecuteCommandOptions): ExecaChildProces
 
   return execaProcess;
 }
+
+export function executeCommandSync(options: ExecuteCommandOptions): string {
+  const { command, args = [], ignoreError = false } = options;
+  try {
+    const commandResult = execaCommandSync([command, ...args].join(' '), getExecaOptions(options));
+    return commandResult.stdout ?? '';
+  } catch (err) {
+    if (!ignoreError) {
+      throw err;
+    }
+    return '';
+  }
+}

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

@@ -94,9 +94,7 @@ export async function storybookDevServer(options: Options) {
     await Promise.resolve();
 
   if (!options.ignorePreview) {
-    if (!options.quiet) {
-      logger.info('Starting preview..');
-    }
+    logger.debug('Starting preview..');
     previewResult = await previewBuilder
       .start({
         startTime: process.hrtime(),