ReviewOS

stacks/stx

stx.d.ts declares a useInterval(fn, ms, options) overload the runtime does not implement

#1941
Closed glennmichael123 opened this 10 days ago ·
10 days ago

@stacksjs/stx ships two contradictory declarations of useInterval, and the one the <script client> typechecker reads describes a function the runtime does not implement. Code written against it compiles clean and throws on mount.

Same shape as #1923 (useEventListener declared target-first against an event-first runtime): one name, two conventions, and no way for the call site to tell which it has.

The two declarations

dist/browser-composables.d.ts:163 — matches the runtime, a reactive counter:

export declare function useInterval(interval: number, options?: { immediate?: boolean }): {
  counter: { readonly value: number }
  pause: () => void
  resume: () => void
  reset: () => void
}

dist/stx.d.ts:358 — what buddy typecheck / stx typecheck resolves for a client script, and what does not exist:

declare function useInterval(_fn: () => void, _ms: number, _options?: { immediate?: boolean }): {
  start: () => void
  stop: () => void
  isActive: StxSignal<boolean>
}

dist/browser-composables.js implements only the first: export function useInterval(interval, options = {}), returning { counter, pause, resume, reset }. There is no start, no stop, no isActive, and no callback parameter. (useTimeout next to it is declared consistently in both files and is fine.)

Failure

<script client>
  const poll = useInterval(() => void refresh(), 60_000, { immediate: false })
  onMount(() => poll.start())
</script>

stx typecheck reports 0 errors. In the browser:

[stx] onMount error: TypeError: poll.start is not a function

Two things go wrong at once, and only the first is visible:

  1. poll.start is undefined, so onMount throws and everything after it in the callback is skipped.
  2. The callback was passed as interval, so the body runs setInterval(tick, someFunction). The delay coerces to NaN, which the platform clamps to 0 — a runaway timer incrementing a counter nobody can read. It also starts unconditionally: the implementation calls start() inside if (typeof window !== 'undefined'), so the { immediate: false } in the call above is ignored even on the correct signature.

Why it is worth fixing rather than documenting

The declared-but-absent overload is the plausible one — a callback-driven interval is what most people reach for, and it is what the composables docs elsewhere suggest. It is also the only useInterval visible to someone writing a client script, because stx.d.ts is the ambient set for that context. I picked it over useIntervalFn specifically because it was the one the framework typed; useIntervalFn worked at runtime at the time but failed the typecheck. The types pointed at the broken call and away from the working one.

The counter form also cannot express "run this every N ms" at all: counter is a plain getter ({ get value() { return count } }), not a signal, so an effect cannot track it either. With this declaration removed there is no interval-with-callback in the package — useTimeout re-armed from its own callback is the workaround, which is what I have shipped.

Suggested fix

Either:

  • Implement the declared overload. Detect a function first argument and branch, the way faker.lorem.sentence already branches on typeof q === 'object'. Keeps both call styles working and makes existing code correct retroactively.
  • Or delete the stx.d.ts declaration and re-export the real one, so the counter form is the only thing that typechecks — and add useIntervalFn(fn, ms, options) for the callback case, since removing the overload leaves a genuine gap.

Either way the two files should not disagree. A consistency test asserting that every name in stx.d.ts's composable registry has a matching arity and return shape in browser-composables.d.ts would catch this class of bug — it is now the second instance.

Also worth honouring options.immediate in the current implementation: it is accepted, documented in the type, and ignored.

Found while upgrading an app that had been calling the phantom overload for weeks with green gates the whole time.

10 days ago

Fixed in 6ab5505, though not by either of the two options you offered — there turned out to be a third shape in play.

stx.d.ts              (fn, ms, opts)  ->  { start, stop, isActive }     <- nothing implements this
runtime signals.ts    (n|fn, ...)     ->  { counter, pause, resume, reset, subscribe }
browser-composables   (n, opts)       ->  { counter: { value }, pause, resume, reset }

The runtime is what a client page actually executes, and it already branched on a leading callback (#1870) — so the working call existed, it just was not the one the types described. That is now the contract the other two state.

The declaration keeps overloads for both call forms. Declaring only the counter form would have left the working call failing to typecheck, which is precisely how you ended up on the phantom one.

The module twin is brought to the same behaviour. You were right that it produced setInterval(tick, NaN) — clamped to 0, a runaway timer incrementing a counter nobody can read. It also had no subscribe, so with counter a plain number there was no supported way to observe a tick at all. And immediate was accepted, documented, and used as an initial paused-flag rather than "tick right away" — fixed, with a test that asserts a tick lands before the first interval could have elapsed.

useTimeout.isPending was declared StxSignal<boolean> and is a plain boolean getter. Same defect one line down, so it is corrected in the same change.

You asked for a consistency test and there are three, because they fail differently:

  • timer-declaration-drift reads the returned key set out of the runtime source and compares it to the declaration — no restated list to fall behind. It also asserts start/stop/isActive are absent by name.
  • interval-surface pins what an author experiences through stx typecheck: phantom call rejected, both real forms accepted. A declaration can match the runtime and still be unusable from a script.
  • use-interval-parity runs both implementations through the same behavioural suite, asserting on tick counts — which is what catches a callback landing in the delay slot. Matching key sets with different semantics behind them is the drift that is hardest to see.

This was the fourth instance of the pattern after #1923, #1929 and #1932, so the guard reads both sides rather than restating either.

Sign in to comment on this issue.