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

[#5675] Reconnect HMR socket without reloading the browser page #16748

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tpresley
Copy link

Description

Fixes issue #5675 by seamlessly replacing the socket object in the client.js module when the server becomes available again after a socket disconnect.

The socket variable used in the code is already in scope to be replaced, but was being masked by a local socket variable of the same name in the setupWebSocket function. This PR simply renames the local socket variable, allowing the top level socket variable to be assigned the new socket object when reconnection is successful.

HMR works properly now after a socket reconnect without needing to reload the page.

Copy link

stackblitz bot commented May 22, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@bluwy
Copy link
Member

bluwy commented May 27, 2024

Is this safe? A file could be edited in between the reconnection, or a different server of a different project could started on the same port, and both cases should trigger a full reload to fetch the new contents?

Maybe I'm missing something. @sapphi-red do you have an idea to support this from the issue? Or is #5675 (comment) the fix you're suggesting? (Seems to be about a different issue though I think)

@sapphi-red
Copy link
Member

I don't think doing this is safe, too.

My suggestion (#5675 (comment)) should fix cases like #10840 (comment), #9007 (comment), #16536, #13125. I think I misunderstood the OP (#5675 (comment)) and thought it's a same thing because it said "Reconnect WebSocket to check HMR status" in the suggested solution.

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.

3 participants