search

brillout / vite-plugin-mdx

Vite Plugin for MDX
MIT License
111 stars 36 forks source link

Fix react-refresh transform #56

Open Jinjiang opened 4 months ago

Jinjiang commented 4 months ago

Change the transform call from with .js suffix into with .jsx in order to ensure the useReactRefresh flag in the React plugin https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/src/index.ts#L184

silvenon commented 4 months ago

Thanks for your contribution, but the plugin you linked to is not the one that this MDX plugin is using, it's referring to @vitejs/plugin-react-refresh, so I don't think that modifying the id extension would make a difference.

If you're using this plugin with the latest version of the React Vite plugin, it's possible that you're also using a modern Vite version that this plugin doesn't support. The maximum supported version of @vitejs/plugin-react is v1.3.2 because vite-plugin-mdx supports up to Vite v2.

The ecosystem has already progressed so far past this plugin, is there anything stopping you from using @mdx-js/rollup? @vitejs/plugin-react even has an example with MDX.

silvenon commented 4 months ago

the plugin you linked to is not the one that this MDX plugin is using, it's referring to @vitejs/plugin-react-refresh, so I don't think that modifying the id extension would make a difference.

My bad, it's also taking the React plugin into account as well, apparently, but if you're using the correct version of @vitejs/plugin-react then it's also using imports to React to detect JSX, it's not only using the extension.

The change in this PR looks safe enough, I just need to find a way to verify whether it fixes a problem, feel free to lend me a hand by providing some context about the problem you were facing, and how you landed on this fix.

But I'd still like to know if you need to use vite-plugin-mdx over @mdx-js/rollup.