chore: upgrade to storybook 7.x #1726
Conversation
- remove extraneous / outdated storybook plugins - upgrade packages to the latest available for storybook - amend documentation pages to support MDX2 format - fix remaining bugs - update test files to support latest story-utils and other dependencies - liberally use `act()` around test setup actions
size-limit report 📦
|
Codecov Report
@@ Coverage Diff @@
## next #1726 +/- ##
=======================================
Coverage 92.27% 92.27%
=======================================
Files 146 146
Lines 2575 2575
Branches 664 664
=======================================
Hits 2376 2376
- Misses 183 198 +15
+ Partials 16 1 -15 see 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -9,16 +10,22 @@ import * as stories from './Accordion.stories'; | |||
| const { Default } = composeStories(stories); | |||
|
|
|||
| describe('<Accordion />', () => { | |||
| generateSnapshots(stories); | |||
| generateSnapshots(stories as StoryFile); | |||
There was a problem hiding this comment.
I notice some usages of generateSnapshots require as StoryFile and others don't, do you know what's happening here?
There was a problem hiding this comment.
not exactly sure, but it could be the type changes related to some of the storybook libraries. The first sweep was to satisfy typescript to restore parity on yarn test, and we can see if something systematic is causing certain story files to need a type
| await act(async () => { | ||
| await user.click(accordionButton); | ||
| }); |
There was a problem hiding this comment.
Why do these events have to be wrapped in act, how'd you come to realize this?
There was a problem hiding this comment.
after upgrading dependencies, yarn test would output gigantic errors in console. what's strange is that for some of these user events, if you add the act() around them, it will complain that it's redundant 👎
For whatever reason, not all user. calls need this, and on older versions of the libraries, none did. It doesnt make obvious sense which is which
| @@ -7,7 +7,7 @@ const defaultArgs = { | |||
| label: 'Checkbox', | |||
| }; | |||
|
|
|||
| export default { | |||
| const meta: Meta<typeof Checkbox> = { | |||
There was a problem hiding this comment.
In cases like this, there were type errors that prevented tests from passing. Following the docs from storybook, this is the recommended typing to apply. What's weird is that types didn't break down on ALL tests, so not sure when types would get lost yet.
It's likely we should make these consistent in one way or the other, and favor aligning with this pattern since it mirrors the docs
There was a problem hiding this comment.
it's likely we should make these consistent in one way or the other, and favor aligning with this pattern since it mirrors the docs
👍
I did something similiar when upgrading the Learning platform to Storybook v7. I went though and changed every as Meta<Args> to satisfies Meta<Args> which is a bit better than as. Not saying satifies is better than const meta: Meta <...> just pointing out I ran into similar weirdness
There was a problem hiding this comment.
Yep yep, i recall that one, and tested it on a few places. I think there may be more issues with the story files since that didn't have an effect in some cases. It's a little bit mysterious...
|
How does testing for the deployed storybook or zeroheight work? |
@jeremiah-clothier I might not be answering your question but we can chat about it. For ZH, the link between it and storybook is either by embedded iframe, OR by the ZH integration, which are keyed on the individual story URLs/names respectively. The testing for deployed storybook itself is pretty straightforward; the github action must be able to pass all the existing checks before it can be published. I'll add a backlog item for making the test signatures consistent, and see if that resolves the need for the |
Summary:
act()around test setup actionsTest Plan: