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

Fire synthetic load events for hydrated elements #11046

Closed
Rich-Harris opened this issue Apr 3, 2024 · 9 comments · Fixed by #11642
Closed

Fire synthetic load events for hydrated elements #11046

Rich-Harris opened this issue Apr 3, 2024 · 9 comments · Fixed by #11642
Milestone

Comments

@Rich-Harris
Copy link
Member

Describe the problem

See #8301 and sveltejs/kit#11933 — if you have an onload handler (for an image or a link or an iframe or script or whatever) it will not fire (at least, not reliably) for elements that are being hydrated.

Describe the proposed solution

This causes sufficient confusion and inconvenience that I have to wonder if we shouldn't just dispatch synthetic load events during hydration, if the element is already loaded.

An alternative possibility is a bind:loaded directive, though this would force people to use effects to do work on load, which isn't ideal.

Importance

nice to have

@7nik
Copy link

7nik commented Apr 3, 2024

Should the same be done with the error event? I had a case similar to #10352 where I needed to fallback third-party images.

@Rich-Harris
Copy link
Member Author

yeah, good point — would be inconsistent if we didn't

@Conduitry
Copy link
Member

I'm worried about the necessary runtime code that this would force on everyone getting out of hand - and I'm also worried that it will never be perfect, and we'll have a constantly moving target. IIRC, at least for the error event on images, there's no guaranteed good way to find that out. The best you can do is to check whether the image is 'complete' but still had a width and height of 0.

I'd prefer, rather than firing synthetic load/error events on every element regardless of whether the user needs them, that we provide actions (or whatever later 5.x thing that replaces actions) that abstract this away as best as possible and call a user callback either immediately or once the item has loaded/errored - without the additional overhead of runtime code for people who don't need it, and without the additional overhead of creating synthetic events for every element.

@Rich-Harris
Copy link
Member Author

load and error don't bubble, so the additional runtime code would only be necessary if a) an onload or onerror handler was specifically added, or b) it's a spread attribute on an applicable element (which may still be a dealbreaker, but definitely seems like less of a dealbreaker).

Being unable to distinguish images that errored vs images that loaded but have a width and height of 0 does feel like a showstopper though. Absolutely farcical if there's truly no way to detect this.

@Conduitry
Copy link
Member

Conduitry commented Apr 3, 2024

The non-bubbling-ness of these events does make me less nervous about constructing these as synthetic events. I'm not sure whether it makes me all the way not nervous about that or not. Maybe.

@sheijne
Copy link

sheijne commented Apr 14, 2024

This is a problem that is older than jQuery, I wonder if this is something that Svelte should try to solve. Maybe this should not be solved by Svelte, but for example by SvelteKit.

One problem is that in Svelte 4 and below it was possible to work around this with a global function and using the onload/onerror attributes with a string value. Svelte 5 makes this a bit more difficult to work around, which means there is no longer a truly reliable (and efficient) way to work around it in userland.

I had two idea's neither are really great:

  1. A very simple workaround would be to do img.src = img.src whenever img.complete is true during hydration. he image is not loaded a second time if successful, sadly the request will fire again in case of error.
<script>
  let success = $state();
  let error = $state();

  $effect(() => {
    if (success.complete) {
      success.src = success.src;
    }
  });

  $effect(() => {
    if (error.complete) {
      error.src = error.src;
    }
  });
</script>

<img bind:this={success} src="https://placehold.co/600x400" onload={() => console.log("loaded")} />
<img bind:this={error} src="woops" onerror={() => console.log("errored")} />
  1. Since this issue is specific to SSR, it would be possible to detect at runtime whether an element has an onload or onerror, this could be used to apply an attribute to the element and inject a script to the html. There is of course some overhead as well, but it would be the most reliable and it would only have to be applied when the attributes/props are actually used, and the url would never be loaded multiple times.

@Rich-Harris
Copy link
Member Author

Rich-Harris commented Apr 15, 2024

Ooh, yeah that's an interesting idea. If we only cared about event handlers added via attributes (or spread attributes) we could do this:

<!-- input -->
<img alt="..." src="..." onload={foo} onerror={bar}>
<!-- server output -->
<img alt="..." src="..." onload="this.__load = event" onerror="this.__error = event">
// client output
if (img.__load) {
  foo(img.__load);
} else {
  $.event('load', img, foo, false);
}

if (img.__error) {
  bar(img.__error);
} else {
  $.event('error', img, bar, false);
}

(Mutatis mutandis for spread attributes.)

Where it gets trickier (or much simpler depending on your perspective) is actions. Say you have an action that fades an image in on load:

<img alt="..." src="..." use:smoothload>

smoothload adds a load event listener but Svelte can't know that, and there's no foo or bar that it can call directly. So we would have to refire the event:

<!-- server output -->
<img alt="..." src="..." onload="this.__e = event" onerror="this.__e = event">
// after the image is mounted (and actions have been attached)
if (hydrating && img.__e) {
  img.dispatchEvent(img.__e);
}

We could add that for every <img> (or video/iframe/whatever) that had an explicit onload/onerror handler or a spread attribute or an action, and handlers would run reliably. But we'd be double-firing the events. Is that bad, or perfectly fine in the case of a non-bubbling non-cancelable event?

@PatrickG
Copy link

I would not refire the events for actions.
If you're writing an action you're working with the raw DOM and probably know they can run after the resource has been loaded/errord.

But caching the events in .__load/.__error and running the handler directly if they exist is a nice solution.

@sheijne
Copy link

sheijne commented Apr 28, 2024

// client output
if (img.__load) {
  foo(img.__load);
} else {
  $.event('load', img, foo, false);
}

if (img.__error) {
  bar(img.__error);
} else {
  $.event('error', img, bar, false);
}

The event listener would still have to be bound, even if the event fired before mount. In the following case the load event would trigger multiple times:

<script>
  let src = 'my-image.png';
  
  async function applyImageFilter(event) {
    src = await makeFacesPretty(event.target);
  }
</script>

<img {src} onload={...} />
<button onclick={applyImageFilter}>Apply some fancy instagram filter</button>

We could add that for every (or video/iframe/whatever) that had an explicit onload/onerror handler or a spread attribute or an action, and handlers would run reliably. But we'd be double-firing the events. Is that bad, or perfectly fine in the case of a non-bubbling non-cancelable event?

Either way the handler will be triggered synthetically which could cause some funky behaviors. It could be better to use dispatchEvent as the behavior would be closer to the native behavior; properties such as eventPhase would get the correct value. On the other hand if someone decided to add onload/onerror attributes using SvelteKit's transformPageChunk it would cause unexpected behaviors. Regardless of the approach, if Svelte decides to synthetically fire events on elements, I think it's safe to say some funky/unexpected behaviors are bound to happen.

Edit: the Event.isTrusted property can be used as escape hatch for people running into issues with the event being triggered twice.

There would be other ways to go about it for actions, but the ones I can image could open doors Svelte might want to leave shut, and they'd add quite a bit of code to the runtime. For example, it would be possible to proxy the node passed to actions and apply the same logic as with listeners bound through props.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants