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

Detect lazy-loaded LCP image performed using a JS library (instead of native lazy loading) #15636

Open
siakaramalegos opened this issue Nov 28, 2023 · 12 comments
Assignees
Labels

Comments

@siakaramalegos
Copy link

Feature request summary

LCP images that are lazy-loaded using a JavaScript library are not detected, and this is a large percentage of the total population of lazy-loaded LCP images - around half:
https://almanac.httparchive.org/en/2021/performance#fig-20
https://almanac.httparchive.org/en/2022/performance#lcp-lazy-loading
https://calendar.perfplanet.com/2022/lazy-loading-lcp-images-why-does-this-anti-pattern-happen/

I think most of the above queries checked for a class name containing the string "lazy". Some might have looked for a data-src attribute.

The numbers might be higher. We use a different query string for Shopify since we have something like 80% of our shops using this antipattern and around 70-95% are using JS libraries:

COUNTIF(REGEXP_CONTAINS(snippet, r'loading="lazy"|lazyload|data-src|swiper-lazy|data-rimg|data-lazy')) AS `Lazy loaded`

What is the motivation or use case for changing this?
The advice is a bit aggressive at promoting adding lazy loading but does not catch probably half the cases where lazy loading went too far. If we could balance the "add lazy loading" advice with better detection of poorly lazy loaded practices, then that would help.

We've done some work on the Shopify platform to help theme devs make the right decisions, but they are still using Lighthouse which can be problematic. For example, with card lists - to not accidentally lazy load on desktop, they need to eager load potentially the first 6-9 cards, for example. However, when stacked in a mobile viewport, those eager loaded images are now more than the px threshold for suggesting to lazy load below-the-fold. So they end up lazy loading them and if they are using lazysizes, Lighthouse never detects that the LCP was lazy loaded.

How is this beneficial to Lighthouse?
Making the audits more accurate and not encouraging potentially more harmful changes (lazy loading above the fold) in favor of trying to accommodate the "lazy load below the fold" audit. We still have a long way to go in educating developers to not lazy load above the fold. This impacts performance across the entire web.

@adamraine
Copy link
Member

prioritize-lcp-image should cover cases where the LCP image is lazily loaded via JS. If the LCP was loaded via JS then it won't be preloaded and so this audit should fail. That being said, we can consider consolidating the explicit loading=lazy advice into the prioritize-lcp-image audit to reduce confusion.

As for deferring offscreen images, images with loading=eager (which is the default behavior) are also ignored as explicitly setting the priority is an indication of some implementation Lighthouse doesn't understand. We're unsure if this is a practice we want to encourage just to make the audit go away though.

Ultimately, it's going to be tricky to come up with a definition for lazy loading via JS, but it is something we think would be useful so let's keep this open.

@siakaramalegos
Copy link
Author

I think we can detect it in the same way we are looking for it in our SQL queries. If we take a look at audits for sites with this problem, we can see that they are only more encouraged to lazy load which is exactly the problem. Devs that use libraries for lazy loading will never see an audit that discourages them from lazy loading the LCP:

https://pagespeed.web.dev/analysis/https-zacbrownband-com/or1mam7rfr?form_factor=mobile
https://pagespeed.web.dev/analysis/https-massie-com-br/rn9hbkz7a0?form_factor=mobile

@tunetheweb
Copy link
Member

tunetheweb commented Jan 2, 2024

Yeah, to me those two sites should fail the Preload Audit and doesn't:

image

You can see it's used the final element (which includes the correct src attribute) for the LCP element but Lighthouse hasn't realised that this was added later and was not in the initial HTML (which just had a dummy placeholder LQIP in the src attribute initially).

So yes the element was in the initial HTML, but not in a usable form (at least for LCP) until the JS kicked in and added the correct src attribute. So preloading it is still good advice here (or even better don't lazy load it, so you don't need to preload it!).

@adamraine
Copy link
Member

@tunetheweb Lighthouse detects the element should be preloaded on https://zacbrownband.com/. Can you provide a repro case for the report in your image we think it may be a bug.

@tunetheweb
Copy link
Member

