Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automigrations: Add migration note about new react-docgen default #26620

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 42 additions & 5 deletions code/lib/cli/src/automigrate/fixes/react-docgen.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,54 +25,91 @@ 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',
reactDocgenTypescriptOptions: undefined,
},
},
})
).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,
});
});
});
50 changes: 38 additions & 12 deletions code/lib/cli/src/automigrate/fixes/react-docgen.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,36 @@
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<Options> = {
id: 'react-docgen',

versionRange: ['<8.0.0-alpha.1', '>=8.0.0-alpha.1'],

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.

Expand All @@ -37,15 +44,34 @@ export const reactDocgen: Fix<Options> = {

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'
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
)} 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?
valentinpalkovic marked this conversation as resolved.
Show resolved Hide resolved
`;
}
},

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');
});
}
},
Expand Down
56 changes: 56 additions & 0 deletions code/lib/cli/src/automigrate/helpers/mainConfigFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest';
import {
getBuilderPackageName,
getFrameworkPackageName,
getRendererName,
getRendererPackageNameFromFramework,
} from './mainConfigFile';

Expand Down Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions code/lib/cli/src/automigrate/helpers/mainConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand Down
Loading