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
janechu pushed a commit that referenced this pull request Jun 10, 2024
* add eslint rules for type imports

* add vite client types for SVG imports

* upgrade to Storybook 8

* migrate Storybook config for v8

* remove storybook manager script

* migrate Storybook preview script for v8

* separate type-only imports

* eslintrc change

* update tests to work with Storybook 8

* restore sideEffects config by using vite glob imports in storybook preview

- use globalThis for html function in tests

* remove unused dev dependencies

- Update csf
- Fix storybook helper types
- update docs

* fix parse5 type imports

* update api-report

* resolve lockfile

* Change files
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.

5 participants