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

LCP Loses Entry Element After SPA Route Change #561

Open
dmitiiv opened this issue Nov 6, 2024 · 12 comments
Open

LCP Loses Entry Element After SPA Route Change #561

dmitiiv opened this issue Nov 6, 2024 · 12 comments

Comments

@dmitiiv
Copy link

dmitiiv commented Nov 6, 2024

When using the web-vitals library to track the Largest Contentful Paint (LCP) metric in a Single Page Application (SPA), we encounter an issue where the LCP entry element is lost upon changing routes. This problem arises because the web-vitals library captures the LCP metric based on the elements rendered on the page at the time of measurement.

In SPAs, content is often loaded dynamically, and when navigating to a new route, the relevant content may not be fully rendered or may change, causing the library to lose track of the largest contentful element. As a result, the reported LCP value may not accurately reflect the user experience, leading to misleading performance metrics.

How to reproduce

  1. Create any default SPA (Vite etc)
  2. Start listerning onLCP (web-vitals or web-vitals/attribution)
  3. Change route before the scroll on any other interaciton
@dmitiiv dmitiiv changed the title Title: LCP Loses Entry Element After SPA Route Change LCP Loses Entry Element After SPA Route Change Nov 6, 2024
@mmocny
Copy link
Member

mmocny commented Nov 6, 2024

This problem is at least partially fundamental to the Performance Timeline. The element references are not held strongly by the performance entry, and so if the page is dynamically updated then the reference may be GC-ed.

However, if you are saying that the web-vitals.js library is buffering the entry and not calling onLCP eagerly enough, I think there is an optional flag value { reportAllChanges: true } where you would be notified eagerly of each (useful) new LCP entry.

Would that help?

@tunetheweb
Copy link
Member

Change route before the scroll on any other interaciton

How can you change route without an interaction? Normally you'd have to click or take some kind of interaction to change the root (which would finalise the LCP for that first page). Without that, the LCP can continue to update (as it would for a non-SPA page for late-loaded content that is larger).

Currently there is a proposal to allow multiple LCP measure measurements for SPAs but even that assumes a route change is the result of an interaction.

@dmitiiv
Copy link
Author

dmitiiv commented Nov 6, 2024

How can you change route without an interaction? Normally you'd have to click or take some kind of interaction to change the root (which would finalise the LCP for that first page). Without that, the LCP can continue to update (as it would for a non-SPA page for late-loaded content that is larger).

You re right. And its related to the explanation above

This problem is at least partially fundamental to the Performance Timeline. The element references are not held strongly by the performance entry, and so if the page is dynamically updated then the reference may be GC-ed.

But when you try to handle the LCP metric in a callback or even early when Web-Vitals onLCP start observation (or LCPattribution) you got empty Element field in the LCP metric object

This problem is at least partially fundamental to the Performance Timeline. The element references are not held strongly by the performance entry, and so if the page is dynamically updated then the reference may be GC-ed.
And yes again. I described that in the issue

I described the way to reproduce the problem. Just try.
The library returns invalid metric I think, coz of it does not contain the target element

In other words... When you interact with a page to change a route, web-vitals start performs stopListening function and then report function and when the work is done and metric object is ready to be passed to client code, the metric object does not have a link on Node and route is changed

@dmitiiv
Copy link
Author

dmitiiv commented Nov 6, 2024

However, if you are saying that the web-vitals.js library is buffering the entry and not calling onLCP eagerly enough, I think there is an optional flag value { reportAllChanges: true } where you would be notified eagerly of each (useful) new LCP entry.

value { reportAllChanges: true } where you would be notified eagerly of each (useful) new LCP entry.

there is the same problem

@tunetheweb
Copy link
Member

tunetheweb commented Nov 6, 2024

OK I think we were confused about "As a result, the reported LCP value may not accurately reflect the user experience". The LCP TIME is correct (and so does accurately reflect the user experience), but the LCP target element may be empty if it has been garbage collected (which makes it more difficult for RUM data to help pinpoint the error). Which is more likely to happen for SPAs. Is that correct?

In #477 we worked around this for INP by saving the target element selector details at the time of the interaction (rather than when we reported the INP later at the end of the page life). We could also do this for LCP. However, this does have some cost as we're no need to run the selector code for every LCP candidate. We felt that was worthwhile for INP elements since it was fairly common there, but for LCP I'm not so sure. As I say, the SPA case (which this library specifically does not handle at present for LCP) is probably the most common reason.

Anyway, if you can confirm this is the issue, and that workaround would help (note you still wouldn't get the Target Element, but at least would get the Target selector), then we can consider whether to add that or not.

@dmitiiv
Copy link
Author

dmitiiv commented Nov 6, 2024

OK I think we were confused about "As a result, the reported LCP value may not accurately reflect the user experience". Rhe LCP TIME is correct (and so does accurately reflect the user experience), but the LCP target element may be empty if it has been garbage collected (which makes it more difficult for RUM data to help pinpoint the error). Which is more likely to happen for SPAs. Is that correct?

At least SPA) I met that in SPA. I'm not sure about the suggested solution for my purposes.

(which makes it more difficult for RUM data to help pinpoint the error)
This is a reason why I've created an issue. Such metric is almost useless as for me.

Anyway, if you can confirm this is the issue, and that workaround would help (note you still wouldn't get the Target Element, but at least would get the Target selector), then we can consider whether to add that or not.

And I'm afraid you cannot prepare selector coz of context (DOM node) is lost at the moment when handleEntries gets an entry but if somehow its possible and the cost is not too high, I think it would be useful for other depelopers

@mmocny
Copy link
Member

mmocny commented Nov 6, 2024

Barry: the same interaction that stops the LCP algorithm might also synchronously trigger the page change. It looks like web-vitals.js will only look specifically at keydown and click, not other interactions or scroll, and then will also wait until whenIdle. That means the page might be updated by the time callback is invoked.

I wonder if you could at least change to capture the query selector of the element synchronously in the event handler, before waiting for whenIdle to fire?

@tunetheweb
Copy link
Member

And I'm afraid you cannot prepare selector coz of context (DOM node) is lost at the moment when handleEntries gets an entry but if somehow its possible and the cost is not too high, I think it would be useful for other depelopers

handleEntries is called when the largest-contentful-paint performance entry is emitted. But that is then not emitted by web-vitals library (unless it's called with reportAllChanges: true) until the page is either hidden or an interaction happens, at which point the attribution happens.

It's still possible for the LCP node to be empty if there's only a short period between handleEntries and onReport, but it's less likely.

However, in that has using it with reportAllChanges: true should be a good test of that. And you're saying you tried this, and it did not help? Because in that case, the workaround would also not help.

@tunetheweb
Copy link
Member

I wonder if you could at least change to capture the query selector of the element synchronously in the event handler, before waiting for whenIdle to fire?

Ah yes the whenIdle won't help things. Capturing it synchronously is what onINP does now.

@tunetheweb
Copy link
Member

@dmitiiv could you test with the code changes in #562 and let us know if that resolves your issue?

@dmitiiv
Copy link
Author

dmitiiv commented Nov 7, 2024

@tunetheweb yes. Seems it works well. Thank you

@tunetheweb
Copy link
Member

Good to hear. I’ll try to get another v5 release candidate with this fix out soon.

Btw this is a work around due to some limitations of these web APIs. So it won’t fix all issues - see #563 (comment) for another example it won’t fix. But should make it better anyway.

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

3 participants