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

[Issue 2507] upgrade storybook to version 8 #2508

Merged
merged 6 commits into from
Nov 12, 2024
Merged

Conversation

doug-s-nava
Copy link
Collaborator

@doug-s-nava doug-s-nava commented Oct 17, 2024

Summary

Fixes #2507

Time to review: 20 mins

Changes proposed

Upgrade to the latest version of Storybook.

This PR is largely needed in order to work around a security issue with esbuild that is present in storybook until 8.4. A workaround was applied in #2502 that can hopefully now be reverted.

Note that to get this working I ran the suggested upgrade script mentioned below, which eventually worked, but then had to fix the reference to our next config, which was not being found for some reason, in order to get storybook to load

https://storybook.js.org/docs/migration-guide

Context for reviewers

Most of our stories are currently broken, so set a baseline expectation on what is currently working before jumping in and assuming that the upgrade broke stuff

Test steps

  1. on main, npm install && npm run storybook
  2. visit http://localhost:6006
  3. get a sense of which stories are working and not working on main
  4. run steps 1 and 2 on this branch
  5. VERIFY: everything that worked on main still works here

Additional information

it looks like Go 1.23 support in esbuild only came along last month in version 0.24 https://github.com/evanw/esbuild/releases/tag/v0.24.0

version 0.24 of esbuild is only supported in storybook core as of v8.4.0-alpha.4 storybookjs/storybook@c40aba2

now that storybook 8.4 is officially released, we can upgrade and hopefully remove the workaround built in #2502

upgrade pitfalls

  • peer dependency issue with @storybook/blocks preventing an npm install
    • resolved by explicitly adding @storybook/blocks: ^8.4.2 as a dev dependency, and deleting package-lock
  • missing @types/lodash
    • previously was required by @storybook/blocks but now isn't anymore, so we need to include it explicitly
  • missing @types/node-fetch
    • previously was required by @storybook/core-common but now isn't anymore, so we need to include it explicitly

@doug-s-nava doug-s-nava changed the title [DNM] [Issue 2507] upgrade storybook to version 8 [Issue 2507] upgrade storybook to version 8 Nov 5, 2024
Copy link
Contributor

@emilycnava emilycnava left a comment

Choose a reason for hiding this comment

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

Verified that storybook between main and this branch are consistent and show similar failing components, unrelated to upgrade

@acouch
Copy link
Collaborator

acouch commented Nov 12, 2024

Testing locally, now that we are on 8 we might be able to include RSCs

@doug-s-nava doug-s-nava merged commit 37226b2 into main Nov 12, 2024
11 checks passed
@doug-s-nava doug-s-nava deleted the ds/storybook-8 branch November 12, 2024 19:15
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.

Upgrade Storybook to v8
3 participants