diff --git a/code/addons/vitest/src/postinstall.ts b/code/addons/vitest/src/postinstall.ts index 5231a1317ddc..fa7446af1a96 100644 --- a/code/addons/vitest/src/postinstall.ts +++ b/code/addons/vitest/src/postinstall.ts @@ -58,11 +58,10 @@ export default async function postInstall(options: PostinstallOptions) { const allDeps = packageManager.getAllDependencies(); // only install these dependencies if they are not already installed - const addonVitestService = new AddonVitestService(); + const addonVitestService = new AddonVitestService(packageManager); // Use AddonVitestService for compatibility validation const compatibilityResult = await addonVitestService.validateCompatibility({ - packageManager, framework: info.framework, builder: info.builder, }); @@ -97,7 +96,7 @@ export default async function postInstall(options: PostinstallOptions) { // Skip all dependency management when flag is set (called from init command) if (!options.skipDependencyManagement) { // Use AddonVitestService for dependency collection - const versionedDependencies = await addonVitestService.collectDependencies(packageManager); + const versionedDependencies = await addonVitestService.collectDependencies(); // Print informational messages for Next.js if (info.framework === SupportedFramework.NEXTJS) { @@ -139,12 +138,9 @@ export default async function postInstall(options: PostinstallOptions) { // Install Playwright browser binaries using AddonVitestService if (!options.skipDependencyManagement) { if (!options.skipInstall) { - const { errors: playwrightErrors } = await addonVitestService.installPlaywright( - packageManager, - { - yes: options.yes, - } - ); + const { errors: playwrightErrors } = await addonVitestService.installPlaywright({ + yes: options.yes, + }); errors.push(...playwrightErrors); } else { logger.warn(dedent` diff --git a/code/core/src/cli/AddonVitestService.test.ts b/code/core/src/cli/AddonVitestService.test.ts index 841d23e4d622..4085fb8b65fe 100644 --- a/code/core/src/cli/AddonVitestService.test.ts +++ b/code/core/src/cli/AddonVitestService.test.ts @@ -24,15 +24,15 @@ describe('AddonVitestService', () => { beforeEach(() => { vi.clearAllMocks(); - service = new AddonVitestService(); - vi.mocked(getProjectRoot).mockReturnValue('/test/project'); - mockPackageManager = { getAllDependencies: vi.fn(), getInstalledVersion: vi.fn(), runPackageCommand: vi.fn(), } as Partial as JsPackageManager; + service = new AddonVitestService(mockPackageManager); + vi.mocked(getProjectRoot).mockReturnValue('/test/project'); + // Setup default mocks for logger and prompt vi.mocked(logger.info).mockImplementation(() => {}); vi.mocked(logger.log).mockImplementation(() => {}); @@ -55,7 +55,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null) // @vitest/coverage-v8 .mockResolvedValueOnce(null); // @vitest/coverage-istanbul - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); expect(deps).toContain('vitest'); // When vitest version is null, defaults to vitest 4+ behavior @@ -75,7 +75,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // @vitest/coverage-v8 .mockResolvedValueOnce(null); // @vitest/coverage-istanbul - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); expect(deps).not.toContain('vitest'); expect(deps).not.toContain('@vitest/browser'); @@ -91,7 +91,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null) // @vitest/coverage-v8 .mockResolvedValueOnce(null); // @vitest/coverage-istanbul - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); // Should only contain base packages, not framework-specific ones expect(deps).toContain('vitest'); @@ -109,7 +109,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null) // @vitest/coverage-v8 .mockResolvedValueOnce(null); // @vitest/coverage-istanbul - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); expect(deps.every((d) => !d.includes('nextjs-vite'))).toBe(true); }); @@ -121,7 +121,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // @vitest/coverage-v8 .mockResolvedValueOnce(null); // @vitest/coverage-istanbul - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); expect(deps.every((d) => !d.includes('coverage'))).toBe(true); }); @@ -133,7 +133,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // @vitest/coverage-istanbul .mockResolvedValueOnce(null); // vitest version - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); expect(deps.every((d) => !d.includes('coverage'))).toBe(true); }); @@ -145,7 +145,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null) // @vitest/coverage-v8 .mockResolvedValueOnce(null); // @vitest/coverage-istanbul - const deps = await service.collectDependencies(mockPackageManager); + const deps = await service.collectDependencies(); expect(deps).toContain('vitest@3.2.0'); // Version 3.2.0 < 4.0.0, so uses @vitest/browser @@ -161,7 +161,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // vitest .mockResolvedValueOnce(null); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(true); expect(result.reasons).toBeUndefined(); @@ -172,7 +172,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('4.0.0') // vitest .mockResolvedValueOnce(null); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(true); expect(result.reasons).toBeUndefined(); @@ -183,7 +183,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('2.5.0') // vitest .mockResolvedValueOnce(null); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(false); expect(result.reasons).toBeDefined(); @@ -195,7 +195,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // vitest .mockResolvedValueOnce('2.0.0'); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(true); }); @@ -205,7 +205,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // vitest .mockResolvedValueOnce('1.9.0'); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(false); expect(result.reasons).toBeDefined(); @@ -217,7 +217,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('3.0.0') // vitest .mockResolvedValueOnce(null); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(true); }); @@ -227,7 +227,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null) // vitest .mockResolvedValueOnce(null); // msw - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(true); }); @@ -237,7 +237,7 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('2.0.0') // vitest <3.0.0 .mockResolvedValueOnce('1.0.0'); // msw <2.0.0 - const result = await service.validatePackageVersions(mockPackageManager); + const result = await service.validatePackageVersions(); expect(result.compatible).toBe(false); expect(result.reasons).toBeDefined(); @@ -253,7 +253,6 @@ describe('AddonVitestService', () => { it('should return compatible for valid Vite-based framework', async () => { const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.REACT_VITE, builder: SupportedBuilder.VITE, }); @@ -263,7 +262,6 @@ describe('AddonVitestService', () => { it('should return compatible for react-vite with Vite builder', async () => { const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.REACT_VITE, builder: SupportedBuilder.VITE, }); @@ -273,7 +271,6 @@ describe('AddonVitestService', () => { it('should return incompatible for non-Vite builder (except Next.js)', async () => { const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.REACT_WEBPACK5, builder: SupportedBuilder.WEBPACK5, }); @@ -288,7 +285,6 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null); // msw const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.NEXTJS, builder: SupportedBuilder.WEBPACK5, }); @@ -300,7 +296,6 @@ describe('AddonVitestService', () => { it('should return incompatible for unsupported framework', async () => { const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.ANGULAR, builder: SupportedBuilder.VITE, }); @@ -317,7 +312,6 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce(null); // msw const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.NEXTJS_VITE, builder: SupportedBuilder.VITE, }); @@ -330,7 +324,6 @@ describe('AddonVitestService', () => { vi.mocked(find.any).mockReturnValueOnce('vitest.workspace.json'); const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.REACT_VITE, builder: SupportedBuilder.VITE, projectRoot: '.storybook', @@ -344,7 +337,6 @@ describe('AddonVitestService', () => { vi.mocked(find.any).mockReturnValueOnce('vitest.workspace.json'); const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.REACT_VITE, builder: SupportedBuilder.VITE, }); @@ -359,7 +351,6 @@ describe('AddonVitestService', () => { .mockResolvedValueOnce('1.0.0'); // msw <2.0.0 const result = await service.validateCompatibility({ - packageManager: mockPackageManager, framework: SupportedFramework.ANGULAR, builder: SupportedBuilder.WEBPACK5, }); @@ -381,7 +372,7 @@ describe('AddonVitestService', () => { vi.mocked(prompt.confirm).mockResolvedValue(true); vi.mocked(prompt.executeTaskWithSpinner).mockResolvedValue(undefined); - const { errors } = await service.installPlaywright(mockPackageManager); + const { errors } = await service.installPlaywright(); expect(errors).toEqual([]); expect(prompt.confirm).toHaveBeenCalledWith({ @@ -409,7 +400,7 @@ describe('AddonVitestService', () => { } ); - await service.installPlaywright(mockPackageManager); + await service.installPlaywright(); expect(mockPackageManager.runPackageCommand).toHaveBeenCalledWith({ args: ['playwright', 'install', 'chromium', '--with-deps'], @@ -424,7 +415,7 @@ describe('AddonVitestService', () => { vi.mocked(prompt.confirm).mockResolvedValue(true); vi.mocked(prompt.executeTaskWithSpinner).mockRejectedValue(error); - const { errors } = await service.installPlaywright(mockPackageManager); + const { errors } = await service.installPlaywright(); expect(errors).toEqual(['Error stack trace']); }); @@ -435,7 +426,7 @@ describe('AddonVitestService', () => { vi.mocked(prompt.confirm).mockResolvedValue(true); vi.mocked(prompt.executeTaskWithSpinner).mockRejectedValue(error); - const { errors } = await service.installPlaywright(mockPackageManager); + const { errors } = await service.installPlaywright(); expect(errors).toEqual(['Installation failed']); }); @@ -444,7 +435,7 @@ describe('AddonVitestService', () => { vi.mocked(prompt.confirm).mockResolvedValue(true); vi.mocked(prompt.executeTaskWithSpinner).mockRejectedValue('String error'); - const { errors } = await service.installPlaywright(mockPackageManager); + const { errors } = await service.installPlaywright(); expect(errors).toEqual(['String error']); }); @@ -452,7 +443,7 @@ describe('AddonVitestService', () => { it('should skip installation when user declines', async () => { vi.mocked(prompt.confirm).mockResolvedValue(false); - const { errors } = await service.installPlaywright(mockPackageManager); + const { errors } = await service.installPlaywright(); expect(errors).toEqual([]); expect(prompt.executeTaskWithSpinner).not.toHaveBeenCalled(); @@ -463,7 +454,7 @@ describe('AddonVitestService', () => { vi.mocked(prompt.confirm).mockResolvedValue(true); vi.mocked(prompt.executeTaskWithSpinner).mockResolvedValue(undefined); - await service.installPlaywright(mockPackageManager); + await service.installPlaywright(); expect(prompt.confirm).toHaveBeenCalled(); expect(prompt.executeTaskWithSpinner).toHaveBeenCalled(); diff --git a/code/core/src/cli/AddonVitestService.ts b/code/core/src/cli/AddonVitestService.ts index 1d72f92f7357..8b15daeaa2a8 100644 --- a/code/core/src/cli/AddonVitestService.ts +++ b/code/core/src/cli/AddonVitestService.ts @@ -9,7 +9,7 @@ import { ErrorCollector } from 'storybook/internal/telemetry'; import type { CallExpression } from '@babel/types'; import * as find from 'empathic/find'; -import { coerce, satisfies } from 'semver'; +import { coerce, minVersion, satisfies, validRange } from 'semver'; import { dedent } from 'ts-dedent'; import { SupportedBuilder, SupportedFramework } from '../types'; @@ -20,7 +20,6 @@ type Result = { }; export interface AddonVitestCompatibilityOptions { - packageManager: JsPackageManager; builder?: SupportedBuilder; framework?: SupportedFramework | null; projectRoot?: string; @@ -37,6 +36,8 @@ export interface AddonVitestCompatibilityOptions { * - Code/lib/create-storybook/src/services/FeatureCompatibilityService.ts */ export class AddonVitestService { + constructor(private readonly packageManager: JsPackageManager) {} + readonly supportedFrameworks: SupportedFramework[] = [ SupportedFramework.HTML_VITE, SupportedFramework.NEXTJS_VITE, @@ -57,16 +58,25 @@ export class AddonVitestService { * - Next.js specific: @storybook/nextjs-vite * - Coverage reporter: @vitest/coverage-v8 */ - async collectDependencies(packageManager: JsPackageManager): Promise { - const allDeps = packageManager.getAllDependencies(); + async collectDependencies(): Promise { + const allDeps = this.packageManager.getAllDependencies(); const dependencies: string[] = []; - // Get vitest version for proper version specifiers - const vitestVersionSpecifier = await packageManager.getInstalledVersion('vitest'); + // Determine Vitest version/range from installed or declared dependency to avoid pulling + // incompatible majors by default. + let vitestVersionSpecifier = await this.packageManager.getInstalledVersion('vitest'); + if (!vitestVersionSpecifier && allDeps['vitest']) { + vitestVersionSpecifier = allDeps['vitest']; + } - const isVitest4OrNewer = vitestVersionSpecifier - ? satisfies(vitestVersionSpecifier, '>=4.0.0') - : true; + let isVitest4OrNewer = true; + if (vitestVersionSpecifier) { + const range = validRange(vitestVersionSpecifier); + const versionToCheck = range + ? minVersion(range)?.version + : coerce(vitestVersionSpecifier)?.version; + isVitest4OrNewer = versionToCheck ? satisfies(versionToCheck, '>=4.0.0') : true; + } // only install these dependencies if they are not already installed const basePackages = [ @@ -83,8 +93,10 @@ export class AddonVitestService { } // Check for coverage reporters - const v8Version = await packageManager.getInstalledVersion('@vitest/coverage-v8'); - const istanbulVersion = await packageManager.getInstalledVersion('@vitest/coverage-istanbul'); + const v8Version = await this.packageManager.getInstalledVersion('@vitest/coverage-v8'); + const istanbulVersion = await this.packageManager.getInstalledVersion( + '@vitest/coverage-istanbul' + ); if (!v8Version && !istanbulVersion) { dependencies.push('@vitest/coverage-v8'); @@ -112,10 +124,7 @@ export class AddonVitestService { * @param options - Installation options * @returns Array of error messages if installation fails */ - async installPlaywright( - packageManager: JsPackageManager, - options: { yes?: boolean } = {} - ): Promise<{ errors: string[] }> { + async installPlaywright(options: { yes?: boolean } = {}): Promise<{ errors: string[] }> { const errors: string[] = []; const playwrightCommand = ['playwright', 'install', 'chromium', '--with-deps']; @@ -137,7 +146,7 @@ export class AddonVitestService { if (shouldBeInstalled) { await prompt.executeTaskWithSpinner( (signal) => - packageManager.runPackageCommand({ + this.packageManager.runPackageCommand({ args: playwrightCommand, stdio: ['inherit', 'pipe', 'pipe'], signal, @@ -196,7 +205,7 @@ export class AddonVitestService { } // Check package versions - const packageVersionResult = await this.validatePackageVersions(options.packageManager); + const packageVersionResult = await this.validatePackageVersions(); if (!packageVersionResult.compatible && packageVersionResult.reasons) { reasons.push(...packageVersionResult.reasons); } @@ -216,11 +225,11 @@ export class AddonVitestService { * Validate package versions for addon-vitest compatibility Public method to allow early * validation before framework detection */ - async validatePackageVersions(packageManager: JsPackageManager): Promise { + async validatePackageVersions(): Promise { const reasons: string[] = []; // Check Vitest version (>=3.0.0 - stricter requirement from postinstall) - const vitestVersionSpecifier = await packageManager.getInstalledVersion('vitest'); + const vitestVersionSpecifier = await this.packageManager.getInstalledVersion('vitest'); const coercedVitestVersion = vitestVersionSpecifier ? coerce(vitestVersionSpecifier) : null; if (coercedVitestVersion && !satisfies(coercedVitestVersion, '>=3.0.0')) { @@ -230,7 +239,7 @@ export class AddonVitestService { } // Check MSW version (>=2.0.0 if installed) - const mswVersionSpecifier = await packageManager.getInstalledVersion('msw'); + const mswVersionSpecifier = await this.packageManager.getInstalledVersion('msw'); const coercedMswVersion = mswVersionSpecifier ? coerce(mswVersionSpecifier) : null; if (coercedMswVersion && !satisfies(coercedMswVersion, '>=2.0.0')) { diff --git a/code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts b/code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts index 2443c4424703..113a073b91f1 100644 --- a/code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts +++ b/code/lib/create-storybook/src/commands/AddonConfigurationCommand.test.ts @@ -19,7 +19,11 @@ vi.mock('../../../cli-storybook/src/postinstallAddon', () => ({ describe('AddonConfigurationCommand', () => { let command: AddonConfigurationCommand; - let mockPackageManager: JsPackageManager; + const mockPackageManager = { + type: 'npm', + getVersionedPackages: vi.fn(), + executeCommand: vi.fn().mockResolvedValue({ stdout: '', stderr: '', exitCode: 0 }), + } as Partial as JsPackageManager; let mockTask: { success: ReturnType; error: ReturnType; @@ -36,19 +40,13 @@ describe('AddonConfigurationCommand', () => { // Mock the AddonVitestService const { AddonVitestService } = await import('storybook/internal/cli'); - mockAddonVitestService = vi.mocked(AddonVitestService); + mockAddonVitestService = vi.mocked(AddonVitestService as any); const mockInstance = { installPlaywright: vi.fn().mockResolvedValue([]), }; mockAddonVitestService.mockImplementation(() => mockInstance); - command = new AddonConfigurationCommand(); - - mockPackageManager = { - type: 'npm', - getVersionedPackages: vi.fn(), - executeCommand: vi.fn().mockResolvedValue({ stdout: '', stderr: '', exitCode: 0 }), - } as Partial as JsPackageManager; + command = new AddonConfigurationCommand(mockPackageManager); mockTask = { success: vi.fn(), @@ -72,7 +70,6 @@ describe('AddonConfigurationCommand', () => { }; const result = await command.execute({ - packageManager: mockPackageManager, dependencyInstallationResult: { status: 'success' }, addons, configDir: '.storybook', @@ -93,7 +90,6 @@ describe('AddonConfigurationCommand', () => { }; const result = await command.execute({ - packageManager: mockPackageManager, dependencyInstallationResult: { status: 'success' }, addons, configDir: '.storybook', @@ -118,7 +114,6 @@ describe('AddonConfigurationCommand', () => { mockPostinstallAddon.mockRejectedValue(error); const result = await command.execute({ - packageManager: mockPackageManager, dependencyInstallationResult: { status: 'success' }, addons, configDir: '.storybook', @@ -140,7 +135,6 @@ describe('AddonConfigurationCommand', () => { }; const result = await command.execute({ - packageManager: mockPackageManager, dependencyInstallationResult: { status: 'success' }, addons, configDir: '.storybook', diff --git a/code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts b/code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts index 989ffcbf1852..09c83de32ba4 100644 --- a/code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts +++ b/code/lib/create-storybook/src/commands/AddonConfigurationCommand.ts @@ -13,7 +13,6 @@ const ADDON_INSTALLATION_INSTRUCTIONS = { } as { [key: string]: string }; type ExecuteAddonConfigurationParams = { - packageManager: JsPackageManager; addons: string[]; options: CommandOptions; configDir?: string; @@ -34,11 +33,13 @@ export type ExecuteAddonConfigurationResult = { * - Handle configuration errors gracefully */ export class AddonConfigurationCommand { - constructor(private readonly addonVitestService = new AddonVitestService()) {} + constructor( + readonly packageManager: JsPackageManager, + private readonly addonVitestService = new AddonVitestService(packageManager) + ) {} /** Execute addon configuration */ async execute({ - packageManager, options, addons, configDir, @@ -57,15 +58,10 @@ export class AddonConfigurationCommand { } try { - const { hasFailures, addonResults } = await this.configureAddons( - packageManager, - configDir, - addons, - options - ); + const { hasFailures, addonResults } = await this.configureAddons(configDir, addons, options); if (addonResults.has('@storybook/addon-vitest')) { - await this.addonVitestService.installPlaywright(packageManager, { + await this.addonVitestService.installPlaywright({ yes: options.yes, }); } @@ -106,24 +102,8 @@ export class AddonConfigurationCommand { } } - private getAddonInstructions(addons: string[]): string { - return addons - .map((addon) => { - const instructions = - ADDON_INSTALLATION_INSTRUCTIONS[addon as keyof typeof ADDON_INSTALLATION_INSTRUCTIONS]; - return instructions ? dedent`- ${addon}: ${instructions}` : null; - }) - .filter(Boolean) - .join('\n'); - } - /** Configure test addons (a11y and vitest) */ - private async configureAddons( - packageManager: JsPackageManager, - configDir: string, - addons: string[], - options: CommandOptions - ) { + private async configureAddons(configDir: string, addons: string[], options: CommandOptions) { // Import postinstallAddon from cli-storybook package const { postinstallAddon } = await import('../../../cli-storybook/src/postinstallAddon'); @@ -141,7 +121,7 @@ export class AddonConfigurationCommand { task.message(`Configuring ${addon}...`); await postinstallAddon(addon, { - packageManager: packageManager.type, + packageManager: this.packageManager.type, configDir, yes: options.yes, skipInstall: true, @@ -179,6 +159,9 @@ export class AddonConfigurationCommand { } } -export const executeAddonConfiguration = (params: ExecuteAddonConfigurationParams) => { - return new AddonConfigurationCommand().execute(params); +export const executeAddonConfiguration = ({ + packageManager, + ...options +}: ExecuteAddonConfigurationParams & { packageManager: JsPackageManager }) => { + return new AddonConfigurationCommand(packageManager).execute(options); }; diff --git a/code/lib/create-storybook/src/commands/DependencyInstallationCommand.test.ts b/code/lib/create-storybook/src/commands/DependencyInstallationCommand.test.ts index de89e07964e4..ed711e46418d 100644 --- a/code/lib/create-storybook/src/commands/DependencyInstallationCommand.test.ts +++ b/code/lib/create-storybook/src/commands/DependencyInstallationCommand.test.ts @@ -8,17 +8,15 @@ import { DependencyInstallationCommand } from './DependencyInstallationCommand'; describe('DependencyInstallationCommand', () => { let command: DependencyInstallationCommand; - let mockPackageManager: JsPackageManager; + const mockPackageManager = { + addDependencies: vi.fn().mockResolvedValue(undefined), + installDependencies: vi.fn().mockResolvedValue(undefined), + } as Partial as JsPackageManager; let dependencyCollector: DependencyCollector; beforeEach(async () => { dependencyCollector = new DependencyCollector(); - command = new DependencyInstallationCommand(dependencyCollector); - - mockPackageManager = { - addDependencies: vi.fn().mockResolvedValue(undefined), - installDependencies: vi.fn().mockResolvedValue(undefined), - } as Partial as JsPackageManager; + command = new DependencyInstallationCommand(dependencyCollector, mockPackageManager); vi.clearAllMocks(); }); @@ -28,7 +26,6 @@ describe('DependencyInstallationCommand', () => { dependencyCollector.addDevDependencies(['storybook@8.0.0']); await command.execute({ - packageManager: mockPackageManager, skipInstall: false, selectedFeatures: new Set([Feature.TEST]), }); @@ -42,7 +39,6 @@ describe('DependencyInstallationCommand', () => { it('should skip installation when skipInstall is true and no packages', async () => { await command.execute({ - packageManager: mockPackageManager, skipInstall: true, selectedFeatures: new Set([Feature.TEST]), }); @@ -55,7 +51,6 @@ describe('DependencyInstallationCommand', () => { dependencyCollector.addDevDependencies(['storybook@8.0.0']); await command.execute({ - packageManager: mockPackageManager, skipInstall: true, selectedFeatures: new Set([Feature.TEST]), }); @@ -71,7 +66,6 @@ describe('DependencyInstallationCommand', () => { dependencyCollector.addDependencies(['react@18.0.0']); await command.execute({ - packageManager: mockPackageManager, skipInstall: true, selectedFeatures: new Set([Feature.TEST]), }); @@ -90,7 +84,6 @@ describe('DependencyInstallationCommand', () => { await expect( command.execute({ - packageManager: mockPackageManager, skipInstall: false, selectedFeatures: new Set([Feature.TEST]), }) @@ -99,7 +92,6 @@ describe('DependencyInstallationCommand', () => { it('should handle empty dependency collector', async () => { await command.execute({ - packageManager: mockPackageManager, skipInstall: false, selectedFeatures: new Set([Feature.TEST]), }); @@ -110,7 +102,6 @@ describe('DependencyInstallationCommand', () => { it('should not collect test dependencies if test feature is not selected', async () => { await command.execute({ - packageManager: mockPackageManager, skipInstall: false, selectedFeatures: new Set([Feature.DOCS]), }); diff --git a/code/lib/create-storybook/src/commands/DependencyInstallationCommand.ts b/code/lib/create-storybook/src/commands/DependencyInstallationCommand.ts index a41d185184ef..36900e956080 100644 --- a/code/lib/create-storybook/src/commands/DependencyInstallationCommand.ts +++ b/code/lib/create-storybook/src/commands/DependencyInstallationCommand.ts @@ -7,7 +7,6 @@ import { Feature } from 'storybook/internal/types'; import type { DependencyCollector } from '../dependency-collector'; type DependencyInstallationCommandParams = { - packageManager: JsPackageManager; skipInstall: boolean; selectedFeatures: Set; }; @@ -23,16 +22,16 @@ type DependencyInstallationCommandParams = { */ export class DependencyInstallationCommand { constructor( - private dependencyCollector: DependencyCollector, - private addonVitestService = new AddonVitestService() + private readonly dependencyCollector: DependencyCollector, + private readonly packageManager: JsPackageManager, + private readonly addonVitestService = new AddonVitestService(packageManager) ) {} /** Execute dependency installation */ async execute({ - packageManager, skipInstall = false, selectedFeatures, }: DependencyInstallationCommandParams): Promise<{ status: 'success' | 'failed' }> { - await this.collectAddonDependencies(packageManager, selectedFeatures); + await this.collectAddonDependencies(selectedFeatures); if (!this.dependencyCollector.hasPackages() && skipInstall) { return { status: 'success' }; @@ -48,7 +47,7 @@ export class DependencyInstallationCommand { if (dependencies.length > 0) { task.message('Adding dependencies:\n' + dependencies.map((dep) => `- ${dep}`).join('\n')); - await packageManager.addDependencies( + await this.packageManager.addDependencies( { type: 'dependencies', skipInstall: true }, dependencies ); @@ -59,7 +58,7 @@ export class DependencyInstallationCommand { 'Adding devDependencies:\n' + devDependencies.map((dep) => `- ${dep}`).join('\n') ); - await packageManager.addDependencies( + await this.packageManager.addDependencies( { type: 'devDependencies', skipInstall: true }, devDependencies ); @@ -69,7 +68,7 @@ export class DependencyInstallationCommand { if (!skipInstall && this.dependencyCollector.hasPackages()) { try { - await packageManager.installDependencies(); + await this.packageManager.installDependencies(); } catch (err) { ErrorCollector.addError(err); return { status: 'failed' }; @@ -80,13 +79,10 @@ export class DependencyInstallationCommand { } /** Collect addon dependencies without installing them */ - private async collectAddonDependencies( - packageManager: JsPackageManager, - selectedFeatures: Set - ): Promise { + private async collectAddonDependencies(selectedFeatures: Set): Promise { try { if (selectedFeatures.has(Feature.TEST)) { - const vitestDeps = await this.addonVitestService.collectDependencies(packageManager); + const vitestDeps = await this.addonVitestService.collectDependencies(); this.dependencyCollector.addDevDependencies(vitestDeps); } } catch (err) { @@ -95,8 +91,11 @@ export class DependencyInstallationCommand { } } -export const executeDependencyInstallation = ( - params: DependencyInstallationCommandParams & { dependencyCollector: DependencyCollector } -) => { - return new DependencyInstallationCommand(params.dependencyCollector).execute(params); -}; +export const executeDependencyInstallation = ({ + dependencyCollector, + packageManager, + ...props +}: DependencyInstallationCommandParams & { + dependencyCollector: DependencyCollector; + packageManager: JsPackageManager; +}) => new DependencyInstallationCommand(dependencyCollector, packageManager).execute(props); diff --git a/code/lib/create-storybook/src/commands/UserPreferencesCommand.test.ts b/code/lib/create-storybook/src/commands/UserPreferencesCommand.test.ts index b02fd343841e..a7ca74bd211f 100644 --- a/code/lib/create-storybook/src/commands/UserPreferencesCommand.test.ts +++ b/code/lib/create-storybook/src/commands/UserPreferencesCommand.test.ts @@ -33,7 +33,7 @@ interface CommandWithPrivates { describe('UserPreferencesCommand', () => { let command: UserPreferencesCommand; - let mockPackageManager: JsPackageManager; + const mockPackageManager = {} as Partial as JsPackageManager; beforeEach(() => { // Provide required CommandOptions to avoid undefined access @@ -42,8 +42,7 @@ describe('UserPreferencesCommand', () => { disableTelemetry: true, }; - command = new UserPreferencesCommand(commandOptions); - mockPackageManager = {} as Partial as JsPackageManager; + command = new UserPreferencesCommand(commandOptions, mockPackageManager); // Mock globalSettings const mockSettings = { @@ -87,7 +86,7 @@ describe('UserPreferencesCommand', () => { describe('execute', () => { it('should return recommended config for new users in non-interactive mode', async () => { - const result = await command.execute(mockPackageManager, { + const result = await command.execute({ framework: null, builder: 'vite' as SupportedBuilder, projectType: ProjectType.REACT, @@ -105,7 +104,7 @@ describe('UserPreferencesCommand', () => { vi.mocked(prompt.select).mockResolvedValueOnce(true); // new user - const result = await command.execute(mockPackageManager, { + const result = await command.execute({ framework: null, builder: 'vite' as SupportedBuilder, projectType: ProjectType.REACT, @@ -128,7 +127,7 @@ describe('UserPreferencesCommand', () => { .mockResolvedValueOnce(false) // not new user .mockResolvedValueOnce('light'); // minimal install - const result = await command.execute(mockPackageManager, { + const result = await command.execute({ framework: null, builder: 'vite' as SupportedBuilder, projectType: ProjectType.REACT, @@ -147,7 +146,7 @@ describe('UserPreferencesCommand', () => { .mockResolvedValueOnce(false) // not new user .mockResolvedValueOnce('light'); // minimal install - const result = await command.execute(mockPackageManager, { + const result = await command.execute({ framework: null, builder: 'vite' as SupportedBuilder, projectType: ProjectType.REACT, @@ -167,14 +166,13 @@ describe('UserPreferencesCommand', () => { compatible: true, }); - await command.execute(mockPackageManager, { + await command.execute({ framework: null, builder: 'vite' as SupportedBuilder, projectType: ProjectType.REACT, }); expect(featureService.validateTestFeatureCompatibility).toHaveBeenCalledWith( - mockPackageManager, null, 'vite', process.cwd() @@ -192,7 +190,7 @@ describe('UserPreferencesCommand', () => { }); vi.mocked(prompt.confirm).mockResolvedValueOnce(true); // continue without test - const result = await command.execute(mockPackageManager, { + const result = await command.execute({ framework: null, builder: 'vite' as SupportedBuilder, projectType: ProjectType.REACT, diff --git a/code/lib/create-storybook/src/commands/UserPreferencesCommand.ts b/code/lib/create-storybook/src/commands/UserPreferencesCommand.ts index d718317e9335..0b5a7a72afd5 100644 --- a/code/lib/create-storybook/src/commands/UserPreferencesCommand.ts +++ b/code/lib/create-storybook/src/commands/UserPreferencesCommand.ts @@ -1,7 +1,6 @@ import type { ProjectType } from 'storybook/internal/cli'; import { globalSettings } from 'storybook/internal/cli'; -import type { JsPackageManager } from 'storybook/internal/common'; -import { isCI } from 'storybook/internal/common'; +import { type JsPackageManager, isCI } from 'storybook/internal/common'; import { logger, prompt } from 'storybook/internal/node-logger'; import type { SupportedBuilder, SupportedFramework } from 'storybook/internal/types'; import { Feature } from 'storybook/internal/types'; @@ -44,26 +43,20 @@ export interface UserPreferencesOptions { * - Track telemetry events */ export class UserPreferencesCommand { - private telemetryService: TelemetryService; - constructor( - private commandOptions: CommandOptions, - private featureService = new FeatureCompatibilityService() - ) { - this.telemetryService = new TelemetryService(commandOptions.disableTelemetry); - } + private readonly commandOptions: CommandOptions, + packageManager: JsPackageManager, + private readonly featureService = new FeatureCompatibilityService(packageManager), + private readonly telemetryService = new TelemetryService(commandOptions.disableTelemetry) + ) {} /** Execute user preferences gathering */ - async execute( - packageManager: JsPackageManager, - options: UserPreferencesOptions - ): Promise { + async execute(options: UserPreferencesOptions): Promise { // Display version information const isInteractive = process.stdout.isTTY && !isCI(); const skipPrompt = !isInteractive || !!this.commandOptions.yes; const isTestFeatureAvailable = await this.isTestFeatureAvailable( - packageManager, options.framework, options.builder ); @@ -212,12 +205,10 @@ export class UserPreferencesCommand { /** Validate test feature compatibility and prompt user if issues found */ private async isTestFeatureAvailable( - packageManager: JsPackageManager, framework: SupportedFramework | null, builder: SupportedBuilder ): Promise { const result = await this.featureService.validateTestFeatureCompatibility( - packageManager, framework, builder, process.cwd() @@ -227,9 +218,10 @@ export class UserPreferencesCommand { } } -export const executeUserPreferences = ( - packageManager: JsPackageManager, - { options, ...restOptions }: UserPreferencesOptions & { options: CommandOptions } -) => { - return new UserPreferencesCommand(options).execute(packageManager, restOptions); +export const executeUserPreferences = ({ + options, + packageManager, + ...restOptions +}: UserPreferencesOptions & { options: CommandOptions; packageManager: JsPackageManager }) => { + return new UserPreferencesCommand(options, packageManager).execute(restOptions); }; diff --git a/code/lib/create-storybook/src/initiate.ts b/code/lib/create-storybook/src/initiate.ts index 6bbc5b1145f0..c600e13c72cf 100644 --- a/code/lib/create-storybook/src/initiate.ts +++ b/code/lib/create-storybook/src/initiate.ts @@ -58,7 +58,8 @@ export async function doInitiate(options: CommandOptions): Promise< ); // Step 4: Get user preferences and feature selections (with framework/builder for validation) - const { newUser, selectedFeatures } = await executeUserPreferences(packageManager, { + const { newUser, selectedFeatures } = await executeUserPreferences({ + packageManager, options, framework, builder, diff --git a/code/lib/create-storybook/src/services/FeatureCompatibilityService.test.ts b/code/lib/create-storybook/src/services/FeatureCompatibilityService.test.ts index 31de0b70be87..664dffc2ac8d 100644 --- a/code/lib/create-storybook/src/services/FeatureCompatibilityService.test.ts +++ b/code/lib/create-storybook/src/services/FeatureCompatibilityService.test.ts @@ -18,11 +18,13 @@ vi.mock('storybook/internal/cli', async () => { describe('FeatureCompatibilityService', () => { let service: FeatureCompatibilityService; - let mockAddonVitestService: AddonVitestService; + const mockPackageManager = { + getInstalledVersion: vi.fn(), + } as Partial as JsPackageManager; + const mockAddonVitestService = new AddonVitestService(mockPackageManager); beforeEach(() => { - mockAddonVitestService = new AddonVitestService(); - service = new FeatureCompatibilityService(mockAddonVitestService); + service = new FeatureCompatibilityService(mockPackageManager, mockAddonVitestService); }); describe('supportsOnboarding', () => { @@ -42,14 +44,9 @@ describe('FeatureCompatibilityService', () => { }); describe('validateTestFeatureCompatibility', () => { - let mockPackageManager: JsPackageManager; let mockValidateCompatibility: ReturnType; beforeEach(() => { - mockPackageManager = { - getInstalledVersion: vi.fn(), - } as Partial as JsPackageManager; - // Get the mocked validateCompatibility method mockValidateCompatibility = vi.mocked(mockAddonVitestService.validateCompatibility); }); @@ -58,7 +55,6 @@ describe('FeatureCompatibilityService', () => { mockValidateCompatibility.mockResolvedValue({ compatible: true }); const result = await service.validateTestFeatureCompatibility( - mockPackageManager, SupportedFramework.REACT_VITE, SupportedBuilder.VITE, '/test' @@ -66,7 +62,6 @@ describe('FeatureCompatibilityService', () => { expect(result.compatible).toBe(true); expect(mockValidateCompatibility).toHaveBeenCalledWith({ - packageManager: mockPackageManager, framework: 'react-vite', builder: SupportedBuilder.VITE, projectRoot: '/test', @@ -80,7 +75,6 @@ describe('FeatureCompatibilityService', () => { }); const result = await service.validateTestFeatureCompatibility( - mockPackageManager, SupportedFramework.REACT_VITE, SupportedBuilder.VITE, '/test' diff --git a/code/lib/create-storybook/src/services/FeatureCompatibilityService.ts b/code/lib/create-storybook/src/services/FeatureCompatibilityService.ts index 3cdb5545e148..b8c52c0cbba8 100644 --- a/code/lib/create-storybook/src/services/FeatureCompatibilityService.ts +++ b/code/lib/create-storybook/src/services/FeatureCompatibilityService.ts @@ -20,7 +20,10 @@ export interface FeatureCompatibilityResult { /** Service for validating feature compatibility with project configurations */ export class FeatureCompatibilityService { - constructor(private readonly addonVitestService = new AddonVitestService()) {} + constructor( + readonly packageManager: JsPackageManager, + private readonly addonVitestService = new AddonVitestService(packageManager) + ) {} /** Check if a project type supports onboarding */ @@ -40,13 +43,11 @@ export class FeatureCompatibilityService { * @returns Compatibility result with reasons if incompatible */ async validateTestFeatureCompatibility( - packageManager: JsPackageManager, framework: SupportedFramework | null | undefined, builder: SupportedBuilder, directory: string ): Promise { const compatibilityResult = await this.addonVitestService.validateCompatibility({ - packageManager, framework, builder, projectRoot: directory,