fix(lifecycle): a mount callback's returned cleanup is honoured only on the component-factory path — discarded by all five other flush sites

Summary

onMount(() => { …; return () => teardown() }) works in exactly one place in the runtime. stx.mountEl's component factory captures the return value and pushes it into the local destroy hooks; every other place that flushes mount callbacks — the DOMContentLoaded page path, the global flush, the stream/hydrate path, the SPA stx:load path, and every scoped __mountCallbacks drain — calls fn() and throws the result away.

The failure is silent. The teardown simply never runs, and there is no warning that a function was returned and dropped.

Impact (real app)

analyticshq polls a realtime endpoint from the dashboard. resources/views/dashboard.stx:1061-1063:

  // Kept as well as the onDestroy hook: this one fires on the fragment-swap path, the
  // hook covers the document-swap path. Neither covers both.
  return () => live.pause()
})

That belief is wrong, and the comment will be copied by the next maintainer. The returned closure never runs on any path. The poll is stopped only by useInterval's internal onDestroy hook (packages/stx/src/signals.ts:4400), drained by the stx:load destroy sweep.

The cost of getting this wrong is on record in the same file (dashboard.stx:1038-1043):

useInterval, not setInterval, and this is a bug fix rather than a style change. […] The hand-rolled timer relied solely on the teardown returned from onMount, and a full-document swap never runs it, so navigating away from the dashboard left the poll hitting /api/sites/:id/realtime every 15 seconds, forever, for a page nobody was looking at.

The earlier version of that poll leaked a request every 15s for the life of the tab because the author reasonably assumed the returned function was the teardown.

Evidence in stx

The one site that honours it — packages/stx/src/signals.ts:5996-6016 (inside mountEl):

var localMountHooks = mountCallbacks.splice(mountStart);
var localDestroyHooks = destroyCallbacks.splice(destroyStart);
…
localMountHooks.forEach(function(fn) {
  try {
    var cleanup = fn();
    if (typeof cleanup === 'function') localDestroyHooks.push(cleanup);
  } catch (e) { console.error('[stx] onMount error:', e); }
});

Every other flush drops it:

And the scoped path, which is where a view-level <script client> registration lands, is the same at signals.ts:1665, :3230-3234, :5578-5581, :6515-6518, :7103-7106:

scopeVars.__mountCallbacks.forEach(function(fn) { fn(); });

onMount (signals.ts:779-787) routes into scope.__mountCallbacks when a lifecycle scope resolves and into the module-level mountCallbacks otherwise — neither is the cleanup-honouring path.

Prior art

• React: useEffect(() => { …; return cleanup }) — the return is the teardown, on every render path.
• Svelte: onMount(() => () => stop()).
• Vue: effectScope() / onScopeDispose(); onUnmounted in every component instance.
• Angular: DestroyRef.onDestroy(fn).

Ask

1. Factor the mount-callback invocation into one helper, e.g. runMountCallbacks(list, destroySink), that captures the return value and registers it as a destroy hook — and route all six flush sites through it (signals.ts:1665, 3230, 5578, 5588, 6428, 6515, 6523, 7103, 7110).
2. For the scoped path, push the captured cleanup onto scopeVars.__destroyCallbacks so it is drained by the same sweep that runs onDestroy.
3. Until (1) lands, warn once when a mount callback returns a function on a path that will discard it, naming the file — a silent drop of a teardown is indistinguishable from a leak.
4. Add a test that asserts a returned cleanup runs on each of: full load, SPA fragment swap, full-document swap, and dynamic :if unmount.

Related: #1811 (module-path lifecycle queues never drained), #1727 (scope leak on :if/:for unmount).

Fixed in 6929576.

Confirmed: only the two component-factory paths captured the return. Nine other flush sites in the generated runtime called fn() and dropped it, so onMount(() => { const id = setInterval(...); return () => clearInterval(id) }) — the documented shape — did nothing, silently, and leaked once per re-render.

Every flush now goes through one helper that keeps the return value. The sink is explicit rather than implied: a scoped flush parks a cleanup on that scope's own __destroyCallbacks, a global flush on the module queue. Sharing one global sink would make destroying one component run another's teardown.

The module-side drain() in signals-api.ts had the same hole, so it is fixed here too — the two implementations have a parity contract (CLAUDE.md item 40) and fixing one side only is exactly how they drift. runDestroyCallbacks deliberately passes no sink: a cleanup returned by a cleanup has nowhere meaningful to go, and parking it back on the same queue would keep it alive across every later drain.

Two incidental improvements fall out of the consolidation: the two sites that ran callbacks with no try/catch at all now contain a throwing callback like the others, and the error message is finally identical everywhere instead of three variations across console.error and console.warn.

Four existing assertions matched the literal runtime text and are updated to the helper. They pin the emitted shape and cannot tell whether the cleanup is ever invoked, so the new tests run the real callbacks instead — 8 tests, 5 of which fail without the change.

Full stx suite: 9011 pass, 1 fail (crosswind-arbitrary-values, which fails identically on a clean tree and is unrelated).

Fixed in the current runtime. All mount-callback flush sites now route through a single runMountCallbacks(list, sink) helper (packages/stx/src/signals.ts:939) that captures a returned cleanup and parks it in the destroy sink — scoped flushes pass that scope's __destroyCallbacks, global flushes pass the module queue. Regression test at packages/stx/test/signals/mount-cleanup.test.ts covers both the returned-cleanup-runs case and the async-callback-returns-a-Promise case (which must NOT be treated as teardown). Closing as done.