-
Notifications
You must be signed in to change notification settings - Fork 18
feat (lazysizes): improves initial page load performance #468
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: master
Are you sure you want to change the base?
Conversation
Adds lazy loading for images to improve initial page load time. Also sets `decoding="async"` attribute to images with `fetchpriority="high"` to further enhance perceived performance.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces lazysizes as a dependency and wires it into the JS entrypoint to enable lazy-loading for images, while also marking high-priority images for asynchronous decoding to improve perceived initial load performance. Sequence diagram for image loading with lazysizes and async decodingsequenceDiagram
participant User
participant Browser
participant IndexJS
participant Lazysizes
User->>Browser: Navigate_to_page
Browser->>Browser: Parse_HTML
Browser->>Browser: Render_picture_with_img(fetchpriority=high,decoding=async)
Browser->>Browser: Schedule_high_priority_image_download
Browser->>Browser: Decode_image_asynchronously
Browser->>IndexJS: Execute_JS_entrypoint
IndexJS->>Lazysizes: Import_and_initialize_lazysizes
Lazysizes-->>Browser: Attach_listeners_to_lazy_images
User->>Browser: Scroll_or_interact
Browser->>Lazysizes: Intersection_or_scroll_event
Lazysizes->>Browser: Swap_in_real_src_for_in_view_lazy_images
Browser->>Browser: Download_and_decode_lazy_images_on_demand
Flow diagram for handling high-priority vs lazily loaded imagesflowchart TD
Start[Image_element_in_template]
IsHighPriority{Has_fetchpriority_high}
HighPriority[Render_img_with_decoding_async]
LazyPriority[Managed_by_lazysizes]
AsyncDecode[Browser_decodes_image_asynchronously]
LazyLoadTrigger[User_scrolls_or_image_enters_viewport]
LoadLazy[Load_and_decode_lazy_image]
End[Image_visible_to_user]
Start --> IsHighPriority
IsHighPriority -->|Yes| HighPriority
IsHighPriority -->|No| LazyPriority
HighPriority --> AsyncDecode --> End
LazyPriority --> LazyLoadTrigger --> LoadLazy --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- The PR description mentions adding lazy loading with lazysizes, but the diff only adds the
decoding="async"attribute and the lazysizes dependency; consider either wiring lazysizes into the templates/JS or updating the description to match the actual change. - Double-check that
decoding="async"onfetchpriority="high"images aligns with your LCP/performance goals, as async decoding on critical (above-the-fold) images can sometimes delay the first render of those images.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PR description mentions adding lazy loading with lazysizes, but the diff only adds the `decoding="async"` attribute and the lazysizes dependency; consider either wiring lazysizes into the templates/JS or updating the description to match the actual change.
- Double-check that `decoding="async"` on `fetchpriority="high"` images aligns with your LCP/performance goals, as async decoding on critical (above-the-fold) images can sometimes delay the first render of those images.
## Individual Comments
### Comment 1
<location> `assets/conf-img/tpl/default-picture-nolazyload-caption.tpl:4` </location>
<code_context>
%%data-location%%
%%sources%%
- <img %%src%% fetchpriority="high" %%attributes%%>
+ <img %%src%% fetchpriority="high" decoding="async" %%attributes%%>
</picture>
-<figcaption>%%caption%%</figcaption>
</code_context>
<issue_to_address>
**issue (performance):** Reconsider using `decoding="async"` on images marked with `fetchpriority="high"`.
On critical, above-the-fold images, `decoding="async"` can delay when the image is decoded and painted, undermining the benefit of `fetchpriority="high"`. Consider either omitting `decoding` so the browser can choose, or using `decoding="sync"` (or making this configurable) for non-lazyloaded LCP candidates to avoid potential regressions in perceived load performance.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| %%data-location%% | ||
| %%sources%% | ||
| <img %%src%% fetchpriority="high" %%attributes%%> | ||
| <img %%src%% fetchpriority="high" decoding="async" %%attributes%%> |
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.
issue (performance): Reconsider using decoding="async" on images marked with fetchpriority="high".
On critical, above-the-fold images, decoding="async" can delay when the image is decoded and painted, undermining the benefit of fetchpriority="high". Consider either omitting decoding so the browser can choose, or using decoding="sync" (or making this configurable) for non-lazyloaded LCP candidates to avoid potential regressions in perceived load performance.
|
Ça fait pas doublon avec WP-Rocket ? Je ne suis pas "pro-rocket", mais si on implément désormais nous-même la feature, il faudra couper celle du plugin ? |
Historiquement, nous utilisions lazySizes sur le BFF pour avoir une compatibilité avec advanced-responsive-images. On a échangé avec Alex pour faire évoluer le plugin et se passer de lazySizes, donc on l'a retiré du BFF mais trop tôt car la dernière version de ARI n'est pas encore compat' sans utilisation de lazySizes. J'ai eu des retours de theme dev qui souhaitent implémenter de nouveau et temporairement la lib js pour que tout fonctionne bien avec ARI. Ça concerne uniquement les images contribuables hors zone Gutenberg (exception pour les champs ACF d'image dans les blocs sur mesure). Mais oui sur le fond tu as raison, ça fait doublon avec WPR. Dans le futur, on aimerait ne plus utiliser lazysizes tout en conservant ARI. |
|
Oui, mais du coup, on combine 2 technos de lazyload sur nos projets ? où Rocket est activé 90% du temps ? |
Adds lazy loading for images to improve initial page load time.
Also sets
decoding="async"attribute to images withfetchpriority="high"to further enhance perceived performance.Summary by Sourcery
Introduce image lazy loading and async decoding to improve initial page load performance.
New Features:
Enhancements:
Build: