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

Add clarification regarding event listener in event batching section #502

Open
mikep-dev opened this issue Jul 3, 2024 · 0 comments
Open

Comments

@mikep-dev
Copy link

mikep-dev commented Jul 3, 2024

I believe the section regarding event batching requires some clarification.
https://github.com/GoogleChrome/web-vitals?tab=readme-ov-file#batch-multiple-reports-together

The provided code works fine, but I feel it's worth emphasizing the importance of attaching listener for the visibilitychange event to the correct target. Check out this demo:
https://stackblitz.com/edit/stackblitz-starters-faxsov?file=index.html

In the demo I've attached event listener to document and window and configured onCLS to add metric to queue and log message to console.

image

As you can see, if you were to use document.addEventListener instead of global addEventListener or window.addEventListener you would lose CLS data, since CLS metric is reported after that.

This happens because onCLS reports data on visibilitychange on document but it attaches its listener inside onFCP callback. That means our listener that sends analytics data will be attached (and executed) earlier if its attached to the same target. The solution is simple: take advantage of event bubbling and attach the listener to window so that it executes after onCLS internal listener.

I've made a mistake and targeted document and wasted time scratching my head wondering why all other metrics are collected on time, but CLS was always late. I believe adding a proper note to README might save some developers time in the future, for example:

Note: to make sure your event listener is executed after all metrics have been reported, attach it to window object with addEventListener or window.addEventListener instead of document.addEventListener, which is internally used by some reporters.

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

No branches or pull requests

1 participant