-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser-utils): bump web-vitals to 5.1.0 #18091
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
base: develop
Are you sure you want to change the base?
Conversation
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
29c8b28 to
019cf99
Compare
| // Remove the above event listener since no longer required. | ||
| // See: https://github.com/GoogleChrome/web-vitals/issues/622 | ||
| removePageListener('visibilitychange', cb, { capture: true }); | ||
| }); |
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.
Bug: Persistent Listeners Leak Memory.
The whenIdleOrHidden function creates a memory leak by calling onHidden(cb) which adds permanent visibilitychange and pagehide event listeners that are never removed. These listeners persist even after the callback has executed via runOnce, causing unnecessary memory consumption. This affects LCP measurement cleanup where stopListening is called via this function but the onHidden listeners remain active.
d9171bb to
dcbfc72
Compare
Bumps the vendored-in web vitals library to include the changes between
5.0.2<->5.1.0from upstreamChanges from upstream
visibilitychangeevent listeners when no longer required #627Our own Changes
addPageListenerandremovePageListenerutilities because the upstream package changed the listeners to be added onwindowinstead ofdocument, so I added those utilities to avoid having to check for window every time we try to add a listener.