ml-in-barcelona / jsoo-react

js_of_ocaml bindings for ReactJS. Based on ReasonReact.
https://ml-in-barcelona.github.io/jsoo-react
MIT License
138 stars 19 forks source link

recommended workflow for hmr/fast refresh #131

Closed joprice closed 2 years ago

joprice commented 2 years ago

Is there a setup that allows for hmr or fast refresh? I enabled 'hot' in my webpack config, but I've run into a few issues: When I change a file in vim (running coc.nvim with ocaml-lsp), the js file is not generated. Explicitly running dune build gets it to generate, but then I see the error below:

[WDS] Hot Module Replacement enabled.
client:52 [WDS] Live Reloading enabled.
client:55 [WDS] App updated. Recompiling...
client:150 [WDS] Errors while compiling. Reload prevented.
Module build failed: Error: ENOENT: no such file or directory, open '../_build/default/src/react-test/main.bc.js'
jchavarri commented 2 years ago

I enabled 'hot' in my webpack config

I don't think this is needed, by default live-reload should work. It's not as fancy as hot reload, but probably more reliable.

If you run make dev from project root and yarn server from example folder, you should see how changing any example component (e.g. Main.re) should trigger Dune to rebuild the necessary files, which will generate a new file in _build/default/example/src/App.bc.js, which will trigger webpack to rebuild. Isn't that the case?

jchavarri commented 2 years ago

I just tried adding hot to webpack config and the steps above keeps working. Maybe the path '../_build/default/src/react-test/main.bc.js' is not ok on your webpack config?

joprice commented 2 years ago

I'm seeing this on my own example repo, where I've based the webpack and dune setup off of this one, only modifying the webpack file in a few places, e.g. setting inject: true on the HtmlWebpackPlugin. I can try to clone and run the example and see if I can repro the above error.

joprice commented 2 years ago

I see similar errors when running the example:

[WDS] App updated. Recompiling...
[WDS] Errors while compiling. Reload prevented.
../_build/default/example/src/App.bc.js
Module build failed: Error: ENOENT: no such file or directory, open '/Users/josephprice/dev/jsoo-react/_build/default/example/src/App.bc.js'
[WDS] App updated. Recompiling...
[WDS] App hot update...
[HMR] Checking for updates on the server...
[HMR] Cannot apply update. Need to do a full reload!
react_devtools_backend.js:4061 [HMR] Error: Aborted because ../_build/default/example/src/App.bc.js is not accepted
Update propagation: ../_build/default/example/src/App.bc.js -> 0
jchavarri commented 2 years ago
[HMR] Cannot apply update. Need to do a full reload!

This is not an error, it's just hot reloader saying it has to do full reload because it doesn't know how to inject new state (the reliability issues I mentioned above). Does the page actually show if you go to localhost:8000?

Another thing I don't understand is how is HMR / hot reloading messages appearing in logs if the example does no use HMR. Could you try with the exact same configuration please? I would try is to remove hot first and try with the simpler webpack configuration that the example uses. Then add hot or other configuration if needed, but starting from a working state, so we can understand what's going on step by step.

jchavarri commented 2 years ago

After more thinking, I am not sure it makes sense to support HMR with jsoo-react in any case, as jsoo generates one single JS file with everything in it, while HMR expects multiple JS files across the app. There is no difference between hot module reloading and live reload when there is only 1 large JS module in the app.

One day, there might be a way to generate individual js files from OCaml modules with jsoo, but until then, I think live reloading (which the example in this repo and the jsoo-react-template use) is the way to go.

As there is nothing actionable from the jsoo-react library perspective, I will close the issue, but feel free to reopen if I misunderstood, or there are any related issues when setting up a project.

joprice commented 2 years ago

That makes sense. I did find this issue, which has a related pr that might eventually lead to the ability to compile to separate modules: https://github.com/ocsigen/js_of_ocaml/issues/1161

joprice commented 2 years ago

I think I narrowed the above webpack error to this line, where the previous file is deleted before moving the new temp file into place: https://github.com/ocsigen/js_of_ocaml/blob/ae78b6ccebbeabb26232e22f8b2361e79b8c5542/compiler/lib/stdlib.ml#L729-L730

davesnx commented 2 years ago

Hey @joprice. Totally, while running dune build (and I think other forms as well) it will nuke the files and re-generate them. This goes against the principle of hot module replacement, where only one module is re-generated and other modules that your bundle are loaded into the page can exchange old implementations with new ones and keep working as they were never changed.

I'm not sure if there's anything we could to do enable it in the current form of jsoo.

PS. The link that you shared points to the Jsoo stdlib module where they wrap a few functions to use while building/linking and other duties of the compiler.