Fix React HMR in story files #53
Conversation
| plugins.push(require('@vitejs/plugin-react-refresh')()); | ||
| plugins.push(require('@vitejs/plugin-react-refresh')({ | ||
| // Do not treat story files as HMR boundaries, storybook itself needs to handle them. | ||
| exclude: [/\.stories\.(t|j)sx?$/, /node_modules/], |
There was a problem hiding this comment.
Probably, also excluding node_modules would lead to bugs with linked dependencies which HMR should be managed by vite
There was a problem hiding this comment.
There should be also ability to override the exclude, as not everyone names files as stories
There was a problem hiding this comment.
Probably, also excluding node_modules would lead to bugs with linked dependencies which HMR should be managed by vite
Excluding node_modules is the default behavior of plugin-react-refresh, so I wanted to keep that behavior for now, until/unless we hear of problems.
There should be also ability to override the exclude, as not everyone names files as stories
There is, it is possible to change any of this config through the use of viteFinal in the storybook configuration file.
There was a problem hiding this comment.
It's also possible to read the list of stories (or the list of file globs) from the config variable, I think?
There was a problem hiding this comment.
Unfortunately it looks like stories from the config is not exposed to the builder. I logged out the options and didn't see it, and it's not in the type definitions, from what I can tell: https://github.com/storybookjs/storybook/blob/next/lib/core-common/src/types.ts#L164
There was a problem hiding this comment.
@shilman should that config option maybe be exposed in the builder API?
One problem is that if Vite is using another glob library than Storybook, there could be a mismatch between the two even if the glob pattern is the same. So neither will be a fool-proof solution.
There was a problem hiding this comment.
After looking into this, it's true that we can get the stories as @shilman suggested. However, those are micromatch globs, and @vitejs/react-fast-refresh uses picomatch, which does not allow brace expansion. We would need to resolve the entire list of files and pass those to the plugin. Since this whole approach is a bit of a bandaid, I propose that we hold off on any more complicated exclusion logic until/unless we hear of problems. Maybe we'll find a better solution entirely that works for more than just the react framework.
|
I think this looks good as a first fix! It solves the 95 % use case, and the rest can be improved upon later? |
Today I spent some time trying to figure out why autoreload for story files doesn't work. This pull request adds remark to README about naming of story files for autoreload to work. This is related to: storybookjs#53
Fixes #3.
This "fixes" HMR in react story files by using the new
excludeoption in@vitejs/plugin-react-refreshto avoid treating story files as HMR boundaries, instead letting changes bubble up into the core storybook client so that it can update the manager when stories are added / deleted / changed. When storybook detects that a story file has changed, it will trigger a reload of the preview iframe. So, it's not exactly full seamless HMR, but it addresses the main problem of story previews not changing upon save.To test, run
yarn install, thencd packages/example-reactandyarn storybook. Open the button story in the browser, then make changes to the button story file (add a story, remove a story, change a story, etc). You should see the changes reflected in the browser without requiring a manual page refresh.