RFC: Expose a `qCleanup` event when component is unmounted

[GrandSchtroumpf]

Champion

@GrandSchtroumpf

What's the motivation for this proposal?

Problems you are trying to solve:

• I want to start listening on event outside of document, window or HTMLElement without downloading core.
• I want to cleanup listener that started within a useOn.

Goals you are trying to achieve:

Wrap Browser based event emitter, without downloading core right away.
Examples:

// wrap navigation.addEventListener('navigate', ...);
useNavigation('navigate', $(event => ...));

// wrap matchMedia('(min-width: 600px)').addEventListener('change', ...)
useMatchMedia('(min-width: 600px)', $(event => ...))

Any other context or information you want to share:

useOn is the best place to achieve that, but it doesn't cleanup, so we need a way to know when the component is unmounted to cleanup.

---

Proposed Solution / Feature

What do you propose?

Emit a new CustomEvent before the component unmount.

Code examples

1. When component is visible, start listeneing on 'navigate' without downloading core.
2. When the event is triggers, it's forwarded to document.
3. When document receive event, trigger the logic from the QRL

const useNavigation = (eventName: string, cb: QRL<(event: Event, element: Element) => any>) => {
  useOnDocument('<custom-event>', cb);
  useOn('qVisible', _qrlSync(
    (e, el) => {
      const handler = (event) => document.dispatchEvent(new CustomEvent('<custom-event>', {detail:event}));
      navigation.addEventListener(eventName, handler);
      el.addEventListener('qCleanup', () => navigation.removeEventListener(eventName, handler), { once: true });
    },
    `(e, el) => {
      const handler = (event) => document.dispatchEvent(new CustomEvent('<custom-event>', {detail:event}));
      navigation.addEventListener('${eventName}', handler);
      el.addEventListener('qCleanup', () => navigation.removeEventListener('${eventName}', handler), { once: true });
    }`,
  ));
};

The goal here is to

• not dowload core too early.
• not adding too much logic in the inline script (addEventListener and removeEventListener).
• make sure all event listeners are removed when the component is removed.

Additional information

The qCleanup event should not impact the rendering process (shouldn't wait for the callbacks to fullfil)

In an ideal world the element should be still in the DOM when the event is received. But it's not mandatory for this RFC:

useOn('qCleanup', $(async (ev, el) => {
  console.log(el.isConnected); // Ideally this should be true, but not mandatory
  await new Promise((res) => setTimeout(res, 100));
  console.log(el.isConnected); // This should be false
})

Another usage of this event would be to run ViewTransition on leaving element (But it would only work if the el is still in the DOM at this moment...)

useOn('qCleanup', $(async (ev, el) => {
  const name = `_${Math.random()}_`;
  el.style.viewTransitionName = name;
  const transition = startViewTransition(() => {
    return new Promise((res) => getPlatform().nextTick(res));
  });
  await transition.ready;
  document.documentElement.animate({ opacity: 0 }, {
    pseudoElement: `view-transition-old(${name})`,
    duration: 100
  })
}))

---

PRs/ Links / References

No response

[wmertens]

Another way to do this would be to attach ._qCleanup() to the dom node in question, then the function is easy to detect and can be awaited.

[GrandSchtroumpf]

It would work well for this usecase also 👍

[ianlet]

From an external point of view, it seems very low level to directly expose an event like qCleanup (and it seems like an antipattern to encourage developers to use it). Though, I really like the idea of exposing high level APIs like the browser ones you showed in your example instead (e.g. useNavigation, useMatchMedia, etc.).

So I'm wondering if that RFC shouldn't be about those APIs rather than simply exposing a cleanup event?

But if we still want to expose something a bit more lower level, maybe introducing a new hook similar to useTask (or useVisibleTask) could be interesting.

Something like:

useSyncTask$(({ element, cleanup }) => {
  // ...
  cleanup(() => /* ... */);
});

Other names could be: useBrowserTask, useVisibleTaskSync, useOnSync, etc.

That way it can be easy to build something on top of it to provide richer API:

function useMatchMedia(query: string, qrl: QRL<(event: MediaQueryListEvent) => void>) {
  useSyncTask$(({ cleanup }) => {
    const mq = window.matchMedia(query);
    mq.addEventListener("change", qrl);
    cleanup(() => mq.removeEventListener("change", qrl));
  });
}

My main concern with the approach I'm suggesting is that it might be confusing to have an extra useTask and to explain the difference between the 3 of them (useTask VS useVisibleTask VS useSyncTask or whatever its name). Especially because it's already confusing for many to know when to use a visible task or not.

These are just suggestions of course and to share some feedback as well 😄

[GrandSchtroumpf]

@ianlet Sorry for the delay. If we can had a new hook that would be the best indeed. I think It cannot be named use*Task as we shouldn't support any tracking mecanism.

The main issue I see with a high-level hook is that Synchronous QRLs functions can't close over any variables. For that we need a generic way to serialize them where they need to be in the second parameter of _qrlSync.

Even with a high-level hook, I think we still need to emit a qcleanup event since we need to catch it in an inlined script.