fix(common): fix LCP image detection with duplicate URLs#67081
fix(common): fix LCP image detection with duplicate URLs#67081SkyZeroZx wants to merge 1 commit intoangular:mainfrom
Conversation
mmalerba
left a comment
There was a problem hiding this comment.
please also add tests for the cases mentioned below
| existingState.count++; | ||
| } else { | ||
| const newObservedImageState: ObservedImageState = { | ||
| priority: isPriority, |
There was a problem hiding this comment.
should priority also be a count? that way we can decrement and realize that it is no longer priority if it hits 0
There was a problem hiding this comment.
Since we also generate preload links when we're in SSR, I'm not sure (as I recall, once it's preloaded it stays in the browser cache), so we can say that it would persist.
| } | ||
| } | ||
|
|
||
| updateImage(originalSrc: string, newSrc: string) { |
There was a problem hiding this comment.
doesn't this need to be updated as well? as is it would move the full count to the new src, I assume its only intended to move one of them?
There was a problem hiding this comment.
Updated, although in this case I don't have a concrete idea of how we could test it as such in an e2e test.
I see we don't have unit tests given the nature of LCPImageObserver
Addresses an issue where the LCP image observer incorrectly identified LCP elements when the same image URL was used multiple times on a page Fixes angular#53278
04a97ef to
2bc2b6f
Compare
This change adds a counter for each image URL. The observer only treats an image as gone when the count hits zero, so LCP stays accurate even with duplicate URLs.
Fixes #53278