Skip to content

Commit

Permalink
Merge pull request #24165 from storybookjs/shilman/default-to-react-d…
Browse files Browse the repository at this point in the history
…ocgen

React: Set `react-docgen` to default TS docgen
  • Loading branch information
shilman authored Nov 30, 2023
2 parents 285d445 + 1fa44fc commit 2e4e691
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 63 deletions.
18 changes: 18 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [UI layout state has changed shape](#ui-layout-state-has-changed-shape)
- [New UI and props for Button and IconButton components](#new-ui-and-props-for-button-and-iconbutton-components)
- [Icons is deprecated](#icons-is-deprecated)
- [React-docgen component analysis by default](#react-docgen-component-analysis-by-default)
- [From version 7.5.0 to 7.6.0](#from-version-750-to-760)
- [CommonJS with Vite is deprecated](#commonjs-with-vite-is-deprecated)
- [Using implicit actions during rendering is deprecated](#using-implicit-actions-during-rendering-is-deprecated)
Expand Down Expand Up @@ -321,6 +322,7 @@
- [Packages renaming](#packages-renaming)
- [Deprecated embedded addons](#deprecated-embedded-addons)


## From version 7.x to 8.0.0

### Implicit actions can not be used during rendering (for example in the play function)
Expand Down Expand Up @@ -442,6 +444,22 @@ The `IconButton` doesn't have any deprecated props but it now uses the new `Butt

In Storybook 8.0 we are introducing a new icon library available with `@storybook/icons`. We are deprecating the `Icons` component in `@storybook/components` and recommend that addon creators and Storybook maintainers use the new `@storybook/icons` component instead.

#### React-docgen component analysis by default

In Storybook 7, we used `react-docgen-typescript` to analyze React component props and auto-generate controls. In Storybook 8, we have moved to `react-docgen` as the new default. `react-docgen` is dramatically more efficient, shaving seconds off of dev startup times. However, it only analyzes basic TypeScript constructs.

We feel `react-docgen` is the right tradeoff for most React projects. However, if you need the full fidelity of `react-docgen-typescript`, you can opt-in using the following setting in `.storybook/main.js`:

```js
export default {
typescript: {
reactDocgen: 'react-docgen-typescript',
}
}
```

For more information see: https://storybook.js.org/docs/react/api/main-config-typescript#reactdocgen

## From version 7.5.0 to 7.6.0

#### CommonJS with Vite is deprecated
Expand Down
40 changes: 6 additions & 34 deletions code/addons/docs/react/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ To learn more about Storybook Docs, read the [general documentation](../README.m
- [Props tables](#props-tables)
- [MDX](#mdx)
- [Inline stories](#inline-stories)
- [TypeScript props with `react-docgen`](#typescript-props-with-react-docgen)
- [TypeScript props with `react-docgen-typescript`](#typescript-props-with-react-docgen-typescript)
- [More resources](#more-resources)

## Installation
Expand Down Expand Up @@ -108,17 +108,17 @@ To do so for all stories, update `.storybook/preview.js`:
export const parameters = { docs: { story: { inline: false } } };
```

## TypeScript props with `react-docgen`
## TypeScript props with `react-docgen-typescript`

If you're using TypeScript, there are two different options for generating props: `react-docgen-typescript` (default) or `react-docgen`.
If you're using TypeScript, there are two different options for generating props: `react-docgen` (default) or `react-docgen-typescript`.

You can add the following lines to your `.storybook/main.js` to switch between the two (or disable docgen):

```js
export default {
typescript: {
// also valid 'react-docgen-typescript' | false
reactDocgen: 'react-docgen',
// also valid 'react-docgen' | false
reactDocgen: 'react-docgen-typescript',
},
};
```
Expand All @@ -129,7 +129,7 @@ Neither option is perfect, so here's everything you should know if you're thinki
| --------------- | ------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- |
| **Features** | **Great**. The analysis produces great results which gives the best props table experience. | **OK**. React-docgen produces basic results that are fine for most use cases. |
| **Performance** | **Slow**. It's doing a lot more work to produce those results, and may also have an inefficient implementation. | **Blazing fast**. Adding it to your project increases build time negligibly. |
| **Bugs** | **Many**. There are a lot of corner cases that are not handled properly, and are annoying for developers. | **Few**. But there's a dealbreaker, which is lack for imported types (see below). |
| **Bugs** | **Some**. There are corner cases that are not handled properly, and are annoying for developers. | **Some**. There are corner cases that are not handled properly, and are annoying for developers. |
| **SB docs** | **Good**. Our prop tables have supported `react-docgen-typescript` results from the beginning, so it's relatively stable. | **OK**. There are some obvious improvements to fully support `react-docgen`, and they're coming soon. |

**Performance** is a common question, so here are build times from a random project to quantify. Your mileage may vary:
Expand All @@ -140,34 +140,6 @@ Neither option is perfect, so here's everything you should know if you're thinki
| react-docgen | 29s |
| none | 28s |

The biggest limitation of `react-docgen` is lack of support for imported types. What that means is that when a component uses a type defined in another file or package, `react-docgen` is unable to extract props information for that type.

```tsx
import React, { FC } from 'react';
import SomeType from './someFile';

type NewType = SomeType & { foo: string };
const MyComponent: FC<NewType> = ...
```

So in the previous example, `SomeType` would simply be ignored! There's an [open PR for this in the `react-docgen` repo](https://github.com/reactjs/react-docgen/pull/352) which you can upvote if it affects you.

Another common pitfall when switching to `react-docgen` is [lack of support for `React.FC`](https://github.com/reactjs/react-docgen/issues/387). This means that the following common pattern **DOESN'T WORK**:

```tsx
import React, { FC } from 'react';
interface IProps { ... };
const MyComponent: FC<IProps> = ({ ... }) => ...
```

Fortunately, the following workaround works:

```tsx
const MyComponent: FC<IProps> = ({ ... }: IProps) => ...
```

Please upvote [the issue](https://github.com/reactjs/react-docgen/issues/387) if this is affecting your productivity, or better yet, submit a fix!

## More resources

Want to learn more? Here are some more articles on Storybook Docs:
Expand Down
2 changes: 1 addition & 1 deletion code/frameworks/react-vite/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type TypescriptOptions = TypescriptOptionsBase & {
/**
* Sets the type of Docgen when working with React and TypeScript
*
* @default `'react-docgen-typescript'`
* @default `'react-docgen'`
*/
reactDocgen: 'react-docgen-typescript' | 'react-docgen' | false;
/**
Expand Down
2 changes: 2 additions & 0 deletions code/lib/cli/src/automigrate/fixes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { angularBuilders } from './angular-builders';
import { incompatibleAddons } from './incompatible-addons';
import { angularBuildersMultiproject } from './angular-builders-multiproject';
import { wrapRequire } from './wrap-require';
import { reactDocgen } from './react-docgen';

export * from '../types';

Expand All @@ -42,6 +43,7 @@ export const allFixes: Fix[] = [
angularBuildersMultiproject,
angularBuilders,
wrapRequire,
reactDocgen,
];

export const initFixes: Fix[] = [missingBabelRc, eslintPlugin];
77 changes: 77 additions & 0 deletions code/lib/cli/src/automigrate/fixes/react-docgen.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import type { StorybookConfig } from '@storybook/types';
import { reactDocgen } from './react-docgen';

const check = async ({
packageManager,
main: mainConfig,
storybookVersion = '7.0.0',
}: {
packageManager: any;
main: Partial<StorybookConfig> & Record<string, unknown>;
storybookVersion?: string;
}) => {
return reactDocgen.check({
packageManager,
configDir: '',
mainConfig: mainConfig as any,
storybookVersion,
});
};

describe('no-ops', () => {
test('typescript.reactDocgen is already set', async () => {
await expect(
check({
packageManager: {},
main: {
typescript: {
// @ts-expect-error assume react
reactDocgen: 'react-docgen-typescript',
},
},
})
).resolves.toBeFalsy();

await expect(
check({
packageManager: {},
main: {
typescript: {
// @ts-expect-error assume react
reactDocgen: false,
},
},
})
).resolves.toBeFalsy();
});
test('typescript.reactDocgen and typescript.reactDocgenTypescriptOptions are both unset', async () => {
await expect(
check({
packageManager: {},
main: {
typescript: {
// @ts-expect-error assume react
reactDocgen: 'react-docgen-typescript',
reactDocgenTypescriptOptions: undefined,
},
},
})
).resolves.toBeFalsy();
});
});

describe('continue', () => {
test('typescript.reactDocgenTypescriptOptions is set', async () => {
await expect(
check({
packageManager: {},
main: {
typescript: {
// @ts-expect-error assume react
reactDocgenTypescriptOptions: {},
},
},
})
).resolves.toBeTruthy();
});
});
49 changes: 49 additions & 0 deletions code/lib/cli/src/automigrate/fixes/react-docgen.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { dedent } from 'ts-dedent';
import { updateMainConfig } from '../helpers/mainConfigFile';
import type { Fix } from '../types';

const logger = console;

interface Options {
reactDocgenTypescriptOptions: any;
}

/**
*/
export const reactDocgen: Fix<Options> = {
id: 'react-docgen',

async check({ mainConfig }) {
// @ts-expect-error assume react
const { reactDocgenTypescriptOptions } = mainConfig.typescript || {};

return reactDocgenTypescriptOptions ? { reactDocgenTypescriptOptions } : null;
},

prompt() {
return dedent`
You have "typescript.reactDocgenTypescriptOptions" configured in your main.js,
but "typescript.reactDocgen" is unset.
In Storybook 8.0, we changed the default React docgen analysis from
"react-docgen-typescript" to "react-docgen", which dramatically faster
but doesn't handle all TypeScript constructs.
We can update your config to continue to use "react-docgen-typescript",
though we recommend giving "react-docgen" for a much faster dev experience.
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#react-docgen-component-analysis-by-default
`;
},

async run({ dryRun, mainConfigPath }) {
if (!dryRun) {
await updateMainConfig({ mainConfigPath, dryRun: !!dryRun }, async (main) => {
logger.info(`✅ Setting typescript.reactDocgen`);
if (!dryRun) {
main.setFieldValue(['typescript', 'reactDocgen'], 'react-docgen-typescript');
}
});
}
},
};
4 changes: 2 additions & 2 deletions code/lib/core-server/src/presets/common-preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ export const previewBody = async (base: any, { configDir, presets }: Options) =>

export const typescript = () => ({
check: false,
// 'react-docgen' faster but produces lower quality typescript results
reactDocgen: 'react-docgen-typescript',
// 'react-docgen' faster than `react-docgen-typescript` but produces lower quality results
reactDocgen: 'react-docgen',
reactDocgenTypescriptOptions: {
shouldExtractLiteralValuesFromEnum: true,
shouldRemoveUndefinedFromOptional: true,
Expand Down
2 changes: 1 addition & 1 deletion code/presets/react-webpack/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export type TypescriptOptions = TypescriptOptionsBase & {
/**
* Sets the type of Docgen when working with React and TypeScript
*
* @default `'react-docgen-typescript'`
* @default `'react-docgen'`
*/
reactDocgen: 'react-docgen-typescript' | 'react-docgen' | false;
/**
Expand Down
24 changes: 1 addition & 23 deletions code/renderers/react/template/stories/ts-argtypes.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import type { Args, Parameters, StoryContext } from '@storybook/types';
import { inferControls } from '@storybook/preview-api';
import { ThemeProvider, themes, convert } from '@storybook/theming';

import { within } from '@storybook/testing-library';
import { component as TsFunctionComponentComponent } from './docgen-components/ts-function-component/input';
import { component as TsFunctionComponentInlineDefaultsComponent } from './docgen-components/ts-function-component-inline-defaults/input';
import { component as TsReactFcGenericsComponent } from './docgen-components/8143-ts-react-fc-generics/input';
Expand Down Expand Up @@ -81,27 +80,6 @@ export const TsJsdoc = { parameters: { component: TsJsdocComponent } };

export const TsFC = { parameters: { component: TsFCComponent } };

const addChromaticIgnore = async (element: HTMLElement) => {
const row = element.parentElement?.parentElement;
if (row?.nodeName === 'TR') {
row.setAttribute('data-chromatic', 'ignore');
} else {
throw new Error('the DOM structure changed, please update this test');
}
};

export const TsTypes: StoryObj = {
parameters: { component: TsTypesComponent },
play: async ({ canvasElement }) => {
// This play function's sole purpose is to add a "chromatic ignore" region to flaky rows.
const canvas = within(canvasElement);
const funcCell = await canvas.findByText('funcWithArgsAndReturns');
addChromaticIgnore(funcCell);
const namedNumericCell = await canvas.findByText('namedNumericLiteralUnion');
addChromaticIgnore(namedNumericCell);
const inlinedNumericCell = await canvas.findByText('inlinedNumericLiteralUnion');
addChromaticIgnore(inlinedNumericCell);
},
};
export const TsTypes: StoryObj = { parameters: { component: TsTypesComponent } };

export const TsHtml = { parameters: { component: TsHtmlComponent } };
3 changes: 1 addition & 2 deletions docs/api/main-config-typescript.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ Type: `'react-docgen' | 'react-docgen-typescript' | false`
Default:

- `false`: if `@storybook/react` is not installed
- `'react-docgen-typescript'`: if `@storybook/react` and `typescript` are installed
- `'react-docgen'`: if `@storybook/react` is installed

Only available for React Storybook projects. Configure which library, if any, Storybook uses to parse React components, [react-docgen](https://github.com/reactjs/react-docgen) or [react-docgen-typescript](https://github.com/styleguidist/react-docgen-typescript). Set to `false` to disable parsing React components.
Only available for React Storybook projects. Configure which library, if any, Storybook uses to parse React components, [react-docgen](https://github.com/reactjs/react-docgen) or [react-docgen-typescript](https://github.com/styleguidist/react-docgen-typescript). Set to `false` to disable parsing React components. `react-docgen-typescript` invokes the TypeScript compiler, which makes it slow but generally accurate. `react-docgen` performs its own analysis, which is much faster but incomplete.

<!-- prettier-ignore-start -->

Expand Down

0 comments on commit 2e4e691

Please sign in to comment.