ReviewOS

stacks/stx

fix(data): the client runtime's useFetch/useQuery construct no AbortController — in-flight requests outlive the page and write into torn-down scopes

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

Summary

stx ships two useFetch implementations. The module one (composables/use-fetch.ts, browser-composables.ts) creates an AbortController, passes signal to fetch, and registers onDestroy(abort). The runtime one — the implementation a <script client> block actually resolves, off window.stx — creates none. Neither does useQuery or useMutation. Nothing is cancelled on unmount or on SPA navigation, so a request issued by a page continues after that page is gone and its .then handler writes into signals belonging to a torn-down scope.

The framework's own parity test records the gap as intentional scope: "the composable additionally exposes status, statusText, and abort (supersets the runtime doesn't implement)". From an application's point of view that superset is the only one that exists, and it is the one that is unreachable.

Impact (real app)

grep -rn 'AbortController\|AbortSignal\|signal:' resources/ app/ across analyticshq returns zero hits — not because we chose not to, but because there is nothing to attach one to.

The dashboard's two client requests both resolve into signals with no cancellation:

// resources/views/dashboard.stx:1031-1034
session.authFetch('/api/me')
  .then(r => r.ok ? r.json() : null)
  .then(d => { if (d && d.pro) isPro.set(true) })   // ← runs after an SPA swap away
  .catch(() => {})

// resources/views/dashboard.stx:1049-1057
const tick = () => session.authFetch(`/api/sites/${…}/realtime`)
  .then(…)
  .then((d) => { if (d && typeof d.current === 'number') liveCount.set(d.current) })

resources/views/account.stx:54-82 has the same shape. The only guard anywhere is .catch(() => {}).

The only way for us to opt in today would be to hand-thread an AbortController through authFetch's init at all 10 call sites (resources/stores/session.ts:215), and then find a lifecycle hook to abort from — which, per the sibling report on onMount cleanup, is itself unreliable on the page path.

Evidence in stx

The module implementations do it — packages/stx/src/browser-composables.ts:964-978:

let abortController: AbortController | null = null
const execute = async (): Promise<void> => {
  abortController?.abort()
  abortController = new AbortController()
  …
  const response = await fetch(resolvedUrl, { ...fetchOptions, signal: abortController.signal })

with onDestroy(abort) at :1005. Same at packages/stx/src/composables/use-fetch.ts:142, 179, 204.

The runtime implementations do not. grep -n 'AbortController\|AbortSignal' packages/stx/src/signals.ts → no match.

  • packages/stx/src/signals.ts:955-969 (useFetch) builds fetchOptions with method, headers, body — no signal — and calls __stxFetch.
  • packages/stx/src/signals.ts:1243-1244 (useQuery) — same.
  • packages/stx/src/signals.ts:1319-1324 (useMutation) — same.
  • packages/stx/src/signals.ts:161-172 (__stxFetch) forwards opts verbatim; there is no place a signal could be injected.

And the divergence is documented as known — packages/stx/test/composables/use-fetch-parity.test.ts:25-28:

 * The composable additionally exposes `status`, `statusText`, and
 * `abort` (supersets the runtime doesn't implement) — those are tested
 * in `use-fetch.test.ts`, not here. This file only covers the shared
 * contract.

Because a <script client> block destructures useFetch from window.stx (signal-processing.ts:1251), the runtime impl is the one every application gets.

Prior art

  • TanStack Query passes signal into every queryFn and aborts on unmount.
  • Nuxt useAsyncData aborts superseded requests on route change.
  • Remix cancels superseded loader calls.
  • Angular takeUntilDestroyed().

Ask

  1. Create an AbortController per request in the runtime useFetch / useQuery / useMutation, pass signal through __stxFetch, and abort the previous one when a new request supersedes it.
  2. Register onDestroy(abort) so a scope teardown — including an SPA fragment swap — cancels its own in-flight requests.
  3. Return abort from the runtime composables so parity with the module implementation is real rather than declared out of scope.
  4. Treat AbortError as a cancellation, not an error: do not set error() and do not write data() for an aborted request.
  5. Add the abort case to use-fetch-parity.test.ts rather than excluding it.

Related: #1726 (the two implementations), #1720 (@async component writing into a detached container with no AbortController — same class).

Sign in to comment on this issue.