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

fix: reload only once on socket reconnect #2340

Merged
merged 2 commits into from
Mar 29, 2021

Conversation

tjk
Copy link
Contributor

@tjk tjk commented Mar 2, 2021

Network without the patch:

Screen Shot 2021-03-02 at 9 12 37 AM

With the patch:

Screen Shot 2021-03-02 at 9 14 53 AM

@@ -172,9 +172,10 @@ async function queueUpdate(p: Promise<(() => void) | undefined>) {
socket.addEventListener('close', ({ wasClean }) => {
if (wasClean) return
console.log(`[vite] server connection lost. polling for restart...`)
setInterval(() => {
const interval = setInterval(() => {
fetch(`${base}__vite_ping`)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this solution is not perfect if the fetch takes longer than the interval... but we can maybe ignore that very unlikely possibility (chain setTimeout would be safer)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think it's better to refactor it to use chained setTimeout instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@Shinigami92 Shinigami92 added p2-nice-to-have Not breaking anything but nice to have (priority) enhancement New feature or request 🔍 review needed labels Mar 23, 2021
@patak-dev patak-dev merged commit d73c1fa into vitejs:main Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants