-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
feat: skip pinging the server when the tab is not shown #12698
feat: skip pinging the server when the tab is not shown #12698
Conversation
Run & review this pull request in StackBlitz Codeflow. |
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.
Nice! I like this change. Looks like we still support safari14, which visibilitychange
event is only supported since 14.1, but the worst case is that the visibilitychange
wait promise never resolves, and I don't think it's a big problem.
Oh, I didn't notice that. It seems Safari 14.0 supports |
packages/vite/src/client/client.ts
Outdated
} | ||
await wait(ms) | ||
} else { | ||
await Promise.race([wait(ms), waitForWindowShow()]) |
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.
This will register a new event handler for visibilitychange
every second while the visibility doesn't change (then resolve all of them at the same time. I think it would be good to create a single waitForWindowShow
promise and use it here until it resolves.
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.
Good catch! I fixed this 👍
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 didn't notice this too, I thought it would wait for window show altogether before continuing the while loop, maybe we could so a simple await waitForWindowShow()
here?
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.
Ah, you're right. I don't know why I wrote it like this 😅
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
Description
This PR makes
@vite/client
to skip pinging the server when the tab is not shown.By doing this, we can reduce non-necessary pings (#5228) and thus reduce the logs output in browser console (#12693).
close #5228
close #12693
similar to #6791
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).