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

React: Set react-docgen to default TS docgen #24165

Merged
merged 23 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b63a043
React: Set react-docgen as the default
shilman Sep 13, 2023
2e395b8
Don't ignore docgen cells
shilman Sep 13, 2023
7df4fc0
Merge branch 'next' into shilman/default-to-react-docgen
shilman Sep 30, 2023
a6823db
React: Fix react-docgen handling for arrays, records, functions
shilman Oct 2, 2023
c54a3ae
Fix types
shilman Oct 3, 2023
3a59f73
Fix snapshots
shilman Oct 3, 2023
9f31bae
Merge branch 'release-8-0' into shilman/default-to-react-docgen
shilman Oct 4, 2023
183e5c9
Merge branch 'release-8-0' into shilman/default-to-react-docgen
valentinpalkovic Oct 5, 2023
4aa689e
Merge branch 'next' into shilman/default-to-react-docgen
shilman Oct 22, 2023
acaa9f9
Merge branch 'release-8-0' into shilman/default-to-react-docgen
shilman Oct 22, 2023
6e40835
Add migration notes & automigration
shilman Nov 2, 2023
1ed5769
Merge branch 'release-8-0' into shilman/default-to-react-docgen
shilman Nov 2, 2023
a3d76db
Update yarn.lock
shilman Nov 2, 2023
626f335
Update .nvmrc
shilman Nov 2, 2023
a1adfdb
Remove React.FC caveat for react-docgen
shilman Nov 2, 2023
2db6744
Merge branch 'shilman/default-to-react-docgen' of github.com:storyboo…
shilman Nov 2, 2023
2f863cc
Fix MIGRATION
shilman Nov 2, 2023
544393e
Merge branch 'release-8-0' into shilman/default-to-react-docgen
shilman Nov 2, 2023
da5d703
Merge branch 'release-8-0' into shilman/default-to-react-docgen
ndelangen Nov 2, 2023
37d0aed
Merge branch 'release-8-0' into shilman/default-to-react-docgen
shilman Nov 3, 2023
7526fee
Review comments
shilman Nov 3, 2023
a1889c5
Merge branch 'next' into shilman/default-to-react-docgen
shilman Nov 30, 2023
1fa44fc
Fix typos & ts check
shilman Nov 30, 2023
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
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-generated control. 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" or a much faster dev experience.
shilman marked this conversation as resolved.
Show resolved Hide resolved

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 }, 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