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

[Storybook] upgrade Storybook to v8 #7659

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Apr 8, 2024

Summary

closes #7646

This PR upgrades Storybook and related dependencies to v^8.x

🔗 Migration documentation

Upgrade testing and findings (main impacting changes)

  1. Storybook v8 changes to be compiler agnostic. Currently available when migrating are SWC and Babel. (link)

    ✅ Storybook builds fine with SWC as well as Babel compilers
    ✅ very few updates required to make them work
    ✅ it's easy to use either or compiler

    1.1. required changes

    • remove actions: { argTypesRegex: '^on[A-Z].*' } preview setting from preview.tsx (link)
    • add waitFor for usage of userEvent.keyboard in test file 🤷‍♀️
    • Babel specific: exclude op_mob >= 1 in the .browserslistrc
  2. Storybook v8 furthermore changes from react-docgen-typescript to react-docgen as default to analyze React component props and auto-generate controls. (link)

    ❗ The main change is around the docgen package.

2.1. react-docgen

example output: EuiButton

Screenshot 2024-04-05 at 16 19 47

🚫 missing props (due to complex/custom types not being resolved)
🚫 completely missing jsdoc strings due to not resolved types
🚫 inferred types based on default prop value is unknown
🚫 inferred (and overwriting) types (based on storybook args)

examples:

// won't allow resolving the included types
type Props = ExclusiveUnion<EuiButtonPropsForAnchor, EuiButtonPropsForButton>;
// doesn't work
type PropsForAnchor<T, P = {}> = T & {
  href?: string;
  onClick?: MouseEventHandler<HTMLAnchorElement>;
} & AnchorHTMLAttributes<HTMLAnchorElement> &
  P;

type EuiButtonPropsForAnchor = PropsForAnchor<
  EuiButtonProps,
  {
    buttonRef?: Ref<HTMLAnchorElement>;
  }
>;

// works if we update types to simplify (extra effort)
// simplifying the AnchorProps
type AnchorProps = {
  href?: string;
  onClick?: MouseEventHandler<HTMLAnchorElement>;
} & AnchorHTMLAttributes<HTMLAnchorElement>;

// removing the wrapping with PropsForAnchor and using simplified AnchorProps instead
type EuiButtonPropsForAnchor = EuiButtonProps &
  AnchorProps & {
    buttonRef?: Ref<HTMLAnchorElement>;
  };
// story config
{
 args: {
  // this will result in the prop type being shown as 'boolean' instead of prop type definition of 
  // `MouseEventHandler<HTMLButtonElement> | MouseEventHandler<HTMLAnchorElement>`
    onClick: false,
 }
}

2.2. react-docgen-typescript

example output: EuiButton

Screenshot 2024-04-05 at 16 32 00

👍 resolved complex/custom types (same as before, not 100% depending on type complexity)
👍 jsdoc strings available (same as before, not 100% but the majority)
👍 better preset control types (due to resolved types)
👍 args do not overwrite prop types

Comparing SWC and Babel performance

Both setups were run locally (M3 Macbook pro with 36 GB memory).

  • both SWC and Babel compilers work running our scripts for dev and build

    • SWC and Babel with react-docgen are about the same when running build
    • SWC with react-docgen-typescript is about 12% faster than Babel when running build
  • SWC

    • build: react-docgen is around 8% faster than with react-docgen-typescript
    • dev: react-docgen is around 17% faster than with react-docgen-typescript
  • Babel

    • build: react-docgen is around 20% faster than with react-docgen-typescript
    • dev: react-docgen is around 18% faster than with react-docgen-typescript

QA

  • ensure Storybook builds and that overall all stories load and their control tables are working as before on production

@mgadewoll mgadewoll added documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog labels Apr 8, 2024
@mgadewoll mgadewoll changed the title [storybook]: upgrade Storybook to v8 [storybook] upgrade Storybook to v8 Apr 8, 2024
@mgadewoll mgadewoll changed the title [storybook] upgrade Storybook to v8 [Storybook] upgrade Storybook to v8 Apr 8, 2024
@mgadewoll
Copy link
Contributor Author

mgadewoll commented Apr 10, 2024

🗒️ Based on first alignments we likely want to first upgrade and ensure parity with the current v7 state.

This would mean using react-docgen-typescript over react-docgen. In terms of SWC vs Babel it looks that we would be ok to use SWC as it works and is slightly faster than Babel.

After that, we could look at additional iterations to update/improve e.g. how types are resolved and storybook controls are auto-generated based on docgen.

@mgadewoll mgadewoll marked this pull request as ready for review April 10, 2024 15:49
@mgadewoll mgadewoll requested a review from a team as a code owner April 10, 2024 15:49
@mgadewoll mgadewoll requested a review from tkajtoch April 10, 2024 15:49
.storybook/main.ts Outdated Show resolved Hide resolved
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

It seems like your branch needs rebasing with latest main :) I tested your PR locally and checked https://eui.elastic.co/pr_7659/storybook to ensure it still builds properly in CI.

@mgadewoll mgadewoll force-pushed the storybook/7646-v8-upgrade branch from c45615e to da8a53b Compare April 12, 2024 14:46
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

This is looking solid! 🚢

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@mgadewoll mgadewoll merged commit 7f8b631 into elastic:main Apr 16, 2024
7 checks passed
@mgadewoll mgadewoll deleted the storybook/7646-v8-upgrade branch April 16, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Storybook 8.x
5 participants