Found while fixing the useClipboard timer leak in #1805.
Summary
packages/stx/src/signals-api.ts declares two module-level arrays:
const mountCallbacks: LifecycleCallback[] = []
const destroyCallbacks: LifecycleCallback[] = []Both are written and never read. onMount pushes to the first, onDestroy pushes to the second, and nothing anywhere drains either:
$ grep -rn 'destroyCallbacks\|mountCallbacks' packages/stx/src --include='*.ts' | grep -v 'signals.ts:'
packages/stx/src/signals-api.ts:132:const mountCallbacks: LifecycleCallback[] = []
packages/stx/src/signals-api.ts:133:const destroyCallbacks: LifecycleCallback[] = []
packages/stx/src/signals-api.ts:492: mountCallbacks.push(callback)
packages/stx/src/signals-api.ts:509: destroyCallbacks.push(callback)The client runtime's equivalents are drained (signals.ts — destroyCallbacks is consumed by cleanupContainer, and scope-level __destroyCallbacks by the scope teardown path). So this is a dual-impl parity gap in the sense of CLAUDE.md item 40: the same public API does something on one side and nothing on the other.
Why it matters
Two distinct consequences:
- Silent no-op. Any composable or store in the module tree that registers cleanup via
onDestroygets no cleanup. The code reads as correct, passes review, and does nothing.useClipboardis the case that surfaced it — I had to add an explicitstop()there because theonDestroyI registered would never have fired. - Unbounded growth. Because nothing ever clears the arrays, every
onMount/onDestroycall on the module path retains its closure for the life of the process. In a long-running SSR server that is a slow leak whose retained objects are exactly the ones a developer expected to be released.
Repro
import { onDestroy } from '@stacksjs/stx'
let ran = false
onDestroy(() => { ran = true })
// there is no exported way to make `ran` trueSuggested direction
Not obvious enough for me to just pick one, so flagging rather than fixing:
- (a) Export drains (
runMountCallbacks()/runDestroyCallbacks()) and call them from whatever owns the module-path lifecycle — SSR render teardown, and tests. Smallest change; needs an owner identified for each entry point. - (b) Scope the queues the way the runtime does (per render/per scope) rather than module-global, so the arrays cannot outlive their unit. Closer to the runtime's semantics, larger change.
- (c) Make them throw or warn outside a scope, so the no-op is loud instead of silent. Weakest, but strictly better than today.
My inclination is (a) plus a parity test asserting both impls actually invoke a registered onDestroy, since (b) is the kind of change that wants its own design pass.
Related
- #1805 — where this surfaced;
useClipboardnow exposesstop()precisely becauseonDestroyalone was dead code there. - CLAUDE.md item 40 — dual reactive implementations must behave equivalently.