search

pmmmwh / react-refresh-webpack-plugin

A Webpack plugin to enable "Fast Refresh" (also previously known as Hot Reloading) for React components.
MIT License
3.13k stars 192 forks source link

Do not override sockProtocol from config #835

Closed darkexone closed 4 months ago

darkexone commented 4 months ago

Hey, I came across a case where sockProtocol is incorrectly overwritten. My application does not use https, but is served through a proxy that uses https. I have https disabled in the webpack configuration, but the protocol read from the window is https. I tried passing the sockProtocol parameter, but found that wss is used all the time, resulting in itegration with overlay not working. I believe that the config passed by the developer (a resourceQuery) should take priority over a failsafe configuration.

A better idea would be to make a mechanism for reading the webSocketURL from the Webpack configuration (as suggested in https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/742 and https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/728), because this fix will help only for part of the problems with connecting to the wrong sockets. Until this mechanism is implemented, we should prefer the plugin configuration over the values read from the path.

codesandbox[bot] commented 4 months ago

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders
Open Preview

pmmmwh commented 4 months ago

A better idea would be to make a mechanism for reading the webSocketURL from the Webpack configuration (as suggested in https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/742 and https://github.com/pmmmwh/react-refresh-webpack-plugin/issues/728), because this fix will help only for part of the problems with connecting to the wrong sockets. Until this mechanism is implemented, we should prefer the plugin configuration over the values read from the path.

With the later versions of WDS v4 and WDS v5 we can actually re-use the "created" server so we no-longer need to parse any of their config. It'll probably come in the next version though.

pmmmwh commented 4 months ago

Thanks for the PR!