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

[WNMGDS-2698] Upgrade Storybook #3003

Merged
merged 17 commits into from
Jul 24, 2024
Merged

Conversation

zarahzachz
Copy link
Collaborator

WNMGDS-2698

Upgraded Storybook 7 to Storybook 8. Comments will explain what changed and why, as some of our features were deprecated.

I was able to remove the @storybook/addon-mdx-gfm package, but ended up adding @storybook/addon-webpack5-compiler-babel as part of the migration, so no obvious big perf wins here.

.storybook/docs/DocumentationTemplate.mdx Show resolved Hide resolved
.storybook/main.ts Show resolved Hide resolved
.storybook/main.ts Show resolved Hide resolved
.storybook/main.ts Show resolved Hide resolved
],
features: {
buildStoriesJson: true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

buildStoriesJson is no longer a valid value for features. Checked out the older documentation to see what it did and couldn't find a 1:1 replacement in v8 of the docs. Removing it didn't seem to impact anything locally, but this is worth testing more vigorously.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this just became a default option, but they've renamed the stories.json file to index.json and changed its structure slightly. We rely on it in storybook-docs.test.ts and stories.test.ts. The import file needs to be updated, and the import { stories as needs to change to import { entries as.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed this, but now there are TypeScript errors.

@@ -156,7 +156,6 @@ const preview: Preview = {
},
},
parameters: {
actions: { argTypesRegex: '^on[A-Z].*' },
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

argsTypesRegex broke the SB migration several times and the only way to continue was in its removal. The migration guide gets into the thick of it, but it seems the preferred way to target specific events in each Story.

packages/docs/content/components/dropdown.mdx Show resolved Hide resolved
@zarahzachz zarahzachz added Type: Internal This item relates to internal tooling/maintenance Type: Breaking This item causes a breaking change. dependencies Pull requests that update a dependency file labels Apr 1, 2024
@zarahzachz zarahzachz requested a review from pwolfert April 1, 2024 03:01
yarn.lock Outdated
Comment on lines 7742 to 8205
"@types/react@*", "@types/react@>=16", "@types/react@^18.2.6":
"@types/react@*", "@types/react@^18.2.6":
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are now TypeScript errors, and they seem to be failing on class components (whether in our project or dependencies). It could possibly come from this version change for @types/react, but I lack evidence to back that hypothesis. When I ran yarn why @type/react in this branch and main, the top result stayed the same. The odd entry present in both of them is this:

=> Found "@types/react-dom#@types/react@17.0.59"

which is probably due to this strange mismatch in types that we've allowed to persist:

"@types/react": "^18.2.6",
"@types/react-dom": "^17.0.10",

Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that changing these entries to the following DOES NOT fix it:

"@types/react": "^17.0.39",
"@types/react-dom": "^17.0.11",

pwolfert and others added 5 commits May 9, 2024 15:16
* WIP: Try upgrading the react types and working through errors

Here's the summary of errors we're starting with:

```
Found 109 errors in 28 files.

Errors  Files
     4  packages/design-system/src/components/Accordion/Accordion.stories.tsx:8
     3  packages/design-system/src/components/Alert/Alert.stories.tsx:6
     1  packages/design-system/src/components/Autocomplete/utils.tsx:15
    11  packages/design-system/src/components/ChoiceList/ChoiceList.stories.tsx:7
     4  packages/design-system/src/components/Drawer/Drawer.stories.tsx:29
    12  packages/design-system/src/components/Dropdown/Dropdown.stories.tsx:8
     6  packages/design-system/src/components/Dropdown/Dropdown.tsx:171
     1  packages/design-system/src/components/Dropdown/DropdownMenuOption.tsx:66
     5  packages/design-system/src/components/HelpDrawer/HelpDrawer.stories.tsx:29
     4  packages/design-system/src/components/IdleTimeout/IdleTimeout.stories.tsx:10
     4  packages/design-system/src/components/PrivacySettingsDialog/PrivacySettingsDialog.stories.tsx:17
     6  packages/design-system/src/components/Spinner/Spinner.stories.tsx:6
    23  packages/design-system/src/components/TextField/TextField.stories.tsx:10
     2  packages/design-system/src/components/Tooltip/Tooltip.tsx:357
     1  packages/design-system/src/components/UsaBanner/UsaBanner.stories.tsx:5
     1  packages/design-system/src/components/web-components/ds-accordion/ds-accordion.tsx:22
     1  packages/design-system/src/components/web-components/preactement/define.test.tsx:35
     1  packages/docs/src/components/content/ContentRenderer.tsx:111
     1  packages/docs/src/components/content/EmbeddedExample.tsx:17
     2  packages/docs/src/components/layout/SideNav/SideNav.tsx:25
     2  packages/docs/src/components/layout/TableOfContents.tsx:43
     1  packages/docs/src/components/page-templates/BlogPage.tsx:18
     1  packages/docs/src/pages/blog.tsx:33
     2  packages/ds-healthcare-gov/src/components/Accordion/Accordion.stories.tsx:8
     2  packages/ds-medicare-gov/src/components/Card/Card.stories.tsx:6
     5  packages/ds-medicare-gov/src/components/HelpDrawer/HelpDrawer.stories.tsx:31
     1  packages/ds-medicare-gov/src/components/SimpleFooter/SimpleFooter.stories.tsx:6
     2  packages/ds-medicare-gov/src/components/Stars/stars.stories.tsx:6
```

* Fixing the first batch of errors (down to 89 from 109)

* Remove custom docs config for Drawer storybook page and fix VRTs

Add new interaction VRTs for dialog and drawer, and don't have any of the stories open them automatically

* Trying to fix missing Drawer story controls but failing

I was hoping fixing these types would do it.

* Ignore errors coming from react-aria types

* Two different ways of getting around the errors for inlining the doc page JSX

* Down to 24 errors

* Finished fixing all the errors in the core package (down to 20 now)

* Huh, the errors just go away after making some edits to the yarn lockfile

I think I'll go back and undo some of my previous commits to see if I need them

* Just realized there was some dead code in here

* Revert "Ignore errors coming from react-aria types"

This reverts commit f4fac88.

* Revert more of the react-aria changes

This reverts part of commit ffc035d.

* I can revert this change too

* Oh no, these new arg tables do not look good

It seems as though the type inference has gotten worse in this version. Instead of the type for Badge `size` being listed as `big` (only supports big), it is now reported as `literal`. For `string | React.ReactNode`, it only says `union`. Instead of `type` being `checkbox` and `radio`, it’s `union`. Instead of `ReactNode` for `React.ReactNode`, it prints out as `ReactReactNode`

* Revert "Oh no, these new arg tables do not look good"

This reverts commit 6811f4b.

* Changing the default settings to use the typescript docgen results in much fewer changes

See storybookjs/storybook#26496
@zarahzachz
Copy link
Collaborator Author

zarahzachz commented Jul 19, 2024

This upgrade requires more work. Union types change on the args table, which results in flaky VRT results. Waiting on SB dependency to resolve this.

* Use the new beta version of `react-docgen-typescript` that has union-member sorting

Right now I'm trying to figure out why the prop table on the Drawer page isn't rendering

* The arg table was gone because it wasn't included in the template
In the interaction tests, we were trying to use features of the story (the toggle) that didn't exist, which is why they were failing. I think the original reason I changed the drawer stories was to make them consistent with the dialog stories, and I was also able to consolidate them down to one. That necessitated the interaction tests because the drawer was no longer open by default for a screenshot to be taken.
@zarahzachz zarahzachz merged commit d0fffd6 into main Jul 24, 2024
1 check passed
@zarahzachz zarahzachz deleted the WNMGDS-2698/upgrade-storybook branch July 24, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Type: Breaking This item causes a breaking change. Type: Internal This item relates to internal tooling/maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants