search

single-spa / create-single-spa

https://single-spa.js.org/docs/create-single-spa
Other
132 stars 59 forks source link

Add react-refresh-webpack-plugin to react projects #258

Open joeldenning opened 3 years ago

joeldenning commented 3 years ago

cc @frehner who has worked on this in the past. I'm happy to add this if you don't have time @frehner - just created the issue to track what we discussed on Slack.

frehner commented 3 years ago

I'm going to implement this in my apps first to check to see if there are any other issues. Should only take an hour or so hopefully

frehner commented 3 years ago

We have discovered some issues that may make integrating this at the moment a headache. It may be worth waiting on this a bit until react-refresh and the webpack plugin are a little more stable.

nilzona commented 3 years ago

Hey @frehner .. just out of curiosity. What was the issues you discovered?

frehner commented 3 years ago

I believe either I or the library author has resolved all the issues that we had; we're using the latest version and are not having any problems at the moment. 🙂

nilzona commented 3 years ago

hey @frehner .. I used the latest create-single-spa to generate some react single-spa applications. I am then using import-map-overrides to point to a local webpack-dev-sever served with react-refresh and react-refresh-webpack-plugin. It doesn't work for me. HMR seems to work, but there's no actual changed applied to the dom.

Do you have any example where you got it working?

I created this issue in hope to get some help.

nilzona commented 3 years ago

The issue mentioned above has been resolved:

https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/502

It is now possible to enable HMR with the latest create-single-spa generated react libraries.

joeldenning commented 3 years ago

I tried this out a bit today and didn't get it working. I don't have further time today to work on it. Based on the comments in the thread you linked to, it seems that react-refresh needs to be a shared dependency or into the root config, and be loaded before react and react-dom? Making react-refresh a shared dependency won't be very easy because react-refresh only publishes CJS bundles to npm and systemjs doesn't load cjs bundles. Specifically, react-refresh references process which is not a real thing in browsers and only works when webpack fakes support for it in browsers, which would not happen for us.

So the next step would be to overcome that problem and get react-refresh loaded in the root config or as a shared dependency, then try again.

frehner commented 3 years ago

Alternatively, you need to have the React DevTools extension installed, and that has the side-effect of fixing the issue with the loading order

joeldenning commented 3 years ago

Ah thanks for pointing that out - after installing the react devtools the hot reloading does work! Would be nice to not require that of every single-spa user, though, or have to constantly document/answer questions about it. So I still think loading react-refresh as a shared dep would be ideal.

nilzona commented 3 years ago

@joeldenning ... I think you had the same issue as I had :) I was banging my head against the wall trying to figure it out and then when I installed React Dev Tools I got it working right away. Yes, it would be nice to not be dependent on React Developer Tools. Any ideas on how that could be a shared dep?

joeldenning commented 3 years ago

Yes but it's just would take a fair amount of effort - either create an esm bundle for it, or do it in the root config with System.set(). I won't get to it today.

alex-enchi commented 11 months ago

with latest webpack for me removing adding hot: true to the dev server worked wonders