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

✨ add storybook v7 support #829

Merged
merged 28 commits into from
Oct 27, 2023
Merged

✨ add storybook v7 support #829

merged 28 commits into from
Oct 27, 2023

Conversation

nilshah98
Copy link
Contributor

@nilshah98 nilshah98 commented Oct 18, 2023

Description -

  • update evalStorybookEnvironmentInfo to grep storybook v7 version
  • update evalStorybookStorySnapshots to support fetching all stories even with storyStorev7: true
  • update packages to storybook v7 and update tests (start-storybook to storybook dev)
  • update minimum node version required to 16 and update github actions to use node 16
  • verify cacheAllCSFFiles with storyStorev7: false and also older storybook version
  • debug flaky tests

@nilshah98 nilshah98 added the ✨ enhancement New feature or request label Oct 18, 2023
@nilshah98 nilshah98 marked this pull request as ready for review October 19, 2023 13:44
@nilshah98 nilshah98 requested a review from a team as a code owner October 19, 2023 13:44
Copy link
Contributor

@itsjwala itsjwala 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 so far, nice!

  • we'll need backward compatibility with v6 and have to figure out how we can add tests for same.
  • we'll create E2E build on Percy and verify visual regression if any with v6 vs v7 and callout.
    • how many customers use node 14? we might need to have CI tests on node 14 as well.

src/utils.js Show resolved Hide resolved
src/start.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@nilshah98
Copy link
Contributor Author

I have noticed that the tests are flaky.
I am not sure if this was the behaviour before as well, might need to debug this.
Example -
Successful run
Unsuccessful run

Both of them are for the same commit ids

Copy link
Contributor

@itsjwala itsjwala 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, some customary changes are pending.

apart from what we discussed in sync.

src/utils.js Show resolved Hide resolved
test/.storybook/main.js Outdated Show resolved Hide resolved
test/.storybook/mainV6.js Outdated Show resolved Hide resolved
test/storybook.test.js Show resolved Hide resolved
Copy link
Contributor

@itsjwala itsjwala left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@nilshah98 nilshah98 merged commit af8fa6c into master Oct 27, 2023
5 checks passed
@nilshah98 nilshah98 deleted the add-storybookv7-support branch October 27, 2023 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants