Async cleanup in Task

[GrandSchtroumpf]

What is it about?

Await for aync callback to resolve in when cleanup task

What's the motivation for this proposal?

Problems you are trying to solve:

External function must be wrap in a QRL to be executed in a task. Since cleanup function is sync we need to do something like that to ensure the task is cleanedup before executing the code:

const signal = useSignal();

const init = useSignal(false);
useVisibleTask$(async ({ track }) => {
  track(signal);
  if (!init.value) {
    init.value = true;
    // Terminate when signal changes only after the initial task
    await terminate$();
  }
  await create$();
});

// Terminate when component is destroyed
useVisibleTask$(() => {
  return () => terminate$();
})

Goals you are trying to achieve:

This code is quite complex to make sure the code is cleaned up. The goal is to improve dexX

Any other context or information you want to share:

In this specific case I'm creating & terminating a web worker.

---

Proposed Solution / Feature

What do you propose?

Ideally cleanup would wait for the callback function to resolve/reject to run the task again. The dev should ensure the cleanup function does resolve. We could have a console.warn after 10s for example to let the user know if some cleanup are pending.

Code examples

useVisibleTask$(async ({ track, cleanup }) => {
  track(signal);
   // cleanup waits for terminate$() to resolve
   // in devmode: console.warn after 10s if cleanup didn't resolve
  cleanup(terminate$);
  await create$();
});

---

Links / References

No response

[wmertens]

From a purity point of view I agree that cleanup results should be awaited.

However I don't understand your example. If the cleanup function is imported it doesn't need to be a QRL.

So my current objection is that I'd rather not complicate core unless there is a valid use case. This means either:

• show that it's trivial to implement and has no performance impact
• show a valid use case that can't be worked around

[GrandSchtroumpf]

Let's take another example without QRL:

const runProcess = (workerId: string, ...args) => {
  const worker = getWebWorker(workerId);
  const requestId = createRequestId();
  return Promise((res) => {
    const handler = ({ data }) => {
      if (data.requestId === requestId) res(data.result);
      worker.removeEventListener('message', handler);
    }
    worker.addEventListener('message', handler);
    worker.postMessage('run', ...args);
  });
}
const closeProcess = (workerId: string) => {
  const worker = getWebWorker(workerId);
  const requestId = createRequestId();
  return Promise(() => {
    const handler = ({ data }) => {
      if (data.requestId === requestId) res(data.result);
      worker.removeEventListener('message', handler);
    }
    worker.addEventListener('message', handler);
    worker.postMessage('close');
  });
}

export const component$(() => {
  const id = useId();
  const signal = useSignal();
  useVisibleTask$(({ track, cleanup }) => {
    track(signal);
    cleanup(() => closeProcess(id))
    await runProcess(id, signal.value);
  });
})

In this case we want to be sure the message come back from the web worker before starting the new process. Else the process will start again, and then it'll close.

[wmertens]

Minimal repro: https://qwik.dev/playground/#f=7VbLCsIwELz7FUGEJjRIBEUE67cUxYMIVUI9xfy7m0zTrq0tXgUvLexjZrddZpcNzXa9%2BzQ0uq9p0YB1PDlQ6DzqcSFIcahzpsvSBkO3UaTVFKTUV3PYHA6ooejqkYa6at1R0rl3Plczcvc1GuokHfa5jtJVPe7Ct3wJklYaAcYwCfbIB3c6NbKaUAO7rTMdUhRCgJrYGHYvPdEPEESz3cJDboxJ1tjokvSINk1eiPK5cJTjSaurcznFEAI4gcdrDC8WBEBWyMqkSsbyblckDT%2FSewFBQ5jIhHsT84qLszu0HL4e%2FgAI89wfLtWpvU73R9vMvmNlebLgkCXB%2BuvVD%2BrVCw

Click the button and see the interleaving.

Side note when you useTask instead of a visible task: The SSR logging won't show the cleanup stop, because SSR also doesn't await the cleanup.

So it's clearly a problem, and we should either disallow Promise returns or fix it.

[shairez]

Thanks @GrandSchtroumpf !

we discussed, @Varixo will take a look when he has time to verify if this is fixed in V2 or not, and we'll decide from there whether this should be promoted or do we need a champion for it