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: Upgrade react-docgen to 6.0.x and improve argTypes #23825

Merged
merged 20 commits into from
Oct 4, 2023

Conversation

shilman
Copy link
Member

@shilman shilman commented Aug 14, 2023

Extends #22324

What I did

NOTE: This only applies to VITE projects, not Webpack ones yet!!!

Extends #22324:

  • Update react-docgen to 6.0.x
  • Add support for fs resolver

Followup:

  • Update webpack too

Also improve argType generation:

  • Add experimental support for converting unions of literals into enum, which generate proper controls
  • Fix table summary types for array, object, and function

How to test in your project

To test this out in a React Vite TS project, install the canary version listed at the top of this PR into a project by manually updating its package.json to update all the @storybook/* packages from this monorepo:

    "@storybook/react": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/react-vite": "0.0.0-pr-23825-sha-7875282b",
    ...

Once you've installed the packages, edit .storybook/main.ts to contain:

  typescript: {
    reactDocgen: 'react-docgen',
  },

The behavior should be the same as before, but running storybook dev should be dramatically faster than before.

If you run into issues, please comment on this PR!

How to test in a development sandbox

  1. Create a development sandbox in this branch, e.g.
yarn task --task sandbox --start-from auto --template react-vite/default-ts
  1. Edit .storybook/main.ts to contain:
  typescript: {
    reactDocgen: 'react-docgen',
  },
  1. Add a new file starbucks-size.ts in src/stories directory:
export type StarbucksSize = 'tall' | 'grande' | 'venti';
  1. Update the CLI-generated Button.tsx component with a new prop:
  /**
   * How large should the button be at Starbucks?
   */
  starbucksSize?: StarbucksSize;
  1. Observe the result in the controls panel:
image

Checklist

Needs tests:

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)

🦋 Canary release

This pull request has been released as version 0.0.0-pr-23825-sha-7875282b. Install it by pinning all your Storybook dependencies to that version.

More information
Published version 0.0.0-pr-23825-sha-7875282b
Triggered by @shilman
Repository storybookjs/storybook
Branch shilman/fix-react-docgen-enum
Commit 7875282b
Datetime Tue Aug 22 10:30:44 UTC 2023 (1692700244)
Workflow run 5937622254

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=23825

@@ -4,7 +4,7 @@
"rootDir": "./src",
"types": ["node"],
"resolveJsonModule": true,
"strict": true
"skipLibCheck": true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was this change necessary for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. This PR is mostly changes from #22324 and I guess this is one of those.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A @ndelangen special, here: a26c431 (#22324)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems wrong, indeed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done, because the TS compiler options here are different than the settings in the react-docgen repo. This lead to one of the CI jobs to fail, because the it was validating react-docgen types with the different options (mostly strict stuff). I fixed a lot of it and enabled some more strict options in react-docgen, so maybe it works now.

@yannbf
Copy link
Member

yannbf commented Oct 3, 2023

React.FC is not yet supported in react-docgen. I will see if I can find some time for this.

Hey @danez did you have any luck with React.FC? Thank you so much for taking a look into it! <3

@shilman shilman changed the title React: Update react-docgen to 6.0.x and improve union handling React: Update react-docgen to 6.0.x and improve data handling Oct 4, 2023
@shilman shilman changed the title React: Update react-docgen to 6.0.x and improve data handling React: Update react-docgen to 6.0.x and improve argTypes Oct 4, 2023
@shilman shilman changed the title React: Update react-docgen to 6.0.x and improve argTypes React: Upgrade react-docgen to 6.0.x and improve argTypes Oct 4, 2023
@shilman shilman merged commit 109eb20 into next Oct 4, 2023
16 checks passed
@shilman shilman deleted the shilman/fix-react-docgen-enum branch October 4, 2023 04:37
@github-actions github-actions bot mentioned this pull request Oct 4, 2023
19 tasks
@shilman
Copy link
Member Author

shilman commented Oct 4, 2023

@danez FYI we're going to evaluate https://www.npmjs.com/package/typescript-react-function-component-props-handler and will report back. If it works well, it could be a good candidate to incorporate in the default settings. cc @valentinpalkovic

@danez
Copy link
Contributor

danez commented Oct 4, 2023

Nice. React.FC is already supported on the main branch of react-docgen but I noticed some other issues that i want to fix first, especially because i had to make a breaking change again.

@valentinpalkovic
Copy link
Contributor

@shilman, @danez I took a quick look at the custom resolver , and it failed pretty quickly

Bildschirmfoto 2023-10-05 um 15 55 10

@danez Do you need some support? I would say I don't spend time fixing the third-party resolve, but instead, I would rather wait and/or support your work on the main branch.

@shilman
Copy link
Member Author

shilman commented Oct 11, 2023

@danez that's great news. please let us know how we can support! we're going to release this upgrade PR in Storybook 7.5 next week, and will merge the companion PR #24165 for 8.0 once FC support is merged & released. Happy to adjust the plans if it helps you out in any way.

@rachelholtz
Copy link

I went to try this out but I am getting an error when running storybook.
I updated all @storybook packages:

    "@storybook/addon-a11y": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/addon-essentials": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/addon-interactions": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/addon-links": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/blocks": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/react": "0.0.0-pr-23825-sha-7875282b",
    "@storybook/react-vite": "0.0.0-pr-23825-sha-7875282b",

Already had typescript: {reactDocgen: 'react-docgen'} in main.ts.

Did a clean install.

When I run storybook I get this error:

@storybook/cli v7.4.3
WARN Failed to load preset: "@storybook/react-vite/preset"
Error: Cannot find module '@storybook/react-vite/preset'
Error: Invariant failed: No builder configured in core.builder

So I tried to specifically install the canary of @storybook/cli and it didn't help.

I have reread the instructions above and I feel like I am missing something. Storybook runs fine with 7.1.0 but was hoping to test the updates for react-docgen. We have issues around pulling in descriptions for arg types when a component Type is extending another component's types.

@yannbf yannbf mentioned this pull request Oct 17, 2023
5 tasks
@danez
Copy link
Contributor

danez commented Oct 19, 2023

I released version 7.0 of react-docgen which supports React.FC and all the other helpers like React.PropsWithChildren. @yannbf The output might still be different for the Button example above because react-docgen does not support styled-components, but it should be at least getting closer.

@ndelangen
Copy link
Member

cc @shilman ☝️

@gluharry
Copy link

First, great work on this excellent product!
Second, do you guys plan on migrating to v7.0 of docgen as released any time soon?

I tried running v7.0 (by npm override) with sb v7.5 and am getting unexpected and strange errors.

@yannbf
Copy link
Member

yannbf commented Oct 30, 2023

Hey @harryyaprakov thank you! We are planning on doing that for Storybook 7.6. This PR might be of interest to you

Can you maybe elaborate and share a reproduction that can display the errors you are mentioning? It will be really useful for us.

cc @shilman

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants