-
Notifications
You must be signed in to change notification settings - Fork 109
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
Fully support Vite 4, remove @vitejs/plugin-react, update @storybook/mdx1-csf #548
Conversation
@joshwooding, any idea what would be causing the parse errors in this PR? I don't recall seeing this before. |
Oh right, we had a problem with the original approach in storybook 7 as well when we tried to support Vite 4. The results of sending MDX through the vite react-plugin still contain JSX at the end, whereas previously all the jsx was converted to javascript. We never really figured out a solution to this in SB 7, we punted and said we'd only support MDX2, and we updated the way that mdx2-csf works so that it can handle jsx on its own (storybookjs/mdx2-csf#30). Maybe we need to do the same thing to mdx1-csf? @ndelangen do you think that would be possible? I also heard that SB 7 might need to continue supporting mdx-1 in an opt-in way, so I think that change to mdx1-csf might be needed regardless. Note: I've confirmed that using |
I'm not sure what's happening in the Workspaces example, something to do with the structure of the monorepo-within-a-monorepo, maybe. With the latest changes, we can support mdx1 and mdx2. I've also removed the addition of |
Fixes #547
This also updates the version of mdx1-csf to one that no longer requires the use of the react vite plugin to compile jsx, it handles compilation itself. So, we no longer need to include
@vitejs/plugin-react
in all projects, and react projects will need to make sure it's installed. That makes this a breaking change.And this also updates Vite to 4 in the examples, and fixes some missing dependencies.