-
-
Notifications
You must be signed in to change notification settings - Fork 136
feat: add reactRefreshHost
option to support module federation HMR
#420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@ArnaudBarre this is my first PR here, let me know if everything looks good 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of the complexity added by module federation, but this small feature doesn't cost much to add so why not.
@sapphi-red @patak-dev @bluwy @hi-ogawa opinions?
Adding this looks fine to me for the same reason with you. I'd like to hear from @underfin what they think about it as they have been working on MF related stuff recently. |
Just wondering as I'm not clear of the relationship of the host & remote server, does relying on Vite's |
@bluwy Interesting, I'll try to add a test setup in the repo for this usecase so that I can try out |
f0a3147
to
cb99053
Compare
Thanks for the quick feedback & good suggestions @ArnaudBarre! I just committed all your requested changes, tested, & works as expected. @sapphi-red I'm super curious about that as well. However for those of us using module federation in a multi build system, this is still necessary. I would love to vite all the things but I'm living in a world that still webpacks all the things 😅 @bluwy Thanks for the suggestion. I don't think that will work for assets on the remote server because if I change the origin, it will look for assets on the host server which won't be there. |
reactRefreshPrefix
to support module federation HMRreactRefreshHost
option to support module federation HMR
@ArnaudBarre I think we are all clear to merge this. Anything else you need me to address? |
@sapphi-red I let you decide when you prefer to merge given your other open PRs. I think we could also add this to other plugins |
cb99053
to
05c0f55
Compare
@sapphi-red @ArnaudBarre I rebased/updated according to your last merged pr moving code to common. Tested & works as expected. Side question, are you planning to do a release anytime soon? Because I haven't seen one since November 2024. |
Thanks for updating your PR! We didn't release anything in the past months because no new bug fix or feature was merged, but we will do a release shortly after merging this. |
05c0f55
to
9ed7d36
Compare
Makes sense 👍 I fixed conflicts again and added changes to plugin-react-swc as well. |
9ed7d36
to
db9d3f4
Compare
Fixed conflicts & edited readme + changelog per review comments. |
I think it is a good idea to support mf react hmr. The host and remote could be use host react-refresh runtime. The previous bug caused by the remote react-refresh runtime is not called |
Description
This resolves HMR not working in a module federation context when using the
@module-federation/vite
plugin.This PR simply adds a new option
reactRefreshPrefix
in a backwards compat way to allow prefixing the react-refresh url which by default is:/@react-refresh
The reason this works is the following example:
Allowing dev to set prefix in the plugin will force the react-refresh plugin from remote files to point at the host react-refresh module:
The react refresh url becomes
http://localhost:3000/@react-refresh
.Screen.Recording.2025-03-25.at.8.23.09.PM.mov
Additional context
module-federation/vite#183 (comment)
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).