From c4230889b13058e8883b1d8e6974b02142d2eac7 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 25 Apr 2025 13:39:54 +0200 Subject: [PATCH 1/7] WIP --- .../fixes/addon-globals-api.test.ts | 690 ++++++++++++++++++ .../automigrate/fixes/addon-globals-api.ts | 300 ++++++++ .../src/automigrate/fixes/index.ts | 2 + 3 files changed, 992 insertions(+) create mode 100644 code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts create mode 100644 code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts new file mode 100644 index 000000000000..473a3ea653d9 --- /dev/null +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -0,0 +1,690 @@ +import * as fsp from 'node:fs/promises'; +import { join } from 'node:path'; + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import * as common from 'storybook/internal/common'; + +// Import common to mock +import dedent from 'ts-dedent'; + +import type { FixResult } from '../types'; +// Import FixResult type +import { addonGlobalsApi } from './addon-globals-api'; + +// Mock fs/promises and common utilities +vi.mock('node:fs/promises', async () => import('../../../../../__mocks__/fs/promises')); +vi.mock('storybook/internal/common', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + scanAndTransformFiles: vi.fn().mockResolvedValue([]), // Mock scanAndTransformFiles + }; +}); + +const previewConfigPath = join('.storybook', 'preview.js'); + +const check = async (previewContents: string) => { + vi.mocked(fsp as any).__setMockFiles({ + [previewConfigPath]: previewContents, + }); + return addonGlobalsApi.check({ + packageManager: {} as any, + configDir: '', + mainConfig: {} as any, + storybookVersion: '9.0.0', // Assume v9 for testing migrations + previewConfigPath, + }); +}; + +// Helper to run the migration for preview file and capture scanAndTransformFiles args +const runMigrationAndGetTransformFn = async (previewContents: string) => { + const result = await check(previewContents); + const mockWriteFile = vi.mocked(fsp.writeFile); + const mockScanAndTransform = vi.mocked(common.scanAndTransformFiles); + + let transformFn: ((filePath: string, content: string) => Promise) | undefined; + let transformOptions: any; + + if (result) { + await addonGlobalsApi.run?.({ + result, + dryRun: false, + packageManager: {} as any, // Add necessary mock properties + } as any); + + if (mockScanAndTransform.mock.calls.length > 0) { + transformFn = mockScanAndTransform.mock.calls[0][0].transformFn; + // Extract options passed to transformStoryFile from the closure + // This is a bit indirect, relying on the implementation detail + transformOptions = { + needsViewportMigration: result.needsViewportMigration, + needsBackgroundsMigration: result.needsBackgroundsMigration, + backgroundValues: result.backgroundsOptions?.values, + }; + } + } + + return { + previewFileContent: mockWriteFile.mock.calls[0]?.[1] as string | undefined, + transformFn, + transformOptions, + migrationResult: result, + }; +}; + +describe('addon-globals-api', () => { + afterEach(() => { + vi.clearAllMocks(); + vi.mocked(common.scanAndTransformFiles).mockClear(); + }); + + describe('check', () => { + it('should return null for empty config', async () => { + await expect(check(`export default { parameters: {} }`)).resolves.toBeFalsy(); + }); + + it('should detect viewport configuration', async () => { + const result = await check(` + export default { + parameters: { + viewport: { + viewports: { + mobile: { name: 'Mobile', width: '320px', height: '568px' }, + tablet: { name: 'Tablet', width: '768px', height: '1024px' } + }, + defaultViewport: 'mobile' + } + } + } + `); + + expect(result).toBeTruthy(); + expect(result?.needsViewportMigration).toBe(true); + expect(result?.needsBackgroundsMigration).toBe(false); + expect(result?.viewportsOptions?.defaultViewport).toBe('mobile'); + }); + + it('should detect backgrounds configuration', async () => { + const result = await check(` + export default { + parameters: { + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#333333' } + ], + default: 'Light', + disable: false + } + } + } + `); + + expect(result).toBeTruthy(); + expect(result?.needsViewportMigration).toBe(false); + expect(result?.needsBackgroundsMigration).toBe(true); + expect(result?.backgroundsOptions?.default).toBe('Light'); + }); + + it('should detect both viewport and backgrounds configuration', async () => { + const result = await check(` + export default { + parameters: { + viewport: { + viewports: { + mobile: { name: 'Mobile', width: '320px', height: '568px' }, + tablet: { name: 'Tablet', width: '768px', height: '1024px' } + }, + defaultViewport: 'tablet' + }, + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#333333' } + ], + default: 'Dark' + } + } + } + `); + + expect(result).toBeTruthy(); + expect(result?.needsViewportMigration).toBe(true); + expect(result?.needsBackgroundsMigration).toBe(true); + expect(result?.viewportsOptions?.defaultViewport).toBe('tablet'); + expect(result?.backgroundsOptions?.default).toBe('Dark'); + }); + + it('should detect both viewport and backgrounds configuration with dynamic values', async () => { + const result = await check(` + import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; + + const backgroundValues = [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#333333' } + ]; + + export default { + parameters: { + viewport: { + viewports: INITIAL_VIEWPORTS, + defaultViewport: 'tablet' + }, + backgrounds: { + values: backgroundValues, + default: 'Dark' + } + } + } + `); + + expect(result).toBeTruthy(); + expect(result?.needsViewportMigration).toBe(true); + expect(result?.needsBackgroundsMigration).toBe(true); + expect(result?.viewportsOptions?.defaultViewport).toBe('tablet'); + expect(result?.backgroundsOptions?.default).toBe('Dark'); + }); + + it('should not detect configurations already using globals API', async () => { + const result = await check(` + export default { + parameters: { + viewport: { + options: { + mobile: { name: 'Mobile', width: '320px', height: '568px' }, + tablet: { name: 'Tablet', width: '768px', height: '1024px' } + } + }, + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' }, + dark: { name: 'Dark', value: '#333333' } + } + } + }, + initialGlobals: { + viewport: { value: 'mobile', isRotated: false }, + backgrounds: { value: 'dark' } + } + } + `); + + // Since there's no defaultViewport or default properties, it should return null (nothing to migrate) + expect(result?.needsViewportMigration).toBeFalsy(); + expect(result?.needsBackgroundsMigration).toBeFalsy(); + }); + }); + + describe('run - preview file', () => { + it('should migrate viewport configuration correctly', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(` + export default { + parameters: { + viewport: { + viewports: { + mobile: { name: 'Mobile', width: '320px', height: '568px' }, + tablet: { name: 'Tablet', width: '768px', height: '1024px' } + }, + defaultViewport: 'mobile' + } + } + } + `); + + expect(previewFileContent).toMatchInlineSnapshot(` + " + export default { + parameters: { + viewport: { + options: { + mobile: { name: 'Mobile', width: '320px', height: '568px' }, + tablet: { name: 'Tablet', width: '768px', height: '1024px' } + } + } + }, + + initialGlobals: { + viewport: { + value: 'mobile', + isRotated: false + } + } + }; + " + `); + }); + + it('should migrate backgrounds configuration correctly', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(` + export default { + parameters: { + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#333333' } + ], + default: 'Light' + } + } + } + `); + + expect(previewFileContent).toMatchInlineSnapshot(` + " + export default { + parameters: { + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' }, + dark: { name: 'Dark', value: '#333333' } + } + } + }, + + initialGlobals: { + backgrounds: { + value: 'light' + } + } + }; + " + `); + }); + + it('should rename backgrounds disable property to disabled', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(` + export default { + parameters: { + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' } + ], + disable: true + } + } + } + `); + + // Verify the transformation results + expect(previewFileContent).toMatchInlineSnapshot(` + " + export default { + parameters: { + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' } + }, + disabled: true + } + } + } + " + `); + }); + + it('should migrate both viewport and backgrounds configurations', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(` + export default { + parameters: { + viewport: { + viewports: { + mobile: { name: 'Mobile', width: '320px', height: '568px' } + }, + defaultViewport: 'mobile' + }, + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#F8F8F8' } + ], + default: 'Light' + } + } + } + `); + + expect(previewFileContent).toMatchInlineSnapshot(` + " + export default { + parameters: { + viewport: { + options: { + mobile: { name: 'Mobile', width: '320px', height: '568px' } + } + }, + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' }, + dark: { name: 'Dark', value: '#F8F8F8' } + } + } + }, + + initialGlobals: { + viewport: { + value: 'mobile', + isRotated: false + }, + + backgrounds: { + value: 'light' + } + } + }; + " + `); + }); + + it('should correctly handle partial configurations', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` + export default { + parameters: { + viewport: { + viewports: { + mobile: { name: 'Mobile', width: '320px', height: '568px' } + } + }, + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' } + ] + } + } + } + `); + + // Verify the transformation results + expect(previewFileContent).toMatchInlineSnapshot(dedent` + "export default { + parameters: { + viewport: { + options: { + mobile: { name: 'Mobile', width: '320px', height: '568px' } + } + }, + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' } + } + } + } + }" + `); + }); + + it('should migrate dynamic values correctly', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` + import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; + + export default { + parameters: { + viewport: { + viewports: INITIAL_VIEWPORTS, + defaultViewport: 'tablet' + }, + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#333333' } + ], + default: 'Light' + } + } + } + `); + + expect(previewFileContent).toMatchInlineSnapshot(` + "import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; + + export default { + parameters: { + viewport: { + options: INITIAL_VIEWPORTS + }, + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' }, + dark: { name: 'Dark', value: '#333333' } + } + } + }, + + initialGlobals: { + viewport: { + value: 'tablet', + isRotated: false + }, + + backgrounds: { + value: 'light' + } + } + };" + `); + }); + }); + + describe('run - story files', () => { + const defaultPreview = dedent` + export default { + parameters: { + viewport: { + viewports: { mobile: {}, tablet: {} }, + defaultViewport: 'mobile' + }, + backgrounds: { + values: [ + { name: 'Light', value: '#F8F8F8' }, + { name: 'Dark', value: '#333333' }, + { name: 'Tweet', value: '#00aced' } + ], + default: 'Light', + disable: false + } + } + } + `; + + it('should migrate parameters.backgrounds.default to globals.backgrounds', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const Default = { + parameters: { + backgrounds: { default: 'Dark' } + } + }; + `; + + const expectedContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const Default = { + globals: { + backgrounds: { value: "dark" } + } + }; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + + it('should migrate parameters.backgrounds.disable: true to disabled: true', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const Disabled = { + parameters: { + backgrounds: { disable: true } + } + }; + `; + const expectedContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const Disabled = { + parameters: { + backgrounds: { disabled: true } + } + }; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + + it('should remove parameters.backgrounds.disable: false', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const Disabled = { + parameters: { + backgrounds: { disable: false } // This should be removed + } + }; + `; + // parameters.backgrounds becomes empty and should be removed + const expectedContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const Disabled = {}; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + + it('should migrate parameters.viewport.defaultViewport to globals.viewport', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const MobileOnly = { + parameters: { + viewport: { defaultViewport: 'mobile' } + } + }; + `; + const expectedContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const MobileOnly = { + globals: { + viewport: { + value: "mobile", + isRotated: false + } + } + }; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + + it('should migrate both viewport and backgrounds in the same story', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const DarkMobile = { + parameters: { + viewport: { defaultViewport: 'mobile' }, + backgrounds: { default: 'Dark' } + } + }; + `; + const expectedContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const DarkMobile = { + globals: { + viewport: { + value: "mobile", + isRotated: false + }, + backgrounds: { value: "dark" } + } + }; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + + it('should handle migration in meta (export default)', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { + component: Button, + parameters: { + backgrounds: { default: 'Tweet' } + } + }; + export const Default = {}; + `; + const expectedContent = dedent` + import Button from './Button'; + export default { + component: Button, + globals: { + backgrounds: { value: "tweet" } + } + }; + export const Default = {}; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + + it('should return null if no changes are needed', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const NoParams = {}; + export const ExistingGlobals = { globals: { backgrounds: { value: 'dark' } } }; + export const ExistingDisabled = { parameters: { backgrounds: { disabled: true } } }; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBeNull(); + }); + + it('should remove empty parameters/backgrounds/viewport objects after migration', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const TestStory = { + parameters: { + otherParam: true, + backgrounds: { default: 'Dark' }, // This will move + viewport: { defaultViewport: 'tablet' } // This will move + } + }; + `; + // parameters.backgrounds and parameters.viewport become empty and are removed + // parameters still has otherParam, so it remains + const expectedContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const TestStory = { + parameters: { + otherParam: true + }, + + globals: { + backgrounds: { value: "dark" }, + viewport: { + value: "tablet", + isRotated: false + } + } + }; + `; + expect(transformFn).toBeDefined(); + await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + }); + }); +}); diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts new file mode 100644 index 000000000000..422519f079b7 --- /dev/null +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts @@ -0,0 +1,300 @@ +import { readFile, writeFile } from 'node:fs/promises'; + +import { types as t } from 'storybook/internal/babel'; +import type { ConfigFile } from 'storybook/internal/csf-tools'; +import { formatConfig, loadConfig } from 'storybook/internal/csf-tools'; + +import type { ArrayExpression, Expression, ObjectExpression } from '@babel/types'; +import picocolors from 'picocolors'; +import { dedent } from 'ts-dedent'; + +import type { Fix } from '../types'; + +const MIGRATION = + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#viewportbackgrounds-addon-synchronized-configuration-and-globals-usage'; + +interface AddonGlobalsApiOptions { + previewConfig: ConfigFile; + previewConfigPath: string; + needsViewportMigration: boolean; + needsBackgroundsMigration: boolean; + viewportsOptions: + | { + defaultViewport?: string; + viewports?: Expression; + } + | undefined; + backgroundsOptions: + | { + default?: string; + values?: Expression; + disable?: boolean; + } + | undefined; +} + +/** + * Migrate viewport and backgrounds addons to use the new globals API in Storybook 9 + * + * - Migrate viewports to use options and initialGlobals + * - Migrate backgrounds to use options and initialGlobals + */ +export const addonGlobalsApi: Fix = { + id: 'addon-globals-api', + versionRange: ['<9.0.0', '^9.0.0-0 || ^9.0.0'], + + async check({ previewConfigPath }) { + if (!previewConfigPath) { + return null; + } + + const previewConfig = loadConfig((await readFile(previewConfigPath)).toString()).parse(); + + const getFieldNode = previewConfig.getFieldNode.bind(previewConfig); + const getFieldValue = previewConfig.getFieldValue.bind(previewConfig); + + // Reusable function to check addon migration status + const checkAddonMigration = (addonName: 'viewport' | 'backgrounds') => { + const paramPath = ['parameters', addonName]; + const addonParams = getFieldNode(paramPath) as ObjectExpression | undefined; + + if (!addonParams) { + return { needsMigration: false }; + } + + const hasOptions = getFieldNode([...paramPath, 'options']) !== undefined; + + // Define fields to check based on addon type + const fieldsToCheck = + addonName === 'viewport' + ? ['viewports', 'defaultViewport'] + : ['values', 'default', 'disable']; + + // Check if any old format fields exist + const hasOldFormat = fieldsToCheck.some( + (field) => getFieldNode([...paramPath, field]) !== undefined + ); + + // Only migrate if using old format and not already migrated + const needsMigration = hasOldFormat && !hasOptions; + + // Collect relevant options from old format + const options: Record = {}; + + if (needsMigration) { + fieldsToCheck.forEach((field) => { + const value = + (addonName === 'viewport' && field === 'viewports') || + (addonName === 'backgrounds' && field === 'values') + ? getFieldNode([...paramPath, field]) + : getFieldValue([...paramPath, field]); + + if (value !== undefined) { + // Convert field names if necessary (maintaining the expected output structure) + const optionKey = addonName === 'viewport' ? field : field; + options[optionKey] = value; + } + }); + } + + return { needsMigration, options }; + }; + + // Check migration status for both addons + const viewportMigration = checkAddonMigration('viewport'); + const backgroundsMigration = checkAddonMigration('backgrounds'); + + // Return null if there's nothing to migrate + if (!viewportMigration.needsMigration && !backgroundsMigration.needsMigration) { + return null; + } + + return { + previewConfig, + previewConfigPath, + needsViewportMigration: viewportMigration.needsMigration, + needsBackgroundsMigration: backgroundsMigration.needsMigration, + viewportsOptions: viewportMigration.options, + backgroundsOptions: backgroundsMigration.options, + }; + }, + + prompt({ needsViewportMigration, needsBackgroundsMigration }) { + return dedent` + We've detected that you're using the ${needsViewportMigration && needsBackgroundsMigration ? 'viewport and backgrounds addons' : needsViewportMigration ? 'viewport addon' : 'backgrounds addon'} with the deprecated configuration API. + + In Storybook 9, ${needsViewportMigration && needsBackgroundsMigration ? 'these addons have' : 'this addon has'} been updated to use the new globals API which ensures a consistent experience while navigating between stories. + + We'll update your configuration to use the new API: + ${needsViewportMigration ? `- ${picocolors.yellow('viewports')} → ${picocolors.yellow('options')} and ${picocolors.yellow('defaultViewport')} → ${picocolors.yellow('initialGlobals.viewport')}\n` : ''}${needsBackgroundsMigration ? `- ${picocolors.yellow('values')} → ${picocolors.yellow('options')} and ${picocolors.yellow('default')} → ${picocolors.yellow('initialGlobals.backgrounds')}\n` : ''} + + Learn more: ${picocolors.cyan(MIGRATION)} + `; + }, + + async run({ dryRun, result }) { + const { + previewConfig, + needsViewportMigration, + needsBackgroundsMigration, + viewportsOptions, + backgroundsOptions, + } = result; + + const getFieldNode = previewConfig.getFieldNode.bind(previewConfig); + + if (needsViewportMigration) { + // Get the viewport parameter object + const viewports = getFieldNode(['parameters', 'viewport', 'viewports']) as ObjectExpression; + + if (viewportsOptions?.viewports) { + // Remove the old viewports property + previewConfig.removeField(['parameters', 'viewport', 'viewports']); + addProperty( + getFieldNode(['parameters', 'viewport']) as ObjectExpression, + 'options', + viewports + ); + } + + // If defaultViewport exists, create initialGlobals.viewport + if (viewportsOptions?.defaultViewport) { + // Remove the old defaultViewport property + const viewportNode = getFieldNode(['parameters', 'viewport']); + removeProperty(viewportNode as ObjectExpression, 'defaultViewport'); + + previewConfig.setFieldValue( + ['initialGlobals', 'viewport', 'value'], + viewportsOptions.defaultViewport + ); + previewConfig.setFieldValue(['initialGlobals', 'viewport', 'isRotated'], false); + } + } + + if (needsBackgroundsMigration) { + if (backgroundsOptions?.values) { + // Transform values array to options object + const optionsObject = transformValuesToOptions( + backgroundsOptions.values as ArrayExpression + ); + + // Remove the old values property + previewConfig.removeField(['parameters', 'backgrounds', 'values']); + addProperty( + getFieldNode(['parameters', 'backgrounds']) as ObjectExpression, + 'options', + optionsObject + ); + } + + // If default exists, create initialGlobals.backgrounds + if (backgroundsOptions?.default) { + // Remove the old default property + removeProperty(getFieldNode(['parameters', 'backgrounds']) as ObjectExpression, 'default'); + + previewConfig.setFieldValue( + ['initialGlobals', 'backgrounds', 'value'], + getKeyFromName(backgroundsOptions.values as ArrayExpression, backgroundsOptions.default) + ); + } + + // If disable exists, rename to disabled + if (backgroundsOptions?.disable === true) { + // Remove the old disable property + removeProperty(getFieldNode(['parameters', 'backgrounds']) as ObjectExpression, 'disable'); + + addProperty( + getFieldNode(['parameters', 'backgrounds']) as ObjectExpression, + 'disabled', + t.booleanLiteral(true) + ); + } + } + + // Write the updated config back to the file + if (!dryRun) { + await writeFile(result.previewConfigPath, formatConfig(previewConfig)); + } + }, +}; + +// Helper functions + +function getObjectProperty(obj: ObjectExpression, propertyName: string): Expression | undefined { + if (!obj || !obj.properties) { + return undefined; + } + + const property = obj.properties.find( + (prop) => + t.isObjectProperty(prop) && + ((t.isIdentifier(prop.key) && prop.key.name === propertyName) || + (t.isStringLiteral(prop.key) && prop.key.value === propertyName)) + ) as t.ObjectProperty; + + return property?.value as Expression; +} + +function removeProperty(obj: ObjectExpression, propertyName: string): void { + if (!obj || !obj.properties) { + return; + } + + const index = obj.properties.findIndex( + (prop) => + t.isObjectProperty(prop) && + ((t.isIdentifier(prop.key) && prop.key.name === propertyName) || + (t.isStringLiteral(prop.key) && prop.key.value === propertyName)) + ); + + if (index !== -1) { + obj.properties.splice(index, 1); + } +} + +function addProperty(obj: ObjectExpression, propertyName: string, value: Expression): void { + if (!obj) { + return; + } + + obj.properties.push(t.objectProperty(t.identifier(propertyName), value)); +} + +function transformValuesToOptions(valuesArray: ArrayExpression): Expression { + // Transform [{ name: 'Light', value: '#FFF' }] to { light: { name: 'Light', value: '#FFF' } } + const optionsObject = t.objectExpression([]); + + if (valuesArray && t.isArrayExpression(valuesArray) && valuesArray.elements) { + valuesArray.elements.forEach((element) => { + if (t.isObjectExpression(element)) { + const nameProperty = getObjectProperty(element, 'name'); + + if (t.isStringLiteral(nameProperty)) { + const key = nameProperty.value.toLowerCase().replace(/\s+/g, '_'); + + optionsObject.properties.push(t.objectProperty(t.identifier(key), element)); + } + } + }); + } + + return optionsObject; +} + +function getKeyFromName(valuesArray: ArrayExpression, name: string): string { + // Generate a key from a name in the values array + if (valuesArray && t.isArrayExpression(valuesArray) && valuesArray.elements) { + for (const element of valuesArray.elements) { + if (t.isObjectExpression(element)) { + const nameProperty = getObjectProperty(element, 'name'); + + if (t.isStringLiteral(nameProperty) && nameProperty.value === name) { + return name.toLowerCase().replace(/\s+/g, '_'); + } + } + } + } + + // If not found, generate a key from the name + return name.toLowerCase().replace(/\s+/g, '_'); +} diff --git a/code/lib/cli-storybook/src/automigrate/fixes/index.ts b/code/lib/cli-storybook/src/automigrate/fixes/index.ts index 66278a0cb2e1..8a577751d0f9 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/index.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/index.ts @@ -4,6 +4,7 @@ import { addonA11yAddonTest } from './addon-a11y-addon-test'; import { addonA11yParameters } from './addon-a11y-parameters'; import { addonEssentialsRemoveDocs } from './addon-essentials-remove-docs'; import { addonExperimentalTest } from './addon-experimental-test'; +import { addonGlobalsApi } from './addon-globals-api'; import { addonMdxGfmRemove } from './addon-mdx-gfm-remove'; import { addonStorysourceRemove } from './addon-storysource-remove'; import { consolidatedImports } from './consolidated-imports'; @@ -24,6 +25,7 @@ export const allFixes: Fix[] = [ addonStorysourceRemove, upgradeStorybookRelatedDependencies, initialGlobals, + addonGlobalsApi, addonA11yAddonTest, consolidatedImports, addonExperimentalTest, From 541e5166b397f0cf1f33b3c84800f92cd7c5d05c Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 29 Sep 2025 13:59:10 +0200 Subject: [PATCH 2/7] Finalize viewport/backgrounds automigration --- .../fixes/addon-globals-api.test.ts | 363 +++++++++++------- .../automigrate/fixes/addon-globals-api.ts | 305 +++++++++++---- .../helpers/addon-a11y-parameters.ts | 50 +-- .../src/automigrate/helpers/ast-utils.ts | 199 ++++++++++ 4 files changed, 665 insertions(+), 252 deletions(-) create mode 100644 code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts index 473a3ea653d9..f32fea4cacd2 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -8,9 +8,8 @@ import * as common from 'storybook/internal/common'; // Import common to mock import dedent from 'ts-dedent'; -import type { FixResult } from '../types'; // Import FixResult type -import { addonGlobalsApi } from './addon-globals-api'; +import { addonGlobalsApi, transformStoryFileSync } from './addon-globals-api'; // Mock fs/promises and common utilities vi.mock('node:fs/promises', async () => import('../../../../../__mocks__/fs/promises')); @@ -34,6 +33,7 @@ const check = async (previewContents: string) => { mainConfig: {} as any, storybookVersion: '9.0.0', // Assume v9 for testing migrations previewConfigPath, + storiesPaths: [], }); }; @@ -43,7 +43,8 @@ const runMigrationAndGetTransformFn = async (previewContents: string) => { const mockWriteFile = vi.mocked(fsp.writeFile); const mockScanAndTransform = vi.mocked(common.scanAndTransformFiles); - let transformFn: ((filePath: string, content: string) => Promise) | undefined; + let transformFn: (filePath: string, content: string) => string | null = () => null; + let transformOptions: any; if (result) { @@ -53,8 +54,16 @@ const runMigrationAndGetTransformFn = async (previewContents: string) => { packageManager: {} as any, // Add necessary mock properties } as any); - if (mockScanAndTransform.mock.calls.length > 0) { - transformFn = mockScanAndTransform.mock.calls[0][0].transformFn; + if (result) { + // Create a transform function that uses the sync version + transformFn = (filePath: string, content: string) => { + return transformStoryFileSync(content, { + needsViewportMigration: result.needsViewportMigration, + needsBackgroundsMigration: result.needsBackgroundsMigration, + viewportsOptions: result.viewportsOptions, + backgroundsOptions: result.backgroundsOptions, + }); + }; // Extract options passed to transformStoryFile from the closure // This is a bit indirect, relying on the implementation detail transformOptions = { @@ -489,153 +498,177 @@ describe('addon-globals-api', () => { it('should migrate parameters.backgrounds.default to globals.backgrounds', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const Default = { - parameters: { - backgrounds: { default: 'Dark' } - } - }; - `; + import Button from './Button'; + export default { component: Button }; + export const Default = { + parameters: { + backgrounds: { default: 'Dark' } + } + }; + `; const expectedContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const Default = { - globals: { - backgrounds: { value: "dark" } - } - }; - `; + import Button from './Button'; + export default { + component: Button + }; + export const Default = { + globals: { + backgrounds: { + value: "dark" + } + } + }; + `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); it('should migrate parameters.backgrounds.disable: true to disabled: true', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const Disabled = { - parameters: { - backgrounds: { disable: true } - } - }; - `; + import Button from './Button'; + export default { component: Button }; + export const Disabled = { + parameters: { + backgrounds: { disable: true } + } + }; + `; const expectedContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const Disabled = { - parameters: { - backgrounds: { disabled: true } - } - }; - `; + import Button from './Button'; + export default { + component: Button + }; + export const Disabled = { + parameters: { + backgrounds: { + disabled: true + } + } + }; + `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); - it('should remove parameters.backgrounds.disable: false', async () => { + it('should rename parameters.backgrounds.disable: false to disabled: false', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const Disabled = { - parameters: { - backgrounds: { disable: false } // This should be removed - } - }; - `; - // parameters.backgrounds becomes empty and should be removed + import Button from './Button'; + export default { component: Button }; + export const Disabled = { + parameters: { + backgrounds: { disable: false } + } + }; + `; + // disable should be renamed to disabled const expectedContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const Disabled = {}; - `; + import Button from './Button'; + export default { + component: Button + }; + export const Disabled = { + parameters: { + backgrounds: { + disabled: false + } + } + }; + `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); it('should migrate parameters.viewport.defaultViewport to globals.viewport', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const MobileOnly = { - parameters: { - viewport: { defaultViewport: 'mobile' } - } - }; - `; + import Button from './Button'; + export default { component: Button }; + export const MobileOnly = { + parameters: { + viewport: { defaultViewport: 'mobile' } + } + }; + `; const expectedContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const MobileOnly = { - globals: { - viewport: { - value: "mobile", - isRotated: false + import Button from './Button'; + export default { + component: Button + }; + export const MobileOnly = { + globals: { + viewport: { + value: "mobile", + isRotated: false + } } - } - }; - `; + }; + `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); it('should migrate both viewport and backgrounds in the same story', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const DarkMobile = { - parameters: { - viewport: { defaultViewport: 'mobile' }, - backgrounds: { default: 'Dark' } - } - }; - `; + import Button from './Button'; + export default { component: Button }; + export const DarkMobile = { + parameters: { + viewport: { defaultViewport: 'mobile' }, + backgrounds: { default: 'Dark' } + } + }; + `; const expectedContent = dedent` - import Button from './Button'; - export default { component: Button }; - export const DarkMobile = { - globals: { - viewport: { - value: "mobile", - isRotated: false - }, - backgrounds: { value: "dark" } - } - }; - `; + import Button from './Button'; + export default { + component: Button + }; + export const DarkMobile = { + globals: { + viewport: { + value: "mobile", + isRotated: false + }, + backgrounds: { + value: "dark" + } + } + }; + `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); it('should handle migration in meta (export default)', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` - import Button from './Button'; - export default { - component: Button, - parameters: { - backgrounds: { default: 'Tweet' } - } - }; - export const Default = {}; - `; + import Button from './Button'; + export default { + component: Button, + parameters: { + backgrounds: { default: 'Tweet' } + } + }; + export const Default = {}; + `; const expectedContent = dedent` - import Button from './Button'; - export default { - component: Button, - globals: { - backgrounds: { value: "tweet" } - } - }; - export const Default = {}; - `; + import Button from './Button'; + export default { + component: Button, + globals: { + backgrounds: { + value: "tweet" + } + } + }; + export const Default = {}; + `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); it('should return null if no changes are needed', async () => { @@ -648,43 +681,105 @@ describe('addon-globals-api', () => { export const ExistingDisabled = { parameters: { backgrounds: { disabled: true } } }; `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBeNull(); + expect(transformFn!('story.js', storyContent)).toBeNull(); }); - it('should remove empty parameters/backgrounds/viewport objects after migration', async () => { + it('should migrate parameters even when other stories have existing globals', async () => { const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); const storyContent = dedent` import Button from './Button'; export default { component: Button }; - export const TestStory = { - parameters: { - otherParam: true, - backgrounds: { default: 'Dark' }, // This will move - viewport: { defaultViewport: 'tablet' } // This will move + export const ExistingGlobals = { globals: { backgrounds: { value: 'dark' } } }; + export const NeedsMigration = { parameters: { backgrounds: { default: 'Dark' } } }; + `; + const expectedContent = dedent` + import Button from './Button'; + export default { + component: Button + }; + export const ExistingGlobals = { + globals: { + backgrounds: { + value: 'dark' + } + } + }; + export const NeedsMigration = { + globals: { + backgrounds: { + value: "dark" + } } }; `; - // parameters.backgrounds and parameters.viewport become empty and are removed - // parameters still has otherParam, so it remains - const expectedContent = dedent` + expect(transformFn).toBeDefined(); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); + }); + + it('should merge new globals with existing globals in the same story', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` import Button from './Button'; export default { component: Button }; - export const TestStory = { - parameters: { - otherParam: true - }, - + export const ExistingAndNeedsMigration = { + globals: { backgrounds: { value: 'light' } }, + parameters: { backgrounds: { default: 'Dark' } } + }; + `; + const expectedContent = dedent` + import Button from './Button'; + export default { + component: Button + }; + export const ExistingAndNeedsMigration = { globals: { - backgrounds: { value: "dark" }, - viewport: { - value: "tablet", - isRotated: false + backgrounds: { + value: 'light' } } }; `; expect(transformFn).toBeDefined(); - await expect(transformFn!('story.js', storyContent)).resolves.toBe(expectedContent); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); + }); + + it('should remove empty parameters/backgrounds/viewport objects after migration', async () => { + const { transformFn } = await runMigrationAndGetTransformFn(defaultPreview); + const storyContent = dedent` + import Button from './Button'; + export default { component: Button }; + export const TestStory = { + parameters: { + otherParam: true, + backgrounds: { default: 'Dark' }, // This will move + viewport: { defaultViewport: 'tablet' } // This will move + } + }; + `; + // parameters.backgrounds and parameters.viewport become empty and are removed + // parameters still has otherParam, so it remains + const expectedContent = dedent` + import Button from './Button'; + export default { + component: Button + }; + export const TestStory = { + parameters: { + otherParam: true + }, + globals: { + viewport: { + value: "tablet", + isRotated: false + }, + backgrounds: { + value: "dark" + } + } + }; + `; + expect(transformFn).toBeDefined(); + expect(transformFn!('story.js', storyContent)).toBe(expectedContent); }); }); }); diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts index 422519f079b7..cf41af3b89de 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts @@ -1,18 +1,28 @@ import { readFile, writeFile } from 'node:fs/promises'; import { types as t } from 'storybook/internal/babel'; -import type { ConfigFile } from 'storybook/internal/csf-tools'; -import { formatConfig, loadConfig } from 'storybook/internal/csf-tools'; +import { scanAndTransformFiles } from 'storybook/internal/common'; +import type { ConfigFile, CsfFile } from 'storybook/internal/csf-tools'; +import { + formatConfig, + formatCsf, + loadConfig, + loadCsf, + writeCsf, +} from 'storybook/internal/csf-tools'; import type { ArrayExpression, Expression, ObjectExpression } from '@babel/types'; -import picocolors from 'picocolors'; -import { dedent } from 'ts-dedent'; +import { + addProperty, + getKeyFromName, + getObjectProperty, + removeProperty, + transformStoryParameters, + transformValuesToOptions, +} from '../helpers/ast-utils'; import type { Fix } from '../types'; -const MIGRATION = - 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#viewportbackgrounds-addon-synchronized-configuration-and-globals-usage'; - interface AddonGlobalsApiOptions { previewConfig: ConfigFile; previewConfigPath: string; @@ -41,7 +51,7 @@ interface AddonGlobalsApiOptions { */ export const addonGlobalsApi: Fix = { id: 'addon-globals-api', - versionRange: ['<9.0.0', '^9.0.0-0 || ^9.0.0'], + link: 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#viewportbackgrounds-addon-synchronized-configuration-and-globals-usage', async check({ previewConfigPath }) { if (!previewConfigPath) { @@ -119,20 +129,11 @@ export const addonGlobalsApi: Fix = { }; }, - prompt({ needsViewportMigration, needsBackgroundsMigration }) { - return dedent` - We've detected that you're using the ${needsViewportMigration && needsBackgroundsMigration ? 'viewport and backgrounds addons' : needsViewportMigration ? 'viewport addon' : 'backgrounds addon'} with the deprecated configuration API. - - In Storybook 9, ${needsViewportMigration && needsBackgroundsMigration ? 'these addons have' : 'this addon has'} been updated to use the new globals API which ensures a consistent experience while navigating between stories. - - We'll update your configuration to use the new API: - ${needsViewportMigration ? `- ${picocolors.yellow('viewports')} → ${picocolors.yellow('options')} and ${picocolors.yellow('defaultViewport')} → ${picocolors.yellow('initialGlobals.viewport')}\n` : ''}${needsBackgroundsMigration ? `- ${picocolors.yellow('values')} → ${picocolors.yellow('options')} and ${picocolors.yellow('default')} → ${picocolors.yellow('initialGlobals.backgrounds')}\n` : ''} - - Learn more: ${picocolors.cyan(MIGRATION)} - `; + prompt() { + return "You're using a deprecated config API for viewport/backgrounds. The globals API will be used instead."; }, - async run({ dryRun, result }) { + async run({ dryRun = false, result }) { const { previewConfig, needsViewportMigration, @@ -215,86 +216,230 @@ export const addonGlobalsApi: Fix = { if (!dryRun) { await writeFile(result.previewConfigPath, formatConfig(previewConfig)); } + + // Update stories + if (needsViewportMigration || needsBackgroundsMigration) { + await scanAndTransformFiles({ + promptMessage: + 'Enter a glob pattern to scan for story files to migrate (or press enter to use default):', + defaultGlob: '**/*.stories.{js,jsx,ts,tsx,mdx}', + dryRun, + transformFn: (files, options, dryRun) => transformStoryFiles(files, options, dryRun), + transformOptions: { + needsViewportMigration, + needsBackgroundsMigration, + viewportsOptions, + backgroundsOptions, + }, + }); + } }, }; -// Helper functions - -function getObjectProperty(obj: ObjectExpression, propertyName: string): Expression | undefined { - if (!obj || !obj.properties) { - return undefined; +// Individual story transformation function for testing +export function transformStoryFileSync( + source: string, + options: { + needsViewportMigration: boolean; + needsBackgroundsMigration: boolean; + viewportsOptions: any; + backgroundsOptions: any; } - - const property = obj.properties.find( - (prop) => - t.isObjectProperty(prop) && - ((t.isIdentifier(prop.key) && prop.key.name === propertyName) || - (t.isStringLiteral(prop.key) && prop.key.value === propertyName)) - ) as t.ObjectProperty; - - return property?.value as Expression; +): string | null { + const result = transformStoryFile(source, options); + return result ? formatCsf(result, {}, source) : null; } -function removeProperty(obj: ObjectExpression, propertyName: string): void { - if (!obj || !obj.properties) { - return; - } - - const index = obj.properties.findIndex( - (prop) => - t.isObjectProperty(prop) && - ((t.isIdentifier(prop.key) && prop.key.name === propertyName) || - (t.isStringLiteral(prop.key) && prop.key.value === propertyName)) +// Story transformation function +async function transformStoryFiles( + files: string[], + options: { + needsViewportMigration: boolean; + needsBackgroundsMigration: boolean; + viewportsOptions: any; + backgroundsOptions: any; + }, + dryRun: boolean +): Promise> { + const errors: Array<{ file: string; error: Error }> = []; + const { default: pLimit } = await import('p-limit'); + const limit = pLimit(10); + + await Promise.all( + files.map((file) => + limit(async () => { + try { + const content = await readFile(file, 'utf-8'); + const transformed = transformStoryFile(content, options); + + if (transformed && !dryRun) { + await writeCsf(transformed, file); + } + } catch (error) { + errors.push({ file, error: error as Error }); + } + }) + ) ); - if (index !== -1) { - obj.properties.splice(index, 1); - } + return errors; } -function addProperty(obj: ObjectExpression, propertyName: string, value: Expression): void { - if (!obj) { - return; +// Transform a single story file +function transformStoryFile( + source: string, + options: { + needsViewportMigration: boolean; + needsBackgroundsMigration: boolean; + viewportsOptions: any; + backgroundsOptions: any; } +): CsfFile | null { + const { needsViewportMigration, needsBackgroundsMigration, backgroundsOptions } = options; + + // Load the story file using CSF tools + const storyConfig = loadCsf(source, { + makeTitle: (title?: string) => title || 'default', + }).parse(); + + // Use the story transformer utility to handle all story iteration + const hasChanges = transformStoryParameters(storyConfig, (parameters, storyObject) => { + let newGlobals: ObjectExpression | undefined; + let viewportParams: ObjectExpression | undefined; + let backgroundsParams: ObjectExpression | undefined; + let storyHasChanges = false; + + // Handle viewport migration + if (needsViewportMigration) { + viewportParams = getObjectProperty(parameters, 'viewport') as ObjectExpression; + if (viewportParams) { + const defaultViewport = getObjectProperty(viewportParams, 'defaultViewport'); + if (defaultViewport && t.isStringLiteral(defaultViewport)) { + // Create globals.viewport + if (!newGlobals) { + newGlobals = t.objectExpression([]); + } - obj.properties.push(t.objectProperty(t.identifier(propertyName), value)); -} - -function transformValuesToOptions(valuesArray: ArrayExpression): Expression { - // Transform [{ name: 'Light', value: '#FFF' }] to { light: { name: 'Light', value: '#FFF' } } - const optionsObject = t.objectExpression([]); + newGlobals.properties.push( + t.objectProperty( + t.identifier('viewport'), + t.objectExpression([ + t.objectProperty(t.identifier('value'), t.stringLiteral(defaultViewport.value)), + t.objectProperty(t.identifier('isRotated'), t.booleanLiteral(false)), + ]) + ) + ); + + // Remove defaultViewport from parameters + removeProperty(viewportParams, 'defaultViewport'); + storyHasChanges = true; + } + } + } - if (valuesArray && t.isArrayExpression(valuesArray) && valuesArray.elements) { - valuesArray.elements.forEach((element) => { - if (t.isObjectExpression(element)) { - const nameProperty = getObjectProperty(element, 'name'); + // Handle backgrounds migration + if (needsBackgroundsMigration) { + backgroundsParams = getObjectProperty(parameters, 'backgrounds') as ObjectExpression; + if (backgroundsParams) { + const defaultBackground = getObjectProperty(backgroundsParams, 'default'); + const disableBackground = getObjectProperty(backgroundsParams, 'disable'); + + if (defaultBackground && t.isStringLiteral(defaultBackground)) { + // Create globals.backgrounds + if (!newGlobals) { + newGlobals = t.objectExpression([]); + } - if (t.isStringLiteral(nameProperty)) { - const key = nameProperty.value.toLowerCase().replace(/\s+/g, '_'); + const backgroundKey = getKeyFromName( + backgroundsOptions?.values as ArrayExpression, + defaultBackground.value + ); + + newGlobals.properties.push( + t.objectProperty( + t.identifier('backgrounds'), + t.objectExpression([ + t.objectProperty(t.identifier('value'), t.stringLiteral(backgroundKey)), + ]) + ) + ); + + // Remove default from parameters + removeProperty(backgroundsParams, 'default'); + storyHasChanges = true; + } - optionsObject.properties.push(t.objectProperty(t.identifier(key), element)); + // Handle disable -> disabled rename + if (disableBackground && t.isBooleanLiteral(disableBackground)) { + removeProperty(backgroundsParams, 'disable'); + // Rename disable to disabled (preserve both true and false values) + addProperty(backgroundsParams, 'disabled', disableBackground); + storyHasChanges = true; } } - }); - } + } - return optionsObject; -} + // Add globals to story if we created any + if (newGlobals && newGlobals.properties.length > 0) { + const existingGlobals = getObjectProperty(storyObject, 'globals') as + | ObjectExpression + | undefined; + + if (existingGlobals) { + // Merge new globals with existing globals + newGlobals.properties.forEach((newGlobal) => { + if (t.isObjectProperty(newGlobal) && t.isIdentifier(newGlobal.key)) { + const globalName = newGlobal.key.name; + const existingGlobal = getObjectProperty(existingGlobals, globalName) as + | ObjectExpression + | undefined; + + if (existingGlobal) { + // Merge properties if both are object expressions + if (t.isObjectExpression(newGlobal.value) && t.isObjectExpression(existingGlobal)) { + newGlobal.value.properties.forEach((newProp) => { + if (t.isObjectProperty(newProp) && t.isIdentifier(newProp.key)) { + const propName = newProp.key.name; + const existingProp = getObjectProperty(existingGlobal, propName); + + if (!existingProp) { + existingGlobal.properties.push(newProp); + } + } + }); + } + } else { + // Add new global to existing globals + existingGlobals.properties.push(newGlobal); + } + } + }); + } else { + // No existing globals, add new ones + storyObject.properties.push(t.objectProperty(t.identifier('globals'), newGlobals)); + } + storyHasChanges = true; + } -function getKeyFromName(valuesArray: ArrayExpression, name: string): string { - // Generate a key from a name in the values array - if (valuesArray && t.isArrayExpression(valuesArray) && valuesArray.elements) { - for (const element of valuesArray.elements) { - if (t.isObjectExpression(element)) { - const nameProperty = getObjectProperty(element, 'name'); + // Clean up empty parameter objects + if (viewportParams && viewportParams.properties.length === 0) { + removeProperty(parameters, 'viewport'); + storyHasChanges = true; + } - if (t.isStringLiteral(nameProperty) && nameProperty.value === name) { - return name.toLowerCase().replace(/\s+/g, '_'); - } - } + if (backgroundsParams && backgroundsParams.properties.length === 0) { + removeProperty(parameters, 'backgrounds'); + storyHasChanges = true; } - } - // If not found, generate a key from the name - return name.toLowerCase().replace(/\s+/g, '_'); + // Remove parameters if it's now empty + if (parameters.properties.length === 0) { + removeProperty(storyObject, 'parameters'); + storyHasChanges = true; + } + + return storyHasChanges; + }); + + return hasChanges ? storyConfig : null; } diff --git a/code/lib/cli-storybook/src/automigrate/helpers/addon-a11y-parameters.ts b/code/lib/cli-storybook/src/automigrate/helpers/addon-a11y-parameters.ts index 9e781438d816..d7751fe9be53 100644 --- a/code/lib/cli-storybook/src/automigrate/helpers/addon-a11y-parameters.ts +++ b/code/lib/cli-storybook/src/automigrate/helpers/addon-a11y-parameters.ts @@ -1,6 +1,8 @@ import { types as t } from 'storybook/internal/babel'; import { type ConfigFile, type CsfFile, loadConfig, loadCsf } from 'storybook/internal/csf-tools'; +import { getObjectProperty, transformStories } from './ast-utils'; + // TODO: this is copied from the codemod, we should move both utilities to the csf-tools package at some point const isStoryAnnotation = (stmt: t.Statement, objectExports: Record) => t.isExpressionStatement(stmt) && @@ -10,18 +12,12 @@ const isStoryAnnotation = (stmt: t.Statement, objectExports: Record objectExports[stmt.expression.left.object.name]; function migrateA11yParameters(obj: t.ObjectExpression): boolean { - const parametersProp = obj.properties.find( - (prop) => t.isObjectProperty(prop) && t.isIdentifier(prop.key) && prop.key.name === 'parameters' - ); + const parametersValue = getObjectProperty(obj, 'parameters') as t.ObjectExpression | undefined; - if (parametersProp && t.isObjectProperty(parametersProp)) { - const parametersValue = parametersProp.value as t.ObjectExpression; - const a11yProp = parametersValue.properties.find( - (prop) => t.isObjectProperty(prop) && t.isIdentifier(prop.key) && prop.key.name === 'a11y' - ); + if (parametersValue) { + const a11yValue = getObjectProperty(parametersValue, 'a11y') as t.ObjectExpression | undefined; - if (a11yProp && t.isObjectProperty(a11yProp)) { - const a11yValue = a11yProp.value as t.ObjectExpression; + if (a11yValue) { const elementProp = a11yValue.properties.find( (prop) => t.isObjectProperty(prop) && t.isIdentifier(prop.key) && prop.key.name === 'element' @@ -39,14 +35,12 @@ function migrateA11yParameters(obj: t.ObjectExpression): boolean { export function transformStoryA11yParameters(code: string): CsfFile | null { const parsed = loadCsf(code, { makeTitle: (title?: string) => title || 'default' }).parse(); - let hasChanges = false; - - if (t.isObjectExpression(parsed._metaNode)) { - if (migrateA11yParameters(parsed._metaNode)) { - hasChanges = true; - } - } + // Use the story transformer utility to handle all story iteration + let hasChanges = transformStories(parsed, (storyObject, storyName, csf) => { + return migrateA11yParameters(storyObject); + }); + // Also handle CSF2-style story annotations parsed._ast.program.body.forEach((stmt: t.Statement) => { const statement = stmt; if ( @@ -72,27 +66,7 @@ export function transformStoryA11yParameters(code: string): CsfFile | null { } }); - Object.values(parsed._storyExports).forEach((declaration) => { - const declarator = declaration as t.VariableDeclarator; - let init = t.isVariableDeclarator(declarator) ? declarator.init : undefined; - - // For type annotations e.g. A in `const Story = {} satisfies A;` - if (t.isTSSatisfiesExpression(init) || t.isTSAsExpression(init)) { - init = init.expression; - } - - if (t.isObjectExpression(init)) { - if (migrateA11yParameters(init)) { - hasChanges = true; - } - } - }); - - if (hasChanges) { - return parsed; - } - - return null; + return hasChanges ? parsed : null; } export function transformPreviewA11yParameters(code: string): ConfigFile | null { diff --git a/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts b/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts new file mode 100644 index 000000000000..36d70b3bdd84 --- /dev/null +++ b/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts @@ -0,0 +1,199 @@ +import { types as t } from 'storybook/internal/babel'; +import type { CsfFile } from 'storybook/internal/csf-tools'; + +import type { Expression, ObjectExpression } from '@babel/types'; + +/** Get a property from an object expression by name */ +export function getObjectProperty( + obj: ObjectExpression, + propertyName: string +): Expression | undefined { + if (!obj || !obj.properties) { + return undefined; + } + + const property = obj.properties.find( + (prop) => + t.isObjectProperty(prop) && + ((t.isIdentifier(prop.key) && prop.key.name === propertyName) || + (t.isStringLiteral(prop.key) && prop.key.value === propertyName)) + ) as t.ObjectProperty; + + return property?.value as Expression; +} + +/** Remove a property from an object expression by name */ +export function removeProperty(obj: ObjectExpression, propertyName: string): void { + if (!obj || !obj.properties) { + return; + } + + const index = obj.properties.findIndex( + (prop) => + t.isObjectProperty(prop) && + ((t.isIdentifier(prop.key) && prop.key.name === propertyName) || + (t.isStringLiteral(prop.key) && prop.key.value === propertyName)) + ); + + if (index !== -1) { + obj.properties.splice(index, 1); + } +} + +/** Add a property to an object expression */ +export function addProperty(obj: ObjectExpression, propertyName: string, value: Expression): void { + if (!obj || !obj.properties) { + return; + } + + obj.properties.push(t.objectProperty(t.identifier(propertyName), value)); +} + +/** Get the story object from a declaration (handles both story exports and meta exports) */ +export function getStoryObject(declaration: t.Node): t.ObjectExpression | undefined { + if (t.isVariableDeclarator(declaration)) { + // Handle story exports: `export const Story = { ... }` + let init = declaration.init; + if (t.isTSSatisfiesExpression(init) || t.isTSAsExpression(init)) { + init = init.expression; + } + if (t.isObjectExpression(init)) { + return init; + } + } else if (t.isExportDefaultDeclaration(declaration)) { + // Handle meta export: `export default { ... }` + let init = declaration.declaration; + if (t.isTSSatisfiesExpression(init) || t.isTSAsExpression(init)) { + init = init.expression; + } + if (t.isObjectExpression(init)) { + return init; + } + } + + return undefined; +} + +/** Transform values array to options object for background keys */ +export function transformValuesToOptions(valuesArray: t.ArrayExpression): t.Expression { + // Transform [{ name: 'Light', value: '#FFF' }] to { light: { name: 'Light', value: '#FFF' } } + const optionsObject = t.objectExpression([]); + + if (valuesArray && t.isArrayExpression(valuesArray) && valuesArray.elements) { + valuesArray.elements.forEach((element) => { + if (t.isObjectExpression(element)) { + const nameProperty = getObjectProperty(element, 'name'); + + if (t.isStringLiteral(nameProperty)) { + const key = nameProperty.value.toLowerCase().replace(/\s+/g, '_'); + + optionsObject.properties.push(t.objectProperty(t.identifier(key), element)); + } + } + }); + } + + return optionsObject; +} + +/** Get key from name using the options mapping */ +export function getKeyFromName(valuesArray: t.ArrayExpression, name: string): string { + // Generate a key from a name in the values array + if (valuesArray && t.isArrayExpression(valuesArray) && valuesArray.elements) { + for (const element of valuesArray.elements) { + if (t.isObjectExpression(element)) { + const nameProperty = getObjectProperty(element, 'name'); + + if (t.isStringLiteral(nameProperty) && nameProperty.value === name) { + return name.toLowerCase().replace(/\s+/g, '_'); + } + } + } + } + + // If not found, generate a key from the name + return name.toLowerCase().replace(/\s+/g, '_'); +} + +/** Options for story transformation */ +export interface StoryTransformOptions { + /** Whether to transform the meta export (export default) */ + includeMeta?: boolean; + /** Whether to transform story exports */ + includeStories?: boolean; +} + +/** Callback function for transforming a story object */ +export type StoryTransformCallback = ( + storyObject: t.ObjectExpression, + storyName: string, + csf: CsfFile +) => boolean; + +/** Callback function for transforming story parameters */ +export type StoryParametersTransformCallback = ( + parameters: t.ObjectExpression, + storyObject: t.ObjectExpression, + storyName: string, + csf: CsfFile +) => boolean; + +/** Callback function for transforming story globals */ +export type StoryGlobalsTransformCallback = ( + globals: t.ObjectExpression, + storyObject: t.ObjectExpression, + storyName: string, + csf: CsfFile +) => boolean; + +/** Transform all stories in a CSF file using a callback function */ +export function transformStories( + csf: CsfFile, + transformCallback: StoryTransformCallback, + options: StoryTransformOptions = { includeMeta: true, includeStories: true } +): boolean { + let hasChanges = false; + + // Transform story exports + if (options.includeStories) { + Object.entries(csf._storyExports).forEach(([storyName, declaration]) => { + const storyObject = getStoryObject(declaration); + if (storyObject) { + if (transformCallback(storyObject, storyName, csf)) { + hasChanges = true; + } + } + }); + } + + // Transform meta export + if (options.includeMeta && csf._metaPath) { + const storyObject = getStoryObject(csf._metaPath.node); + if (storyObject) { + if (transformCallback(storyObject, 'meta', csf)) { + hasChanges = true; + } + } + } + + return hasChanges; +} + +/** Transform stories with parameters-specific logic */ +export function transformStoryParameters( + csf: CsfFile, + transformParameters: StoryParametersTransformCallback, + options: StoryTransformOptions = { includeMeta: true, includeStories: true } +): boolean { + return transformStories( + csf, + (storyObject, storyName, csf) => { + const parameters = getObjectProperty(storyObject, 'parameters') as t.ObjectExpression; + if (parameters) { + return transformParameters(parameters, storyObject, storyName, csf); + } + return false; + }, + options + ); +} From 6d01ccff1238fa9382fb7e06872dfde56475914d Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Mon, 29 Sep 2025 14:45:38 +0200 Subject: [PATCH 3/7] Refactor addonGlobalsApi to remove common dependency and update migration logic with storiesPaths parameter --- .../automigrate/fixes/addon-globals-api.test.ts | 16 +++------------- .../src/automigrate/fixes/addon-globals-api.ts | 16 ++++++---------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts index f32fea4cacd2..7e7601edd982 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -3,23 +3,14 @@ import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import * as common from 'storybook/internal/common'; - // Import common to mock import dedent from 'ts-dedent'; // Import FixResult type import { addonGlobalsApi, transformStoryFileSync } from './addon-globals-api'; -// Mock fs/promises and common utilities +// Mock fs/promises vi.mock('node:fs/promises', async () => import('../../../../../__mocks__/fs/promises')); -vi.mock('storybook/internal/common', async (importOriginal) => { - const original = await importOriginal(); - return { - ...original, - scanAndTransformFiles: vi.fn().mockResolvedValue([]), // Mock scanAndTransformFiles - }; -}); const previewConfigPath = join('.storybook', 'preview.js'); @@ -37,11 +28,10 @@ const check = async (previewContents: string) => { }); }; -// Helper to run the migration for preview file and capture scanAndTransformFiles args +// Helper to run the migration for preview file and capture transform function const runMigrationAndGetTransformFn = async (previewContents: string) => { const result = await check(previewContents); const mockWriteFile = vi.mocked(fsp.writeFile); - const mockScanAndTransform = vi.mocked(common.scanAndTransformFiles); let transformFn: (filePath: string, content: string) => string | null = () => null; @@ -51,6 +41,7 @@ const runMigrationAndGetTransformFn = async (previewContents: string) => { await addonGlobalsApi.run?.({ result, dryRun: false, + storiesPaths: ['**/*.stories.{js,jsx,ts,tsx,mdx}'], // Mock stories paths packageManager: {} as any, // Add necessary mock properties } as any); @@ -85,7 +76,6 @@ const runMigrationAndGetTransformFn = async (previewContents: string) => { describe('addon-globals-api', () => { afterEach(() => { vi.clearAllMocks(); - vi.mocked(common.scanAndTransformFiles).mockClear(); }); describe('check', () => { diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts index cf41af3b89de..0752060c951c 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts @@ -1,7 +1,6 @@ import { readFile, writeFile } from 'node:fs/promises'; import { types as t } from 'storybook/internal/babel'; -import { scanAndTransformFiles } from 'storybook/internal/common'; import type { ConfigFile, CsfFile } from 'storybook/internal/csf-tools'; import { formatConfig, @@ -133,7 +132,7 @@ export const addonGlobalsApi: Fix = { return "You're using a deprecated config API for viewport/backgrounds. The globals API will be used instead."; }, - async run({ dryRun = false, result }) { + async run({ dryRun = false, result, storiesPaths }) { const { previewConfig, needsViewportMigration, @@ -219,19 +218,16 @@ export const addonGlobalsApi: Fix = { // Update stories if (needsViewportMigration || needsBackgroundsMigration) { - await scanAndTransformFiles({ - promptMessage: - 'Enter a glob pattern to scan for story files to migrate (or press enter to use default):', - defaultGlob: '**/*.stories.{js,jsx,ts,tsx,mdx}', - dryRun, - transformFn: (files, options, dryRun) => transformStoryFiles(files, options, dryRun), - transformOptions: { + await transformStoryFiles( + storiesPaths, + { needsViewportMigration, needsBackgroundsMigration, viewportsOptions, backgroundsOptions, }, - }); + dryRun + ); } }, }; From 3a9018a2e8fa5937b3cfdeb4521f16f8c0a457ac Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 30 Sep 2025 11:19:38 +0200 Subject: [PATCH 4/7] Add logic to cover background and viewport configuration which contains special characters in value name --- .../fixes/addon-globals-api.test.ts | 38 +++++++++++++++++++ .../src/automigrate/helpers/ast-utils.ts | 8 +++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts index 7e7601edd982..60bdbd0bec1c 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -462,6 +462,44 @@ describe('addon-globals-api', () => { };" `); }); + + it('should migrate complex backgrounds configuration with dots and brackets in names', async () => { + const { previewFileContent } = await runMigrationAndGetTransformFn(` + export default { + parameters: { + backgrounds: { + default: 'palette.neutral[100]', + values: [ + { + name: 'palette.neutral[100]', + value: palette.neutral[100], + }, + ], + } + } + } + `); + + expect(previewFileContent).toMatchInlineSnapshot(` + " + export default { + parameters: { + backgrounds: { + options: { + 'palette.neutral[100]': { name: 'palette.neutral[100]', value: palette.neutral[100] }, + } + }, + }, + + initialGlobals: { + backgrounds: { + value: 'palette.neutral[100]' + } + } + }; + " + `); + }); }); describe('run - story files', () => { diff --git a/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts b/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts index 36d70b3bdd84..9521068c4010 100644 --- a/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts +++ b/code/lib/cli-storybook/src/automigrate/helpers/ast-utils.ts @@ -87,7 +87,13 @@ export function transformValuesToOptions(valuesArray: t.ArrayExpression): t.Expr if (t.isStringLiteral(nameProperty)) { const key = nameProperty.value.toLowerCase().replace(/\s+/g, '_'); - optionsObject.properties.push(t.objectProperty(t.identifier(key), element)); + // For complex names with dots, brackets, or other special characters, use string literal + // For simple names, use identifier + const keyNode = /^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(key) + ? t.identifier(key) + : t.stringLiteral(nameProperty.value); + + optionsObject.properties.push(t.objectProperty(keyNode, element)); } } }); From 0f359941894ccc6d8addf2455cf69e69b94ce7a7 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 30 Sep 2025 13:50:56 +0200 Subject: [PATCH 5/7] Update addon-globals-api tests to use dedent for inline snapshots and improve formatting for better readability --- .../fixes/addon-globals-api.test.ts | 217 +++++++++--------- 1 file changed, 105 insertions(+), 112 deletions(-) diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts index 60bdbd0bec1c..d718abc0db56 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -217,7 +217,7 @@ describe('addon-globals-api', () => { describe('run - preview file', () => { it('should migrate viewport configuration correctly', async () => { - const { previewFileContent } = await runMigrationAndGetTransformFn(` + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` export default { parameters: { viewport: { @@ -231,31 +231,29 @@ describe('addon-globals-api', () => { } `); - expect(previewFileContent).toMatchInlineSnapshot(` - " - export default { - parameters: { - viewport: { - options: { - mobile: { name: 'Mobile', width: '320px', height: '568px' }, - tablet: { name: 'Tablet', width: '768px', height: '1024px' } - } - } - }, - - initialGlobals: { - viewport: { - value: 'mobile', - isRotated: false - } - } - }; - " - `); + expect(previewFileContent).toMatchInlineSnapshot(dedent` + "export default { + parameters: { + viewport: { + options: { + mobile: { name: 'Mobile', width: '320px', height: '568px' }, + tablet: { name: 'Tablet', width: '768px', height: '1024px' } + } + } + }, + + initialGlobals: { + viewport: { + value: 'mobile', + isRotated: false + } + } + };" +`); }); it('should migrate backgrounds configuration correctly', async () => { - const { previewFileContent } = await runMigrationAndGetTransformFn(` + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` export default { parameters: { backgrounds: { @@ -269,30 +267,28 @@ describe('addon-globals-api', () => { } `); - expect(previewFileContent).toMatchInlineSnapshot(` - " - export default { - parameters: { - backgrounds: { - options: { - light: { name: 'Light', value: '#F8F8F8' }, - dark: { name: 'Dark', value: '#333333' } - } - } - }, - - initialGlobals: { - backgrounds: { - value: 'light' - } - } - }; - " - `); + expect(previewFileContent).toMatchInlineSnapshot(dedent` + "export default { + parameters: { + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' }, + dark: { name: 'Dark', value: '#333333' } + } + } + }, + + initialGlobals: { + backgrounds: { + value: 'light' + } + } + };" +`); }); it('should rename backgrounds disable property to disabled', async () => { - const { previewFileContent } = await runMigrationAndGetTransformFn(` + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` export default { parameters: { backgrounds: { @@ -306,24 +302,22 @@ describe('addon-globals-api', () => { `); // Verify the transformation results - expect(previewFileContent).toMatchInlineSnapshot(` - " - export default { - parameters: { - backgrounds: { - options: { - light: { name: 'Light', value: '#F8F8F8' } - }, - disabled: true - } - } - } - " + expect(previewFileContent).toMatchInlineSnapshot(dedent` + "export default { + parameters: { + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' } + }, + disabled: true + } + } + }" `); }); it('should migrate both viewport and backgrounds configurations', async () => { - const { previewFileContent } = await runMigrationAndGetTransformFn(` + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` export default { parameters: { viewport: { @@ -335,7 +329,7 @@ describe('addon-globals-api', () => { backgrounds: { values: [ { name: 'Light', value: '#F8F8F8' }, - { name: 'Dark', value: '#F8F8F8' } + { name: 'Dark', value: '#333333' } ], default: 'Light' } @@ -343,36 +337,34 @@ describe('addon-globals-api', () => { } `); - expect(previewFileContent).toMatchInlineSnapshot(` - " - export default { - parameters: { - viewport: { - options: { - mobile: { name: 'Mobile', width: '320px', height: '568px' } - } - }, - backgrounds: { - options: { - light: { name: 'Light', value: '#F8F8F8' }, - dark: { name: 'Dark', value: '#F8F8F8' } - } - } - }, - - initialGlobals: { - viewport: { - value: 'mobile', - isRotated: false - }, - - backgrounds: { - value: 'light' - } - } - }; - " - `); + expect(previewFileContent).toMatchInlineSnapshot(dedent` + "export default { + parameters: { + viewport: { + options: { + mobile: { name: 'Mobile', width: '320px', height: '568px' } + } + }, + backgrounds: { + options: { + light: { name: 'Light', value: '#F8F8F8' }, + dark: { name: 'Dark', value: '#333333' } + } + } + }, + + initialGlobals: { + viewport: { + value: 'mobile', + isRotated: false + }, + + backgrounds: { + value: 'light' + } + } + };" +`); }); it('should correctly handle partial configurations', async () => { @@ -433,7 +425,7 @@ describe('addon-globals-api', () => { } `); - expect(previewFileContent).toMatchInlineSnapshot(` + expect(previewFileContent).toMatchInlineSnapshot(dedent` "import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport'; export default { @@ -464,7 +456,7 @@ describe('addon-globals-api', () => { }); it('should migrate complex backgrounds configuration with dots and brackets in names', async () => { - const { previewFileContent } = await runMigrationAndGetTransformFn(` + const { previewFileContent } = await runMigrationAndGetTransformFn(dedent` export default { parameters: { backgrounds: { @@ -480,25 +472,26 @@ describe('addon-globals-api', () => { } `); - expect(previewFileContent).toMatchInlineSnapshot(` - " - export default { - parameters: { - backgrounds: { - options: { - 'palette.neutral[100]': { name: 'palette.neutral[100]', value: palette.neutral[100] }, - } - }, - }, - - initialGlobals: { - backgrounds: { - value: 'palette.neutral[100]' - } - } - }; - " - `); + expect(previewFileContent).toMatchInlineSnapshot(dedent` + "export default { + parameters: { + backgrounds: { + options: { + "palette.neutral[100]": { + name: 'palette.neutral[100]', + value: palette.neutral[100], + } + } + } + }, + + initialGlobals: { + backgrounds: { + value: 'palette.neutral[100]' + } + } + };" +`); }); }); @@ -705,8 +698,8 @@ describe('addon-globals-api', () => { import Button from './Button'; export default { component: Button }; export const NoParams = {}; - export const ExistingGlobals = { globals: { backgrounds: { value: 'dark' } } }; - export const ExistingDisabled = { parameters: { backgrounds: { disabled: true } } }; + export const ExistingGlobals = { globals: { backgrounds: { value: 'dark' } } }; + export const ExistingDisabled = { parameters: { backgrounds: { disabled: true } } }; `; expect(transformFn).toBeDefined(); expect(transformFn!('story.js', storyContent)).toBeNull(); From b31cae217d7c2047ec0cfb564ea892ae4e8680ea Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 30 Sep 2025 14:21:16 +0200 Subject: [PATCH 6/7] Refactor transform function type in addon-globals-api tests for improved type safety --- .../src/automigrate/fixes/addon-globals-api.test.ts | 5 ++++- .../cli-storybook/src/automigrate/fixes/addon-globals-api.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts index d718abc0db56..f39c2532758f 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -33,7 +33,10 @@ const runMigrationAndGetTransformFn = async (previewContents: string) => { const result = await check(previewContents); const mockWriteFile = vi.mocked(fsp.writeFile); - let transformFn: (filePath: string, content: string) => string | null = () => null; + let transformFn: ( + filePath: string, + content: string + ) => ReturnType = () => null; let transformOptions: any; diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts index 0752060c951c..e7d59be0a7fd 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.ts @@ -241,7 +241,7 @@ export function transformStoryFileSync( viewportsOptions: any; backgroundsOptions: any; } -): string | null { +) { const result = transformStoryFile(source, options); return result ? formatCsf(result, {}, source) : null; } From fc539b3c708e24005814e599d1365a7d90213738 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 30 Sep 2025 15:09:32 +0200 Subject: [PATCH 7/7] Using single quotes in tests for recast results --- .../src/automigrate/fixes/addon-globals-api.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts index f39c2532758f..7eb0ffc0b894 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/addon-globals-api.test.ts @@ -12,6 +12,17 @@ import { addonGlobalsApi, transformStoryFileSync } from './addon-globals-api'; // Mock fs/promises vi.mock('node:fs/promises', async () => import('../../../../../__mocks__/fs/promises')); +vi.mock(import('storybook/internal/babel'), async (actualImport) => { + const actual = await actualImport(); + return { + ...actual, + recast: { + ...actual.recast, + print: (ast, options) => actual.recast.print(ast, { ...options, quote: 'single' }), + }, + }; +}); + const previewConfigPath = join('.storybook', 'preview.js'); const check = async (previewContents: string) => { @@ -480,7 +491,7 @@ describe('addon-globals-api', () => { parameters: { backgrounds: { options: { - "palette.neutral[100]": { + 'palette.neutral[100]': { name: 'palette.neutral[100]', value: palette.neutral[100], }