search

vitejs / vite-plugin-react-swc

Speed up your Vite dev server with SWC
MIT License
778 stars 50 forks source link

use with pre-compiled code #152

Closed joprice closed 1 month ago

joprice commented 9 months ago

I'm using reason-react to compile code to js, so my input does have contain raw jsx but pre-compiled calls to the react apis. I found that my files were being ignored, but with a few small tweaks I could get this plugin to work: One is passing js files through by adding

...
 : id.endsWith(".js")
    ? { syntax: "ecmascript", jsx: true}

and the other ignoring reason-react's compiler-generated makeProps function, to avoid hmr where fast-refresh is possible:

 if (isLikelyComponentType(value) || key == 'makeProps') return true;

Seeing as reason-react is a bit of a niche community and passing all js files through is probably not what most users would expect (especially as it may have performance implications), what would you recommend for supporting a workflow like this? Is there perhaps a way to do something like this as a plugin?

ArnaudBarre commented 9 months ago

Hum yeah I think this is better for the reason-react compiler to output files with ids (even virtual) that are not .js.

I'm not sure I get the second point. Can you setup a small example so I can look at the output and try to tweak the vite config to avoid the first issue?

joprice commented 9 months ago

The quickest setup I have to show as an example is still experimental and is might be painful to build if you're not using nix. It's using some overridden dependencies, pulling together a few different libs to try out reason-react SSR with a ocaml multi-core server, which take a while to build when not using a nix binary cache https://github.com/joprice/server-reason-react-example.

On the second point, the reason-react compiler produces files that don't only contain component exports. They can contain a pair of functions, make and makeProps, which in most cases get compiled away https://reasonml.github.io/reason-react/docs/en/components, but in some cases they are retained. This causes warnings that the file is not eligible for fast refresh, which is why I added the extra exclusion mentioned above.

ArnaudBarre commented 9 months ago

Thanks for providing the repro. I didn't try it for now because I want to better understand the second point before.

Do you know if there is more details documentation in this makeProps? For what I understand, makeProps will evolve in time when props signature change, and if you block the propagation of this function (by adding key == 'makeProps') then you will have have a mismatch between your component runtime and the function being effectively used to create props (because some other parts of the runtime will still have the reference to the initial makeProps fn, you can watch my talk at ViteConf later today to better understand what happen under the hood). There is technically ways to work around it, the simplest one being to invalidate only when the toString() output on the function changed.

I need to came up with an API to support this kind of edge case like the React Router loader functions pattern.

joprice commented 9 months ago

That makes sense. I’ll definitely check out the talk. For now this has been working for me but I need to add a case that demonstrates the custom props issue directly, and modify it as you’re saying to check for changes being ignored.

ArnaudBarre commented 1 month ago

I'm closing this issue for now. Since then I've added the parser option config that fix part of the problem. I've also started shipping undocumented and unstable API for hooking into HMR checks for Remix, and I can make it more official and more flexible if I have another usecase for it.

For anyone wanting better HMR for new compilation output / meta-framework feature, please open an issue with example and explanation on how you make sure that the extra import doesn't lead to stale code/broken HMR.