fix(signals): effect() calls its callback's return value as a cleanup without checking it is a function — a concise arrow body crashes on its second run

effect() stores whatever the callback returns and calls it on the next run, guarded only by truthiness. An effect body is an expression at least as often as it is a block, and an expression has a value.

Repro

import { effect, state } from '@stacksjs/stx'

const name = state('a')
effect(() => el.textContent = name())   // concise arrow: RETURNS the assigned string
name.set('b')

first run ok, textContent = a
CRASH on second run: cleanup is not a function. (In 'cleanup()', 'cleanup' is "a")

Assigning to the DOM inside an effect is the single most common effect body in any framework, and el.textContent = x evaluates to the assigned string. if (cleanup) is truthy for any non-empty string, so the next run invokes it.

It only fails on the second run, so it survives review and survives any test that sets the signal once.

Writing the same effect with braces returns undefined and is fine — so whether your code works depends on whether you typed {}, which is not a distinction the API documents or intends.

Both implementations are affected

The runtime copy is the worse of the two. Its cleanup() call at :739 sits ABOVE the try block that wraps fn(), so the TypeError is not caught by the effect's own error handling. It escapes runEffect entirely and propagates out of whatever .set() triggered the notification — so one bad effect takes down unrelated subscribers in the same update, and the .set() call itself throws.

Verified by executing both: the module path via a direct import, the runtime path by installing generateSignalsRuntimeDev() in happy-dom and calling window.stx.effect.

Fix

Guard at the assignment rather than at the two call sites, so "cleanup is a function or undefined" becomes an invariant and both the re-run and the dispose path are correct for free:

const result = fn()
cleanup = typeof result === 'function' ? result : undefined

This is the same missing typeof cleanup === 'function' check that #1857 fixed for mount callbacks, one file away — worth a sweep for other places a returned value is assumed callable.

Found while auditing #1885.

Fixed in fd5fdb6.

Guarded at the assignment rather than at the two call sites, so "cleanup is a function or undefined" becomes an invariant and both the re-run path and the dispose path are correct without repeating the check:

const result = fn()
cleanup = typeof result === 'function' ? result : undefined

Applied to both implementations. 7 tests; 5 fail without the change. The 2 that pass either way are the controls — a genuine function cleanup must still run before the next pass and on dispose, which the guard must not break. The runtime side is additionally pinned by asserting the emitted text no longer contains an unguarded cleanup = fn().

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

I did not do the sweep for other places a returned value is assumed callable — that is still worth doing, and #1857 plus this one suggest it is a recurring shape rather than two coincidences.