Skip to content

Commit

Permalink
Merge pull request #26620 from storybookjs/valentin/react-docgen-auto…
Browse files Browse the repository at this point in the history
…migration-improvements

Automigrations: Add migration note about new react-docgen default
  • Loading branch information
valentinpalkovic committed Mar 26, 2024
2 parents 2118e9a + c366e03 commit de173e8
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 17 deletions.
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.0, ${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');
});
}
},
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

0 comments on commit de173e8

Please sign in to comment.