From 96b431435a495df0384f742d45b08a49c345931d Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 22 Mar 2024 14:42:36 +0100 Subject: [PATCH 1/3] Add getRendererName function to mainConfigFile --- .../helpers/mainConfigFile.test.ts | 56 +++++++++++++++++++ .../src/automigrate/helpers/mainConfigFile.ts | 18 ++++++ 2 files changed, 74 insertions(+) diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts index a4f0e7e5e92f..8f2f0fde245f 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { getBuilderPackageName, getFrameworkPackageName, + getRendererName, getRendererPackageNameFromFramework, } from './mainConfigFile'; @@ -133,6 +134,61 @@ describe('getFrameworkPackageName', () => { }); }); +describe('getRendererName', () => { + it('should return null when mainConfig is undefined', () => { + const rendererName = getRendererName(undefined); + expect(rendererName).toBeNull(); + }); + + it('should return null when framework package name or path is not found', () => { + const mainConfig = {}; + + const rendererName = getRendererName(mainConfig as any); + expect(rendererName).toBeNull(); + }); + + it('should return renderer name when framework is a string', () => { + const frameworkPackage = '@storybook/react-webpack5'; + const mainConfig = { + framework: frameworkPackage, + }; + + const rendererName = getRendererName(mainConfig as any); + expect(rendererName).toBe('react'); + }); + + it('should return renderer name when framework.name contains valid framework package name', () => { + const frameworkPackage = '@storybook/react-vite'; + const packageNameOrPath = `/path/to/${frameworkPackage}`; + const mainConfig = { + framework: { name: packageNameOrPath }, + }; + + const rendererName = getRendererName(mainConfig as any); + expect(rendererName).toBe('react'); + }); + + it('should return renderer name when framework.name contains windows backslash paths', () => { + const packageNameOrPath = 'c:\\path\\to\\@storybook\\sveltekit'; + const mainConfig = { + framework: { name: packageNameOrPath }, + }; + + const rendererName = getRendererName(mainConfig as any); + expect(rendererName).toBe('svelte'); + }); + + it(`should return undefined when framework does not contain the name of a valid framework package`, () => { + const packageNameOrPath = '@my-org/storybook-framework'; + const mainConfig = { + framework: packageNameOrPath, + }; + + const rendererName = getRendererName(mainConfig as any); + expect(rendererName).toBeUndefined(); + }); +}); + describe('getRendererPackageNameFromFramework', () => { it('should return null when given no package name', () => { // @ts-expect-error (Argument of type 'undefined' is not assignable) diff --git a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts index 2ba805c95614..3fd56fc8107b 100644 --- a/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli/src/automigrate/helpers/mainConfigFile.ts @@ -13,6 +13,7 @@ import dedent from 'ts-dedent'; import path from 'path'; import type { JsPackageManager } from '@storybook/core-common'; import { getCoercedStorybookVersion } from '@storybook/core-common'; +import { frameworkToRenderer } from '../../helpers'; const logger = console; @@ -36,6 +37,23 @@ export const getFrameworkPackageName = (mainConfig?: StorybookConfigRaw) => { ); }; +/** + * Given a Storybook configuration object, retrieves the inferred renderer name from the framework. + * @param mainConfig - The main Storybook configuration object to lookup. + * @returns - The renderer name. If not found, returns null. + */ +export const getRendererName = (mainConfig?: StorybookConfigRaw) => { + const frameworkPackageName = getFrameworkPackageName(mainConfig); + + if (!frameworkPackageName) { + return null; + } + + const frameworkName = frameworkPackages[frameworkPackageName]; + + return frameworkToRenderer[frameworkName as keyof typeof frameworkToRenderer]; +}; + /** * Given a Storybook configuration object, retrieves the package name or file path of the builder. * @param mainConfig - The main Storybook configuration object to lookup. From 5e2f1e7f9c3749dd8d5770cf312e652ba2559167 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Fri, 22 Mar 2024 15:16:35 +0100 Subject: [PATCH 2/3] Adjust react-docgen automigration to add possibility to migrate back to the previous default --- .../automigrate/fixes/react-docgen.test.ts | 47 +++++++++++++++-- .../cli/src/automigrate/fixes/react-docgen.ts | 50 ++++++++++++++----- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/code/lib/cli/src/automigrate/fixes/react-docgen.test.ts b/code/lib/cli/src/automigrate/fixes/react-docgen.test.ts index ba46fd8ee77b..22461bc18aeb 100644 --- a/code/lib/cli/src/automigrate/fixes/react-docgen.test.ts +++ b/code/lib/cli/src/automigrate/fixes/react-docgen.test.ts @@ -25,31 +25,35 @@ describe('no-ops', () => { check({ packageManager: {}, main: { + framework: '@storybook/react-vite', typescript: { // @ts-expect-error assume react reactDocgen: 'react-docgen-typescript', }, }, }) - ).resolves.toBeFalsy(); + ).resolves.toBeNull(); await expect( check({ packageManager: {}, main: { + framework: '@storybook/react-vite', typescript: { // @ts-expect-error assume react reactDocgen: false, }, }, }) - ).resolves.toBeFalsy(); + ).resolves.toBeNull(); }); + it('typescript.reactDocgen and typescript.reactDocgenTypescriptOptions are both unset', async () => { await expect( check({ packageManager: {}, main: { + framework: '@storybook/react-vite', typescript: { // @ts-expect-error assume react reactDocgen: 'react-docgen-typescript', @@ -57,22 +61,55 @@ describe('no-ops', () => { }, }, }) - ).resolves.toBeFalsy(); + ).resolves.toBeNull(); + }); + + it('typescript.reactDocgen is undefined and it is not a react framework', async () => { + await expect( + check({ + packageManager: {}, + main: { + framework: '@storybook/sveltekit', + }, + }) + ).resolves.toBeNull(); }); }); describe('continue', () => { + it('should resolve if the framework is using a react renderer', async () => { + await expect( + check({ + packageManager: {}, + main: { + framework: '@storybook/nextjs', + }, + }) + ).resolves.toEqual({ + reactDocgenTypescriptOptions: undefined, + reactDocgen: undefined, + }); + }); + it('typescript.reactDocgenTypescriptOptions is set', async () => { await expect( check({ packageManager: {}, main: { + framework: '@storybook/react-vite', typescript: { // @ts-expect-error assume react - reactDocgenTypescriptOptions: {}, + reactDocgenTypescriptOptions: { + someOption: true, + }, }, }, }) - ).resolves.toBeTruthy(); + ).resolves.toEqual({ + reactDocgenTypescriptOptions: { + someOption: true, + }, + reactDocgen: undefined, + }); }); }); diff --git a/code/lib/cli/src/automigrate/fixes/react-docgen.ts b/code/lib/cli/src/automigrate/fixes/react-docgen.ts index ef89a24915ff..1af8fe6d0a62 100644 --- a/code/lib/cli/src/automigrate/fixes/react-docgen.ts +++ b/code/lib/cli/src/automigrate/fixes/react-docgen.ts @@ -1,15 +1,15 @@ import { dedent } from 'ts-dedent'; -import { updateMainConfig } from '../helpers/mainConfigFile'; +import { getRendererName, updateMainConfig } from '../helpers/mainConfigFile'; import type { Fix } from '../types'; +import chalk from 'chalk'; const logger = console; interface Options { - reactDocgenTypescriptOptions: any; + reactDocgenTypescriptOptions?: any; + reactDocgen?: 'react-docgen-typescript' | 'react-docgen' | false; } -/** - */ export const reactDocgen: Fix = { id: 'react-docgen', @@ -17,13 +17,20 @@ export const reactDocgen: Fix = { async check({ mainConfig }) { // @ts-expect-error assume react - const { reactDocgenTypescriptOptions } = mainConfig.typescript || {}; + const { reactDocgenTypescriptOptions, reactDocgen: rDocgen } = mainConfig.typescript || {}; - return reactDocgenTypescriptOptions ? { reactDocgenTypescriptOptions } : null; + const rendererName = getRendererName(mainConfig); + + if (rendererName !== 'react' || rDocgen !== undefined) { + return null; + } + + return { reactDocgenTypescriptOptions, reactDocgen: rDocgen }; }, - prompt() { - return dedent` + prompt({ reactDocgenTypescriptOptions }) { + if (reactDocgenTypescriptOptions) { + return dedent` You have "typescript.reactDocgenTypescriptOptions" configured in your main.js, but "typescript.reactDocgen" is unset. @@ -37,15 +44,34 @@ export const reactDocgen: Fix = { https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#react-docgen-component-analysis-by-default `; + } else { + return dedent` + Since Storybook 8, ${chalk.cyan( + 'react-docgen' + )} is now the default for generating component controls, replacing ${chalk.cyan( + 'react-docgen-typescript' + )}. + This offers better performance and suits most cases. + However, for complex TypeScript types or specific type features, the generated controls might not be as precise. + + For more on this change, check the migration guide: + ${chalk.yellow( + 'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#react-docgen-component-analysis-by-default' + )} + + For known "react-docgen" limitations, see: + ${chalk.yellow('https://github.com/storybookjs/storybook/issues/26606')} + + Would you like to switch back to ${chalk.cyan('react-docgen-typescript')} in your Storybook? + `; + } }, - async run({ dryRun, mainConfigPath }) { + async run({ dryRun, mainConfigPath, result }) { if (!dryRun) { await updateMainConfig({ mainConfigPath, dryRun: !!dryRun }, async (main) => { logger.info(`✅ Setting typescript.reactDocgen`); - if (!dryRun) { - main.setFieldValue(['typescript', 'reactDocgen'], 'react-docgen-typescript'); - } + main.setFieldValue(['typescript', 'reactDocgen'], 'react-docgen-typescript'); }); } }, From c366e03c77203abc4062bae49b92a5fd7aca9618 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 26 Mar 2024 14:38:23 +0100 Subject: [PATCH 3/3] Make versioning consistent in prompt --- code/lib/cli/src/automigrate/fixes/react-docgen.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/cli/src/automigrate/fixes/react-docgen.ts b/code/lib/cli/src/automigrate/fixes/react-docgen.ts index 1af8fe6d0a62..f791be0ab068 100644 --- a/code/lib/cli/src/automigrate/fixes/react-docgen.ts +++ b/code/lib/cli/src/automigrate/fixes/react-docgen.ts @@ -46,7 +46,7 @@ export const reactDocgen: Fix = { `; } else { return dedent` - Since Storybook 8, ${chalk.cyan( + Since Storybook 8.0, ${chalk.cyan( 'react-docgen' )} is now the default for generating component controls, replacing ${chalk.cyan( 'react-docgen-typescript'