On PSI it doesn't detect that it should be preloaded as the audit passes?
https://pagespeed.web.dev/analysis/https-zacbrownband-com/or1mam7rfr?form_factor=mobile

image
@adamraine
Copy link
Member

Ah gotcha I was using Lighthouse directly. Will investigate.

@adamraine
Copy link
Member

adamraine commented Jan 8, 2024

Update: PSI is still using the old Lighthouse scoring methodology. We detect that the LCP request should be preloaded and so it is listed in the table. If the LCP request does not need to be preloaded then the table would be empty. For whatever reason though, our load simulator detects that preloading the request would have no impact on LCP.

In the old scoring methodology 0 savings means the audit will pass but this isn't the case in the latest version of LH which PSI has not been updated to. Example report. So I think this is working as intended, it's just a matter of updating PSI to the latest version.

@tunetheweb
Copy link
Member

Good news. I wonder if the 0ms estimated saving is somehow due to the fact the element is in the initial HTML and it hasn't realised it's with a different src? Cause that is definitely odd (and potentially confusing to a user who might decide to ignore this audit because of that!).

@adamraine
Copy link
Member

I opened #15737 as the root cause of the 0 savings is related to some other issues we have been seeing with LCP graphs.

@siakaramalegos
Copy link
Author

siakaramalegos commented Jan 9, 2024

For me, I don't see how suggesting to preload that image is a good solution. I understand how it could help overcome part of the technical problem, but the impact will be you're propagating a new bad idea on top of the original bad idea. To explain more:

  • By not telling them to stop lazy loading the LCP, they aren't getting the message that lazy-loading images above the fold is bad. So they will keep doing 100% lazy loading on all images.
  • By telling them instead to preload visible images, you're starting to suggest to them to start preloading more. If you have 9 visible images, that's now 9 preloads and some are probably not as important as others. And now you have bandwidth contention issues. And they'll start to preload other things they think are "important" even though they are discoverable in HTML, messing up the priorities even more.

What's the difficulty in detecting the root cause? My guess is it's not a technical difficulty since I was able to detect it with my bookmarklet. See https://github.com/siakaramalegos/priorities-bookmarklet/blob/main/script.js#L9. Help me understand the reluctance here a bit more?

@tunetheweb
Copy link
Member

@siakaramalegos we're in the middle of reengineering the LCP audits as detailed in #13738 . After that it will be more about prioritizing the LCP image appropriately (also reflected in the suggested rename of the audit from preload-lcp-image to prioritize-lcp-image).

After that lands Lighthouse should flag when the LCP image is not available in the initial HTML (as it is not here) and warn you that it should be. That MAY include warning about lazy loading (see discussion that issue) and that might be just native lazy loading or also the JS lazy loading your raised here.

Anyway thats all work in progess. The point @adamraine was raising was the current version of that audit (called preload-lcp-image and concentrating JUST on preload) should have caught this, which would mean it's at least going to be considered in the new version of the audit. And it kind of did and kind of didn't. But does more so in the later version on LH than is on PSI. So that's good news.

That's not to say all the work for that is done yet (note we have not closed this issue) but it's at least in scope for that new audit in some form and it's good that it is catching it in the new version of Lighthouse. I also wanted more investigation into why it was 0ms. I agree with you that preloading is the wrong answer here, but it may be we need to accept that in the short term until we add support for JS lazy loading.

Anyway, point is there's a lot of work in progress, so adding another new audit for JS library lazy loading of LCP images isn't something we should do at this point IMHO, but it's definitely something we should consider for the combined, new and improved, LCP audit we're working on (though no promising it will land in the first iteration of that!).

Hope that makes sense.

@brendankenny
Copy link
Member

There was a reluctance in the past to enter into the world of custom-lazy-loading detection heuristics independent of LCP performance advice that we've maybe carried with us, but I think you're completely right that as the preload LCP audit has morphed into the prioritize LCP audit this becomes unavoidable.

The current audit's only advice being to preload is a bug and tracked in #13738 (comment), but resolving that is still not sufficient. Suggesting that the developer inline the image in their HTML could be confusing in this case since it's already inline, they just need to stop using the lazy loading part. Adding heuristics to detect that case seems like the only good way to handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6 participants