-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Update Vite builder docs a bit #18187
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 5a62b13. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
|
||
Vite automatically restarts and begins the prebundling process if it detects a new dependency. This pattern breaks with Storybook and throws confusing error messages. If you see the following message in your terminal: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These issues have been straightened out in vite 2.9+, and I'd like to avoid the extra confusion that this workaround causes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IanVS thanks for clearing this up, appreciate it, looking at the builder's Readme we need to also update it as well to avoid some confusion. I'll follow up with you on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, there are a few things I need to clean up there. 👍
|
||
### HMR seems flaky | ||
|
||
Saving a component story does not initiate a hot module replacement. Instead, a complete reload happens. You can update your component file or save it to see the changes in effect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't quite correct. Changes are always shown, but when editing story files, a reload occurs rather than true HMR. There's a flicker of the screen, but that's about it, and it still usually only takes ~1 second, which is still quite fast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clearing that up, appreciate it.
docs/builders/vite.md
Outdated
|
||
<!-- prettier-ignore-start --> | ||
|
||
<CodeSnippets | ||
paths={[ | ||
'common/storybook-vite-builder-error-optimized.js.mdx', | ||
'common/storybook-vite-builder-react-docgen.ts.mdx', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React and Vue3 projects have automatic docgen. One tricky part is how we choose whether to use typescript or non-typescript docgen for react, so I've expanded on that a bit, with an example of how to configure it.
port, | ||
server: devServer, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual integration between the vite server and storybook dev server, so I think it's important to show it.
resolve: { | ||
alias: { exampleAlias: 'something' }, | ||
// Use the same "resolve" configuration as your app | ||
resolve: (await import('../vite.config.js')).default.resolve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an example of how to re-use configuration between the app's config, and the storybook config, and the resolve
setting is something that normally needs to be the same between the two.
resolve: (await import('../vite.config.js')).default.resolve | ||
// Add dependencies to pre-optimization | ||
optimizeDeps: { | ||
include: ['storybook-dark-mode'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using mergeConfig
, it becomes easy to extend the pre-optimized dependencies, so it's nice to show that here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IanVS thank you very much for taking the time to help us improve the documentation. Really appreciate it 🙏 ! Left one small item to address and we're good to go in merging this! Would love to hear your thoughts on it.
Hope you have a great day!
Stay safe
docs/builders/vite.md
Outdated
|
||
Update your `viteFinal` configuration and include the new dependencies as follows: | ||
Storybook’s [automatic argType inference](https://storybook.js.org/docs/react/api/argtypes#automatic-argtype-inference) is currently only available for React and Vue3 projects. In react projects, if typescript is found in the project's package.json, we assume the components are written in typescript, and use `react-docgen-typescript`. If this causes problems, you can use `react-docgen` by configuring the `typescript` option in your `.storybook/main.js`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IanVS we have room for improvement here. We can probably go with:
Currently, [automatic argType inference](../api/argtypes.md#automatic-argtype-inference) is only available for React and Vue3. With React, the Vite builder defaults to `react-docgen-typescript` if TypeScript is listed as a dependency. If you run into any issues, you can revert to `react-docgen` by updating your Storybook configuration file (in `.storybook/main.js|ts`) as follows:
Merging! |
Issue:
What I did
This makes a few tweaks to the new docs for the vite builder (https://storybook.js.org/docs/6.5/react/builders/vite). I'll leave some inline comments to explain my reasoning.
How to test
If your answer is yes to any of these, please make sure to include it in your PR.