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

feat(storybook): storybook 7 #2533

Merged
merged 25 commits into from Jan 18, 2024
Merged

Conversation

jpandersen87
Copy link
Contributor

Summary

Upgrades project's Storybook to version 7.1. Storybook's auto-migrations were ran which affected configuration and stories (function to object-based stories).

In addition:

  • internal-only stories had their filenames changed (*.stories.tsx -> *.stories.internal.tsx) to prevent Storybook from attempting to load them with the matching glob pattern (which caused warnings as Storybook would fail to load the commented-out default export).
  • The USWDS svg referenced from USWDS' github hosting has been copied to a .storybook/public folder in order to reference locally instead (in the documentation story, and as the branding logo in USWDS.js)
  • adds manual version resolution for jackspeak to fix yarn v1 bug with storybook 7.1 (see [Bug]: string-width dependency stops storybook from executing storybookjs/storybook#22431 (comment) )
  • updates typescript to 5.1.6 (fixes transient dependency minimum typescript issues)
    • some new type errors in project were corrected that appeared from update
  • updates @typescript-eslint dependencies to 6.2.0
  • updates jest to 29.6.1
  • updates ts-jest to 29.1.1
  • adds jest-environment-jsdom

Related Issues or PRs

Closes #2266

How To Test

Run storybook locally via yarn run storybook

Copy link
Contributor

@gidjin gidjin left a comment

Choose a reason for hiding this comment

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

looks good from a code perspective, I'll hold off approval until one of our designers can do a once over.

@@ -46,7 +46,7 @@ export const useModal = (isInitiallyOpen?: boolean): ModalHook => {

export const getScrollbarWidth = (): string => {
// Only run in browser
if (typeof document !== undefined) {
if (typeof document !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: nice catch

@gidjin gidjin requested a review from shkeating July 31, 2023 21:32
@shkeating
Copy link
Contributor

It's the removal of the icon in the custom controls alert intentional? See below.

Screenshot_20230801-093651

@jpandersen87
Copy link
Contributor Author

It's the removal of the icon in the custom controls alert intentional? See below.

Screenshot_20230801-093651

Appears to have been an issue with defaultValues passed to the story (I think sb 7 deprecated defaultValue in favor of the args property). I made an update that should have the custom control SiteAlert story appear correctly.

custom.d.ts Outdated Show resolved Hide resolved
.happo.js Outdated Show resolved Hide resolved
.github/workflows/deploy-storybook.yml Outdated Show resolved Hide resolved
.github/workflows/build-and-test.yml Outdated Show resolved Hide resolved
@brandonlenz
Copy link
Contributor

@jpandersen87 do you mind resolving the conflicts on this PR? That should let CI run so we can move forward with these changes. Thanks for the contributions!

If you'd like to be added to all-contributors, we can also do that by pinging the all-contributors bot in a comment 😄

shkeating
shkeating previously approved these changes Aug 9, 2023
@werdnanoslen
Copy link
Member

werdnanoslen commented Jan 17, 2024

@jpandersen87 thanks for the updates! Could you take a look at these new package conflicts please? 👇

@jpandersen87
Copy link
Contributor Author

@jpandersen87 thanks for the updates! Could you take a look at these new package conflicts please? 👇

Done. Also fixed the babel error that came up after initial merge (updated package.json versions to prevent transitive dependency mangling, always a fun issue XD).

- fix eslint config for webpack resolve to prevent module import eslint errors
- replace deprecated ComponentStory with StoryFn
@brandonlenz
Copy link
Contributor

@jpandersen87 Looks like all that's still failing is Prettier. Can you run yarn format:fix? It should update LanguageSelector. Looks like that was the only offender.

@jpandersen87
Copy link
Contributor Author

@jpandersen87 Looks like all that's still failing is Prettier. Can you run yarn format:fix? It should update LanguageSelector. Looks like that was the only offender.

Fixed. Apologies for the minor issue churn.

@jpandersen87
Copy link
Contributor Author

jpandersen87 commented Jan 18, 2024

Also to note: This will get the project to SB major version 7 but will still be out of date. Due to the delicate nature of transient dependency versions (especially since react-uswds is still using React 17) I'm sticking with the version in this branch as a stable point that can be followed by anyone with a PR to catch SB up to latest 7.X.X.

@shkeating shkeating merged commit 8f65ec4 into trussworks:main Jan 18, 2024
7 checks passed
@brandonlenz
Copy link
Contributor

@all-contributors please add @jpandersen87 for code, maintenance

Copy link
Contributor

@brandonlenz

I've put up a pull request to add @jpandersen87! 🎉

@jpandersen87
Copy link
Contributor Author

jpandersen87 commented Jan 18, 2024

I appreciate the patience with my churn as I ensured my contributions follow react-uswds norms!

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

Successfully merging this pull request may close these issues.

[feat] Upgrade Storybook (version 7+)
5 participants