ReviewOS

stacks/stx

No background refetch: refetch() always sets loading, so a polling view cannot use useFetch/useQuery (and useQuery returns isStale/invalidate that the type omits)

#1929
Closed glennmichael123 opened this 10 days ago · {{ comments.length }} {{ comments.length === 1 ? 'comment' : 'comments' }}
10 days ago

A polling refresh cannot use useFetch or useQuery, because refetch() always sets loading, and a spinner bound to it flashes on every poll. There is no first-load / background-refresh distinction.

Where it bites

A listening view polls every 60 seconds and deliberately keeps the loading state out of it:

const loadData = async ({ quiet = false } = {}) => {
  if (!quiet) loading.set(true)
  ...
}

onMount(() => {
  void loadData()
  setInterval(() => void loadData({ quiet: true }), 60_000)
})

That is the one thing the hand-rolled version does that the composable cannot express, and it is why this view is the only one in the app that could not move onto useFetch. Converting it would put a spinner on screen once a minute, forever.

Cause

fetchData opens with an unconditional loading.set(true) in both primitives — signals.ts:1140 for useFetch, signals.ts:1552 for useQuery — and neither takes a background / quiet / silent option. Grepping the whole of useFetch for any of those names, or for isFetching, returns nothing.

useQuery almost has the answer, but it is not reachable

useQuery tracks isStale and returns it (signals.ts:1634):

return {
  data: data,
  loading: loading,
  error: error,
  isStale: isStale,
  refetch: fetchData,
  invalidate: ...
}

but StxQueryResult in dist/stx.d.ts declares only:

interface StxQueryResult<T> {
  data: StxSignal<T | null>
  loading: StxSignal<boolean>
  error: StxSignal<Error | null>
  refetch: () => Promise<void>
}

So isStale and invalidate both exist at runtime and are invisible to the type — reading either is a type error on a value that is really there. That is the same class as #1923 and the navigate / useRoute.fullPath cases: stx.d.ts describing the registry by hand and drifting from signals.ts.

Suggested

Two separate things, and the first is the one that unblocks the app:

  1. A background: true (or { silent: true }) option on refetch, so a poll updates data without touching loading. The convention elsewhere is a second flag — loading for the first load, isFetching for any in-flight request — which would also let a view show a subtle indicator during a poll rather than nothing at all.
  2. Add isStale and invalidate to StxQueryResult so useQuery's existing surface is usable.
10 days ago

Both halves are in d544c757e3.

1. Background refetch. refetch({ background: true }) updates data without touching loading, and a new isFetching signal is true while any request is open — the loading / isFetching split you suggested:

const { data, loading, isFetching, refetch } = useFetch('/api/rows')

onMount(() => {
  const id = setInterval(() => refetch({ background: true }), 60_000)
  onDestroy(() => clearInterval(id))
})
<Spinner :if="loading" />
<span :if="isFetching" class="text-xs opacity-60">refreshing…</span>

refetchInterval and refetchOnFocus are now background by default. Both refresh data that is already on screen, and leaving them on the foreground path would have meant the composable's own polling option still could not be used for polling — which is the complaint. If you want the old flash back, bind isFetching.

One thing worth knowing because it is easy to miss: a background run also does not clear the current error on the way in. Blanking a message that is still true is the same disturbance as flashing a spinner. It clears when the background request actually succeeds — otherwise one failed poll would leave the message up for the life of the page.

Changed in both useFetch implementations together (the module one and the generated runtime's), per CLAUDE.md item 40 — a background refetch that worked on only one path would be the same crash-depending-on-your-bundler shape #1726 fixed. test/composables/background-refetch.test.ts runs the same suite against both, and watches loading through an effect rather than sampling it, so a spinner that flashed true and back inside one tick could not pass unnoticed.

2. The query surface. Split into StxFetchResult and a StxQueryResult that extends it with isStale and invalidate, rather than widening the shared interface. Widening would have traded one drift for a worse one: useFetch does not return invalidate, so declaring it there would type-check a call to undefined — the checker introducing exactly the error it exists to catch. Both directions are pinned in test/typecheck/query-result-surface.test.ts.

refetch and invalidate both accept { background?: boolean } in the declaration too, so the option is discoverable rather than folklore.

Sign in to comment on this issue.