Skip to content
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

Do not override sockProtocol from config #835

Merged
merged 2 commits into from
Apr 25, 2024
Merged

Do not override sockProtocol from config #835

merged 2 commits into from
Apr 25, 2024

Conversation

darkexone
Copy link
Contributor

@darkexone darkexone commented Apr 16, 2024

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 #742 and #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.

Copy link

codesandbox bot commented Apr 16, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 25, 2024

A better idea would be to make a mechanism for reading the webSocketURL from the Webpack configuration (as suggested in #742 and #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 pmmmwh merged commit 88e1441 into pmmmwh:main Apr 25, 2024
5 of 8 checks passed
@pmmmwh
Copy link
Owner

pmmmwh commented Apr 25, 2024

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants