Compatibily with @vitejs/react-plugin #3
Replies: 3 comments 14 replies
-
Thanks for the detailed write-up of your current rationale @ArnaudBarre! About configurations options, my take is that we could release the plugin without most options as you propose, and then wait for users to request them with proper use cases. Supporting only the automatic jsx runtime for example may be a good idea, given that users can still fall back to the current react plugin to support legacy. About configuring SWC, I see your point in pushing to try to keep it as standard as possible so it is easier to later change things on our side. But we have seen that providing an escape hatch like About the check for refresh boundaries, keeping HMR and a warning by default doesn't sound bad if it is what Next and other react frameworks are doing. I would like to hear the opinion of others here (cc @IanVS, @cyco130, @frandiox, @aleclarson) |
Beta Was this translation helpful? Give feedback.
-
vite exports |
Beta Was this translation helpful? Give feedback.
-
As far as I know, vite's If you need it instead, you will be able to provide swc's minify like vite-plugin-swc-only that I just found. |
Beta Was this translation helpful? Give feedback.
-
Configurations options
For reference here are the current options in 3.0.0-alpha.0:
include / exclude
This could be easily implemented, but I'm wondering if there is a usage for it.
For exclude, maybe some people want to ship un-transpiled non jsx files directly to the browser. But given that in a real world React codebase most of the code is in JSX or even TS/TSX, I'm questioning the need.
For include, I think this is because people want to transpile node_modules? But I think that a lot of work has been done supporting non-standard syntax in dependencies pre-bundling via esbuild.
The complexity in the plugin is low, but the runtime cost is not zero. The current implementation for the babel plugin uses various regex compare to path.extname + map access in this plugin
fastRefresh
This is the whole point of downloading 60mb of Rust. For punctual opt-out, if this plugin has very few options, people can just comment the plugin and get back to transpiling via esbuild.
jsxRuntime
This is already supported but by looking at the esbuild configuration. This can be changed for simplicity and compatibility.
jsxImportSource
Easy to implement but for people using TS it can be a duplication with TSConfig. Reading it like most tools does is a possibility but a non-zero cost when done in JS.
jsxPure
From my research this is not supported by SWC. This was added very recently do esbuild for a use case of a still WIP library. I'm not sure if I want to bother the maintainer of SWC for this myself. Plus this is a build only requirement, which is not an issue if SWC is dev-only.
babel
Obviously this doesn’t make sense. But the possibility to configure SWC is an open question. See conclusion
Use SWC for builds
esbuild is very well integrated in Vite, and with automatic jsx runtime support, there is no big need to use SWC in build mode. This makes the plugin a bit simpler.
I did an experiment to be able to benchmark but the issue is that disabling esbuild also disable minification. The build time went from 35s to ~44s on my professional project, but it's difficult to know the cause without fixing the minification issue. I will try to fix it in the coming days to make a fair benchmark (if you have a fix let me know).
For reference, the Babel plugin uses a mix of both in v3
Check for refresh boundaries
React refresh is an awesome tool, but it comes with some limitations, that sadly, are not shared the React core team. It's only on the React Native docs, but Next and Gatsby also made their own version.
The frustrating part as a React dev is that some tools tend to hide these limitations and just throw a full page reload at you without any notice (CRA, Vite, Bun). Next is the only good student here and logs a warning (with a stack trace 😕) in the terminal and in the browser (but it's not visible if logs are preserved). Gatsby gets around it by bundling every page and forbids export from pages.
IMO the limitations are worth the benefits, and they should be enforced by tools. There is nothing more painful that starting to debug a component written by a coworker and getting full-reload because the file also export a function at the bottom. So I made this ESLint plugin (And I'm still very surprised that the core team didn't make one compared to well-designed ESLint rules for hooks).
Side note: There is the same issue in Vite core with circular imports. It triggers a full page reload without explanations.
In the short term, I understand that not everyone want to install ESLint on every React project, so I could add some runtime checks like IanVS did for the Babel version. But I think that both plugins should at least log a warning. My opinion would be to keep HMR and display an overlay.
In the long term, we should push react-refresh implementations (only Babel and SWC in my knowledge, Bun has one but not usable outside the runtime currently) to take into account these limitations so that toolchain don't need to re-analyse all exports via additional plugins and fragile naming conventions. In my opinion they should throw an error pointing to the exact export that is causing an issue, but they could also give back a transpiled version without fast refresh enable and a warning that the toolchain can log. In that second case, the current check for
code.includes("$RefreshReg$")
would be enough and won't require additional checks and post invalidation.Conclusion
SWC and esbuild are both incredible tools that support out of the box a lot of features like automatic jsx runtime and satisfies operator even before TS 4.9 is stable. I think that writing custom AST plugins attach to a specific compiler is not worth it anymore and that this plugin should have a very small API surface so that we can make performance improvements (ex caching), use a different compiler for builds or make it easy to migrate to a new one later. For this reason I don't think the plugin should allow custom configuration for SWC. People with special needs can still fork it (it's currently only 120 lines long) or patch it.
I would even go one step further and only support automatic jsx runtime so that the plugin don't have to do some
gymnastic to allow people to skip React imports. This also removes the need for the
jsxImportSource
option.I think that keeping esbuild for build is safe and (still to be confirmed) even make builds faster. Also, keeping the possibility to use the same native tool for both transpiling and bundling is a game changer in terms of build speed. The web app I mentioned earlier that builds in 35s can be bundled in ~3s with a custom script using esbuild (most time being spent on loading and running PostCSS).
I'm open to discuss all the points above and know more about different usage of React toolchains. The only thing I'm sure of is that given the extensibility of Vite plugin system, multiple plugins with different trade-offs can exist. The main question is what should be the default trade-offs that the core team recommends?
Beta Was this translation helpful? Give feedback.
All reactions