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

Upgrade to Storybook 8 in FAST Foundation #6929

Merged
merged 15 commits into from
May 20, 2024

Conversation

radium-v
Copy link
Collaborator

@radium-v radium-v commented Mar 19, 2024

Pull Request

πŸ“– Description

This PR updates the FAST Foundation project to use Storybook 8.

  • Switch the Storybook configuration from Webpack to Vite (@storybook/html-vite)
  • Update the ESLint config in Foundation to force type-only imports to use import type ... from ...
  • Removes "sideEffects": false from the package.json side effect config is no longer affected

🎫 Issues

#6698

πŸ‘©β€πŸ’» Reviewer Notes

Most of the changes in this PR can be categorized into a few manageable piles:

  • All type-only imports and exports have to be explicitly separated out into their own statements.
  • Storybook changed its root element from #root to #storybook-root, and sometimes Playwright thinks it's okay to continue even though the page hasn't finished defining the components.

πŸ“‘ Test Plan

All tests in fast-foundation should pass as expected, though some individual test modules might be flaky in different environments.

βœ… Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

#6855

@radium-v
Copy link
Collaborator Author

I checked to see if anything we generate is affected by the removal of sideEffects: false - the only file that changed in dist/ was custom-elements.json, and all that changed was the sort order.

I'm still investigating if consumers of @microsoft/fast-foundation rely on us to have sideEffects: false for tree-shaking to work for them.

Copy link
Collaborator

@bheston bheston left a comment

Choose a reason for hiding this comment

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

Wow, thanks for doing this! I'm surprised there weren't more changes needed in the Stories or helper functions.

packages/web-components/fast-foundation/package.json Outdated Show resolved Hide resolved
@radium-v radium-v force-pushed the users/jokreitl/upgrade-storybook-8 branch from 9de2a4c to a1806d9 Compare March 25, 2024 20:03
@radium-v radium-v force-pushed the users/jokreitl/upgrade-storybook-8 branch from a1806d9 to 3411cac Compare May 20, 2024 18:23
@radium-v radium-v requested a review from janechu as a code owner May 20, 2024 18:23
@radium-v radium-v force-pushed the users/jokreitl/upgrade-storybook-8 branch from 3411cac to b9eb5f4 Compare May 20, 2024 18:25
@radium-v radium-v merged commit 5dac26f into master May 20, 2024
10 checks passed
@radium-v radium-v deleted the users/jokreitl/upgrade-storybook-8 branch May 20, 2024 18:55
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.

None yet

5 participants