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

[LiveComponent] Lazy load LiveComponent #1515

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

smnandre
Copy link
Member

@smnandre smnandre commented Feb 18, 2024

UPDATED COMMENT

This PR add loading support for LiveComponent

  • loading="defer" to load the component on page load
  • loading="lazy" to load the component when the component is visible
{# HTML Syntax #}
<twig:HeavyComponent loading="lazy" /> 

{# Twig Syntax #}
{{ component('HeavyComponent, { loading: "lazy" }) }}

The deferattribute is deprecated for this solution.

See more details in the document.


ORIGINAL PR:

I have some live component stuff in progress, so let's post this one now to start the discussion.

This PR implements a volontarely-minimal loading="lazy" mode on LiveComponents, based on top of the defer one.

⚠️ Live Component

This feature is dedicated to Live Component (as there is some particularities due to keeping "state").

(if you need some lazy loading with classic twig components, there are already methods out there to handle such things)

Defer ? Lazy ?

Both defer and lazy attributes allow to ... defer the full rendering of the component. So when the page first renders, it contains only a skeletton / empty component.

Then, depending of the attribute used, the component will fetch its content via a fetch request:

  • defer: when the page loads (it react to the "connect" Stimulus)
  • lazy: when the component enters the viewport (so as soon it's at least 1px visible in it)

I precise "visible" because if you use some tabbed content, horizontal sliders, etc... the "intersect" is not triggered until the component is actually.... visible.

Usage

<twig:Acme foo="bar" lazy />

Note: lazy implies defer, so this is equivalent to

<twig:Acme foo="bar" defer lazy />

So.. defer or lazy ?

As i see it:

  • defer should be used for any content that will 100% be visible in the main content, but too heavy to be computed with the original request (think Facebook/Insta first results of the timeline)
  • Lazy should be used for anything really not in the viewport, and that you're not sure will be rendered to the user (comments, tabbed content, ...)

In both situation, this is not a good idea to have too many lazy live components in the page, it's probably a sign that you should use Turbo instead!

(i'll add some doc about defer/lazy and layout shift too, but the idea is: better "keep the space you'll need")

Rendering

IntersectionObserver

The loading is triggered with a dedicated intersection observer.

It happens only once and the the element in unregistered.

No settings allowed (as images, iframes, etc.. in the browser)

TODO: optimize threesholds.

Loading=lazy

I chose to use the loading="lazy" attribute tag on the rendered HTML, as the goal is to have the same behaviour on the browser side.

<div
    loading="lazy"
    data-....
    class="foo"
>

It is standard for image, iframes, maybe portals soon, and i think it's self-explanatory.

Example: https://web.dev/articles/browser-level-image-lazy-loading

For browsers and for seo , this seems to be a good way to hint what this div is (i'll see aria stuff later as it touches many things on the codebase)

DX

Boolean attribute values

This is how we document the defer feature

{# expression can be a bool, a var, .... #}
{{ component('acme', {foo: "bar, defer: expression}) }}

For better DX, i add a small bool cast for both lazy and defer attributes values. It allows to have some dynamism on the calling side with HTML syntax too.

{# Defer: true #}
<twig:Acme foo="bar" defer />
<twig:Acme foo="bar" defer="defer" />
<twig:Acme foo="bar" defer="true" />
<twig:Acme foo="bar" defer="{{ 8 < 42 }}" />
<twig:Acme foo="bar" :defer="true" />

{# Also defer: true ! #} 
<twig:Acme foo="bar" defer="false" />

{# Defer: false #}
<twig:Acme foo="bar" defer="" />
<twig:Acme foo="bar" defer="{{ 8 > 42 }}" />
<twig:Acme foo="bar" :defer="false" />

(Tests / doc incoming, demo(s) in progress)

-- IN PROGRESS --

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Feb 18, 2024
@94noni
Copy link
Contributor

94noni commented Feb 19, 2024

I use on some projects https://www.stimulus-components.com/docs/stimulus-content-loader/
and I would love to have this in symfony/ux 👍🏻

@kbond
Copy link
Member

kbond commented Feb 19, 2024

To clarify?

  1. defer = on page load
  2. lazy = when in view

@WebMamba
Copy link
Collaborator

If I follow the logic:
for every component with a lazy attribute, we add the element into the IntersectionObserver.
So now when a component is on the viewport the IntersionObserver executes a callback that finds all the components present in the viewport and dispatches an event 'live:appear' for each of them.

Based on what I understand I have a question:
Can I trigger this live:appear before it's trigger by IntersionObserver without having the IntersionObjerver to trigger a live:appear a second time?
Do you see what I mean?

@smnandre
Copy link
Member Author

lazy = when in view

Yep i'll edit the description you're right :)

@smnandre
Copy link
Member Author

So now when a component is on the viewport the IntersionObserver

It's not "when a component is on the viewport" it can be a lot of things (a component is "no more intersecting" for instance)

So let's say "at a given time" for now :)

executes a callback that finds all the components present in the viewport and dispatches an event 'live:appear' for each of them.

Yes

Can I trigger this live:appear before it's trigger by IntersionObserver

You can dispatch any event you want :)

without having the IntersionObjerver to trigger a live:appear a second time?

In that particular scenario, as the intersection observer is still obeserving the component, it would indeed trigger a "live:appear" event when the component start intersecting the viewport, and then unobserve it.

Now my question is: why would you do that ? :)

I mean, if you trigger manually some "disconnect" event to a stimulus component, while not really disconnecting it, that would create some weird side effects... same for all the "live:*" events in fact :)

If someone voluntarily call internal/prive code, there is not much we can do to anticipate it / handle it, and there is nothing we can do to prevent it.

Did you have a specific / particular case in mind ?

@smnandre smnandre changed the title [LiveComponent] Lazy load component [LiveComponent] Lazy load LiveComponent Feb 20, 2024
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's quite an elegant, simple solution. Turbo also uses loading="lazy" on Turbo Frames, so I like that attribute.

Though, then doing <twig:CustomThing loading="lazy"> would be more consistent than having a lazy boolean attribute. Doing loading="lazy" might also help clarify loading vs defer. The explanation would be something like:

To defer loading your component until later, add the defer attribute. The component will render empty (or with some loading markup) then immediately make an AJAX call to load the real content.
To not load the content until the element becomes visible, add loading="lazy".

So, like Turbo, there would be loading="eager" (the default) and loading="lazy". And we expose this detail to the user vs hiding it from them.

src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Feb 26, 2024
@kbond
Copy link
Member

kbond commented Feb 26, 2024

Doing loading="lazy" might also help clarify loading vs defer. The explanation would be something like:

Should we then support loading="defer" to be consistent (and maybe deprecate the defer attribute)?

@smnandre
Copy link
Member Author

Should we then support loading="defer" to be consistent (and maybe deprecate the defer attribute)?

In HTML, loading="defer" does not exist.

  • defer attribute on scripts (in "opposition" to async )
    * loading="lazy" on image, iframe ..

As lazy implies defer here, your suggestion makes perfect sense... but it complexifies a bit the DX

<twig:MyComponent defer lazy="{{ loop.index > 4 }}" />
<twig:MyComponent loading="{{ loop.index > 4 ? 'lazy'  : 'defer' }}" />

Can we keep "defer" and "lazy" as some kind of aliases of "loading=XXX" ? Or is it not worth it ?

@weaverryan
Copy link
Member

Yea... just having a loading (set to defer or lazy), indeed simplifies things.

Can we keep "defer" and "lazy" as some kind of aliases of "loading=XXX" ? Or is it not worth it ?

The loading="{{ loop.index > 4 ? 'lazy' : 'defer' }}" is indeed longer, but the expression (loop.index > 4) is the same, so it feels like both have equal complexity. This longer version also still reads very clear. I'd say we do NOT keep these aliases.

@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Feb 29, 2024
@smnandre
Copy link
Member Author

Updated for loading="lazy" / loading="defer"

  • deprecate "defer" attribute
  • doc

Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs/backend code looks good!

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Feb 29, 2024
@WebMamba
Copy link
Collaborator

WebMamba commented Mar 1, 2024

Hey, I just have a question in a previous project we were using lazy loading in a specific way. The user was able to scroll a list of full-page images and we started the lazy load 3 images before the current image. I don't know if this PR can cover such a need. And if we should care about such a need?

@smnandre
Copy link
Member Author

smnandre commented Mar 1, 2024

Hey, I just have a question in a previous project we were using lazy loading in a specific way. The user was able to scroll a list of full-page images and we started the lazy load 3 images before the current image. I don't know if this PR can cover such a need. And if we should care about such a need?

I dont think this will be necessary. With a placeholder, and knowing your component will load probably in less than 200ms so it will more look like an animation than an ajax request.

An image is often much larger than a live component request.

I’d suggest we release without any customization and lets see if the need presents itself IRL ?

Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fantastic - a few last tweaks only!

src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
src/LiveComponent/doc/index.rst Outdated Show resolved Hide resolved
unset($data['defer']);
}

if (\array_key_exists('loading', $data)) {
if (\in_array($data['loading'], ['defer', 'lazy'], true)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, what if the user does loading="foo"? On the one hand, I think we should throw an exception. On the other hand, maybe we should allow that and render it as an attribute. Right now, I think we're doing neither - as it looks like loading="foo" would not be rendered due to the unset() below, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only HTML element that supports the loading attribute today are img and iframe, probably portal soon.

None of them can be a LiveComponent so i think we can ignore this for now. And decide what to do if someone raises some concern about that ?

wdyt ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, throwing an exception would be the most helpful for the user and the most conservative (if we removed the exception and allowed the attribute to render later, there is no way that will affect anyone).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we should ignore false and '' value, keep lazy or defer, and throw elsewhen.. right ?

@carsonbot carsonbot added Status: Needs Work Additional work is needed Status: Needs Review Needs to be reviewed and removed Status: Reviewed Has been reviewed by a maintainer Status: Needs Work Additional work is needed labels Mar 6, 2024
@smnandre
Copy link
Member Author

smnandre commented Mar 6, 2024

  • Updated the doc following feedback
  • Updated CHANGELOG.md
  • Updated UPGRADE.md

All is ready for me :)

@weaverryan
Copy link
Member

One last comment (hopefully) and a phpcs failure :)

@smnandre
Copy link
Member Author

smnandre commented Mar 6, 2024

phpcs due to rebase, they are fixed in: #1579

@weaverryan
Copy link
Member

Fantastic! Thanks Simon!

@weaverryan weaverryan merged commit 2dcfc13 into symfony:2.x Mar 6, 2024
8 checks passed
@smnandre
Copy link
Member Author

smnandre commented Mar 6, 2024

Thanks to @jakubtobiasz for paving the way with #1143 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants