From 84c80aacfffe349f8641befc8c800d3e3fcaf2c4 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 02:47:50 -0400 Subject: [PATCH 01/18] perf(fetchium): microtask refetches, prefetch/retain leases, useSuspenseQuery Read-path latency: - A refetch with no debounce (stale reactivation, Signal param change) now starts on a microtask instead of setTimeout(0). On React Native a zero timer goes through the native timing module and can wait a frame. Stale reactivation goes from 1 macrotask turn to 0, a param change from 2 to 1 (the remaining one is Signalium's flush), a staggered reactivation from 2 to 1. Zero-delay refetches requested in one task still coalesce. - Invalidation keeps the timer, so a query whose last reader left in the same task is deactivated by Signalium's flush instead of refetched and aborted. New API: - QueryClient.prefetch(QueryClass, params, { ttl }) starts a query and keeps it active for ttl (default 10 s). A reader that mounts inside the window joins it: no second request, data on the first render if it has arrived. - QueryClient.retain(fn, { ttl }) keeps the queries fn reads or returns active until release() or ttl. Built on public Signalium 3.0.3 APIs (watcher + addListener), mirroring the unreleased signalium retain() but bound to the client's QueryClientContext. destroy() releases leases. - useSuspenseQuery (fetchium/react) suspends only on a cold miss, never on a refetch; it decides before subscribing and has the client hold the query while suspended, so a discarded render leaks no watch. A failed cold fetch is thrown to the error boundary; a boundary reset retries. Docs: useQuery's .value never suspended and a rejected .value never threw; the agent doc's `{ suspended }` option never existed. Fixed, and documented the new API. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/fast-delivery-prefetch-retain.md | 11 + docs/src/app/api/fetchium-react/page.md | 55 ++- docs/src/app/api/fetchium/page.md | 34 +- docs/src/app/guides/error-handling/page.md | 12 +- packages/fetchium/plugin/agents/fetchium.md | 4 +- packages/fetchium/src/QueryClient.ts | 255 +++++++++++++- packages/fetchium/src/QueryResult.ts | 78 ++++- .../src/__tests__/read-path-latency.test.ts | 208 +++++++++++ .../src/__tests__/retain-prefetch.test.ts | 323 +++++++++++++++++ packages/fetchium/src/index.ts | 4 +- .../__tests__/prefetch-suspense.test.tsx | 328 ++++++++++++++++++ packages/fetchium/src/react/index.ts | 1 + .../fetchium/src/react/use-suspense-query.ts | 62 ++++ 13 files changed, 1336 insertions(+), 39 deletions(-) create mode 100644 .changeset/fast-delivery-prefetch-retain.md create mode 100644 packages/fetchium/src/__tests__/read-path-latency.test.ts create mode 100644 packages/fetchium/src/__tests__/retain-prefetch.test.ts create mode 100644 packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx create mode 100644 packages/fetchium/src/react/use-suspense-query.ts diff --git a/.changeset/fast-delivery-prefetch-retain.md b/.changeset/fast-delivery-prefetch-retain.md new file mode 100644 index 0000000..a70c38d --- /dev/null +++ b/.changeset/fast-delivery-prefetch-retain.md @@ -0,0 +1,11 @@ +--- +'fetchium': minor +--- + +Adds `queryClient.prefetch(QueryClass, params, { ttl })` and `queryClient.retain(fn, { ttl })`. Both keep queries active without a reader and return a `release` function. A reader that mounts while the lease is held joins the running query: no second request, and the data on its first render if it has arrived. `prefetch` defaults to a 10 s lease (`DEFAULT_PREFETCH_TTL`) and lets a fetch in flight at expiry finish; `retain` lasts until released. + +Adds `useSuspenseQuery` to `fetchium/react`. It suspends only when the query has never produced a value (in memory or in a synchronous store), never for a refetch, and throws a failed cold fetch to the error boundary. + +A refetch with no `debounce` (a stale query reactivating, a Signal param change) now starts on a microtask instead of after `setTimeout(0)`, which on React Native can wait a frame. Invalidation still waits a task, so a query whose last reader left in the same task is deactivated rather than refetched. + +Docs no longer claim that reading `.value` on a pending or rejected query suspends or throws; it does neither. diff --git a/docs/src/app/api/fetchium-react/page.md b/docs/src/app/api/fetchium-react/page.md index b02089a..98f2c51 100644 --- a/docs/src/app/api/fetchium-react/page.md +++ b/docs/src/app/api/fetchium-react/page.md @@ -8,7 +8,7 @@ description: API reference for the fetchium React integration. React hooks for using Fetchium queries in React components. Built on top of Signalium's `useReactive` hook. ```ts -import { useQuery } from 'fetchium/react'; +import { useQuery, useSuspenseQuery } from 'fetchium/react'; ``` --- @@ -39,14 +39,14 @@ React hook for fetching a query. Subscribes the component to the query's reactiv The returned promise object has the following properties: -| Property | Type | Description | -| ------------ | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | -| `value` | `QueryResult` | The resolved query result. Reading this while pending triggers React Suspense. Returns a deep clone to avoid accidental mutation of cached entities. | -| `isReady` | `boolean` | `true` once the query has loaded a value at least once. Use for type narrowing on `value`. | -| `isPending` | `boolean` | `true` while the query is loading. Also true during refetches, even if a value already exists. | -| `isResolved` | `boolean` | `true` when the most recent execution resolved successfully. | -| `isRejected` | `boolean` | `true` when the most recent execution failed. | -| `error` | `unknown` | The error if `isRejected` is `true`. | +| Property | Type | Description | +| ------------ | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | +| `value` | `QueryResult` | The resolved query result, or `undefined` until the first value arrives. Returns a deep clone to avoid accidental mutation of cached entities. | +| `isReady` | `boolean` | `true` once the query has loaded a value at least once. Use for type narrowing on `value`. | +| `isPending` | `boolean` | `true` while the query is loading. Also true during refetches, even if a value already exists. | +| `isResolved` | `boolean` | `true` when the most recent execution resolved successfully. | +| `isRejected` | `boolean` | `true` when the most recent execution failed. | +| `error` | `unknown` | The error if `isRejected` is `true`. | The resolved `QueryResult` includes pagination helpers: @@ -133,4 +133,39 @@ const SearchResults = component(() => { - `useQuery` is a thin wrapper around Signalium v3's `useReactive`, which is deep-by-default. It returns a **structurally-shared snapshot** of the query result, so memoized children that receive subtrees as props keep stable references when the underlying data is unchanged. - Fetchium registers a custom snapshot for entity proxies so the snapshot walks into entities (instead of returning them by reference), giving you correct re-rendering on entity field changes. - The snapshot is a plain-object copy of the query result to prevent accidental mutation of the entity cache. Use `draft()` from `fetchium` if you need a mutable copy for mutations. -- When used with React Suspense, reading `.value` on a pending query will suspend the component. +- `useQuery` never suspends. Reading `.value` while pending returns `undefined`. Use [`useSuspenseQuery`](#usesuspensequery) to suspend on a cold miss. + +--- + +### `useSuspenseQuery` + +```ts +function useSuspenseQuery( + QueryClass: new () => T, + params?: ExtractQueryParams, +): ReadyReactivePromise>; +``` + +`useQuery` for components under a React `` boundary. It suspends only on a **cold miss**: the query has never produced a value, neither in memory nor, with a synchronous store, in the persisted cache. Anything else renders at once, including a stale value whose refetch is in flight. Refetches never suspend. Because the hook only returns once a value exists, `value` is always defined. + +A cold fetch that fails is thrown to the nearest error boundary. Resetting the boundary (remounting the component) tries the fetch again. + +Changing params to ones with no value yet is a cold miss, so the component suspends. Wrap the change in `startTransition` to keep showing the previous result. + +Call it from a plain function component. Like `useQuery`, it cannot run inside a Signalium `component()`, which is a reactive context. + +```tsx +import { Suspense } from 'react'; +import { useSuspenseQuery } from 'fetchium/react'; + +function UserName({ id }: { id: number }) { + const user = useSuspenseQuery(GetUser, { id }); + return {user.value.name}; +} + +}> + +; +``` + +While suspended, the client keeps the query active, since React discards a suspended render without subscribing to anything. The component's commit takes over. diff --git a/docs/src/app/api/fetchium/page.md b/docs/src/app/api/fetchium/page.md index 4ef56cb..85f40bd 100644 --- a/docs/src/app/api/fetchium/page.md +++ b/docs/src/app/api/fetchium/page.md @@ -245,7 +245,37 @@ new QueryClient(config: QueryClientConfig) | `getContext` | `(): QueryContext` | Returns the `QueryContext` passed at construction. | | `applyMutationEvent` | `(event: MutationEvent): void` | Applies an external mutation event (create/update/delete) to the entity store. | | `invalidateQueries` | `(targets: ReadonlyArray): void` | Marks matching query instances as stale. Accepts query classes and optional param subsets for filtering. | -| `destroy` | `(): void` | Tears down the GC manager, network manager, and all caches. | +| `prefetch` | `(QueryClass, params?, { ttl? }?): () => void` | Starts a query now and keeps it active for `ttl` ms (default `DEFAULT_PREFETCH_TTL`, 10 s). See below. | +| `retain` | `(fn: () => unknown, { ttl? }?): () => void` | Keeps the queries `fn` reads active until the returned `release` is called, or for `ttl` ms. See below. | +| `destroy` | `(): void` | Tears down the GC manager, network manager, and all caches, and releases outstanding leases. | + +#### `prefetch` and `retain` + +Both take a lease: the queries stay active (fetched, subscribed, exempt from GC) without a component reading them. A reader that mounts while the lease is held joins the active query, so it starts no request of its own and renders the data on its first render if it has arrived. Both return an idempotent `release` function. Releasing never interrupts other readers; once the last one goes, the query deactivates and its `gcTime` starts as usual. Calling `release` while a fetch is in flight, with no reader, aborts that fetch. A `prefetch` whose `ttl` runs out mid-fetch instead waits for the fetch to settle before letting go. + +`prefetch` is for the moment a user commits to a navigation, such as a tap. Call it in the handler, before navigating: + +```ts +function onPressToken(id: string) { + queryClient.prefetch(GetTokenDetail, { id }, { ttl: 5_000 }); + router.push(`/token/${id}`); +} +``` + +With a synchronous store, a cached result is applied inside the call. A fresh one makes no request, and a stale one is refetched, as on any activation. + +`retain` keeps a set of queries warm, for example the queries of a screen that is hidden but likely to come back. `fn` runs immediately, with the client as `QueryClientContext`, and again when what it reads changes. It holds a query when it reads one of the query's fields, or when it returns the query's promise or an array of them: + +```ts +const release = queryClient.retain(() => [ + fetchQuery(GetPortfolio), + fetchQuery(GetPrices, { ids }), +]); +// when the surface is gone for good: +release(); +``` + +Without a `ttl`, a `retain` lease lasts until `release` is called. --- @@ -625,7 +655,7 @@ The `t` object provides a declarative type definition DSL for describing query p | ------------------------- | ---------------------------------- | --------------------------- | -------------------------------------------------------------------------------------------------------------------------- | | `gcTime` | `number` | `5` | In-memory eviction time in minutes. `0` = next-tick, `Infinity` = never. | | `staleTime` | `number` | `0` | Milliseconds data is considered fresh. `0` = always stale. | -| `debounce` | `number` | `0` | Milliseconds to debounce param-change refetches. | +| `debounce` | `number` | `0` | Milliseconds to debounce refetches. At `0`, a refetch starts on a microtask (invalidation: on the next task). | | `networkMode` | `NetworkMode` | `NetworkMode.Online` | When to allow fetching. | | `retry` | `RetryConfig \| number \| boolean` | `3` (client) / `0` (server) | Retry configuration. | | `refreshStaleOnReconnect` | `boolean` | `true` | Whether to refetch stale queries when network reconnects. | diff --git a/docs/src/app/guides/error-handling/page.md b/docs/src/app/guides/error-handling/page.md index 03b3605..edbdbb4 100644 --- a/docs/src/app/guides/error-handling/page.md +++ b/docs/src/app/guides/error-handling/page.md @@ -295,7 +295,7 @@ if (result.isRejected) { ### Opting into throw behavior -If you _want_ error boundaries to catch query failures --- for example, when using React Suspense or when you prefer a centralized error UI --- you can read `.value` directly. Reading `.value` on a rejected `ReactivePromise` throws the error, which will propagate up to the nearest error boundary. +If you _want_ error boundaries to catch query failures --- for example, when using React Suspense or when you prefer a centralized error UI --- throw the error yourself. Reading `.value` does not throw: on a rejected query it holds the last successful value, or `undefined` if there never was one. ```tsx import { ErrorBoundary } from 'react-error-boundary'; @@ -303,13 +303,13 @@ import { ErrorBoundary } from 'react-error-boundary'; function UserProfile({ userId }: { userId: number }) { const result = useQuery(GetUser, { id: userId }); - // This throws if the query is rejected, - // which the ErrorBoundary above will catch - const user = result.value; + // Hand a failure with nothing to show to the ErrorBoundary above + if (result.isRejected && !result.isReady) throw result.error; + if (!result.isReady) return ; return (
-

{user.name}

+

{result.value.name}

); } @@ -323,6 +323,8 @@ function App() { } ``` +`useSuspenseQuery` from `fetchium/react` does this for you: it suspends while a query with no value loads, and throws a failed cold fetch to the error boundary. + This is a conscious opt-in. The default explicit-checking pattern (`isRejected` + `error`) is recommended for most use cases because it gives you the most flexibility. Error boundaries are best reserved for catching truly unexpected failures that shouldn't be handled inline. --- diff --git a/packages/fetchium/plugin/agents/fetchium.md b/packages/fetchium/plugin/agents/fetchium.md index b873c30..2d8189a 100644 --- a/packages/fetchium/plugin/agents/fetchium.md +++ b/packages/fetchium/plugin/agents/fetchium.md @@ -137,7 +137,9 @@ function UserProfile() { } ``` -`useQuery` returns `ReactivePromise` with: `value`, `isReady`, `isPending`, `isResolved`, `isRejected`, `error`. Pass `{ suspended: true/false }` to control whether the query is active. +`useQuery` returns `ReactivePromise` with: `value`, `isReady`, `isPending`, `isResolved`, `isRejected`, `error`. It never suspends; `value` is `undefined` until the first value arrives. `useSuspenseQuery` (same arguments) suspends on a cold miss only, never for a refetch, and throws a failed cold fetch to the error boundary. + +To start a destination's query when the user taps, call `queryClient.prefetch(QueryClass, params, { ttl })` before navigating; the screen's `useQuery` then joins the running query. `queryClient.retain(() => [fetchQuery(A), fetchQuery(B)], { ttl })` keeps a set of queries active until its returned `release()` is called. ## React + Signalium Usage diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index 064d7dd..f0b102a 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -1,4 +1,4 @@ -import { context, watcher, ReactiveTask, type Context } from 'signalium'; +import { context, watcher, withContexts, ReactivePromise, ReactiveTask, type Context } from 'signalium'; import { hashValue } from 'signalium/utils'; import { EntityDef, @@ -35,6 +35,8 @@ import { queryKeyFor, } from './query-types.js'; import { SyncQueryStore, MemoryPersistentStore } from './stores/sync.js'; +import type { ExtractType } from './types.js'; +import type { Optionalize, Signalize } from './type-utils.js'; export interface QueryClientConfig { store?: QueryStore; @@ -84,6 +86,42 @@ export interface QueryClientConfig { pollResumeJitterMs?: number; } +export interface RetainOptions { + /** + * Milliseconds after which the lease releases itself. Omit to hold it until + * the returned `release` is called. + */ + ttl?: number; +} + +export interface PrefetchOptions { + /** + * Milliseconds to keep the query active if nothing else reads it. Default: + * `DEFAULT_PREFETCH_TTL` (10 s). + */ + ttl?: number; +} + +/** How long `prefetch()` keeps a query active when no `ttl` is given. */ +export const DEFAULT_PREFETCH_TTL = 10_000; + +/** + * How long a suspense hold outlives its fetch settling when no reader commits + * to claim it (the suspended tree was abandoned). + */ +const SUSPENSE_HOLD_TTL = 10_000; + +interface SuspenseHold { + release: () => void; + /** Resolves (never rejects) once the cold fetch settles. */ + settled: Promise | undefined; + /** The fetch this hold suspended on has settled. */ + done: boolean; + /** That fetch failed and its error has been handed to a render. */ + failed: boolean; + timer: ReturnType | undefined; +} + export { type QueryContext, type ActivitySource, @@ -122,6 +160,11 @@ export class QueryClient { private staggerQueue = new Set>(); private staggerTimer: ReturnType | undefined = undefined; + /** Release functions of outstanding `retain()` / `prefetch()` leases. */ + private leases = new Set<() => void>(); + /** Leases taken by `useSuspenseQuery` for cold misses, by query instance key. */ + private suspenseHolds = new Map(); + private context!: QueryContext; private typenameRegistry = new Map[]>(); private constraintRegistry = new Map(); @@ -328,6 +371,195 @@ export class QueryClient { return queryInstance.relay; } + // ====================================================== + // Leases (retain / prefetch) + // ====================================================== + + /** + * Keeps the queries `fn` reads active (fetched, subscribed, and exempt from + * GC) until the returned `release` is called, or for `ttl` milliseconds, + * whichever comes first. Use it to keep a hidden surface's queries warm, or + * to start the queries a likely next screen needs. + * + * `fn` runs immediately with this client as the `QueryClientContext`, and + * again whenever what it reads changes (for example a Signal param). A query + * is held when `fn` reads one of its fields (`isReady`, `value`, ...), or + * when `fn` returns its promise or an array of them: + * + * ```ts + * const release = client.retain(() => [fetchQuery(GetTokens), fetchQuery(GetPrices, { ids })]); + * // later, when the surface is gone for good: + * release(); + * ``` + * + * A reader that mounts while the lease is held joins the already active + * query: no new request, and the data (if it has arrived) on its first + * render. Releasing never interrupts other readers; when the last one goes, + * the query deactivates and its `gcTime` starts as usual. `release` is + * idempotent. + */ + retain(fn: () => unknown, options?: RetainOptions): () => void { + const w = withContexts([[QueryClientContext, this]], () => watcher(() => holdReturned(fn()))); + const unsubscribe = w.addListener(noop); + + let timer: ReturnType | undefined; + let released = false; + + const release = (): void => { + if (released) return; + released = true; + clearTimeout(timer); + this.leases.delete(release); + unsubscribe(); + }; + + this.leases.add(release); + + try { + // Run now, so the queries activate (and a synchronous store hydrates + // them) before this returns, not on Signalium's next flush. + void w.value; + } catch (error) { + release(); + throw error; + } + + const ttl = options?.ttl; + if (ttl !== undefined && Number.isFinite(ttl)) { + timer = setTimeout(release, Math.max(0, ttl)); + } + + return release; + } + + /** + * Starts `QueryClass` with `params` now and keeps it active for `ttl` + * milliseconds (default {@link DEFAULT_PREFETCH_TTL}), or until the returned + * `release` is called. A fetch still in flight when `ttl` runs out is let + * finish before the lease goes; `release()` lets go at once. Meant for the moment a user commits to a navigation + * (a tap): the destination's reader, mounting within the window, reuses the + * in-flight or finished fetch instead of starting its own, and renders the + * data on its first render if it has arrived. + * + * Cached data counts: with a synchronous store a cached, fresh result is + * applied without a request. A stale one is shown and refetched, as on any + * activation. + */ + prefetch( + QueryClass: new () => T, + params?: Optionalize>>, + options?: PrefetchOptions, + ): () => void { + const def = QueryDefinition.for(QueryClass); + let relay: QueryPromise | undefined; + const release = this.retain(() => (relay = this.getQuery(def, params as QueryParams | undefined))); + + const ttl = options?.ttl ?? DEFAULT_PREFETCH_TTL; + if (Number.isFinite(ttl)) { + const timer = setTimeout( + () => { + // Expiring mid-fetch would abort a request that is about to land: + // hold until it settles. An explicit release() still lets go at once. + if (relay?.isPending) relay.then(release, release); + else release(); + }, + Math.max(0, ttl), + ); + return () => { + clearTimeout(timer); + release(); + }; + } + + return release; + } + + /** + * For `useSuspenseQuery`. Returns a promise to suspend on when the query has + * never produced a value (a cold miss), or `undefined` when it has one to + * render (in memory, or hydrated now from a synchronous store). + * + * A cold miss takes a hold that keeps the query active while the render is + * suspended, since React discards a suspended render without subscribing. + * The reader's commit releases it (`releaseSuspenseHold`); otherwise it + * releases itself `SUSPENSE_HOLD_TTL` after the fetch settles. A query whose + * last fetch failed is refetched once per hold: when that attempt fails too, + * `error` is set and the hold is dropped, so the caller can throw it. + * + * @internal + */ + suspendOnColdMiss( + def: QueryDefinition, + params: QueryParams | undefined, + ): { promise: Promise | undefined; failed?: true; error?: unknown; key: number } { + const key = queryKeyFor(def, params); + const relay = this.getQuery(def, params); + if (relay.isReady) return { promise: undefined, key }; + + let hold = this.suspenseHolds.get(key); + if (hold === undefined) { + const release = this.retain(() => this.getQuery(def, params)); + hold = { release, settled: undefined, done: false, failed: false, timer: undefined }; + this.suspenseHolds.set(key, hold); + } + + if (relay.isReady) { + // Hydrated by the activation: render it, and keep the hold until the reader commits. + this.expireSuspenseHold(key, hold); + return { promise: undefined, key }; + } + + if (!relay.isPending) { + if (hold.done) { + // The fetch this hold waited for failed. Keep the hold until the next + // task: React re-renders a throwing component once more before giving + // up, and that render must throw the same error, not refetch. A later + // mount (an error boundary reset) gets a fresh hold and a new attempt. + const error = relay.error; + if (!hold.failed) { + hold.failed = true; + clearTimeout(hold.timer); + hold.timer = setTimeout(() => { + if (this.suspenseHolds.get(key) === hold) this.releaseSuspenseHold(key); + }, 0); + } + return { promise: undefined, failed: true, error, key }; + } + // A failed or not-yet-started fetch: start it now rather than after the activation's refetch hop. + this.queryInstances.get(key)?.refetch(); + } + + if (hold.settled === undefined) { + const current = hold; + current.settled = new Promise(resolve => { + const settle = (): void => { + current.done = true; + this.expireSuspenseHold(key, current); + resolve(); + }; + relay.then(settle, settle); + }); + } + + return { promise: hold.settled, key }; + } + + /** Releases a `useSuspenseQuery` hold once its reader has committed. @internal */ + releaseSuspenseHold(key: number): void { + const hold = this.suspenseHolds.get(key); + if (hold === undefined) return; + this.suspenseHolds.delete(key); + clearTimeout(hold.timer); + hold.release(); + } + + private expireSuspenseHold(key: number, hold: SuspenseHold): void { + if (hold.timer !== undefined || this.suspenseHolds.get(key) !== hold) return; + hold.timer = setTimeout(() => { + if (this.suspenseHolds.get(key) === hold) this.releaseSuspenseHold(key); + }, SUSPENSE_HOLD_TTL); + } + /** * Gets or creates a MutationResult for the given mutation definition. * Mutations are cached by their definition ID. @@ -606,6 +838,8 @@ export class QueryClient { } destroy(): void { + for (const key of [...this.suspenseHolds.keys()]) this.releaseSuspenseHold(key); + for (const release of [...this.leases]) release(); clearTimeout(this.staggerTimer); this.staggerTimer = undefined; this.staggerQueue.clear(); @@ -627,6 +861,25 @@ export class QueryClient { export const QueryClientContext: Context = context(undefined); +const noop = (): void => {}; + +/** + * Reads the query promises `retain`'s callback returned, so the lease's + * watcher depends on them and keeps them active. `isReady` changes once per + * query, so the watcher reruns at most once for each. + */ +function holdReturned(returned: unknown): void { + if (Array.isArray(returned)) { + for (const item of returned) holdOne(item); + } else { + holdOne(returned); + } +} + +function holdOne(value: unknown): void { + if (value instanceof ReactivePromise) void value.isReady; +} + function nonNegative(value: unknown): number { return typeof value === 'number' && value > 0 ? value : 0; } diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index c5441a0..0f0cc49 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -52,6 +52,11 @@ export class QueryInstance { private wasPaused: boolean = false; private currentParams: QueryParams | undefined = undefined; private debounceTimer: ReturnType | undefined = undefined; + /** + * Bumped to schedule or cancel a zero-delay refetch, which runs on a + * microtask and so can't be cleared like a timer. + */ + private debounceGeneration: number = 0; /** The last fetch ended in an error (not an abort). Disables the reactivation grace. */ private lastFetchFailed: boolean = false; /** @@ -146,8 +151,7 @@ export class QueryInstance { const deactivate = ({ isPausing = false }: DeactivateOptions = {}) => { this._isActive = false; - clearTimeout(this.debounceTimer); - this.debounceTimer = undefined; + this.cancelDebounced(); this._abortController?.abort(); this._abortController = undefined; @@ -474,17 +478,51 @@ export class QueryInstance { this.relayState.setPromise(this.runQuery()); } - private runDebounced(extraDelay: number = 0): void { + /** + * Starts a refetch after the query's `debounce` (plus `extraDelay`). Calls + * made before it starts are coalesced into one fetch. + * + * With no delay the fetch starts on a microtask rather than a timer. That + * still runs outside the reactive computation that asked for it (a relay + * update) and still coalesces every call made in the same task, but doesn't + * wait for the next macrotask. On React Native a zero timer goes through the + * native timing module and can wait up to a frame. + * + * `nextTask` keeps the zero-delay fetch on a timer. Invalidation uses it: a + * query invalidated in the same task its last watcher left is still active + * until Signalium's deactivation flush, and the timer lets that flush cancel + * the fetch rather than start and abort it. + */ + private runDebounced(extraDelay: number = 0, nextTask: boolean = false): void { if (this.relayState.isPending) return; - const debounce = this.config?.debounce ?? 0; + const delay = (this.config?.debounce ?? 0) + extraDelay; - clearTimeout(this.debounceTimer); + this.cancelDebounced(); - this.debounceTimer = setTimeout(() => { - this.debounceTimer = undefined; + if (delay > 0 || nextTask) { + this.debounceTimer = setTimeout(() => { + this.debounceTimer = undefined; + this.runQueryImmediately(); + }, delay); + return; + } + + const generation = this.debounceGeneration; + queueMicrotask(() => { + if (generation !== this.debounceGeneration) return; + this.debounceGeneration++; + // Another path started a fetch in the meantime (refetch(), activation). + if (this.relayState.isPending && this._abortController !== undefined) return; this.runQueryImmediately(); - }, debounce + extraDelay); + }); + } + + /** Cancels a refetch scheduled by runDebounced(). */ + private cancelDebounced(): void { + this.debounceGeneration++; + clearTimeout(this.debounceTimer); + this.debounceTimer = undefined; } /** @@ -500,15 +538,19 @@ export class QueryInstance { const queuedAt = this.reactivationQueuedAt; if (this.fetchStarts !== queuedAt) return; - clearTimeout(this.debounceTimer); - this.debounceTimer = setTimeout( - () => { - this.debounceTimer = undefined; - if (this.fetchStarts !== queuedAt) return; - this.runQueryImmediately(); - }, - (this.config?.debounce ?? 0) + delay, - ); + const totalDelay = (this.config?.debounce ?? 0) + delay; + if (totalDelay === 0) { + // Within the flush task, on runDebounced()'s microtask. + this.runDebounced(); + return; + } + + this.cancelDebounced(); + this.debounceTimer = setTimeout(() => { + this.debounceTimer = undefined; + if (this.fetchStarts !== queuedAt) return; + this.runQueryImmediately(); + }, totalDelay); } /** Records that the subscription delivered data. See `lastPushAt`. */ @@ -529,7 +571,7 @@ export class QueryInstance { markStale(): void { this.updatedAt = 0; if (this._isActive && !this.isPaused) { - this.runDebounced(); + this.runDebounced(0, true); } } diff --git a/packages/fetchium/src/__tests__/read-path-latency.test.ts b/packages/fetchium/src/__tests__/read-path-latency.test.ts new file mode 100644 index 0000000..20b6878 --- /dev/null +++ b/packages/fetchium/src/__tests__/read-path-latency.test.ts @@ -0,0 +1,208 @@ +/* eslint-disable @typescript-eslint/no-unused-expressions */ +import { describe, it, expect, afterEach } from 'vitest'; +import { signal, watcher, withContexts } from 'signalium'; +import { MemoryPersistentStore, SyncQueryStore } from '../stores/sync.js'; +import { QueryClient, QueryClientContext } from '../QueryClient.js'; +import { RESTQuery, RESTQueryAdapter } from '../rest/index.js'; +import { t } from '../typeDefs.js'; +import { fetchQuery } from '../query.js'; +import { createMockFetch, sleep } from './utils.js'; + +/** + * Read-path latency, counted in macrotask turns. Each turn drains the + * microtask queue, checks the condition, then yields one `setTimeout(0)`. + * Turn 0 means the condition held before any timer ran. + * + * On React Native a zero timer goes through the native timing module and can + * wait up to a frame, so a turn saved here is up to ~16 ms on device. The + * logged `ms` values are Node timers (about 1 ms per turn). + */ + +async function flushMicrotasks(count = 30): Promise { + for (let i = 0; i < count; i++) await Promise.resolve(); +} + +async function turnsUntil(cond: () => boolean, max = 10): Promise { + for (let turn = 0; turn <= max; turn++) { + await flushMicrotasks(); + if (cond()) return turn; + await new Promise(resolve => setTimeout(resolve, 0)); + } + return Infinity; +} + +class GetItem extends RESTQuery { + path = '/item'; + result = { n: t.number }; + config = { staleTime: 0 }; +} + +class GetFreshItem extends RESTQuery { + path = '/fresh'; + result = { n: t.number }; + config = { staleTime: 60_000 }; +} + +class GetUser extends RESTQuery { + params = { id: t.number }; + path = `/users/${this.params.id}`; + result = { n: t.number }; +} + +let clients: QueryClient[] = []; + +afterEach(() => { + for (const c of clients) c.destroy(); + clients = []; +}); + +function setup() { + const mockFetch = createMockFetch(); + let n = 0; + mockFetch.get('/item', () => ({ n: ++n })); + mockFetch.get('/fresh', () => ({ n: ++n })); + mockFetch.get('/users/[id]', () => ({ n: ++n })); + const client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], + }); + clients.push(client); + return { client, mockFetch }; +} + +/** Watches `read` and runs it synchronously, so activation happens now. */ +function watchNow(client: QueryClient, read: () => T): { w: { value: T }; unsub: () => void } { + const w = withContexts([[QueryClientContext, client]], () => watcher(read)); + const unsub = w.addListener(() => {}); + w.value; + return { w: w as { value: T }, unsub }; +} + +async function loadThenDeactivate(client: QueryClient, Q: new () => RESTQuery): Promise { + const { unsub } = watchNow(client, () => fetchQuery(Q as any).value); + await sleep(10); + unsub(); + // Let Signalium's deactivation flush run. + await sleep(10); +} + +describe('read-path latency (macrotask turns)', () => { + it('cold miss: the first fetch starts without a macrotask', async () => { + const { client, mockFetch } = setup(); + const started = performance.now(); + const { unsub } = watchNow(client, () => fetchQuery(GetItem).value); + const turns = await turnsUntil(() => mockFetch.calls.length === 1); + console.log(`[latency] cold miss -> fetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`); + expect(turns).toBe(0); + unsub(); + }); + + it('in-memory cache hit: the value is readable in the activating read', async () => { + const { client, mockFetch } = setup(); + await loadThenDeactivate(client, GetFreshItem); + expect(mockFetch.calls).toHaveLength(1); + + const { w, unsub } = watchNow(client, () => fetchQuery(GetFreshItem).value); + // Same synchronous read that activated the query: no microtask, no timer. + expect(w.value).toMatchObject({ n: 1 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + unsub(); + }); + + it('stale reactivation: the refetch starts without a macrotask', async () => { + const { client, mockFetch } = setup(); + await loadThenDeactivate(client, GetItem); + expect(mockFetch.calls).toHaveLength(1); + + const started = performance.now(); + const { w, unsub } = watchNow(client, () => fetchQuery(GetItem).value); + // The cached value shows immediately while the refetch runs. + expect(w.value).toMatchObject({ n: 1 }); + const turns = await turnsUntil(() => mockFetch.calls.length === 2); + console.log( + `[latency] stale reactivation -> refetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`, + ); + expect(turns).toBe(0); + unsub(); + }); + + it('staggered reactivation: the first refetch starts in the stagger flush', async () => { + const mockFetch = createMockFetch(); + let n = 0; + mockFetch.get('/item', () => ({ n: ++n })); + const client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], + reactivationStaggerMs: 300, + }); + clients.push(client); + await loadThenDeactivate(client, GetItem); + + const started = performance.now(); + const { unsub } = watchNow(client, () => fetchQuery(GetItem).value); + const turns = await turnsUntil(() => mockFetch.calls.length === 2); + console.log( + `[latency] staggered reactivation -> first refetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`, + ); + // One timer for the stagger flush, which groups the task's reactivations. + expect(turns).toBe(1); + unsub(); + }); + + it('Signal param change: the new fetch starts in the flush that applies it', async () => { + const { client, mockFetch } = setup(); + const id = signal(1); + const { w, unsub } = watchNow(client, () => fetchQuery(GetUser, { id }).value); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + + const started = performance.now(); + id.value = 2; + w.value; + const turns = await turnsUntil(() => mockFetch.calls.length === 2); + console.log( + `[latency] param change -> fetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`, + ); + // A watched relay reruns its update in Signalium's flush (one timer); the + // fetch then starts on a microtask of that flush rather than another timer. + expect(turns).toBe(1); + expect(mockFetch.calls[1].url).toContain('/users/2'); + unsub(); + }); + + it('coalesces zero-delay refetches requested in the same task', async () => { + const { client, mockFetch } = setup(); + const id = signal(1); + const { w, unsub } = watchNow(client, () => fetchQuery(GetUser, { id }).value); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + + id.value = 2; + w.value; + id.value = 3; + w.value; + await sleep(10); + expect(mockFetch.calls).toHaveLength(2); + expect(mockFetch.calls[1].url).toContain('/users/3'); + unsub(); + }); + + it('a deactivation in the same task aborts the zero-delay refetch', async () => { + const { client, mockFetch } = setup(); + await loadThenDeactivate(client, GetItem); + mockFetch.reset(); + mockFetch.get('/item', { n: 2 }, { delay: 30 }); + const { unsub } = watchNow(client, () => fetchQuery(GetItem).value); + unsub(); + // Before the response (30 ms). + await sleep(10); + // Signalium deactivates on its next flush (a timer), after the microtask + // that starts the refetch, so the refetch starts and deactivation cancels + // it. A mount and unmount within one task is the cost of not waiting a + // timer on every reactivation. + expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); + await sleep(30); + }); +}); diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts new file mode 100644 index 0000000..0f3f8c2 --- /dev/null +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -0,0 +1,323 @@ +/* eslint-disable @typescript-eslint/no-unused-expressions */ +import { describe, it, expect, afterEach } from 'vitest'; +import { signal, watcher, withContexts } from 'signalium'; +import { MemoryPersistentStore, SyncQueryStore } from '../stores/sync.js'; +import { QueryClient, QueryClientContext, DEFAULT_PREFETCH_TTL } from '../QueryClient.js'; +import { RESTQuery, RESTQueryAdapter } from '../rest/index.js'; +import { t } from '../typeDefs.js'; +import { fetchQuery, QueryDefinition } from '../query.js'; +import type { MutationEvent } from '../types.js'; +import { createMockFetch, sleep } from './utils.js'; + +/** + * client.retain() and client.prefetch(): app-level leases that keep queries + * active without a reader, so a reader that mounts inside the lease joins the + * running query instead of starting its own. + */ + +async function flushMicrotasks(count = 30): Promise { + for (let i = 0; i < count; i++) await Promise.resolve(); +} + +class GetItem extends RESTQuery { + path = '/item'; + result = { n: t.number }; + config = { staleTime: 0 }; +} + +class GetOther extends RESTQuery { + path = '/other'; + result = { n: t.number }; + config = { staleTime: 0 }; +} + +class GetUser extends RESTQuery { + params = { id: t.number }; + path = `/users/${this.params.id}`; + result = { n: t.number }; +} + +class GetFreshUser extends RESTQuery { + params = { id: t.number }; + path = `/users/${this.params.id}`; + result = { n: t.number }; + config = { staleTime: 60_000 }; +} + +let subscribed = 0; +let unsubscribed = 0; + +class GetStreamed extends RESTQuery { + path = '/streamed'; + result = { n: t.number }; + config = { + subscribe: (_onEvent: (event: MutationEvent) => void) => { + subscribed++; + return () => { + unsubscribed++; + }; + }, + }; +} + +let clients: QueryClient[] = []; + +afterEach(() => { + for (const c of clients) c.destroy(); + clients = []; + subscribed = 0; + unsubscribed = 0; +}); + +function setup(kv: MemoryPersistentStore = new MemoryPersistentStore()) { + const mockFetch = createMockFetch(); + let n = 0; + for (const path of ['/item', '/other', '/users/[id]', '/streamed']) { + mockFetch.get(path, () => ({ n: ++n })); + } + const client = new QueryClient({ + store: new SyncQueryStore(kv), + adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], + }); + clients.push(client); + return { client, mockFetch, kv }; +} + +/** Mounts a reader: watches `read` and runs it now, like a React render. */ +function mountReader(client: QueryClient, read: () => T): { w: { value: T }; unsub: () => void } { + const w = withContexts([[QueryClientContext, client]], () => watcher(read)); + const unsub = w.addListener(() => {}); + w.value; + return { w: w as { value: T }, unsub }; +} + +describe('client.prefetch()', () => { + it('starts the fetch before returning control to a timer', async () => { + const { client, mockFetch } = setup(); + client.prefetch(GetItem); + expect(mockFetch.calls).toHaveLength(0); + await flushMicrotasks(); + expect(mockFetch.calls).toHaveLength(1); + }); + + it('a reader mounting after the data arrives renders it on its first read, with no second request', async () => { + const { client, mockFetch } = setup(); + client.prefetch(GetItem, undefined, { ttl: 1_000 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + + // staleTime 0, yet the query is still active, so mounting is not a reactivation. + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + expect(w.value).toMatchObject({ n: 1 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + unsub(); + }); + + it('a reader mounting while the fetch is in flight joins it', async () => { + const { client, mockFetch } = setup(); + mockFetch.reset(); + mockFetch.get('/item', { n: 7 }, { delay: 30 }); + client.prefetch(GetItem, undefined, { ttl: 1_000 }); + await sleep(5); + + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + expect(w.value).toBeUndefined(); + await sleep(50); + expect(w.value).toMatchObject({ n: 7 }); + expect(mockFetch.calls).toHaveLength(1); + unsub(); + }); + + it('passes params, and keys on them', async () => { + const { client, mockFetch } = setup(); + client.prefetch(GetUser, { id: 4 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls[0].url).toContain('/users/4'); + + const { w, unsub } = mountReader(client, () => fetchQuery(GetUser, { id: 4 }).value); + expect(w.value).toMatchObject({ n: 1 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + unsub(); + }); + + it('serves fresh cached data from a synchronous store without a request', async () => { + const kv = new MemoryPersistentStore(); + const seed = setup(kv); + seed.client.prefetch(GetFreshUser, { id: 1 }); + await sleep(10); + expect(seed.mockFetch.calls).toHaveLength(1); + seed.client.destroy(); + + const { client, mockFetch } = setup(kv); + client.prefetch(GetFreshUser, { id: 1 }); + // Hydrated by the activation inside prefetch(), before it returned. + expect(client.getQuery(QueryDefinition.for(GetFreshUser), { id: 1 }).value).toMatchObject({ n: 1 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(0); + }); + + it('expires after ttl: the query deactivates, and a later mount is a reactivation', async () => { + const { client, mockFetch } = setup(); + client.prefetch(GetItem, undefined, { ttl: 20 }); + await sleep(60); + expect(mockFetch.calls).toHaveLength(1); + + // staleTime 0 and no grace: reactivating refetches. + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + expect(w.value).toMatchObject({ n: 1 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(2); + unsub(); + }); + + it('a ttl that runs out mid-fetch waits for the fetch instead of aborting it', async () => { + const { client, mockFetch } = setup(); + mockFetch.reset(); + mockFetch.get('/streamed', { n: 9 }, { delay: 40 }); + client.prefetch(GetStreamed, undefined, { ttl: 10 }); + await sleep(25); + // Past the ttl, before the response (40 ms): still held, request not cancelled. + expect(unsubscribed).toBe(0); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(false); + await sleep(60); + expect(mockFetch.calls).toHaveLength(1); + // The fetch landed (an aborted one applies nothing), then the lease went. + const relay = client.getQuery(QueryDefinition.for(GetStreamed), undefined); + expect(relay.value).toMatchObject({ n: 9 }); + expect(unsubscribed).toBe(1); + }); + + it('defaults ttl to DEFAULT_PREFETCH_TTL', () => { + expect(DEFAULT_PREFETCH_TTL).toBe(10_000); + }); + + it('release() while a reader is mounted leaves the reader running', async () => { + const { client, mockFetch } = setup(); + mockFetch.reset(); + mockFetch.get('/item', { n: 3 }, { delay: 30 }); + const release = client.prefetch(GetItem); + await sleep(5); + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + release(); + release(); + await sleep(50); + expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(false); + expect(w.value).toMatchObject({ n: 3 }); + unsub(); + }); + + it('release() with no reader aborts an in-flight fetch', async () => { + const { client, mockFetch } = setup(); + mockFetch.reset(); + mockFetch.get('/item', { n: 3 }, { delay: 30 }); + const release = client.prefetch(GetItem); + await sleep(5); + release(); + // Still before the response (30 ms): the request has been cancelled. + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); + await sleep(40); + }); +}); + +describe('client.retain()', () => { + it('holds the query promises the callback returns', async () => { + const { client, mockFetch } = setup(); + const release = client.retain(() => [fetchQuery(GetItem), fetchQuery(GetOther)]); + await sleep(10); + expect(mockFetch.calls.map(c => new URL(c.url).pathname).sort()).toEqual(['/item', '/other']); + + const { w, unsub } = mountReader(client, () => [fetchQuery(GetItem).value, fetchQuery(GetOther).value]); + expect(w.value).toEqual([expect.objectContaining({ n: expect.any(Number) }), expect.any(Object)]); + await sleep(10); + expect(mockFetch.calls).toHaveLength(2); + unsub(); + release(); + }); + + it('holds queries whose fields the callback reads', async () => { + const { client, mockFetch } = setup(); + const release = client.retain(() => { + fetchQuery(GetItem).isReady; + }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + expect(w.value).toMatchObject({ n: 1 }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + unsub(); + release(); + }); + + it('keeps a subscription running until released', async () => { + const { client } = setup(); + const release = client.retain(() => fetchQuery(GetStreamed)); + await sleep(10); + expect(subscribed).toBe(1); + + // A reader coming and going doesn't restart it. + const reader = mountReader(client, () => fetchQuery(GetStreamed).value); + reader.unsub(); + await sleep(10); + expect(subscribed).toBe(1); + expect(unsubscribed).toBe(0); + + release(); + await sleep(10); + expect(unsubscribed).toBe(1); + }); + + it('follows Signal params', async () => { + const { client, mockFetch } = setup(); + const id = signal(1); + const release = client.retain(() => fetchQuery(GetUser, { id })); + await sleep(10); + id.value = 2; + await sleep(20); + expect(mockFetch.calls.map(c => new URL(c.url).pathname)).toEqual(['/users/1', '/users/2']); + release(); + }); + + it('without ttl, holds until released', async () => { + const { client } = setup(); + const release = client.retain(() => fetchQuery(GetStreamed)); + await sleep(50); + expect(unsubscribed).toBe(0); + release(); + await sleep(10); + expect(unsubscribed).toBe(1); + }); + + it('with ttl, releases itself', async () => { + const { client } = setup(); + client.retain(() => fetchQuery(GetStreamed), { ttl: 20 }); + await sleep(10); + expect(subscribed).toBe(1); + await sleep(40); + expect(unsubscribed).toBe(1); + }); + + it('destroy() releases outstanding leases', async () => { + const { client } = setup(); + client.retain(() => fetchQuery(GetStreamed)); + await sleep(10); + client.destroy(); + await sleep(10); + expect(unsubscribed).toBe(1); + }); + + it('releases and rethrows when the callback throws', () => { + const { client } = setup(); + expect(() => + client.retain(() => { + throw new Error('boom'); + }), + ).toThrow('boom'); + }); +}); diff --git a/packages/fetchium/src/index.ts b/packages/fetchium/src/index.ts index 350503c..92e665b 100644 --- a/packages/fetchium/src/index.ts +++ b/packages/fetchium/src/index.ts @@ -1,7 +1,7 @@ export * from './types.js'; -export { QueryClient, QueryClientContext } from './QueryClient.js'; -export type { QueryContext, QueryClientConfig, ActivitySource } from './QueryClient.js'; +export { QueryClient, QueryClientContext, DEFAULT_PREFETCH_TTL } from './QueryClient.js'; +export type { QueryContext, QueryClientConfig, ActivitySource, RetainOptions, PrefetchOptions } from './QueryClient.js'; export { QueryAdapter } from './QueryAdapter.js'; export type { IQueryClientForAdapter } from './QueryAdapter.js'; export { t, registerFormat } from './typeDefs.js'; diff --git a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx new file mode 100644 index 0000000..cee950a --- /dev/null +++ b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx @@ -0,0 +1,328 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { render } from 'vitest-browser-react'; +import { ContextProvider } from 'signalium/react'; +import React, { Suspense } from 'react'; +import { MemoryPersistentStore, SyncQueryStore } from '../../stores/sync.js'; +import { QueryClient, QueryClientContext } from '../../QueryClient.js'; +import { t } from '../../typeDefs.js'; +import { RESTQuery } from '../../rest/index.js'; +import { RESTQueryAdapter } from '../../rest/RESTQueryAdapter.js'; +import { createMockFetch, sleep } from '../../__tests__/utils.js'; +import type { MutationEvent } from '../../types.js'; +import { useQuery } from '../use-query.js'; +import { useSuspenseQuery } from '../use-suspense-query.js'; + +class GetItem extends RESTQuery { + path = '/item'; + result = { name: t.string }; + config = { staleTime: 0 }; +} + +class GetFreshItem extends RESTQuery { + path = '/fresh'; + result = { name: t.string }; + config = { staleTime: 60_000 }; +} + +let subscribed = 0; +let unsubscribed = 0; + +class GetStreamed extends RESTQuery { + path = '/streamed'; + result = { name: t.string }; + config = { + subscribe: (_onEvent: (event: MutationEvent) => void) => { + subscribed++; + return () => { + unsubscribed++; + }; + }, + }; +} + +let clients: QueryClient[] = []; + +afterEach(() => { + for (const c of clients) c.destroy(); + clients = []; + subscribed = 0; + unsubscribed = 0; +}); + +function makeClient(mockFetch: ReturnType, kv = new MemoryPersistentStore()): QueryClient { + const client = new QueryClient({ + store: new SyncQueryStore(kv), + adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], + }); + clients.push(client); + return client; +} + +class Boundary extends React.Component< + { children: React.ReactNode; onError: (e: unknown) => void }, + { error: unknown } +> { + state = { error: undefined as unknown }; + static getDerivedStateFromError(error: unknown) { + return { error }; + } + componentDidCatch(error: unknown) { + this.props.onError(error); + } + render() { + return this.state.error !== undefined ?
Boundary caught
: this.props.children; + } +} + +describe('client.prefetch() with React', () => { + it('a screen mounting after a tap-time prefetch renders the data on its first render, with one request', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', { name: 'prefetched' }, { delay: 20 }); + const client = makeClient(mockFetch); + + // The tap. + client.prefetch(GetItem, undefined, { ttl: 5_000 }); + // The navigation animation. + await sleep(60); + + const renders: Array = []; + function Item(): React.ReactNode { + const item = useQuery(GetItem); + const label = item.isReady ? item.value.name : undefined; + renders.push(label); + return
{label ?? 'Loading...'}
; + } + + const { getByText } = render( + + + , + ); + + await expect.element(getByText('prefetched')).toBeInTheDocument(); + await sleep(50); + expect(renders[0]).toBe('prefetched'); + expect(renders).not.toContain(undefined); + expect(mockFetch.calls).toHaveLength(1); + }); + + it('a screen mounting before the prefetch lands joins the in-flight request', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', { name: 'joined' }, { delay: 60 }); + const client = makeClient(mockFetch); + + client.prefetch(GetItem); + await sleep(10); + + function Item(): React.ReactNode { + const item = useQuery(GetItem); + return
{item.isReady ? item.value.name : 'Loading...'}
; + } + + const { getByText } = render( + + + , + ); + + await expect.element(getByText('joined')).toBeInTheDocument(); + expect(mockFetch.calls).toHaveLength(1); + }); +}); + +describe('useSuspenseQuery', () => { + it('suspends on a cold miss, then renders the data, with one request', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', { name: 'loaded' }, { delay: 20 }); + const client = makeClient(mockFetch); + + const renders: string[] = []; + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetItem); + renders.push(item.value.name); + return
{item.value.name}
; + } + + const { getByText } = render( + + Suspended}> + + + , + ); + + await expect.element(getByText('Suspended')).toBeInTheDocument(); + await expect.element(getByText('loaded')).toBeInTheDocument(); + await sleep(30); + expect(renders.every(r => r === 'loaded')).toBe(true); + expect(mockFetch.calls).toHaveLength(1); + }); + + it('does not suspend when the value is in memory, and does not suspend for the refetch', async () => { + const mockFetch = createMockFetch(); + let n = 0; + mockFetch.get('/item', () => ({ name: `v${++n}` }), { delay: 20 }); + const client = makeClient(mockFetch); + + // Warm it, then let it go inactive. + const release = client.prefetch(GetItem); + await sleep(40); + release(); + await sleep(20); + expect(mockFetch.calls).toHaveLength(1); + + let fallbacks = 0; + function Fallback(): React.ReactNode { + fallbacks++; + return
Suspended
; + } + const renders: string[] = []; + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetItem); + renders.push(item.value.name); + return
{item.value.name}
; + } + + const { getByText } = render( + + }> + + + , + ); + + // staleTime 0: the reactivation refetches, and the refetch must not suspend. + await expect.element(getByText('v2')).toBeInTheDocument(); + expect(renders[0]).toBe('v1'); + expect(fallbacks).toBe(0); + expect(mockFetch.calls).toHaveLength(2); + }); + + it('does not suspend when a synchronous store has the value', async () => { + const kv = new MemoryPersistentStore(); + const seedFetch = createMockFetch(); + seedFetch.get('/fresh', { name: 'persisted' }); + const seed = makeClient(seedFetch, kv); + seed.prefetch(GetFreshItem); + await sleep(20); + seed.destroy(); + + const mockFetch = createMockFetch(); + const client = makeClient(mockFetch, kv); + + let fallbacks = 0; + function Fallback(): React.ReactNode { + fallbacks++; + return
Suspended
; + } + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetFreshItem); + return
{item.value.name}
; + } + + const { getByText } = render( + + }> + + + , + ); + + await expect.element(getByText('persisted')).toBeInTheDocument(); + expect(fallbacks).toBe(0); + expect(mockFetch.calls).toHaveLength(0); + }); + + it('throws a failed cold fetch to the error boundary', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', null, { status: 500 }); + const client = makeClient(mockFetch); + + class GetNoRetry extends RESTQuery { + path = '/item'; + result = { name: t.string }; + config = { retry: false }; + } + + const errors: unknown[] = []; + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetNoRetry); + return
{item.value.name}
; + } + + const { getByText } = render( + + errors.push(e)}> + Suspended}> + + + + , + ); + + await expect.element(getByText('Boundary caught')).toBeInTheDocument(); + expect(errors.length).toBeGreaterThan(0); + expect(mockFetch.calls).toHaveLength(1); + }); + + it('retries a failed cold fetch when the error boundary resets', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', { error: 'down' }, { status: 500 }); + const client = makeClient(mockFetch); + + class GetNoRetry extends RESTQuery { + path = '/item'; + result = { name: t.string }; + config = { retry: false }; + } + + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetNoRetry); + return
{item.value.name}
; + } + + const tree = (attempt: number) => ( + + {}}> + Suspended}> + + + + + ); + + const screen = render(tree(0)); + await expect.element(screen.getByText('Boundary caught')).toBeInTheDocument(); + await sleep(10); + + mockFetch.get('/item', { name: 'recovered' }); + screen.rerender(tree(1)); + await expect.element(screen.getByText('recovered')).toBeInTheDocument(); + expect(mockFetch.calls).toHaveLength(2); + }); + + it('hands the query to the reader on commit, so unmounting deactivates it', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/streamed', { name: 'live' }, { delay: 10 }); + const client = makeClient(mockFetch); + + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetStreamed); + return
{item.value.name}
; + } + + const screen = render( + + Suspended}> + + + , + ); + + await expect.element(screen.getByText('live')).toBeInTheDocument(); + expect(subscribed).toBe(1); + screen.unmount(); + await sleep(30); + expect(unsubscribed).toBe(1); + }); +}); diff --git a/packages/fetchium/src/react/index.ts b/packages/fetchium/src/react/index.ts index ee11804..a01c17b 100644 --- a/packages/fetchium/src/react/index.ts +++ b/packages/fetchium/src/react/index.ts @@ -1 +1,2 @@ export { useQuery } from './use-query.js'; +export { useSuspenseQuery } from './use-suspense-query.js'; diff --git a/packages/fetchium/src/react/use-suspense-query.ts b/packages/fetchium/src/react/use-suspense-query.ts new file mode 100644 index 0000000..9ce80d9 --- /dev/null +++ b/packages/fetchium/src/react/use-suspense-query.ts @@ -0,0 +1,62 @@ +import { useEffect } from 'react'; +import type { ReadyReactivePromise } from 'signalium'; +import { useContext } from 'signalium/react'; +import { ExtractType, QueryResult } from '../types.js'; +import { Query, QueryDefinition } from '../query.js'; +import { QueryClientContext, type QueryParams } from '../QueryClient.js'; +import { HasRequiredKeys, Optionalize, Signalize } from '../type-utils.js'; +import { useQuery } from './use-query.js'; + +/** + * `useQuery` for components rendered under a React `` boundary. + * + * Suspends only on a cold miss: the query has never produced a value, neither + * in memory nor (with a synchronous store) in the persisted cache. Anything + * else renders immediately, including a stale value whose refetch is in + * flight; refetches never suspend. A cold fetch that fails is thrown to the + * nearest error boundary; resetting the boundary retries it. + * + * Changing params to ones with no value yet is a cold miss, so it suspends. + * Wrap the change in `startTransition` to keep showing the previous result. + * + * Call it from a plain function component. Like `useQuery`, it can't run + * inside a Signalium `component()` (that is a reactive context), where + * `fetchQuery` is the way to read queries. + */ +export function useSuspenseQuery( + QueryClass: new () => T, + ...args: HasRequiredKeys> extends true + ? [params: Optionalize>>] + : [params?: Optionalize>> | undefined] +): ReadyReactivePromise> { + const client = useContext(QueryClientContext); + + if (client === undefined) { + throw new Error('QueryClient not found'); + } + + // Decide before useQuery: a render that suspends is discarded without a + // commit, so a subscription taken by it would never be cleaned up. The + // client holds the query active while suspended instead. + const { promise, failed, error, key } = client.suspendOnColdMiss( + QueryDefinition.for(QueryClass), + args[0] as QueryParams | undefined, + ); + + if (failed) { + throw error; + } + + if (promise !== undefined) { + throw promise; + } + + const result = useQuery(QueryClass, ...args); + + // useQuery's subscription now keeps the query active. + useEffect(() => { + client.releaseSuspenseHold(key); + }, [client, key]); + + return result as ReadyReactivePromise>; +} From 1e0b42ba6508377c26ecb8efa9bd9e54709b92d7 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 03:02:59 -0400 Subject: [PATCH 02/18] perf(fetchium): skip live value notify when a reducer returns the current value A stream reconnect re-delivers every entity as an update event. Entities, writes and live-array membership were already quiet for identical data, but every update event still ran the live value's onUpdate reducer and notified unconditionally, even when the reducer returned the value it was given. Only notify when the reduced value changed, as LiveValueInstance.reset already does. Adds tests pinning identical re-delivery as a no-op across query results, mutation events, topic events and nested entities (no entity notify, no version bump, no write, no live collection notify, no React re-render), and that a real change still notifies exactly once. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/LiveCollection.ts | 6 +- .../__tests__/identical-redelivery.test.ts | 350 ++++++++++++++++++ .../identical-redelivery-react.test.tsx | 148 ++++++++ 3 files changed, 503 insertions(+), 1 deletion(-) create mode 100644 packages/fetchium/src/__tests__/identical-redelivery.test.ts create mode 100644 packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx diff --git a/packages/fetchium/src/LiveCollection.ts b/packages/fetchium/src/LiveCollection.ts index 9262243..34ae819 100644 --- a/packages/fetchium/src/LiveCollection.ts +++ b/packages/fetchium/src/LiveCollection.ts @@ -435,6 +435,7 @@ export class LiveValueInstance { entityData: Record, eventType: 'create' | 'update' | 'delete', ): void { + const prev = this._value; switch (eventType) { case 'create': if (this._createdKeys.has(entityKey)) return; @@ -450,7 +451,10 @@ export class LiveValueInstance { this._value = this._onDelete(this._value, entity ?? entityData); break; } - this._notifier.notify(); + // Update events route here even when they changed nothing (a stream + // re-delivering current data). A reducer that returns the current value is + // a no-op, as in `reset`. + if (this._value !== prev) this._notifier.notify(); } getValue(): unknown { diff --git a/packages/fetchium/src/__tests__/identical-redelivery.test.ts b/packages/fetchium/src/__tests__/identical-redelivery.test.ts new file mode 100644 index 0000000..4b1a32b --- /dev/null +++ b/packages/fetchium/src/__tests__/identical-redelivery.test.ts @@ -0,0 +1,350 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { hashValue } from 'signalium/utils'; +import { t } from '../typeDefs.js'; +import { Entity } from '../proxy.js'; +import { EntityInstance } from '../EntityInstance.js'; +import { LiveCollectionBinding } from '../LiveCollection.js'; +import { QueryClient } from '../QueryClient.js'; +import { RESTQuery } from '../rest/index.js'; +import { fetchQuery } from '../query.js'; +import { MemoryPersistentStore, SyncQueryStore } from '../stores/sync.js'; +import { TopicQuery } from '../topic/TopicQuery.js'; +import { TopicQueryAdapter } from '../topic/TopicQueryAdapter.js'; +import type { MutationEvent, QueryPromise } from '../types.js'; +import { testWithClient, setupTestClient } from './utils.js'; + +/** + * Identical Re-delivery + * + * A stream reconnect re-snapshots every topic, so every entity on screen is + * re-delivered with the data the store already holds — as a query result, as a + * mutation event, or as a topic event. None of those may notify an entity, bump + * its version, write it, or notify a live collection. A real change still + * notifies exactly once. + */ + +/** Records `typename:id` for every entity notify. */ +function recordNotifies() { + const notified: string[] = []; + const original = EntityInstance.prototype.notify; + const spy = vi.spyOn(EntityInstance.prototype, 'notify').mockImplementation(function (this: EntityInstance) { + notified.push(`${this.typename}:${this.id}`); + original.call(this); + }); + return { notified, restore: () => spy.mockRestore() }; +} + +function savesOf(spy: ReturnType, typename: string) { + return spy.mock.calls.filter(([, value]) => (value as Record)?.__typename === typename); +} + +class Owner extends Entity { + __typename = t.typename('Owner'); + id = t.id; + name = t.string; +} + +class Token extends Entity { + __typename = t.typename('Token'); + id = t.id; + walletId = t.string; + symbol = t.string; + price = t.number; + issued = t.format('date-time'); + metadata = t.object({ logo: t.string, tags: t.array(t.string) }); + owner = t.entity(Owner); +} + +class Wallet extends Entity { + __typename = t.typename('Wallet'); + id = t.id; + tokens = t.liveArray(Token, { constraints: { walletId: (this as unknown as { id: string }).id } }); + tokenCount = t.liveValue(t.number, Token, { + constraints: { walletId: (this as unknown as { id: string }).id }, + onCreate: (v: number) => v + 1, + onUpdate: (v: number) => v, + onDelete: (v: number) => v - 1, + }); +} + +function token(i: number, overrides: { price?: number; owner?: string } = {}) { + return { + __typename: 'Token', + id: `tok-${i}`, + walletId: 'w-1', + symbol: `SYM${i}`, + price: overrides.price ?? i, + issued: '2026-01-01T00:00:00.000Z', + metadata: { logo: `logo-${i}.png`, tags: ['defi', 'l1'] }, + owner: { __typename: 'Owner', id: 'o-1', name: overrides.owner ?? 'Ann' }, + }; +} + +function wallet(count: number, override?: (i: number) => { price?: number; owner?: string }) { + return { + wallet: { + __typename: 'Wallet', + id: 'w-1', + tokenCount: count, + tokens: Array.from({ length: count }, (_, i) => token(i, override?.(i))), + }, + }; +} + +const tokenKey = (i: number) => hashValue(['Token', `tok-${i}`]); +const ownerKey = hashValue(['Owner', 'o-1']); +const walletKey = hashValue(['Wallet', 'w-1']); + +function versions(client: QueryClient, keys: number[]) { + return keys.map(k => client.entityMap.getEntity(k)!.version); +} + +/** Spies on the notifiers of a wallet's live array and live value. */ +function liveNotifies(client: QueryClient) { + const data = client.entityMap.getEntity(walletKey)!.data; + const tokens = data.tokens as LiveCollectionBinding; + const count = data.tokenCount as LiveCollectionBinding; + return { + tokens: vi.spyOn((tokens.instance as unknown as { _notifier: { notify(): void } })._notifier, 'notify'), + count: vi.spyOn((count.instance as unknown as { _notifier: { notify(): void } })._notifier, 'notify'), + }; +} + +// ====================================================== +// Query results +// ====================================================== + +class GetWallet extends RESTQuery { + path = '/wallet'; + result = { wallet: t.entity(Wallet) }; +} + +describe('Identical re-delivery', () => { + const getClient = setupTestClient(); + let recorder: ReturnType | undefined; + + afterEach(() => { + recorder?.restore(); + recorder = undefined; + vi.restoreAllMocks(); + }); + + async function loadWallet(count: number) { + const tc = getClient(); + tc.mockFetch.get('/wallet', wallet(count)); + let query!: QueryPromise; + await testWithClient(tc.client, async () => { + query = fetchQuery(GetWallet); + await query; + }); + return { ...tc, query }; + } + + describe('(a) query result', () => { + it('is a no-op when a refetch re-delivers identical data', async () => { + const { client, mockFetch, store, query } = await loadWallet(3); + const keys = [walletKey, ownerKey, tokenKey(0), tokenKey(1), tokenKey(2)]; + const before = versions(client, keys); + + const saveEntity = vi.spyOn(store, 'saveEntity'); + const live = liveNotifies(client); + recorder = recordNotifies(); + + await testWithClient(client, async () => { + mockFetch.get('/wallet', wallet(3)); + (query.value as unknown as { __refetch(): void }).__refetch(); + await query; + }); + + expect(recorder.notified).toEqual([]); + expect(versions(client, keys)).toEqual(before); + expect(saveEntity).not.toHaveBeenCalled(); + expect(live.tokens).not.toHaveBeenCalled(); + expect(live.count).not.toHaveBeenCalled(); + }); + + it('(d) notifies only the changed parent when its nested entity is unchanged', async () => { + const { client, mockFetch, store, query } = await loadWallet(2); + const [ownerBefore, tok0Before, tok1Before] = versions(client, [ownerKey, tokenKey(0), tokenKey(1)]); + + const saveEntity = vi.spyOn(store, 'saveEntity'); + const live = liveNotifies(client); + recorder = recordNotifies(); + + await testWithClient(client, async () => { + mockFetch.get( + '/wallet', + wallet(2, i => (i === 1 ? { price: 99 } : {})), + ); + (query.value as unknown as { __refetch(): void }).__refetch(); + await query; + }); + + expect(recorder.notified).toEqual(['Token:tok-1']); + expect(versions(client, [ownerKey, tokenKey(0), tokenKey(1)])).toEqual([ownerBefore, tok0Before, tok1Before + 1]); + expect(savesOf(saveEntity, 'Token')).toHaveLength(1); + expect(savesOf(saveEntity, 'Owner')).toHaveLength(0); + expect(savesOf(saveEntity, 'Wallet')).toHaveLength(0); + expect(live.tokens).not.toHaveBeenCalled(); + expect(live.count).not.toHaveBeenCalled(); + }); + }); + + // ====================================================== + // Mutation events + // ====================================================== + + describe('(b) mutation event', () => { + it('is a no-op for a full update carrying identical data, nested values included', async () => { + const { client, store } = await loadWallet(2); + const keys = [walletKey, ownerKey, tokenKey(0), tokenKey(1)]; + const before = versions(client, keys); + const metadata = client.entityMap.getEntity(tokenKey(0))!.data.metadata; + + const saveEntity = vi.spyOn(store, 'saveEntity'); + const live = liveNotifies(client); + recorder = recordNotifies(); + + client.applyMutationEvent({ type: 'update', typename: 'Token', data: token(0) }); + client.applyMutationEvent({ type: 'update', typename: 'Owner', data: { id: 'o-1', name: 'Ann' } }); + + expect(recorder.notified).toEqual([]); + expect(versions(client, keys)).toEqual(before); + expect(saveEntity).not.toHaveBeenCalled(); + expect(live.tokens).not.toHaveBeenCalled(); + expect(live.count).not.toHaveBeenCalled(); + + // A `create` for an entity the store already holds is the same no-op for + // the entity and the live array. (The live value's `onCreate` reducer + // still runs: it is told about a create, not about a change.) + client.applyMutationEvent({ type: 'create', typename: 'Token', data: token(0) }); + client.applyMutationEvent({ type: 'create', typename: 'Owner', data: { id: 'o-1', name: 'Ann' } }); + + expect(recorder.notified).toEqual([]); + expect(versions(client, keys)).toEqual(before); + expect(saveEntity).not.toHaveBeenCalled(); + expect(live.tokens).not.toHaveBeenCalled(); + // The unchanged nested object keeps its identity. + expect(client.entityMap.getEntity(tokenKey(0))!.data.metadata).toBe(metadata); + }); + + it('notifies exactly once for a real field change', async () => { + const { client, store } = await loadWallet(2); + const [ownerBefore, tokBefore] = versions(client, [ownerKey, tokenKey(0)]); + + const saveEntity = vi.spyOn(store, 'saveEntity'); + recorder = recordNotifies(); + + client.applyMutationEvent({ type: 'update', typename: 'Token', data: token(0, { price: 42 }) }); + + expect(recorder.notified).toEqual(['Token:tok-0']); + expect(versions(client, [ownerKey, tokenKey(0)])).toEqual([ownerBefore, tokBefore + 1]); + expect(savesOf(saveEntity, 'Token')).toHaveLength(1); + expect(savesOf(saveEntity, 'Owner')).toHaveLength(0); + expect(client.entityMap.getEntity(tokenKey(0))!.data.price).toBe(42); + }); + + it('(d) notifies only the nested entity that changed inside an unchanged parent', async () => { + const { client, store } = await loadWallet(2); + const [ownerBefore, tokBefore] = versions(client, [ownerKey, tokenKey(0)]); + + const saveEntity = vi.spyOn(store, 'saveEntity'); + recorder = recordNotifies(); + + client.applyMutationEvent({ type: 'update', typename: 'Token', data: token(0, { owner: 'Bea' }) }); + + expect(recorder.notified).toEqual(['Owner:o-1']); + expect(versions(client, [ownerKey, tokenKey(0)])).toEqual([ownerBefore + 1, tokBefore]); + expect(savesOf(saveEntity, 'Owner')).toHaveLength(1); + expect(savesOf(saveEntity, 'Token')).toHaveLength(0); + }); + }); +}); + +// ====================================================== +// Topic events +// ====================================================== + +class MockTopicAdapter extends TopicQueryAdapter { + snapshots = new Map(); + + subscribe(topic: string): void { + this.fulfillTopic(topic, this.snapshots.get(topic)); + } + + unsubscribe(topic: string): void { + this.clearTopic(topic); + } + + emit(event: MutationEvent): void { + this.sendMutationEvent(event); + } +} + +class WalletTopic extends TopicQuery { + static override adapter = MockTopicAdapter; + topic = 'wallet:w-1'; + result = { wallet: t.entity(Wallet) }; +} + +describe('(c) topic events', () => { + let client: QueryClient; + let adapter: MockTopicAdapter; + let store: SyncQueryStore; + let recorder: ReturnType | undefined; + + afterEach(() => { + recorder?.restore(); + recorder = undefined; + vi.restoreAllMocks(); + client?.destroy(); + }); + + async function loadTopic(count: number) { + store = new SyncQueryStore(new MemoryPersistentStore()); + adapter = new MockTopicAdapter(); + adapter.snapshots.set('wallet:w-1', wallet(count)); + client = new QueryClient({ store, adapters: [adapter] } as never); + await testWithClient(client, async () => { + const query = fetchQuery(WalletTopic); + await query; + expect((query.value as unknown as { wallet: { tokens: unknown[] } }).wallet.tokens).toHaveLength(count); + }); + } + + it('is a no-op when a reconnect re-snapshots every entity as topic events', async () => { + await loadTopic(3); + const keys = [walletKey, ownerKey, tokenKey(0), tokenKey(1), tokenKey(2)]; + const before = versions(client, keys); + + const saveEntity = vi.spyOn(store, 'saveEntity'); + const live = liveNotifies(client); + recorder = recordNotifies(); + + for (let i = 0; i < 3; i++) adapter.emit({ type: 'update', typename: 'Token', data: token(i) }); + adapter.emit({ type: 'update', typename: 'Wallet', data: { id: 'w-1' } }); + + expect(recorder.notified).toEqual([]); + expect(versions(client, keys)).toEqual(before); + expect(saveEntity).not.toHaveBeenCalled(); + expect(live.tokens).not.toHaveBeenCalled(); + expect(live.count).not.toHaveBeenCalled(); + }); + + it('notifies exactly once for a topic event that changes a field', async () => { + await loadTopic(2); + const [walletBefore, tokBefore] = versions(client, [walletKey, tokenKey(1)]); + + const saveEntity = vi.spyOn(store, 'saveEntity'); + const live = liveNotifies(client); + recorder = recordNotifies(); + + adapter.emit({ type: 'update', typename: 'Token', data: { id: 'tok-1', price: 7 } }); + + expect(recorder.notified).toEqual(['Token:tok-1']); + expect(versions(client, [walletKey, tokenKey(1)])).toEqual([walletBefore, tokBefore + 1]); + expect(savesOf(saveEntity, 'Token')).toHaveLength(1); + expect(live.tokens).not.toHaveBeenCalled(); + expect(client.entityMap.getEntity(tokenKey(1))!.data.price).toBe(7); + }); +}); diff --git a/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx b/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx new file mode 100644 index 0000000..46a4efe --- /dev/null +++ b/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx @@ -0,0 +1,148 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { render } from 'vitest-browser-react'; +import { ContextProvider, component } from 'signalium/react'; +import React from 'react'; +import { MemoryPersistentStore, SyncQueryStore } from '../../stores/sync.js'; +import { QueryClient, QueryClientContext } from '../../QueryClient.js'; +import { t } from '../../typeDefs.js'; +import { Entity } from '../../proxy.js'; +import { fetchQuery } from '../../query.js'; +import { sleep } from '../../__tests__/utils.js'; +import { TopicQuery } from '../../topic/TopicQuery.js'; +import { TopicQueryAdapter } from '../../topic/TopicQueryAdapter.js'; +import type { MutationEvent } from '../../types.js'; +import { createRenderCounter } from './utils.js'; + +/** + * A stream reconnect re-delivers every on-screen entity with the data it + * already holds. Readers of those entities must not re-render for it; a real + * change re-renders them once. + */ + +class MockTopicAdapter extends TopicQueryAdapter { + snapshots = new Map(); + + subscribe(topic: string): void { + this.fulfillTopic(topic, this.snapshots.get(topic)); + } + + unsubscribe(topic: string): void { + this.clearTopic(topic); + } + + emit(event: MutationEvent): void { + this.sendMutationEvent(event); + } +} + +class Owner extends Entity { + __typename = t.typename('Owner'); + id = t.id; + name = t.string; +} + +class Token extends Entity { + __typename = t.typename('Token'); + id = t.id; + walletId = t.string; + price = t.number; + metadata = t.object({ logo: t.string, tags: t.array(t.string) }); + owner = t.entity(Owner); +} + +class Wallet extends Entity { + __typename = t.typename('Wallet'); + id = t.id; + tokens = t.liveArray(Token, { constraints: { walletId: (this as unknown as { id: string }).id } }); +} + +class WalletTopic extends TopicQuery { + static override adapter = MockTopicAdapter; + topic = 'wallet:w-1'; + result = { wallet: t.entity(Wallet) }; +} + +function token(i: number, price = i) { + return { + __typename: 'Token', + id: `tok-${i}`, + walletId: 'w-1', + price, + metadata: { logo: `logo-${i}.png`, tags: ['defi'] }, + owner: { __typename: 'Owner', id: 'o-1', name: 'Ann' }, + }; +} + +type TokenProxy = { id: string; price: number; metadata: { logo: string }; owner: { name: string } }; + +describe('React readers on identical re-delivery', () => { + let client: QueryClient; + let adapter: MockTopicAdapter; + + beforeEach(() => { + client?.destroy(); + adapter = new MockTopicAdapter(); + adapter.snapshots.set('wallet:w-1', { + wallet: { __typename: 'Wallet', id: 'w-1', tokens: [token(0), token(1)] }, + }); + client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [adapter], + } as never); + }); + + it('does not re-render entity readers when every entity is re-delivered unchanged', async () => { + const Row = createRenderCounter( + ({ token }: { token: TokenProxy }) => ( + + {token.price}:{token.metadata.logo}:{token.owner.name} + + ), + component, + ); + + const ListCounter = createRenderCounter(() => { + const result = fetchQuery(WalletTopic); + if (!result.isReady) return
Loading...
; + const tokens = result.value.wallet.tokens as unknown as TokenProxy[]; + return ( +
+ {tokens.map(tok => ( + + ))} +
+ ); + }, component); + + const { getByTestId } = render( + + + , + ); + + await expect.element(getByTestId('row-tok-1')).toHaveTextContent('1:logo-1.png:Ann'); + await sleep(20); + const rowRendersBefore = Row.renderCount; + const listRendersBefore = ListCounter.renderCount; + + // Reconnect: every entity re-delivered with the data the store holds. + for (let pass = 0; pass < 3; pass++) { + adapter.emit({ type: 'update', typename: 'Wallet', data: { id: 'w-1' } }); + adapter.emit({ type: 'update', typename: 'Token', data: token(0) }); + adapter.emit({ type: 'update', typename: 'Token', data: token(1) }); + adapter.emit({ type: 'update', typename: 'Owner', data: { id: 'o-1', name: 'Ann' } }); + } + await sleep(50); + + expect(Row.renderCount).toBe(rowRendersBefore); + expect(ListCounter.renderCount).toBe(listRendersBefore); + + // A real change re-renders the row that reads it, once. (The list re-renders + // too: its constrained live array re-filters on any member change.) + adapter.emit({ type: 'update', typename: 'Token', data: token(1, 99) }); + await expect.element(getByTestId('row-tok-1')).toHaveTextContent('99:logo-1.png:Ann'); + await sleep(50); + + expect(Row.renderCount).toBe(rowRendersBefore + 1); + }); +}); From 8ed72a8e789536837f52c65e30f0fc4a08f2fbd0 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 04:04:49 -0400 Subject: [PATCH 03/18] perf(fetchium): start prefetch/retain fetches synchronously A query activated by a read leaves its subscription and first fetch for a microtask, since that work runs adapter and app code that must not run while the relay's computation is the current consumer. Leases paid the same hop: a tap handler that called prefetch() and then navigated had React's sync render microtask (queued by the press) run before the request went out. retain() and prefetch() now collect the queries their first run left a start for (a cold start from initialize(), or a zero-delay runDebounced() refetch on reactivation), and run those starts once the watcher's read has returned, outside any computation. The queued microtask then does nothing. Reader activation is unchanged, as are the stagger queue and debounce timers, the reactivation grace, and sync-store hydration (fresh cached data still issues no request). useSuspenseQuery's internal hold keeps the deferred start, since it leases from inside a render. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryClient.ts | 49 ++++++- packages/fetchium/src/QueryResult.ts | 41 +++++- .../src/__tests__/retain-prefetch.test.ts | 135 +++++++++++++++++- 3 files changed, 218 insertions(+), 7 deletions(-) diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index f0b102a..bd8b2be 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -162,6 +162,12 @@ export class QueryClient { /** Release functions of outstanding `retain()` / `prefetch()` leases. */ private leases = new Set<() => void>(); + /** + * While a lease's callback first runs: the queries it reached that left + * their first fetch (or a zero-delay refetch) for a microtask. The lease + * starts them before returning. See `retain()`. + */ + private leaseStarts: Set> | undefined = undefined; /** Leases taken by `useSuspenseQuery` for cold misses, by query instance key. */ private suspenseHolds = new Map(); @@ -368,6 +374,10 @@ export class QueryClient { this.queryInstances.set(queryKey, queryInstance as QueryInstance); } + // Already active with its start still queued (a reader activated it earlier + // in this task): the lease starts it too. + this.leaseStarts?.add(queryInstance); + return queryInstance.relay; } @@ -397,8 +407,25 @@ export class QueryClient { * render. Releasing never interrupts other readers; when the last one goes, * the query deactivates and its `gcTime` starts as usual. `release` is * idempotent. + * + * The fetches the first run of `fn` needs start before `retain` returns: a + * request goes out inside the call, ahead of anything already queued on the + * microtask queue (such as a render React scheduled for the same tap). A + * query that depends on another's result (`fn` reads an id, then fetches by + * it) starts once that result arrives, as with any reader. Call `retain` + * from an event handler or effect, not from inside a reactive computation: + * starting a fetch runs adapter code. */ retain(fn: () => unknown, options?: RetainOptions): () => void { + return this.lease(fn, options, true); + } + + /** + * `retain()`, with `startNow` choosing whether the first run's fetches start + * before returning or on their usual microtask. `useSuspenseQuery` leases + * from inside a render, where adapter code must not run, so it passes false. + */ + private lease(fn: () => unknown, options: RetainOptions | undefined, startNow: boolean): () => void { const w = withContexts([[QueryClientContext, this]], () => watcher(() => holdReturned(fn()))); const unsubscribe = w.addListener(noop); @@ -415,6 +442,9 @@ export class QueryClient { this.leases.add(release); + const outerStarts = this.leaseStarts; + const starts = startNow ? new Set>() : undefined; + this.leaseStarts = starts; try { // Run now, so the queries activate (and a synchronous store hydrates // them) before this returns, not on Signalium's next flush. @@ -422,6 +452,13 @@ export class QueryClient { } catch (error) { release(); throw error; + } finally { + this.leaseStarts = outerStarts; + } + + // Outside the watcher's computation now, so adapter code may run. + if (starts !== undefined) { + for (const instance of starts) instance.startPendingNow(); } const ttl = options?.ttl; @@ -498,7 +535,7 @@ export class QueryClient { let hold = this.suspenseHolds.get(key); if (hold === undefined) { - const release = this.retain(() => this.getQuery(def, params)); + const release = this.lease(() => this.getQuery(def, params), undefined, false); hold = { release, settled: undefined, done: false, failed: false, timer: undefined }; this.suspenseHolds.set(key, hold); } @@ -722,6 +759,16 @@ export class QueryClient { // Reactivation stagger // ====================================================== + /** + * Called by a query that queued its start (or a zero-delay refetch) on a + * microtask. During a lease's first run, the lease starts it before returning. + * + * @internal + */ + noteDeferredStart(instance: QueryInstance): void { + this.leaseStarts?.add(instance); + } + /** * Queues a reactivation refetch. Everything queued in the same task is * started from one flush, spread evenly across `reactivationStaggerMs` in diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 0f0cc49..63f59d4 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -57,6 +57,10 @@ export class QueryInstance { * microtask and so can't be cleared like a timer. */ private debounceGeneration: number = 0; + /** The zero-delay refetch runDebounced() queued, until it runs or is cancelled. */ + private pendingDebouncedRun: (() => void) | undefined = undefined; + /** initialize() queued startSubscriptionAndFetch() on a microtask that hasn't run yet. */ + private startPending: boolean = false; /** The last fetch ended in an error (not an abort). Disables the reactivation grace. */ private lastFetchFailed: boolean = false; /** @@ -298,6 +302,10 @@ export class QueryInstance { * * With an asynchronous store (AsyncQueryStore), the cache resolves on a later * tick, outside the activating read, and everything runs from there. + * + * A lease (`retain()` / `prefetch()`) doesn't wait for the microtask: it + * calls startPendingNow() once its activating read has returned, so the + * request goes out inside the call, ahead of any render already queued. */ private initialize(): void { this.initialized = true; @@ -324,10 +332,32 @@ export class QueryInstance { ); } else { this.hydrate(loaded); - queueMicrotask(() => this.startSubscriptionAndFetch()); + this.startPending = true; + this.queryClient.noteDeferredStart(this); + queueMicrotask(this.runPendingStart); } } + private runPendingStart = (): void => { + if (!this.startPending) return; + this.startPending = false; + this.startSubscriptionAndFetch(); + }; + + /** + * Runs, now, the start initialize() or a zero-delay runDebounced() left for + * a microtask, if it hasn't run yet. The microtask then does nothing. Called + * by a lease after its activating read, outside any reactive computation. + * + * @internal + */ + startPendingNow(): void { + this.runPendingStart(); + const run = this.pendingDebouncedRun; + this.pendingDebouncedRun = undefined; + run?.(); + } + private discardCorruptCache(error: unknown): void { const qc = this.queryClient; qc.store.deleteQuery(this.storageKey); @@ -509,18 +539,23 @@ export class QueryInstance { } const generation = this.debounceGeneration; - queueMicrotask(() => { + const run = (): void => { if (generation !== this.debounceGeneration) return; this.debounceGeneration++; + this.pendingDebouncedRun = undefined; // Another path started a fetch in the meantime (refetch(), activation). if (this.relayState.isPending && this._abortController !== undefined) return; this.runQueryImmediately(); - }); + }; + this.pendingDebouncedRun = run; + this.queryClient.noteDeferredStart(this); + queueMicrotask(run); } /** Cancels a refetch scheduled by runDebounced(). */ private cancelDebounced(): void { this.debounceGeneration++; + this.pendingDebouncedRun = undefined; clearTimeout(this.debounceTimer); this.debounceTimer = undefined; } diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts index 0f3f8c2..165bd43 100644 --- a/packages/fetchium/src/__tests__/retain-prefetch.test.ts +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -69,13 +69,29 @@ afterEach(() => { unsubscribed = 0; }); -function setup(kv: MemoryPersistentStore = new MemoryPersistentStore()) { +class GetIdentity extends RESTQuery { + path = '/identity'; + result = { id: t.number }; +} + +class GetHoldings extends RESTQuery { + params = { owner: t.number }; + path = `/holdings/${this.params.owner}`; + result = { n: t.number }; +} + +function setup( + kv: MemoryPersistentStore = new MemoryPersistentStore(), + options: { reactivationStaggerMs?: number } = {}, +) { const mockFetch = createMockFetch(); let n = 0; - for (const path of ['/item', '/other', '/users/[id]', '/streamed']) { + for (const path of ['/item', '/other', '/users/[id]', '/streamed', '/holdings/[owner]']) { mockFetch.get(path, () => ({ n: ++n })); } + mockFetch.get('/identity', { id: 42 }); const client = new QueryClient({ + ...options, store: new SyncQueryStore(kv), adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], }); @@ -92,12 +108,88 @@ function mountReader(client: QueryClient, read: () => T): { w: { value: T }; } describe('client.prefetch()', () => { - it('starts the fetch before returning control to a timer', async () => { + it('issues the request before returning', async () => { const { client, mockFetch } = setup(); client.prefetch(GetItem); + expect(mockFetch.calls).toHaveLength(1); + await flushMicrotasks(); + expect(mockFetch.calls).toHaveLength(1); + }); + + it('issues the request ahead of a render already queued on the microtask queue', async () => { + const { client, mockFetch } = setup(); + let callsAtRender = -1; + // React queues its sync render for a press as a microtask before the handler's prefetch. + queueMicrotask(() => { + callsAtRender = mockFetch.calls.length; + }); + client.prefetch(GetItem); + await flushMicrotasks(); + expect(callsAtRender).toBe(1); + }); + + it('a reader activating a query leaves its fetch for the microtask, as before', async () => { + const { client, mockFetch } = setup(); + const { unsub } = mountReader(client, () => fetchQuery(GetItem).value); expect(mockFetch.calls).toHaveLength(0); await flushMicrotasks(); expect(mockFetch.calls).toHaveLength(1); + unsub(); + }); + + it('starts a query a reader activated earlier in the same task, once', async () => { + const { client, mockFetch } = setup(); + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + expect(mockFetch.calls).toHaveLength(0); + client.prefetch(GetItem); + expect(mockFetch.calls).toHaveLength(1); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + expect(w.value).toMatchObject({ n: 1 }); + unsub(); + }); + + it('a reader mounting in the same task joins the request, with no second one', async () => { + const { client, mockFetch } = setup(); + mockFetch.reset(); + mockFetch.get('/item', { n: 5 }, { delay: 20 }); + client.prefetch(GetItem); + expect(mockFetch.calls).toHaveLength(1); + + const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); + expect(w.value).toBeUndefined(); + await sleep(40); + expect(w.value).toMatchObject({ n: 5 }); + expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(false); + unsub(); + }); + + it('refetches a stale, inactive query before returning', async () => { + const { client, mockFetch } = setup(); + const release = client.prefetch(GetItem); + await sleep(10); + release(); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + + client.prefetch(GetItem); + expect(mockFetch.calls).toHaveLength(2); + }); + + it('leaves a stale reactivation to the stagger when reactivationStaggerMs is set', async () => { + const { client, mockFetch } = setup(undefined, { reactivationStaggerMs: 100 }); + const release = client.prefetch(GetItem); + expect(mockFetch.calls).toHaveLength(1); + await sleep(10); + release(); + await sleep(10); + + client.prefetch(GetItem); + await flushMicrotasks(); + expect(mockFetch.calls).toHaveLength(1); + await sleep(10); + expect(mockFetch.calls).toHaveLength(2); }); it('a reader mounting after the data arrives renders it on its first read, with no second request', async () => { @@ -155,6 +247,7 @@ describe('client.prefetch()', () => { client.prefetch(GetFreshUser, { id: 1 }); // Hydrated by the activation inside prefetch(), before it returned. expect(client.getQuery(QueryDefinition.for(GetFreshUser), { id: 1 }).value).toMatchObject({ n: 1 }); + expect(mockFetch.calls).toHaveLength(0); await sleep(10); expect(mockFetch.calls).toHaveLength(0); }); @@ -226,6 +319,42 @@ describe('client.prefetch()', () => { }); describe('client.retain()', () => { + it('issues the first request of a chained lease before returning, and the next when its input arrives', async () => { + const { client, mockFetch } = setup(); + const release = client.retain(() => { + const identity = fetchQuery(GetIdentity); + if (!identity.isReady) return identity; + return fetchQuery(GetHoldings, { owner: identity.value.id }); + }); + expect(mockFetch.calls.map(c => new URL(c.url).pathname)).toEqual(['/identity']); + await sleep(10); + expect(mockFetch.calls.map(c => new URL(c.url).pathname)).toEqual(['/identity', '/holdings/42']); + + const { w, unsub } = mountReader(client, () => fetchQuery(GetHoldings, { owner: 42 }).value); + expect(w.value).toMatchObject({ n: expect.any(Number) }); + await sleep(10); + expect(mockFetch.calls).toHaveLength(2); + unsub(); + release(); + }); + + it('issues both requests of a chained lease before returning when the first is cached and fresh', async () => { + const kv = new MemoryPersistentStore(); + const seed = setup(kv); + seed.client.retain(() => fetchQuery(GetFreshUser, { id: 1 })); + await sleep(10); + seed.client.destroy(); + + const { client, mockFetch } = setup(kv); + const release = client.retain(() => { + const user = fetchQuery(GetFreshUser, { id: 1 }); + if (!user.isReady) return user; + return fetchQuery(GetHoldings, { owner: user.value.n }); + }); + expect(mockFetch.calls.map(c => new URL(c.url).pathname)).toEqual(['/holdings/1']); + release(); + }); + it('holds the query promises the callback returns', async () => { const { client, mockFetch } = setup(); const release = client.retain(() => [fetchQuery(GetItem), fetchQuery(GetOther)]); From b053d83d11407775276ea7f2e480250aa3c6be92 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 06:33:37 -0400 Subject: [PATCH 04/18] fix(fetchium): do not retry 4xx client errors; add shouldRetry A failed attempt whose HTTP status is 4xx (except 408 and 429) now fails at once instead of retrying three times. The status comes from the adapter's error response (ctx.response set during the attempt) or from status, statusCode or response.status on the error. QueryClientConfig.shouldRetry and RetryConfig.shouldRetry override the default. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/skip-client-error-retries.md | 7 + docs/src/app/api/fetchium/page.md | 31 +-- docs/src/app/guides/error-handling/page.md | 11 + packages/fetchium/src/MutationResult.ts | 25 +- packages/fetchium/src/QueryClient.ts | 14 + packages/fetchium/src/QueryResult.ts | 30 ++- .../src/__tests__/retry-should-retry.test.ts | 250 ++++++++++++++++++ packages/fetchium/src/index.ts | 2 + packages/fetchium/src/query.ts | 8 +- packages/fetchium/src/retry.ts | 56 ++++ packages/fetchium/src/types.ts | 12 +- 11 files changed, 416 insertions(+), 30 deletions(-) create mode 100644 .changeset/skip-client-error-retries.md create mode 100644 packages/fetchium/src/__tests__/retry-should-retry.test.ts diff --git a/.changeset/skip-client-error-retries.md b/.changeset/skip-client-error-retries.md new file mode 100644 index 0000000..ee46ed9 --- /dev/null +++ b/.changeset/skip-client-error-retries.md @@ -0,0 +1,7 @@ +--- +'fetchium': minor +--- + +Queries no longer retry client errors. A failed attempt whose HTTP status is 4xx (other than 408 and 429) now fails at once instead of being retried three times; network errors and 5xx responses still retry. The status comes from the response the adapter received (a REST error response whose body fails validation) or from `status`, `statusCode` or `response.status` on the thrown error. + +Adds `shouldRetry(error, attempt, status)` to `QueryClientConfig` and to `RetryConfig` (per query or mutation, overriding the client's) to decide retries yourself, and exports the default as `defaultShouldRetry` along with `getErrorStatus`. `RetryConfig.retries` is now optional and defaults as `retry: true` does. diff --git a/docs/src/app/api/fetchium/page.md b/docs/src/app/api/fetchium/page.md index 85f40bd..3e2b20c 100644 --- a/docs/src/app/api/fetchium/page.md +++ b/docs/src/app/api/fetchium/page.md @@ -225,18 +225,19 @@ Central coordinator for queries, mutations, entity storage, caching, and garbage new QueryClient(config: QueryClientConfig) ``` -| Field | Type | Default | Description | -| ----------------------- | ----------------------------- | ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `store` | `QueryStore` | In-memory | Persistent storage backend. Defaults to `SyncQueryStore(MemoryPersistentStore)`. | -| `adapters` | `QueryAdapter[]` | `[]` | Transport adapters (e.g. `new RESTQueryAdapter({ fetch, baseUrl })`). | -| `log` | `LogContext \| undefined` | `console` | Logger with `error`, `warn`, `info`, `debug` methods. | -| `evictionMultiplier` | `number \| undefined` | `1` | Scales all GC times for testing. Set to `0.001` to make timers fire in milliseconds. | -| `networkManager` | `NetworkManager` | `new NetworkManager()` | Tracks network connectivity. | -| `gcManager` | `GcManager \| NoOpGcManager` | Auto-detected | GC manager. Uses `NoOpGcManager` on the server. | -| `reactivationGraceMs` | `number` | `0` | Milliseconds. A query that reactivates (a watcher returns, or a paused scope resumes) with data younger than this is not refetched, even if stale. Data a subscription pushed to (a `subscribe` stream event, or a topic event sent with its topic) counts as fresh from the last push; a running `poll()` counts through the fetches it makes. Network reconnects, `__refetch()` and invalidation still refetch. Per-query override: `reactivationGraceMs` in the query config. | -| `reactivationStaggerMs` | `number` | `0` | Milliseconds. Reactivation refetches that start in the same task are spread evenly across this window, in activation order, instead of all starting at once. | -| `activity` | `ActivitySource \| undefined` | — | `{ isActive(), subscribe(listener) }` foreground source. When set, `poll()` stops its timers while inactive and resumes them on activation. | -| `pollResumeJitterMs` | `number` | `0` | Milliseconds. A `poll()` tick that is overdue on resume (or whose timer fired over a second late, as after JS timers were frozen in the background) is rescheduled at a random point in this window instead of firing at once. Per-poll override: `poll({ interval, resumeJitterMs })`. | +| Field | Type | Default | Description | +| ----------------------- | ------------------------------------- | ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `store` | `QueryStore` | In-memory | Persistent storage backend. Defaults to `SyncQueryStore(MemoryPersistentStore)`. | +| `adapters` | `QueryAdapter[]` | `[]` | Transport adapters (e.g. `new RESTQueryAdapter({ fetch, baseUrl })`). | +| `log` | `LogContext \| undefined` | `console` | Logger with `error`, `warn`, `info`, `debug` methods. | +| `evictionMultiplier` | `number \| undefined` | `1` | Scales all GC times for testing. Set to `0.001` to make timers fire in milliseconds. | +| `networkManager` | `NetworkManager` | `new NetworkManager()` | Tracks network connectivity. | +| `gcManager` | `GcManager \| NoOpGcManager` | Auto-detected | GC manager. Uses `NoOpGcManager` on the server. | +| `reactivationGraceMs` | `number` | `0` | Milliseconds. A query that reactivates (a watcher returns, or a paused scope resumes) with data younger than this is not refetched, even if stale. Data a subscription pushed to (a `subscribe` stream event, or a topic event sent with its topic) counts as fresh from the last push; a running `poll()` counts through the fetches it makes. Network reconnects, `__refetch()` and invalidation still refetch. Per-query override: `reactivationGraceMs` in the query config. | +| `reactivationStaggerMs` | `number` | `0` | Milliseconds. Reactivation refetches that start in the same task are spread evenly across this window, in activation order, instead of all starting at once. | +| `shouldRetry` | `(error, attempt, status) => boolean` | `defaultShouldRetry` | Whether a failed attempt is retried. `attempt` starts at 0; `status` is the attempt's HTTP status when known (the adapter's error response, or `status` / `statusCode` / `response.status` on the error). The default retries network errors and 5xx but not 4xx other than 408 and 429. Per-query override: `retry.shouldRetry`. | +| `activity` | `ActivitySource \| undefined` | — | `{ isActive(), subscribe(listener) }` foreground source. When set, `poll()` stops its timers while inactive and resumes them on activation. | +| `pollResumeJitterMs` | `number` | `0` | Milliseconds. A `poll()` tick that is overdue on resume (or whose timer fired over a second late, as after JS timers were frozen in the background) is rescheduled at a random point in this window instead of firing at once. Per-poll override: `poll({ interval, resumeJitterMs })`. | #### Methods @@ -690,9 +691,9 @@ The `t` object provides a declarative type definition DSL for describing query p ### Retry types -| Type | Definition | Description | -| ------------- | -------------------------------------------------------------------- | ------------------------------------------------------------------------------- | -| `RetryConfig` | `{ retries: number; retryDelay?: (attemptIndex: number) => number }` | Retry configuration. Default delay: exponential backoff (`1000ms * 2^attempt`). | +| Type | Definition | Description | +| ------------- | ------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------- | +| `RetryConfig` | `{ retries?: number; retryDelay?: (attemptIndex: number) => number; shouldRetry?: (error, attempt, status) => boolean }` | Retry configuration. Default delay: exponential backoff (`1000ms * 2^attempt`). `shouldRetry` overrides the client's `shouldRetry`. | ### Utility types diff --git a/docs/src/app/guides/error-handling/page.md b/docs/src/app/guides/error-handling/page.md index edbdbb4..2da961f 100644 --- a/docs/src/app/guides/error-handling/page.md +++ b/docs/src/app/guides/error-handling/page.md @@ -147,6 +147,17 @@ class GetUser extends RESTQuery { The default retry delay uses exponential backoff --- each successive attempt waits longer than the last (roughly 1s, 2s, 4s, ...). This avoids hammering a struggling server with rapid retries. +Client errors are not retried: a `4xx` response fails on the first attempt, because repeating the same request gets the same answer. `408 Request Timeout` and `429 Too Many Requests` are the exceptions, and network errors and `5xx` responses are retried. The status comes from the response the adapter received (a REST error response whose body doesn't match `result`), or from `status`, `statusCode` or `response.status` on the thrown error. To decide yourself, pass `shouldRetry`, either per query in `retry` or for every query on the `QueryClient`: + +```ts +const client = new QueryClient({ + adapters: [new RESTQueryAdapter({ baseUrl })], + // attempt starts at 0; status is undefined for network errors + shouldRetry: (error, attempt, status) => + status === undefined || status >= 500, +}); +``` + To disable retries entirely: ```ts diff --git a/packages/fetchium/src/MutationResult.ts b/packages/fetchium/src/MutationResult.ts index 234c706..3410413 100644 --- a/packages/fetchium/src/MutationResult.ts +++ b/packages/fetchium/src/MutationResult.ts @@ -112,16 +112,21 @@ export class MutationResultImpl { ); } - return withRetry(async () => { - const abortController = new AbortController(); - const ctx = createExecutionContext( - this.def.captured, - (request ?? {}) as Record, - this.queryClient.getContext(), - ); - - return (await adapter.sendMutation!(ctx, abortController.signal)) as Result; - }, retryConfig); + return withRetry( + async () => { + const abortController = new AbortController(); + const ctx = createExecutionContext( + this.def.captured, + (request ?? {}) as Record, + this.queryClient.getContext(), + ); + + return (await adapter.sendMutation!(ctx, abortController.signal)) as Result; + }, + retryConfig, + undefined, + { shouldRetry: this.queryClient.shouldRetry }, + ); } } diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index bd8b2be..c356848 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -26,6 +26,7 @@ import { ValidatorDef } from './typeDefs.js'; import { ConstraintMatcher, EVENT_SOURCE_FIELD } from './ConstraintMatcher.js'; import { LiveCollectionBinding } from './LiveCollection.js'; import { QueryAdapter, type QueryAdapterClass } from './QueryAdapter.js'; +import { defaultShouldRetry, type ShouldRetry } from './retry.js'; import { type QueryContext, type QueryStore, @@ -62,6 +63,15 @@ export interface QueryClientConfig { * still refetch. Default: 0 (every stale query refetches on reactivation). */ reactivationGraceMs?: number; + /** + * Decides whether a failed query attempt (or a mutation attempt, when the + * mutation enables retries) is retried. Receives the error, the attempt index + * (starting at 0) and the attempt's HTTP status when known. A query's or + * mutation's own `retry.shouldRetry` overrides it. Default: + * `defaultShouldRetry`, which retries network errors and 5xx responses but + * not 4xx responses other than 408 and 429. + */ + shouldRetry?: ShouldRetry; /** * Milliseconds. Reactivation refetches that start in the same task (for * example every query on a screen that just resumed) are spread evenly @@ -155,6 +165,8 @@ export class QueryClient { readonly reactivationGraceMs: number; /** See `QueryClientConfig.reactivationStaggerMs`. */ readonly reactivationStaggerMs: number; + /** See `QueryClientConfig.shouldRetry`. */ + readonly shouldRetry: ShouldRetry; /** Queries whose reactivation refetch waits for the current task's stagger flush. */ private staggerQueue = new Set>(); @@ -186,6 +198,7 @@ export class QueryClient { evictionMultiplier, reactivationGraceMs, reactivationStaggerMs, + shouldRetry, adapters: _c, networkManager: _n, gcManager: _g, @@ -195,6 +208,7 @@ export class QueryClient { this.store = store; this.reactivationGraceMs = nonNegative(reactivationGraceMs); this.reactivationStaggerMs = nonNegative(reactivationStaggerMs); + this.shouldRetry = shouldRetry ?? defaultShouldRetry; // `activity` and `pollResumeJitterMs` ride along in `rest`: poll() reads them from the context. this.context = { ...rest, log: log ?? console, evictionMultiplier }; this.gcManager = diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 63f59d4..249c4f7 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -21,7 +21,7 @@ import { GcKeyType } from './GcManager.js'; import { Query, QueryDefinition, type ResolvedRetryConfig, resolveRetryConfig } from './query.js'; import { EntityInstance } from './EntityInstance.js'; import { hashValue } from 'signalium/utils'; -import { withRetry } from './retry.js'; +import { getFailedResponseStatus, withRetry, type WithRetryOptions } from './retry.js'; function isThenable(value: MaybePromise): value is Promise { return typeof (value as { then?: unknown } | null | undefined)?.then === 'function'; @@ -469,10 +469,12 @@ export class QueryInstance { const ctx = this.getOrCreateExecutionContext(); const adapter = this.queryClient.getAdapter(def.statics.adapterClass); const signal = this._abortController?.signal ?? new AbortController().signal; + const attempt = this.attemptStatusTracker(ctx); try { const result = await withRetry( async () => { + attempt.start(); try { const freshData = await adapter.send(ctx, signal); this.updatedAt = Date.now(); @@ -489,6 +491,7 @@ export class QueryInstance { }, this.retryConfig, signal, + attempt.options, ); this.lastFetchFailed = false; return result; @@ -665,9 +668,11 @@ export class QueryInstance { const ctx = this.getOrCreateExecutionContext(); ctx.resultData = this.rootEntity!.data; const adapter = this.queryClient.getAdapter(def.statics.adapterClass); + const attempt = this.attemptStatusTracker(ctx); return withRetry( async () => { + attempt.start(); const freshData = await adapter.sendNext!(ctx, signal); this.updatedAt = Date.now(); @@ -678,9 +683,32 @@ export class QueryInstance { }, this.retryConfig, signal, + attempt.options, ); } + /** + * Retry options that report the HTTP status of a failed attempt. Adapters + * that expose their response as `ctx.response` (RESTQueryAdapter does) set + * it before the body is parsed and validated, so an error response whose + * body fails validation surfaces as a schema error, not an HTTP error. A + * response assigned during the failed attempt supplies its status. + */ + private attemptStatusTracker(ctx: Query): { start: () => void; options: WithRetryOptions } { + const holder = ctx as unknown as { response?: unknown }; + let responseBefore: unknown; + return { + start: () => { + responseBefore = holder.response; + }, + options: { + shouldRetry: this.queryClient.shouldRetry, + getAttemptStatus: () => + holder.response !== responseBefore ? getFailedResponseStatus(holder.response) : undefined, + }, + }; + } + // ====================================================== // Internal computed properties // ====================================================== diff --git a/packages/fetchium/src/__tests__/retry-should-retry.test.ts b/packages/fetchium/src/__tests__/retry-should-retry.test.ts new file mode 100644 index 0000000..d53abaf --- /dev/null +++ b/packages/fetchium/src/__tests__/retry-should-retry.test.ts @@ -0,0 +1,250 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { QueryClient } from '../QueryClient.js'; +import { SyncQueryStore, MemoryPersistentStore } from '../stores/sync.js'; +import { RESTQuery, RESTMutation } from '../rest/index.js'; +import { RESTQueryAdapter } from '../rest/RESTQueryAdapter.js'; +import { fetchQuery } from '../query.js'; +import { getMutation } from '../mutation.js'; +import { NetworkManager } from '../NetworkManager.js'; +import { defaultShouldRetry, getErrorStatus, type ShouldRetry } from '../retry.js'; +import { createMockFetch, testWithClient } from './utils.js'; +import { t } from '../typeDefs.js'; + +const user = { id: '1', name: 'Alice' }; + +/** An error shaped like a fetch wrapper's HTTP error (e.g. axios / request builders). */ +function httpError(status: number): Error { + return Object.assign(new Error(`HTTP ${status}`), { response: { status } }); +} + +describe('shouldRetry', () => { + let mockFetch: ReturnType; + let client: QueryClient; + + function createClient(shouldRetry?: ShouldRetry): QueryClient { + return new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], + networkManager: new NetworkManager(true), + shouldRetry, + }); + } + + class GetUser extends RESTQuery { + path = '/users/1'; + result = t.object({ id: t.string, name: t.string }); + config = { retry: { retries: 3, retryDelay: () => 1 } }; + } + + async function runFailing(query: new () => RESTQuery = GetUser): Promise { + let caught: unknown; + await testWithClient(client, async () => { + try { + await fetchQuery(query as typeof GetUser); + } catch (error) { + caught = error; + } + }); + return caught; + } + + beforeEach(() => { + mockFetch = createMockFetch(); + client = createClient(); + }); + + afterEach(() => { + client.destroy(); + }); + + describe('default', () => { + for (const status of [400, 401, 403, 404, 422]) { + it(`does not retry a ${status} response whose body fails validation`, async () => { + mockFetch.get('/users/1', { error: 'nope' }, { status }); + + const error = await runFailing(); + + expect(error).toBeDefined(); + expect(mockFetch.calls).toHaveLength(1); + }); + } + + it('does not retry an error that carries a 4xx status', async () => { + mockFetch.get('/users/1', null, { error: httpError(401) }); + + const error = await runFailing(); + + expect(error).toMatchObject({ response: { status: 401 } }); + expect(mockFetch.calls).toHaveLength(1); + }); + + for (const status of [408, 429, 500, 503]) { + it(`retries a ${status} response`, async () => { + mockFetch.get('/users/1', { error: 'busy' }, { status }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(4); + }); + } + + it('retries a 503 whose body is not JSON, then succeeds', async () => { + mockFetch.get('/users/1', null, { status: 503, jsonError: new SyntaxError('Unexpected token <') }); + mockFetch.get('/users/1', user); + + await testWithClient(client, async () => { + const result = fetchQuery(GetUser); + await result; + expect(result.value).toMatchObject(user); + }); + expect(mockFetch.calls).toHaveLength(2); + }); + + it('retries an error that carries a 429 status', async () => { + mockFetch.get('/users/1', null, { error: httpError(429) }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(4); + }); + + it('retries network errors', async () => { + mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(4); + }); + + it('retries a 2xx response whose body fails validation', async () => { + mockFetch.get('/users/1', { unexpected: true }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(4); + }); + + it('does not take the status of an earlier response for a later network error', async () => { + // Fetch 1 succeeds; the refetch fails with a network error, which must + // not inherit a status from the response assigned by fetch 1. + mockFetch.get('/users/1', user); + mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); + + await testWithClient(client, async () => { + const result = fetchQuery(GetUser); + await result; + await result.value!.__refetch().catch(() => {}); + }); + + expect(mockFetch.calls).toHaveLength(5); + }); + + it('does not retry a 4xx page in fetchNext', async () => { + class GetItems extends RESTQuery { + path = '/items'; + result = { items: t.array(t.string), nextPage: t.optional(t.number) }; + fetchNext = { searchParams: { page: this.result.nextPage } }; + config = { retry: { retries: 3, retryDelay: () => 1 } }; + } + mockFetch.get('/items', { items: ['a'], nextPage: 2 }); + + await testWithClient(client, async () => { + const result = fetchQuery(GetItems); + await result; + mockFetch.get('/items', { error: 'bad page' }, { status: 400 }); + await expect(result.value!.__fetchNext()).rejects.toBeDefined(); + }); + + expect(mockFetch.calls).toHaveLength(2); + }); + }); + + describe('custom', () => { + it('client-level shouldRetry receives error, attempt and status', async () => { + const seen: Array<[number, number | undefined]> = []; + client.destroy(); + client = createClient((_error, attempt, status) => { + seen.push([attempt, status]); + return attempt < 1; + }); + mockFetch.get('/users/1', { error: 'nope' }, { status: 404 }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(2); + expect(seen).toEqual([ + [0, 404], + [1, 404], + ]); + }); + + it('query-level shouldRetry overrides the client default', async () => { + class GetUserRetry4xx extends RESTQuery { + path = '/users/1'; + result = t.object({ id: t.string, name: t.string }); + config = { retry: { retries: 2, retryDelay: () => 1, shouldRetry: () => true } }; + } + mockFetch.get('/users/1', { error: 'nope' }, { status: 400 }); + + await runFailing(GetUserRetry4xx); + + expect(mockFetch.calls).toHaveLength(3); + }); + + it('query-level shouldRetry can stop retries the default would make', async () => { + class GetUserNoRetry extends RESTQuery { + path = '/users/1'; + result = t.object({ id: t.string, name: t.string }); + config = { retry: { retries: 3, retryDelay: () => 1, shouldRetry: () => false } }; + } + mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); + + await runFailing(GetUserNoRetry); + + expect(mockFetch.calls).toHaveLength(1); + }); + + it('applies to mutations', async () => { + class CreateUser extends RESTMutation { + readonly path = '/users'; + readonly method = 'POST' as const; + readonly params = { name: t.string }; + readonly body = { name: this.params.name }; + readonly result = { id: t.number }; + config = { retry: { retries: 2, retryDelay: () => 1 } }; + } + mockFetch.post('/users', null, { error: httpError(422) }); + + await testWithClient(client, async () => { + const mut = getMutation(CreateUser); + await expect(mut.run({ name: 'Test' })).rejects.toBeDefined(); + }); + + expect(mockFetch.calls).toHaveLength(1); + }); + }); +}); + +describe('defaultShouldRetry', () => { + it('classifies statuses', () => { + const err = new Error('x'); + expect(defaultShouldRetry(err, 0, undefined)).toBe(true); + expect(defaultShouldRetry(err, 0, 400)).toBe(false); + expect(defaultShouldRetry(err, 0, 404)).toBe(false); + expect(defaultShouldRetry(err, 0, 408)).toBe(true); + expect(defaultShouldRetry(err, 0, 429)).toBe(true); + expect(defaultShouldRetry(err, 0, 500)).toBe(true); + }); +}); + +describe('getErrorStatus', () => { + it('reads status, statusCode and response.status', () => { + expect(getErrorStatus({ status: 404 })).toBe(404); + expect(getErrorStatus({ statusCode: 401 })).toBe(401); + expect(getErrorStatus({ response: { status: 503 } })).toBe(503); + expect(getErrorStatus({ response: null })).toBeUndefined(); + expect(getErrorStatus({ status: 'failed' })).toBeUndefined(); + expect(getErrorStatus(new Error('x'))).toBeUndefined(); + expect(getErrorStatus(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/fetchium/src/index.ts b/packages/fetchium/src/index.ts index 92e665b..a11b55b 100644 --- a/packages/fetchium/src/index.ts +++ b/packages/fetchium/src/index.ts @@ -13,3 +13,5 @@ export type { Draft } from './utils.js'; export { NetworkManager, NoOpNetworkManager, defaultNetworkManager, NetworkManagerContext } from './NetworkManager.js'; export { GcManager, NoOpGcManager } from './GcManager.js'; export { Entity } from './proxy.js'; +export { defaultShouldRetry, getErrorStatus } from './retry.js'; +export type { ShouldRetry } from './retry.js'; diff --git a/packages/fetchium/src/query.ts b/packages/fetchium/src/query.ts index d0c397e..c2b041c 100644 --- a/packages/fetchium/src/query.ts +++ b/packages/fetchium/src/query.ts @@ -25,6 +25,8 @@ import type { QueryAdapter, QueryAdapterClass } from './QueryAdapter.js'; export interface ResolvedRetryConfig { retries: number; retryDelay: (attempt: number) => number; + /** Undefined defers to the client's `shouldRetry`. */ + shouldRetry?: (error: unknown, attempt: number, status: number | undefined) => boolean; } export function resolveRetryConfig( @@ -40,7 +42,7 @@ export function resolveRetryConfig( } else if (typeof retryOption === 'number') { retries = retryOption; } else { - retries = retryOption.retries; + retries = retryOption.retries ?? (isServer ? 0 : 3); } const retryDelay = @@ -48,7 +50,9 @@ export function resolveRetryConfig( ? retryOption.retryDelay : (attempt: number) => 1000 * Math.pow(2, attempt); - return { retries, retryDelay }; + const shouldRetry = typeof retryOption === 'object' ? retryOption.shouldRetry : undefined; + + return { retries, retryDelay, shouldRetry }; } // ================================ diff --git a/packages/fetchium/src/retry.ts b/packages/fetchium/src/retry.ts index 221b853..451ab2c 100644 --- a/packages/fetchium/src/retry.ts +++ b/packages/fetchium/src/retry.ts @@ -1,5 +1,57 @@ import type { ResolvedRetryConfig } from './query.js'; +/** + * Decides whether a failed attempt is retried. Consulted only while retries + * remain. + * + * @param error What the attempt threw. + * @param attempt Index of the failed attempt, starting at 0 (the first request). + * @param status HTTP status of the failed attempt, when known: the status the + * adapter received (`ctx.response.status` for a REST query whose response + * body failed to parse or validate), or else `status`, `statusCode` or + * `response.status` on the error. Undefined for network errors. + */ +export type ShouldRetry = (error: unknown, attempt: number, status: number | undefined) => boolean; + +function asHttpStatus(value: unknown): number | undefined { + return typeof value === 'number' && value >= 100 && value <= 599 ? value : undefined; +} + +/** + * The HTTP status an error carries, read from `status`, `statusCode`, or + * `response.status`. Undefined when none holds an HTTP status code. + */ +export function getErrorStatus(error: unknown): number | undefined { + if (typeof error !== 'object' || error === null) return undefined; + const e = error as { status?: unknown; statusCode?: unknown; response?: { status?: unknown } | null }; + return asHttpStatus(e.status) ?? asHttpStatus(e.statusCode) ?? asHttpStatus(e.response?.status); +} + +/** The status of an error response (4xx/5xx), or undefined. */ +export function getFailedResponseStatus(response: unknown): number | undefined { + if (typeof response !== 'object' || response === null) return undefined; + const status = asHttpStatus((response as { status?: unknown }).status); + return status !== undefined && status >= 400 ? status : undefined; +} + +/** + * Retries network errors, unknown errors and server errors (5xx). Does not + * retry client errors (4xx), which a repeat of the same request would hit + * again, except 408 Request Timeout and 429 Too Many Requests. + */ +export const defaultShouldRetry: ShouldRetry = (_error, _attempt, status) => + status === undefined || status < 400 || status >= 500 || status === 408 || status === 429; + +export interface WithRetryOptions { + /** Used when the retry config has no `shouldRetry`. Default: `defaultShouldRetry`. */ + shouldRetry?: ShouldRetry; + /** + * Called after an attempt fails. Returns the HTTP status the attempt + * received, for errors that don't carry one themselves. + */ + getAttemptStatus?: () => number | undefined; +} + /** * Safely retrieve the abort reason from a signal. Falls back to an AbortError * for engines (like Hermes) where `signal.reason` is not implemented. @@ -36,11 +88,13 @@ export async function withRetry( fn: () => Promise, config: ResolvedRetryConfig, signal?: AbortSignal, + options?: WithRetryOptions, ): Promise { if (IS_DEV && config.retries < 0) { throw new Error('retries must be non-negative'); } const retries = Math.max(0, config.retries); + const shouldRetry = config.shouldRetry ?? options?.shouldRetry ?? defaultShouldRetry; let lastError: unknown; for (let attempt = 0; attempt <= retries; attempt++) { if (signal?.aborted) { @@ -51,6 +105,8 @@ export async function withRetry( } catch (error) { lastError = error; if (attempt >= retries) throw error; + const status = getErrorStatus(error) ?? options?.getAttemptStatus?.(); + if (!shouldRetry(error, attempt, status)) throw error; await sleep(config.retryDelay(attempt), signal); } } diff --git a/packages/fetchium/src/types.ts b/packages/fetchium/src/types.ts index 0004474..e44d3f4 100644 --- a/packages/fetchium/src/types.ts +++ b/packages/fetchium/src/types.ts @@ -58,14 +58,22 @@ export enum NetworkMode { export interface RetryConfig { /** - * Number of retry attempts + * Number of retry attempts. Default: 3 for queries on the client, 0 on the + * server and for mutations. */ - retries: number; + retries?: number; /** * Optional custom delay function (receives attempt index starting at 0) * Default: exponential backoff (1000ms * 2^attempt) */ retryDelay?: (attemptIndex: number) => number; + /** + * Decides whether a failed attempt is retried, from the error, the attempt + * index (starting at 0) and the attempt's HTTP status when known. Overrides + * `QueryClientConfig.shouldRetry`. Default: `defaultShouldRetry`, which does + * not retry 4xx responses other than 408 and 429. + */ + shouldRetry?: (error: unknown, attempt: number, status: number | undefined) => boolean; } export const enum Mask { From 6702797b87d455d421ca8a5f2fc8f15e529f695e Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 22:11:40 -0400 Subject: [PATCH 05/18] fix(fetchium): notify a live value after every reducer run again Skipping the notify when a reducer returned the value it was given broke reducers that mutate in place: one that pushes onto its array and returns it changed the value without notifying, so readers kept the old length. A reference check can't tell that from a no-op, so every routed event that runs a reducer notifies again. Entities, writes and live arrays stay quiet on identical re-delivery. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/LiveCollection.ts | 9 ++-- .../__tests__/identical-redelivery.test.ts | 8 ++- .../fetchium/src/__tests__/live-value.test.ts | 51 +++++++++++++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/packages/fetchium/src/LiveCollection.ts b/packages/fetchium/src/LiveCollection.ts index 34ae819..673c54a 100644 --- a/packages/fetchium/src/LiveCollection.ts +++ b/packages/fetchium/src/LiveCollection.ts @@ -435,7 +435,6 @@ export class LiveValueInstance { entityData: Record, eventType: 'create' | 'update' | 'delete', ): void { - const prev = this._value; switch (eventType) { case 'create': if (this._createdKeys.has(entityKey)) return; @@ -451,10 +450,10 @@ export class LiveValueInstance { this._value = this._onDelete(this._value, entity ?? entityData); break; } - // Update events route here even when they changed nothing (a stream - // re-delivering current data). A reducer that returns the current value is - // a no-op, as in `reset`. - if (this._value !== prev) this._notifier.notify(); + // Notify after every reducer run, even one that returns the value it was + // given: a reducer may mutate the value in place (push onto an array and + // return it), and a reference check can't tell that from a no-op. + this._notifier.notify(); } getValue(): unknown { diff --git a/packages/fetchium/src/__tests__/identical-redelivery.test.ts b/packages/fetchium/src/__tests__/identical-redelivery.test.ts index 4b1a32b..0afa3d9 100644 --- a/packages/fetchium/src/__tests__/identical-redelivery.test.ts +++ b/packages/fetchium/src/__tests__/identical-redelivery.test.ts @@ -212,7 +212,10 @@ describe('Identical re-delivery', () => { expect(versions(client, keys)).toEqual(before); expect(saveEntity).not.toHaveBeenCalled(); expect(live.tokens).not.toHaveBeenCalled(); - expect(live.count).not.toHaveBeenCalled(); + // The live value's onUpdate reducer runs for the Token event and the + // value notifies: a reducer may mutate in place, so a returned identical + // reference doesn't prove nothing changed. + expect(live.count).toHaveBeenCalledTimes(1); // A `create` for an entity the store already holds is the same no-op for // the entity and the live array. (The live value's `onCreate` reducer @@ -328,7 +331,8 @@ describe('(c) topic events', () => { expect(versions(client, keys)).toEqual(before); expect(saveEntity).not.toHaveBeenCalled(); expect(live.tokens).not.toHaveBeenCalled(); - expect(live.count).not.toHaveBeenCalled(); + // Each Token event runs the live value's reducer, which notifies (see (b)). + expect(live.count).toHaveBeenCalledTimes(3); }); it('notifies exactly once for a topic event that changes a field', async () => { diff --git a/packages/fetchium/src/__tests__/live-value.test.ts b/packages/fetchium/src/__tests__/live-value.test.ts index 3bd4455..f6cc650 100644 --- a/packages/fetchium/src/__tests__/live-value.test.ts +++ b/packages/fetchium/src/__tests__/live-value.test.ts @@ -241,6 +241,57 @@ describe('LiveValue', () => { // ============================================================ describe('entity-level liveValue lifecycle', () => { + it('notifies when a reducer mutates the value in place and returns it', async () => { + const { client, mockFetch } = getClient(); + class Item extends Entity { + __typename = t.typename('Item'); + id = t.id; + listId = t.string; + } + + class List extends Entity { + __typename = t.typename('List'); + id = t.id; + ids = t.liveValue(t.array(t.string), Item, { + constraints: { listId: (this as any).id }, + onCreate: (v: string[], item: any) => { + v.push(item.id); + return v; + }, + onUpdate: (v: string[]) => v, + onDelete: (v: string[]) => v, + }); + } + + class GetList extends RESTQuery { + params = { id: t.id }; + path = `/list/${this.params.id}`; + result = { list: t.entity(List) }; + } + + mockFetch.get('/list/[id]', { list: { __typename: 'List', id: '1', ids: [] } }); + + let length!: () => number; + await testWithClient(client, async () => { + const relay = fetchQuery(GetList, { id: '1' }); + await relay; + const list = relay.value!.list; + length = reactive(() => (list.ids as string[]).length); + expect(length()).toBe(0); + }); + + client.applyMutationEvent({ + type: 'create', + typename: 'Item', + data: { __typename: 'Item', id: 'x', listId: '1' }, + }); + await sleep(5); + + await testWithClient(client, async () => { + expect(length()).toBe(1); + }); + }); + it('should increment on create and decrement on delete via applyEntityData', async () => { const { client, mockFetch } = getClient(); class Item extends Entity { From b8528e41c415a4436d53d9d9b59b22adbac80bd7 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 22:15:08 -0400 Subject: [PATCH 06/18] fix(fetchium): suspend on the current fetch when a suspense hold's fetch already settled When the fetch a useSuspenseQuery hold waited for settled and another fetch started before the reader rendered (a refetch from elsewhere, a reconnect), the hold kept handing out its already-resolved promise. React re-rendered at once on each throw, spinning tens of thousands of renders until the new fetch landed. A hold whose fetch settled now starts over on the fetch in flight, with a fresh promise. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryClient.ts | 11 +++++ .../__tests__/prefetch-suspense.test.tsx | 47 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index c356848..08b7024 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -560,6 +560,17 @@ export class QueryClient { return { promise: undefined, key }; } + if (relay.isPending && hold.done) { + // The fetch this hold waited for settled, and another has started since + // (a refetch from elsewhere). Suspend on that one: the settled promise + // has already resolved, and throwing it again would re-render at once. + clearTimeout(hold.timer); + hold.timer = undefined; + hold.done = false; + hold.failed = false; + hold.settled = undefined; + } + if (!relay.isPending) { if (hold.done) { // The fetch this hold waited for failed. Keep the hold until the next diff --git a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx index cee950a..f6fc7a6 100644 --- a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx +++ b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx @@ -11,6 +11,8 @@ import { createMockFetch, sleep } from '../../__tests__/utils.js'; import type { MutationEvent } from '../../types.js'; import { useQuery } from '../use-query.js'; import { useSuspenseQuery } from '../use-suspense-query.js'; +import { withContexts } from 'signalium'; +import { fetchQuery } from '../../query.js'; class GetItem extends RESTQuery { path = '/item'; @@ -301,6 +303,51 @@ describe('useSuspenseQuery', () => { expect(mockFetch.calls).toHaveLength(2); }); + it('suspends on a refetch that started after the cold fetch failed, without re-rendering in a loop', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', { error: 'down' }, { status: 500, delay: 20 }); + mockFetch.get('/item', { name: 'second' }, { delay: 400 }); + const client = makeClient(mockFetch); + + class GetNoRetry extends RESTQuery { + path = '/item'; + result = { name: t.string }; + config = { retry: false }; + } + + client.prefetch(GetNoRetry); + const relay = withContexts([[QueryClientContext, client]], () => fetchQuery(GetNoRetry)); + // Something else (another reader's retry, a reconnect) refetches as soon + // as the first attempt fails. + relay.then( + () => {}, + () => { + client.queryInstances.values().next().value!.refetch(); + }, + ); + + let renders = 0; + function Item(): React.ReactNode { + renders++; + const item = useSuspenseQuery(GetNoRetry); + return
{item.value.name}
; + } + + const screen = render( + + {}}> + Suspended}> + + + + , + ); + + await expect.element(screen.getByText('second'), { timeout: 3000 }).toBeInTheDocument(); + expect(renders).toBeLessThan(10); + expect(mockFetch.calls).toHaveLength(2); + }); + it('hands the query to the reader on commit, so unmounting deactivates it', async () => { const mockFetch = createMockFetch(); mockFetch.get('/streamed', { name: 'live' }, { delay: 10 }); From bc0b555cde6141da58c23218bfe05f5ea436f8e9 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 22:15:15 -0400 Subject: [PATCH 07/18] fix(fetchium): give a remount a new attempt after an abandoned suspense hold failed A hold's failed fetch was kept for SUSPENSE_HOLD_TTL (10 s) whether or not a render ever claimed the error. When the suspended tree was abandoned (the user navigated away) and the fetch then failed, mounting the screen again within that window threw the stale error with no new attempt, though a later mount is promised a fresh one. React retries a suspended render within a task or two of its promise settling, so a failure no render claims within 50 ms now drops the hold. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryClient.ts | 34 ++++++++++++---- .../__tests__/prefetch-suspense.test.tsx | 39 +++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index 08b7024..4a00126 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -121,6 +121,14 @@ export const DEFAULT_PREFETCH_TTL = 10_000; */ const SUSPENSE_HOLD_TTL = 10_000; +/** + * How long a failed cold fetch's error waits for a render to claim it. React + * retries a suspended render within a task or two of its promise settling; an + * error nobody claimed by then belongs to an abandoned tree, and a later mount + * makes a fresh attempt instead of inheriting it. + */ +const UNCLAIMED_FAILURE_TTL = 50; + interface SuspenseHold { release: () => void; /** Resolves (never rejects) once the cold fetch settles. */ @@ -535,7 +543,9 @@ export class QueryClient { * The reader's commit releases it (`releaseSuspenseHold`); otherwise it * releases itself `SUSPENSE_HOLD_TTL` after the fetch settles. A query whose * last fetch failed is refetched once per hold: when that attempt fails too, - * `error` is set and the hold is dropped, so the caller can throw it. + * `error` is set and the hold is dropped, so the caller can throw it. A + * failure no render claims within `UNCLAIMED_FAILURE_TTL` (the suspended + * tree was abandoned) drops the hold, so a later mount makes a new attempt. * * @internal */ @@ -594,12 +604,22 @@ export class QueryClient { if (hold.settled === undefined) { const current = hold; current.settled = new Promise(resolve => { - const settle = (): void => { - current.done = true; - this.expireSuspenseHold(key, current); - resolve(); - }; - relay.then(settle, settle); + relay.then( + () => { + current.done = true; + this.expireSuspenseHold(key, current); + resolve(); + }, + () => { + current.done = true; + // Dropped unless the retried render claims the error first. + clearTimeout(current.timer); + current.timer = setTimeout(() => { + if (this.suspenseHolds.get(key) === current && !current.failed) this.releaseSuspenseHold(key); + }, UNCLAIMED_FAILURE_TTL); + resolve(); + }, + ); }); } diff --git a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx index f6fc7a6..09160e9 100644 --- a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx +++ b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx @@ -348,6 +348,45 @@ describe('useSuspenseQuery', () => { expect(mockFetch.calls).toHaveLength(2); }); + it('makes a new attempt when a tree abandoned while suspended is mounted again after the fetch failed', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/item', { error: 'down' }, { status: 500, delay: 20 }); + const client = makeClient(mockFetch); + + class GetNoRetry extends RESTQuery { + path = '/item'; + result = { name: t.string }; + config = { retry: false }; + } + + function Item(): React.ReactNode { + const item = useSuspenseQuery(GetNoRetry); + return
{item.value.name}
; + } + + const errors: unknown[] = []; + const tree = (show: boolean) => ( + + errors.push(e)}> + Suspended}>{show ? :
Other
} + + + ); + + const screen = render(tree(true)); + await expect.element(screen.getByText('Suspended')).toBeInTheDocument(); + // Navigated away while suspended; the fetch fails with no reader. + screen.rerender(tree(false)); + await sleep(150); + expect(mockFetch.calls).toHaveLength(1); + + mockFetch.get('/item', { name: 'recovered' }); + screen.rerender(tree(true)); + await expect.element(screen.getByText('recovered')).toBeInTheDocument(); + expect(mockFetch.calls).toHaveLength(2); + expect(errors).toEqual([]); + }); + it('hands the query to the reader on commit, so unmounting deactivates it', async () => { const mockFetch = createMockFetch(); mockFetch.get('/streamed', { name: 'live' }, { delay: 10 }); From 9003c44d2d7ca42b4af5c14cbcad369fa9d93282 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 22:18:26 -0400 Subject: [PATCH 08/18] fix(fetchium): resubscribe a never-loaded query before refetching it A TopicQuery whose first fetch was aborted (a prefetch or retain released before the snapshot arrived, now routine) never loaded. Reactivation skipped reconcileSubscription() for a query with no data and restarted the fetch from inside the activating read; TopicQueryAdapter.send() then waited on a deferred that only the torn-down subscription could fulfil. A topic whose snapshot was rejected hung the same way on its next activation. runQuery() now puts the subscription back before sending, and the restart of an aborted fetch takes the relay's promise at once (so the doomed one can't settle it) but runs the subscription and the request on a microtask, or when a lease starts them, outside the read. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryResult.ts | 52 +++++++++-- .../src/__tests__/topic-query.test.ts | 87 ++++++++++++++++++- 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 249c4f7..cf232e4 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -61,6 +61,8 @@ export class QueryInstance { private pendingDebouncedRun: (() => void) | undefined = undefined; /** initialize() queued startSubscriptionAndFetch() on a microtask that hasn't run yet. */ private startPending: boolean = false; + /** The fetch restartAbortedFetch() queued on a microtask, until it runs. */ + private pendingRestart: (() => void) | undefined = undefined; /** The last fetch ended in an error (not an abort). Disables the reactivation grace. */ private lastFetchFailed: boolean = false; /** @@ -206,13 +208,12 @@ export class QueryInstance { } // If the relay shows pending but the abort controller is gone, the - // previous fetch was aborted during deactivation. runDebounced() - // would bail out because isPending is still true from the doomed - // promise. Force an immediate refetch so the new setPromise() call - // replaces _promise, causing the stale AbortError rejection to hit - // the `promise !== this._promise` guard and be silently ignored. + // previous fetch was aborted during deactivation (or never started). + // runDebounced() would bail out because isPending is still true from + // the doomed promise, which may also never settle (a topic query's + // send() waits on a subscription that is gone). if (this.relayState.isPending && this._abortController === undefined) { - this.runQueryImmediately(); + this.restartAbortedFetch(); } else { const refreshStaleOnReconnect = this.config?.refreshStaleOnReconnect ?? true; // The grace covers a relay resuming, not a network reconnect: data @@ -356,6 +357,7 @@ export class QueryInstance { const run = this.pendingDebouncedRun; this.pendingDebouncedRun = undefined; run?.(); + this.pendingRestart?.(); } private discardCorruptCache(error: unknown): void { @@ -467,6 +469,9 @@ export class QueryInstance { } const ctx = this.getOrCreateExecutionContext(); + // Put back a subscription a deactivation or pause tore down before + // sending: a topic query's send() waits for data its subscription brings. + this.reconcileSubscription(); const adapter = this.queryClient.getAdapter(def.statics.adapterClass); const signal = this._abortController?.signal ?? new AbortController().signal; const attempt = this.attemptStatusTracker(ctx); @@ -501,6 +506,41 @@ export class QueryInstance { } } + /** + * Replaces a fetch that a deactivation aborted, from inside the activating + * read. The relay takes the new promise now, so the doomed one can no longer + * settle it, but the subscription and the request start on a microtask (or + * when a lease starts them), outside the read: both run adapter code. + */ + private restartAbortedFetch(): void { + this.fetchStarts++; + this._fetchNextAbort?.abort(); + this._fetchNextAbort = undefined; + this._fetchNextPromise = undefined; + const controller = new AbortController(); + this._abortController = controller; + + let resolve!: (value: QueryResult) => void; + let reject!: (error: unknown) => void; + const promise = new Promise>((res, rej) => { + resolve = res; + reject = rej; + }); + const run = (): void => { + if (this.pendingRestart !== run) return; + this.pendingRestart = undefined; + if (controller.signal.aborted) { + reject(controller.signal.reason); + return; + } + this.runQuery().then(resolve, reject); + }; + this.pendingRestart = run; + this.queryClient.noteDeferredStart(this); + queueMicrotask(run); + this.relayState.setPromise(promise); + } + private runQueryImmediately(): void { this.fetchStarts++; this._abortController?.abort(); diff --git a/packages/fetchium/src/__tests__/topic-query.test.ts b/packages/fetchium/src/__tests__/topic-query.test.ts index 49a2adb..51ec769 100644 --- a/packages/fetchium/src/__tests__/topic-query.test.ts +++ b/packages/fetchium/src/__tests__/topic-query.test.ts @@ -1,14 +1,14 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { reactive } from 'signalium'; +import { reactive, withContexts } from 'signalium'; import { SyncQueryStore, MemoryPersistentStore } from '../stores/sync.js'; -import { QueryClient } from '../QueryClient.js'; +import { QueryClient, QueryClientContext } from '../QueryClient.js'; import { t } from '../typeDefs.js'; import { Entity } from '../proxy.js'; import { fetchQuery } from '../query.js'; import { getMutation } from '../mutation.js'; import { RESTMutation } from '../rest/index.js'; import { reifyValue } from '../fieldRef.js'; -import { createMockFetch, testWithClient, sleep, getEntityMapSize } from './utils.js'; +import { createMockFetch, createTestWatcher, testWithClient, sleep, getEntityMapSize } from './utils.js'; import { TopicQuery } from '../topic/TopicQuery.js'; import { TopicQueryAdapter } from '../topic/TopicQueryAdapter.js'; import { RESTQueryAdapter } from '../rest/RESTQueryAdapter.js'; @@ -2273,3 +2273,84 @@ describe('TopicQuery', () => { }); }); }); + +describe('TopicQuery reactivation before it ever loaded', () => { + class SlowTopicAdapter extends TopicQueryAdapter { + subscribes = 0; + reject = false; + private timers = new Map>(); + + subscribe(topic: string): void { + this.subscribes++; + this.timers.set( + topic, + setTimeout(() => { + if (this.reject) this.rejectTopic(topic, new Error('topic unavailable')); + else this.fulfillTopic(topic, { n: 1 }); + }, 30), + ); + } + + unsubscribe(topic: string): void { + clearTimeout(this.timers.get(topic)); + this.clearTopic(topic); + } + } + + class GetCount extends TopicQuery { + static override adapter = SlowTopicAdapter; + topic = 'count'; + result = { n: t.number }; + } + + let client: QueryClient; + let adapter: SlowTopicAdapter; + + beforeEach(() => { + adapter = new SlowTopicAdapter(); + client = new QueryClient({ store: new SyncQueryStore(new MemoryPersistentStore()), adapters: [adapter] }); + }); + + afterEach(() => { + client.destroy(); + }); + + function mount(): () => void { + return withContexts([[QueryClientContext, client]], () => createTestWatcher(() => fetchQuery(GetCount).value)) + .unsub; + } + + function relay() { + return withContexts([[QueryClientContext, client]], () => fetchQuery(GetCount)); + } + + it('loads when a reader mounts after a prefetch was released before the snapshot arrived', async () => { + const release = client.prefetch(GetCount); + await sleep(5); + release(); + await sleep(10); + + const unmount = mount(); + await sleep(100); + expect(adapter.subscribes).toBe(2); + expect(relay().isReady).toBe(true); + expect(relay().value!.n).toBe(1); + unmount(); + }); + + it('subscribes again before refetching a topic whose snapshot was rejected', async () => { + adapter.reject = true; + const unmount = mount(); + await sleep(60); + expect(relay().isRejected).toBe(true); + unmount(); + await sleep(10); + + adapter.reject = false; + const remount = mount(); + await sleep(100); + expect(adapter.subscribes).toBe(2); + expect(relay().isReady).toBe(true); + remount(); + }); +}); From 642baa934945a3ebca6b0e0d32ad8335e1ab8868 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Sat, 3 Oct 2026 22:22:07 -0400 Subject: [PATCH 09/18] fix(fetchium): bound lease lifetimes and clean them up on destroy - prefetch()'s ttl is now an upper bound. A lease whose fetch was in flight at expiry waited for it to settle, so a fetch that never settles (offline, a topic nobody fulfils) kept the query active forever. It now releases at the ttl, which aborts the fetch only when no reader has joined it. - destroy() cleared leases but not prefetch's separate ttl timer; prefetch now uses retain()'s ttl, which release() clears. - Releasing leases in destroy() deactivates their queries on Signalium's next flush, after gcManager.destroy(), and that deactivation created a fresh GC bucket interval nothing would clear. GcManager.schedule() is a no-op once destroyed. - retain() called inside a reactive computation took a new lease on every rerun. It now warns once in development, starts its fetches on their usual microtask instead of inside the computation, and releases the leases the computation's previous run took. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/fast-delivery-prefetch-retain.md | 2 +- docs/src/app/api/fetchium/page.md | 2 +- packages/fetchium/src/GcManager.ts | 5 +- packages/fetchium/src/QueryClient.ts | 83 +++++++--- .../src/__tests__/retain-prefetch.test.ts | 148 ++++++++++++++++-- 5 files changed, 201 insertions(+), 39 deletions(-) diff --git a/.changeset/fast-delivery-prefetch-retain.md b/.changeset/fast-delivery-prefetch-retain.md index a70c38d..a31fde9 100644 --- a/.changeset/fast-delivery-prefetch-retain.md +++ b/.changeset/fast-delivery-prefetch-retain.md @@ -2,7 +2,7 @@ 'fetchium': minor --- -Adds `queryClient.prefetch(QueryClass, params, { ttl })` and `queryClient.retain(fn, { ttl })`. Both keep queries active without a reader and return a `release` function. A reader that mounts while the lease is held joins the running query: no second request, and the data on its first render if it has arrived. `prefetch` defaults to a 10 s lease (`DEFAULT_PREFETCH_TTL`) and lets a fetch in flight at expiry finish; `retain` lasts until released. +Adds `queryClient.prefetch(QueryClass, params, { ttl })` and `queryClient.retain(fn, { ttl })`. Both keep queries active without a reader and return a `release` function. A reader that mounts while the lease is held joins the running query: no second request, and the data on its first render if it has arrived. `prefetch` defaults to a 10 s lease (`DEFAULT_PREFETCH_TTL`), an upper bound even with a fetch in flight; `retain` lasts until released. Adds `useSuspenseQuery` to `fetchium/react`. It suspends only when the query has never produced a value (in memory or in a synchronous store), never for a refetch, and throws a failed cold fetch to the error boundary. diff --git a/docs/src/app/api/fetchium/page.md b/docs/src/app/api/fetchium/page.md index 3e2b20c..1321712 100644 --- a/docs/src/app/api/fetchium/page.md +++ b/docs/src/app/api/fetchium/page.md @@ -252,7 +252,7 @@ new QueryClient(config: QueryClientConfig) #### `prefetch` and `retain` -Both take a lease: the queries stay active (fetched, subscribed, exempt from GC) without a component reading them. A reader that mounts while the lease is held joins the active query, so it starts no request of its own and renders the data on its first render if it has arrived. Both return an idempotent `release` function. Releasing never interrupts other readers; once the last one goes, the query deactivates and its `gcTime` starts as usual. Calling `release` while a fetch is in flight, with no reader, aborts that fetch. A `prefetch` whose `ttl` runs out mid-fetch instead waits for the fetch to settle before letting go. +Both take a lease: the queries stay active (fetched, subscribed, exempt from GC) without a component reading them. A reader that mounts while the lease is held joins the active query, so it starts no request of its own and renders the data on its first render if it has arrived. Both return an idempotent `release` function. Releasing never interrupts other readers; once the last one goes, the query deactivates and its `gcTime` starts as usual. Calling `release` while a fetch is in flight, with no reader, aborts that fetch. A `ttl` is an upper bound: the lease goes when it runs out even if the fetch is still in flight (offline, or a topic never fulfilled), which aborts the fetch unless a reader has joined it. Call `retain` and `prefetch` from event handlers or effects, not from inside a reactive computation; called from one, they warn in development and keep only the leases from the computation's latest run. `prefetch` is for the moment a user commits to a navigation, such as a tap. Call it in the handler, before navigating: diff --git a/packages/fetchium/src/GcManager.ts b/packages/fetchium/src/GcManager.ts index c3793b7..ba786b6 100644 --- a/packages/fetchium/src/GcManager.ts +++ b/packages/fetchium/src/GcManager.ts @@ -62,6 +62,8 @@ export class GcManager { private _nextTickScheduled = false; private _onEvict: (key: number, type: GcKeyType) => void; private _multiplier: number; + /** Set by destroy(). Queries that deactivate afterwards schedule nothing. */ + private _destroyed = false; constructor(onEvict: (key: number, type: GcKeyType) => void, multiplier: number = 1) { this._onEvict = onEvict; @@ -69,7 +71,7 @@ export class GcManager { } schedule(key: number, gcTime: number, type: GcKeyType): void { - if (gcTime === Infinity) return; + if (gcTime === Infinity || this._destroyed) return; if (gcTime === 0) { const { _nextTickEntries } = this; @@ -111,6 +113,7 @@ export class GcManager { }; destroy(): void { + this._destroyed = true; const { _buckets, _nextTickEntries } = this; for (const bucket of _buckets.values()) { bucket.destroy(); diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index 4a00126..159bd97 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -1,4 +1,4 @@ -import { context, watcher, withContexts, ReactivePromise, ReactiveTask, type Context } from 'signalium'; +import { context, signal, watcher, withContexts, ReactivePromise, ReactiveTask, type Context } from 'signalium'; import { hashValue } from 'signalium/utils'; import { EntityDef, @@ -188,6 +188,12 @@ export class QueryClient { * starts them before returning. See `retain()`. */ private leaseStarts: Set> | undefined = undefined; + /** + * Leases taken by `retain()` calls inside a reactive computation, by the + * computation, with the run that took them. See `retain()`. + */ + private reactiveLeases = new WeakMap void> }>(); + private warnedReactiveRetain = false; /** Leases taken by `useSuspenseQuery` for cold misses, by query instance key. */ private suspenseHolds = new Map(); @@ -436,10 +442,32 @@ export class QueryClient { * query that depends on another's result (`fn` reads an id, then fetches by * it) starts once that result arrives, as with any reader. Call `retain` * from an event handler or effect, not from inside a reactive computation: - * starting a fetch runs adapter code. + * starting a fetch runs adapter code, and a computation that reruns would + * take a new lease on every run. Called from one anyway, it warns in + * development, starts its fetches on their usual microtask, and releases the + * leases the computation's previous run took. */ retain(fn: () => unknown, options?: RetainOptions): () => void { - return this.lease(fn, options, true); + const owner = currentReactiveOwner(); + if (owner === undefined) return this.lease(fn, options, true); + + if (IS_DEV && !this.warnedReactiveRetain) { + this.warnedReactiveRetain = true; + this.context.log?.warn?.( + 'QueryClient.retain() (or prefetch()) was called inside a reactive computation. Call it from an event handler or effect instead.', + ); + } + + const release = this.lease(fn, options, false); + const previous = this.reactiveLeases.get(owner.ref); + if (previous === undefined || previous.run !== owner.run) { + this.reactiveLeases.set(owner.ref, { run: owner.run, releases: [release] }); + // After taking the new lease, so queries both runs read stay active. + if (previous !== undefined) for (const prior of previous.releases) prior(); + } else { + previous.releases.push(release); + } + return release; } /** @@ -494,8 +522,10 @@ export class QueryClient { /** * Starts `QueryClass` with `params` now and keeps it active for `ttl` * milliseconds (default {@link DEFAULT_PREFETCH_TTL}), or until the returned - * `release` is called. A fetch still in flight when `ttl` runs out is let - * finish before the lease goes; `release()` lets go at once. Meant for the moment a user commits to a navigation + * `release` is called. The `ttl` is an upper bound: the lease goes when it + * runs out even if the fetch is still in flight (offline, or a topic that is + * never fulfilled), which aborts that fetch unless a reader has joined it. + * Meant for the moment a user commits to a navigation * (a tap): the destination's reader, mounting within the window, reuses the * in-flight or finished fetch instead of starting its own, and renders the * data on its first render if it has arrived. @@ -510,27 +540,9 @@ export class QueryClient { options?: PrefetchOptions, ): () => void { const def = QueryDefinition.for(QueryClass); - let relay: QueryPromise | undefined; - const release = this.retain(() => (relay = this.getQuery(def, params as QueryParams | undefined))); - - const ttl = options?.ttl ?? DEFAULT_PREFETCH_TTL; - if (Number.isFinite(ttl)) { - const timer = setTimeout( - () => { - // Expiring mid-fetch would abort a request that is about to land: - // hold until it settles. An explicit release() still lets go at once. - if (relay?.isPending) relay.then(release, release); - else release(); - }, - Math.max(0, ttl), - ); - return () => { - clearTimeout(timer); - release(); - }; - } - - return release; + return this.retain(() => this.getQuery(def, params as QueryParams | undefined), { + ttl: options?.ttl ?? DEFAULT_PREFETCH_TTL, + }); } /** @@ -955,6 +967,25 @@ export const QueryClientContext: Context = context {}; +/** + * The reactive computation (and its run) currently executing, if any. Signalium + * has no public API for this: a signal read records its consumer, so read a + * throwaway signal and look at what it recorded. Undefined when the shape + * isn't the expected one. + */ +function currentReactiveOwner(): { ref: object; run: number } | undefined { + try { + const probe = signal(0); + void probe.value; + const subs = (probe as unknown as { _subs?: Map })._subs; + if (!(subs instanceof Map) || subs.size === 0) return undefined; + const [ref, run] = subs.entries().next().value!; + return typeof ref === 'object' && ref !== null && typeof run === 'number' ? { ref, run } : undefined; + } catch { + return undefined; + } +} + /** * Reads the query promises `retain`'s callback returned, so the lease's * watcher depends on them and keeps them active. `isReady` changes once per diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts index 165bd43..8d19757 100644 --- a/packages/fetchium/src/__tests__/retain-prefetch.test.ts +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -1,12 +1,14 @@ /* eslint-disable @typescript-eslint/no-unused-expressions */ -import { describe, it, expect, afterEach } from 'vitest'; -import { signal, watcher, withContexts } from 'signalium'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { reactive, signal, watcher, withContexts } from 'signalium'; import { MemoryPersistentStore, SyncQueryStore } from '../stores/sync.js'; import { QueryClient, QueryClientContext, DEFAULT_PREFETCH_TTL } from '../QueryClient.js'; import { RESTQuery, RESTQueryAdapter } from '../rest/index.js'; import { t } from '../typeDefs.js'; import { fetchQuery, QueryDefinition } from '../query.js'; import type { MutationEvent } from '../types.js'; +import { GcManager, type GcKeyType } from '../GcManager.js'; +import { NetworkManager } from '../NetworkManager.js'; import { createMockFetch, sleep } from './utils.js'; /** @@ -266,21 +268,69 @@ describe('client.prefetch()', () => { unsub(); }); - it('a ttl that runs out mid-fetch waits for the fetch instead of aborting it', async () => { + it('a ttl that runs out mid-fetch releases the lease, aborting the fetch with no reader', async () => { const { client, mockFetch } = setup(); mockFetch.reset(); mockFetch.get('/streamed', { n: 9 }, { delay: 40 }); client.prefetch(GetStreamed, undefined, { ttl: 10 }); await sleep(25); - // Past the ttl, before the response (40 ms): still held, request not cancelled. - expect(unsubscribed).toBe(0); - expect(mockFetch.calls[0].options.signal?.aborted).toBe(false); + // The ttl is an upper bound: past it, before the response, the lease is gone. + expect(unsubscribed).toBe(1); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); + await sleep(40); + }); + + it('a ttl that runs out mid-fetch leaves a reader that joined the fetch running', async () => { + const { client, mockFetch } = setup(); + mockFetch.reset(); + mockFetch.get('/streamed', { n: 9 }, { delay: 40 }); + client.prefetch(GetStreamed, undefined, { ttl: 10 }); + await sleep(5); + const { w, unsub } = mountReader(client, () => fetchQuery(GetStreamed).value); await sleep(60); expect(mockFetch.calls).toHaveLength(1); - // The fetch landed (an aborted one applies nothing), then the lease went. - const relay = client.getQuery(QueryDefinition.for(GetStreamed), undefined); - expect(relay.value).toMatchObject({ n: 9 }); - expect(unsubscribed).toBe(1); + expect(mockFetch.calls[0].options.signal?.aborted).toBe(false); + expect(w.value).toMatchObject({ n: 9 }); + unsub(); + }); + + it('a ttl caps a lease whose fetch never settles', async () => { + vi.useFakeTimers(); + try { + const networkManager = new NetworkManager(false); + const client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: createMockFetch() as any, baseUrl: 'http://localhost' })], + networkManager, + }); + clients.push(client); + // Offline: the relay stays pending. + client.prefetch(GetStreamed, undefined, { ttl: 1_000 }); + await vi.advanceTimersByTimeAsync(10); + expect(client.getQuery(QueryDefinition.for(GetStreamed), undefined).isPending).toBe(true); + await vi.advanceTimersByTimeAsync(2_000); + expect((client as unknown as { leases: Set }).leases.size).toBe(0); + client.destroy(); + clients = []; + await vi.advanceTimersByTimeAsync(1_000); + } finally { + vi.useRealTimers(); + } + }); + + it('destroy() clears the ttl timer', async () => { + vi.useFakeTimers(); + try { + const { client } = setup(); + client.prefetch(GetItem, undefined, { ttl: 60_000 }); + await vi.advanceTimersByTimeAsync(20); + client.destroy(); + clients = []; + await vi.advanceTimersByTimeAsync(50); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } }); it('defaults ttl to DEFAULT_PREFETCH_TTL', () => { @@ -441,6 +491,84 @@ describe('client.retain()', () => { expect(unsubscribed).toBe(1); }); + it('destroy() leaves no GC timer behind for the queries it releases', async () => { + const created: unknown[] = []; + const cleared = new Set(); + const realSetInterval = globalThis.setInterval; + const realClearInterval = globalThis.clearInterval; + vi.spyOn(globalThis, 'setInterval').mockImplementation(((fn: () => void, ms?: number) => { + const id = realSetInterval(fn, ms); + created.push(id); + return id; + }) as typeof setInterval); + vi.spyOn(globalThis, 'clearInterval').mockImplementation(((id: ReturnType) => { + cleared.add(id); + realClearInterval(id); + }) as typeof clearInterval); + + try { + const { client } = setup(); + // A real GcManager: the tests' window-less environment gets the no-op one. + (client as unknown as { gcManager: GcManager }).gcManager = new GcManager( + (client as unknown as { handleEviction: (key: number, type: GcKeyType) => void }).handleEviction, + 1, + ); + client.retain(() => fetchQuery(GetItem)); + await sleep(20); + client.destroy(); + clients = []; + // The released query deactivates on Signalium's next flush, after destroy(). + await sleep(50); + const leaked = created.filter(id => !cleared.has(id)); + for (const id of leaked) realClearInterval(id as ReturnType); + expect(leaked).toHaveLength(0); + } finally { + vi.restoreAllMocks(); + } + }); + + it('warns, and keeps one lease, when called from a reactive computation that reruns', async () => { + const warn = vi.fn(); + const mockFetch = createMockFetch(); + mockFetch.get('/item', () => ({ n: 1 })); + const client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: mockFetch as any, baseUrl: 'http://localhost' })], + log: { warn }, + }); + clients.push(client); + const leases = (client as unknown as { leases: Set }).leases; + + const tick = signal(0); + const computation = reactive(() => { + const value = tick.value; + client.retain(() => fetchQuery(GetItem)); + return value; + }); + const { w, unsub } = mountReader(client, () => computation()); + expect(warn).toHaveBeenCalledTimes(1); + for (let i = 1; i <= 5; i++) { + tick.value = i; + w.value; + await sleep(10); + } + expect(leases.size).toBe(1); + expect(mockFetch.calls).toHaveLength(1); + unsub(); + }); + + it('does not warn when called outside a reactive computation', () => { + const warn = vi.fn(); + const client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: createMockFetch() as any, baseUrl: 'http://localhost' })], + log: { warn }, + }); + clients.push(client); + client.retain(() => fetchQuery(GetItem))(); + expect(warn).not.toHaveBeenCalled(); + }); + it('releases and rethrows when the callback throws', () => { const { client } = setup(); expect(() => From 86577fa0a4a55b7901f9784bb2c6e0f697071d2c Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Mon, 5 Oct 2026 11:17:50 -0400 Subject: [PATCH 10/18] fix(fetchium): retry every failed attempt by default; keep shouldRetry opt-in Restores the previous retry default: every failed attempt is retried with the existing backoff, whatever its status, including all 4xx responses. The shouldRetry hook stays (QueryClientConfig.shouldRetry and per-query or per-mutation RetryConfig.shouldRetry, with getErrorStatus for reading a status), so an app can opt out per query where it knows a 4xx is permanent. Not retrying 4xx by default removed several seconds of retries that apps were relying on to recover queries that failed before their first load and had no other retry path. The burst of 400s that motivated the change was fixed at its source, so the default no longer needs to guard against it. defaultShouldRetry is removed (it was new in this branch); without a hook, withRetry no longer computes a status at all. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/should-retry-hook.md | 5 + .changeset/skip-client-error-retries.md | 7 - docs/src/app/api/fetchium/page.md | 2 +- docs/src/app/guides/error-handling/page.md | 19 +- packages/fetchium/src/QueryClient.ts | 12 +- .../src/__tests__/retry-should-retry.test.ts | 234 +++++++++++++----- packages/fetchium/src/index.ts | 2 +- packages/fetchium/src/retry.ts | 20 +- packages/fetchium/src/types.ts | 4 +- 9 files changed, 200 insertions(+), 105 deletions(-) create mode 100644 .changeset/should-retry-hook.md delete mode 100644 .changeset/skip-client-error-retries.md diff --git a/.changeset/should-retry-hook.md b/.changeset/should-retry-hook.md new file mode 100644 index 0000000..1aa0909 --- /dev/null +++ b/.changeset/should-retry-hook.md @@ -0,0 +1,5 @@ +--- +'fetchium': minor +--- + +Adds an optional `shouldRetry(error, attempt, status)` hook to `QueryClientConfig` and to `RetryConfig` (per query or mutation, overriding the client's). Return `false` to stop retrying an error you know is permanent, such as a 4xx from a given endpoint. `status` is the attempt's HTTP status when known: the response the adapter received (a REST error response whose body fails validation) or `status`, `statusCode` or `response.status` on the thrown error. Also exports `getErrorStatus`. Without a hook, retries are unchanged: every failed attempt is retried with the existing backoff. `RetryConfig.retries` is now optional and defaults as `retry: true` does. diff --git a/.changeset/skip-client-error-retries.md b/.changeset/skip-client-error-retries.md deleted file mode 100644 index ee46ed9..0000000 --- a/.changeset/skip-client-error-retries.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'fetchium': minor ---- - -Queries no longer retry client errors. A failed attempt whose HTTP status is 4xx (other than 408 and 429) now fails at once instead of being retried three times; network errors and 5xx responses still retry. The status comes from the response the adapter received (a REST error response whose body fails validation) or from `status`, `statusCode` or `response.status` on the thrown error. - -Adds `shouldRetry(error, attempt, status)` to `QueryClientConfig` and to `RetryConfig` (per query or mutation, overriding the client's) to decide retries yourself, and exports the default as `defaultShouldRetry` along with `getErrorStatus`. `RetryConfig.retries` is now optional and defaults as `retry: true` does. diff --git a/docs/src/app/api/fetchium/page.md b/docs/src/app/api/fetchium/page.md index 1321712..bcf3f80 100644 --- a/docs/src/app/api/fetchium/page.md +++ b/docs/src/app/api/fetchium/page.md @@ -235,7 +235,7 @@ new QueryClient(config: QueryClientConfig) | `gcManager` | `GcManager \| NoOpGcManager` | Auto-detected | GC manager. Uses `NoOpGcManager` on the server. | | `reactivationGraceMs` | `number` | `0` | Milliseconds. A query that reactivates (a watcher returns, or a paused scope resumes) with data younger than this is not refetched, even if stale. Data a subscription pushed to (a `subscribe` stream event, or a topic event sent with its topic) counts as fresh from the last push; a running `poll()` counts through the fetches it makes. Network reconnects, `__refetch()` and invalidation still refetch. Per-query override: `reactivationGraceMs` in the query config. | | `reactivationStaggerMs` | `number` | `0` | Milliseconds. Reactivation refetches that start in the same task are spread evenly across this window, in activation order, instead of all starting at once. | -| `shouldRetry` | `(error, attempt, status) => boolean` | `defaultShouldRetry` | Whether a failed attempt is retried. `attempt` starts at 0; `status` is the attempt's HTTP status when known (the adapter's error response, or `status` / `statusCode` / `response.status` on the error). The default retries network errors and 5xx but not 4xx other than 408 and 429. Per-query override: `retry.shouldRetry`. | +| `shouldRetry` | `(error, attempt, status) => boolean` | — | Optional. Whether a failed attempt is retried. Without it, every failed attempt is retried, whatever its status. `attempt` starts at 0; `status` is the attempt's HTTP status when known (the adapter's error response, or `status` / `statusCode` / `response.status` on the error). Return `false` to stop retrying errors you know are permanent, such as a 4xx. Per-query override: `retry.shouldRetry`. | | `activity` | `ActivitySource \| undefined` | — | `{ isActive(), subscribe(listener) }` foreground source. When set, `poll()` stops its timers while inactive and resumes them on activation. | | `pollResumeJitterMs` | `number` | `0` | Milliseconds. A `poll()` tick that is overdue on resume (or whose timer fired over a second late, as after JS timers were frozen in the background) is rescheduled at a random point in this window instead of firing at once. Per-poll override: `poll({ interval, resumeJitterMs })`. | diff --git a/docs/src/app/guides/error-handling/page.md b/docs/src/app/guides/error-handling/page.md index 2da961f..c44362b 100644 --- a/docs/src/app/guides/error-handling/page.md +++ b/docs/src/app/guides/error-handling/page.md @@ -147,17 +147,22 @@ class GetUser extends RESTQuery { The default retry delay uses exponential backoff --- each successive attempt waits longer than the last (roughly 1s, 2s, 4s, ...). This avoids hammering a struggling server with rapid retries. -Client errors are not retried: a `4xx` response fails on the first attempt, because repeating the same request gets the same answer. `408 Request Timeout` and `429 Too Many Requests` are the exceptions, and network errors and `5xx` responses are retried. The status comes from the response the adapter received (a REST error response whose body doesn't match `result`), or from `status`, `statusCode` or `response.status` on the thrown error. To decide yourself, pass `shouldRetry`, either per query in `retry` or for every query on the `QueryClient`: +Every failed attempt is retried, whatever its status, including `4xx` responses. When you know an error is permanent, opt out with `shouldRetry`, either per query in `retry` or for every query on the `QueryClient`. It receives the error, the attempt index and the attempt's HTTP status when known: the response the adapter received (a REST error response whose body doesn't match `result`), or `status`, `statusCode` or `response.status` on the thrown error. A query's `shouldRetry` overrides the client's. ```ts -const client = new QueryClient({ - adapters: [new RESTQueryAdapter({ baseUrl })], - // attempt starts at 0; status is undefined for network errors - shouldRetry: (error, attempt, status) => - status === undefined || status >= 500, -}); +class GetUser extends RESTQuery { + // ... + config = { + retry: { + // A 404 won't change on retry; status is undefined for network errors + shouldRetry: (error, attempt, status) => status !== 404, + }, + }; +} ``` +`getErrorStatus(error)` reads the status from an error, if you need it elsewhere. + To disable retries entirely: ```ts diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index 159bd97..82dc8b0 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -26,7 +26,7 @@ import { ValidatorDef } from './typeDefs.js'; import { ConstraintMatcher, EVENT_SOURCE_FIELD } from './ConstraintMatcher.js'; import { LiveCollectionBinding } from './LiveCollection.js'; import { QueryAdapter, type QueryAdapterClass } from './QueryAdapter.js'; -import { defaultShouldRetry, type ShouldRetry } from './retry.js'; +import type { ShouldRetry } from './retry.js'; import { type QueryContext, type QueryStore, @@ -67,9 +67,9 @@ export interface QueryClientConfig { * Decides whether a failed query attempt (or a mutation attempt, when the * mutation enables retries) is retried. Receives the error, the attempt index * (starting at 0) and the attempt's HTTP status when known. A query's or - * mutation's own `retry.shouldRetry` overrides it. Default: - * `defaultShouldRetry`, which retries network errors and 5xx responses but - * not 4xx responses other than 408 and 429. + * mutation's own `retry.shouldRetry` overrides it. Optional: without it, + * every failed attempt is retried, whatever its status. Use it to stop + * retrying errors you know are permanent, such as a 4xx from an endpoint. */ shouldRetry?: ShouldRetry; /** @@ -174,7 +174,7 @@ export class QueryClient { /** See `QueryClientConfig.reactivationStaggerMs`. */ readonly reactivationStaggerMs: number; /** See `QueryClientConfig.shouldRetry`. */ - readonly shouldRetry: ShouldRetry; + readonly shouldRetry: ShouldRetry | undefined; /** Queries whose reactivation refetch waits for the current task's stagger flush. */ private staggerQueue = new Set>(); @@ -222,7 +222,7 @@ export class QueryClient { this.store = store; this.reactivationGraceMs = nonNegative(reactivationGraceMs); this.reactivationStaggerMs = nonNegative(reactivationStaggerMs); - this.shouldRetry = shouldRetry ?? defaultShouldRetry; + this.shouldRetry = shouldRetry; // `activity` and `pollResumeJitterMs` ride along in `rest`: poll() reads them from the context. this.context = { ...rest, log: log ?? console, evictionMultiplier }; this.gcManager = diff --git a/packages/fetchium/src/__tests__/retry-should-retry.test.ts b/packages/fetchium/src/__tests__/retry-should-retry.test.ts index d53abaf..9ded5a9 100644 --- a/packages/fetchium/src/__tests__/retry-should-retry.test.ts +++ b/packages/fetchium/src/__tests__/retry-should-retry.test.ts @@ -6,7 +6,7 @@ import { RESTQueryAdapter } from '../rest/RESTQueryAdapter.js'; import { fetchQuery } from '../query.js'; import { getMutation } from '../mutation.js'; import { NetworkManager } from '../NetworkManager.js'; -import { defaultShouldRetry, getErrorStatus, type ShouldRetry } from '../retry.js'; +import { getErrorStatus, type ShouldRetry } from '../retry.js'; import { createMockFetch, testWithClient } from './utils.js'; import { t } from '../typeDefs.js'; @@ -36,6 +36,15 @@ describe('shouldRetry', () => { config = { retry: { retries: 3, retryDelay: () => 1 } }; } + class CreateUser extends RESTMutation { + readonly path = '/users'; + readonly method = 'POST' as const; + readonly params = { name: t.string }; + readonly body = { name: this.params.name }; + readonly result = { id: t.number }; + config = { retry: { retries: 2, retryDelay: () => 1 } }; + } + async function runFailing(query: new () => RESTQuery = GetUser): Promise { let caught: unknown; await testWithClient(client, async () => { @@ -58,36 +67,26 @@ describe('shouldRetry', () => { }); describe('default', () => { - for (const status of [400, 401, 403, 404, 422]) { - it(`does not retry a ${status} response whose body fails validation`, async () => { + for (const status of [400, 401, 403, 404, 408, 422, 429, 500, 503]) { + it(`retries a ${status} response whose body fails validation`, async () => { mockFetch.get('/users/1', { error: 'nope' }, { status }); const error = await runFailing(); expect(error).toBeDefined(); - expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls).toHaveLength(4); }); } - it('does not retry an error that carries a 4xx status', async () => { + it('retries an error that carries a 4xx status', async () => { mockFetch.get('/users/1', null, { error: httpError(401) }); const error = await runFailing(); expect(error).toMatchObject({ response: { status: 401 } }); - expect(mockFetch.calls).toHaveLength(1); + expect(mockFetch.calls).toHaveLength(4); }); - for (const status of [408, 429, 500, 503]) { - it(`retries a ${status} response`, async () => { - mockFetch.get('/users/1', { error: 'busy' }, { status }); - - await runFailing(); - - expect(mockFetch.calls).toHaveLength(4); - }); - } - it('retries a 503 whose body is not JSON, then succeeds', async () => { mockFetch.get('/users/1', null, { status: 503, jsonError: new SyntaxError('Unexpected token <') }); mockFetch.get('/users/1', user); @@ -100,14 +99,6 @@ describe('shouldRetry', () => { expect(mockFetch.calls).toHaveLength(2); }); - it('retries an error that carries a 429 status', async () => { - mockFetch.get('/users/1', null, { error: httpError(429) }); - - await runFailing(); - - expect(mockFetch.calls).toHaveLength(4); - }); - it('retries network errors', async () => { mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); @@ -124,22 +115,7 @@ describe('shouldRetry', () => { expect(mockFetch.calls).toHaveLength(4); }); - it('does not take the status of an earlier response for a later network error', async () => { - // Fetch 1 succeeds; the refetch fails with a network error, which must - // not inherit a status from the response assigned by fetch 1. - mockFetch.get('/users/1', user); - mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); - - await testWithClient(client, async () => { - const result = fetchQuery(GetUser); - await result; - await result.value!.__refetch().catch(() => {}); - }); - - expect(mockFetch.calls).toHaveLength(5); - }); - - it('does not retry a 4xx page in fetchNext', async () => { + it('retries a 4xx page in fetchNext', async () => { class GetItems extends RESTQuery { path = '/items'; result = { items: t.array(t.string), nextPage: t.optional(t.number) }; @@ -155,30 +131,111 @@ describe('shouldRetry', () => { await expect(result.value!.__fetchNext()).rejects.toBeDefined(); }); - expect(mockFetch.calls).toHaveLength(2); + expect(mockFetch.calls).toHaveLength(5); + }); + + it('retries a mutation that enables retries, whatever the status', async () => { + mockFetch.post('/users', null, { error: httpError(422) }); + + await testWithClient(client, async () => { + const mut = getMutation(CreateUser); + await expect(mut.run({ name: 'Test' })).rejects.toBeDefined(); + }); + + expect(mockFetch.calls).toHaveLength(3); }); }); describe('custom', () => { + /** Opts out of retrying client errors other than 408 and 429. */ + const skipClientErrors: ShouldRetry = (_error, _attempt, status) => + status === undefined || status < 400 || status >= 500 || status === 408 || status === 429; + + function useSkipClientErrors(): void { + client.destroy(); + client = createClient(skipClientErrors); + } + it('client-level shouldRetry receives error, attempt and status', async () => { - const seen: Array<[number, number | undefined]> = []; + const seen: Array<[unknown, number, number | undefined]> = []; client.destroy(); - client = createClient((_error, attempt, status) => { - seen.push([attempt, status]); + client = createClient((error, attempt, status) => { + seen.push([error, attempt, status]); return attempt < 1; }); mockFetch.get('/users/1', { error: 'nope' }, { status: 404 }); - await runFailing(); + const error = await runFailing(); expect(mockFetch.calls).toHaveLength(2); - expect(seen).toEqual([ + expect(seen.map(([, attempt, status]) => [attempt, status])).toEqual([ [0, 404], [1, 404], ]); + expect(seen[1][0]).toBe(error); }); - it('query-level shouldRetry overrides the client default', async () => { + it('passes the status carried by the error', async () => { + const statuses: Array = []; + client.destroy(); + client = createClient((_error, _attempt, status) => { + statuses.push(status); + return true; + }); + mockFetch.get('/users/1', null, { error: httpError(401) }); + + await runFailing(); + + expect(statuses).toEqual([401, 401, 401]); + }); + + for (const status of [400, 401, 404, 422]) { + it(`client-level shouldRetry returning false for ${status} makes one attempt`, async () => { + useSkipClientErrors(); + mockFetch.get('/users/1', { error: 'nope' }, { status }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(1); + }); + } + + it('client-level shouldRetry returning false for a 4xx error makes one attempt', async () => { + useSkipClientErrors(); + mockFetch.get('/users/1', null, { error: httpError(401) }); + + const error = await runFailing(); + + expect(error).toMatchObject({ response: { status: 401 } }); + expect(mockFetch.calls).toHaveLength(1); + }); + + for (const status of [408, 429, 503]) { + it(`client-level shouldRetry still retries what it allows (${status})`, async () => { + useSkipClientErrors(); + mockFetch.get('/users/1', { error: 'busy' }, { status }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(4); + }); + } + + it('query-level shouldRetry returning false for 4xx makes one attempt', async () => { + class GetUserSkip4xx extends RESTQuery { + path = '/users/1'; + result = t.object({ id: t.string, name: t.string }); + config = { retry: { retries: 3, retryDelay: () => 1, shouldRetry: skipClientErrors } }; + } + mockFetch.get('/users/1', { error: 'nope' }, { status: 404 }); + + await runFailing(GetUserSkip4xx); + + expect(mockFetch.calls).toHaveLength(1); + }); + + it('query-level shouldRetry overrides the client-level one', async () => { + useSkipClientErrors(); class GetUserRetry4xx extends RESTQuery { path = '/users/1'; result = t.object({ id: t.string, name: t.string }); @@ -191,7 +248,7 @@ describe('shouldRetry', () => { expect(mockFetch.calls).toHaveLength(3); }); - it('query-level shouldRetry can stop retries the default would make', async () => { + it('query-level shouldRetry can stop retries of a network error', async () => { class GetUserNoRetry extends RESTQuery { path = '/users/1'; result = t.object({ id: t.string, name: t.string }); @@ -204,15 +261,68 @@ describe('shouldRetry', () => { expect(mockFetch.calls).toHaveLength(1); }); - it('applies to mutations', async () => { - class CreateUser extends RESTMutation { - readonly path = '/users'; - readonly method = 'POST' as const; - readonly params = { name: t.string }; - readonly body = { name: this.params.name }; - readonly result = { id: t.number }; - config = { retry: { retries: 2, retryDelay: () => 1 } }; + it('does not pass an earlier response status for a later network error', async () => { + // Attempt 0 gets a 404 response; the later attempts fail with network + // errors, which must report no status rather than the earlier 404. + const statuses: Array = []; + client.destroy(); + client = createClient((_error, _attempt, status) => { + statuses.push(status); + return true; + }); + mockFetch.get('/users/1', { error: 'nope' }, { status: 404 }); + mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); + + await runFailing(); + + expect(mockFetch.calls).toHaveLength(4); + expect(statuses).toEqual([404, undefined, undefined]); + }); + + it('does not pass the status of an earlier fetch for a refetch network error', async () => { + // Fetch 1 succeeds; the refetch fails with a network error, which must + // not inherit a status from the response assigned by fetch 1. + const statuses: Array = []; + client.destroy(); + client = createClient((_error, _attempt, status) => { + statuses.push(status); + return true; + }); + mockFetch.get('/users/1', user); + mockFetch.get('/users/1', null, { error: new TypeError('Network request failed') }); + + await testWithClient(client, async () => { + const result = fetchQuery(GetUser); + await result; + await result.value!.__refetch().catch(() => {}); + }); + + expect(mockFetch.calls).toHaveLength(5); + expect(statuses).toEqual([undefined, undefined, undefined]); + }); + + it('client-level shouldRetry returning false for a 4xx page in fetchNext makes one attempt', async () => { + useSkipClientErrors(); + class GetItems extends RESTQuery { + path = '/items'; + result = { items: t.array(t.string), nextPage: t.optional(t.number) }; + fetchNext = { searchParams: { page: this.result.nextPage } }; + config = { retry: { retries: 3, retryDelay: () => 1 } }; } + mockFetch.get('/items', { items: ['a'], nextPage: 2 }); + + await testWithClient(client, async () => { + const result = fetchQuery(GetItems); + await result; + mockFetch.get('/items', { error: 'bad page' }, { status: 400 }); + await expect(result.value!.__fetchNext()).rejects.toBeDefined(); + }); + + expect(mockFetch.calls).toHaveLength(2); + }); + + it('applies to mutations', async () => { + useSkipClientErrors(); mockFetch.post('/users', null, { error: httpError(422) }); await testWithClient(client, async () => { @@ -225,18 +335,6 @@ describe('shouldRetry', () => { }); }); -describe('defaultShouldRetry', () => { - it('classifies statuses', () => { - const err = new Error('x'); - expect(defaultShouldRetry(err, 0, undefined)).toBe(true); - expect(defaultShouldRetry(err, 0, 400)).toBe(false); - expect(defaultShouldRetry(err, 0, 404)).toBe(false); - expect(defaultShouldRetry(err, 0, 408)).toBe(true); - expect(defaultShouldRetry(err, 0, 429)).toBe(true); - expect(defaultShouldRetry(err, 0, 500)).toBe(true); - }); -}); - describe('getErrorStatus', () => { it('reads status, statusCode and response.status', () => { expect(getErrorStatus({ status: 404 })).toBe(404); diff --git a/packages/fetchium/src/index.ts b/packages/fetchium/src/index.ts index a11b55b..b482323 100644 --- a/packages/fetchium/src/index.ts +++ b/packages/fetchium/src/index.ts @@ -13,5 +13,5 @@ export type { Draft } from './utils.js'; export { NetworkManager, NoOpNetworkManager, defaultNetworkManager, NetworkManagerContext } from './NetworkManager.js'; export { GcManager, NoOpGcManager } from './GcManager.js'; export { Entity } from './proxy.js'; -export { defaultShouldRetry, getErrorStatus } from './retry.js'; +export { getErrorStatus } from './retry.js'; export type { ShouldRetry } from './retry.js'; diff --git a/packages/fetchium/src/retry.ts b/packages/fetchium/src/retry.ts index 451ab2c..37877ed 100644 --- a/packages/fetchium/src/retry.ts +++ b/packages/fetchium/src/retry.ts @@ -2,7 +2,7 @@ import type { ResolvedRetryConfig } from './query.js'; /** * Decides whether a failed attempt is retried. Consulted only while retries - * remain. + * remain. Without one, every failed attempt is retried. * * @param error What the attempt threw. * @param attempt Index of the failed attempt, starting at 0 (the first request). @@ -34,16 +34,8 @@ export function getFailedResponseStatus(response: unknown): number | undefined { return status !== undefined && status >= 400 ? status : undefined; } -/** - * Retries network errors, unknown errors and server errors (5xx). Does not - * retry client errors (4xx), which a repeat of the same request would hit - * again, except 408 Request Timeout and 429 Too Many Requests. - */ -export const defaultShouldRetry: ShouldRetry = (_error, _attempt, status) => - status === undefined || status < 400 || status >= 500 || status === 408 || status === 429; - export interface WithRetryOptions { - /** Used when the retry config has no `shouldRetry`. Default: `defaultShouldRetry`. */ + /** Used when the retry config has no `shouldRetry`. Without either, every failed attempt is retried. */ shouldRetry?: ShouldRetry; /** * Called after an attempt fails. Returns the HTTP status the attempt @@ -94,7 +86,7 @@ export async function withRetry( throw new Error('retries must be non-negative'); } const retries = Math.max(0, config.retries); - const shouldRetry = config.shouldRetry ?? options?.shouldRetry ?? defaultShouldRetry; + const shouldRetry = config.shouldRetry ?? options?.shouldRetry; let lastError: unknown; for (let attempt = 0; attempt <= retries; attempt++) { if (signal?.aborted) { @@ -105,8 +97,10 @@ export async function withRetry( } catch (error) { lastError = error; if (attempt >= retries) throw error; - const status = getErrorStatus(error) ?? options?.getAttemptStatus?.(); - if (!shouldRetry(error, attempt, status)) throw error; + if (shouldRetry !== undefined) { + const status = getErrorStatus(error) ?? options?.getAttemptStatus?.(); + if (!shouldRetry(error, attempt, status)) throw error; + } await sleep(config.retryDelay(attempt), signal); } } diff --git a/packages/fetchium/src/types.ts b/packages/fetchium/src/types.ts index e44d3f4..9b873bb 100644 --- a/packages/fetchium/src/types.ts +++ b/packages/fetchium/src/types.ts @@ -70,8 +70,8 @@ export interface RetryConfig { /** * Decides whether a failed attempt is retried, from the error, the attempt * index (starting at 0) and the attempt's HTTP status when known. Overrides - * `QueryClientConfig.shouldRetry`. Default: `defaultShouldRetry`, which does - * not retry 4xx responses other than 408 and 429. + * `QueryClientConfig.shouldRetry`. Without either, every failed attempt is + * retried. */ shouldRetry?: (error: unknown, attempt: number, status: number | undefined) => boolean; } From 887d5f86aedb507ed8278a765a880f0d62a5f8e2 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Tue, 6 Oct 2026 13:00:48 -0400 Subject: [PATCH 11/18] chore(fetchium): rewrite this change's changesets for React developers Lead with what changes for an app using useQuery or fetchQuery, keep migration notes to a line or two, and drop internal detail. No source change. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/fast-delivery-prefetch-retain.md | 9 ++++----- .changeset/should-retry-hook.md | 4 +++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.changeset/fast-delivery-prefetch-retain.md b/.changeset/fast-delivery-prefetch-retain.md index a31fde9..7a5a507 100644 --- a/.changeset/fast-delivery-prefetch-retain.md +++ b/.changeset/fast-delivery-prefetch-retain.md @@ -2,10 +2,9 @@ 'fetchium': minor --- -Adds `queryClient.prefetch(QueryClass, params, { ttl })` and `queryClient.retain(fn, { ttl })`. Both keep queries active without a reader and return a `release` function. A reader that mounts while the lease is held joins the running query: no second request, and the data on its first render if it has arrived. `prefetch` defaults to a 10 s lease (`DEFAULT_PREFETCH_TTL`), an upper bound even with a fetch in flight; `retain` lasts until released. +New `queryClient.prefetch(Query, params, { ttl })` and `queryClient.retain(fn, { ttl })` start queries before any component reads them. A component that mounts while one is held reuses its request and renders the data on its first render if it has arrived. Both return a `release` function; `prefetch` lasts 10 seconds by default and `retain` until released. -Adds `useSuspenseQuery` to `fetchium/react`. It suspends only when the query has never produced a value (in memory or in a synchronous store), never for a refetch, and throws a failed cold fetch to the error boundary. +New `useSuspenseQuery` in `fetchium/react`. It suspends only while the query has no data yet, and sends a failed first fetch to the error boundary. -A refetch with no `debounce` (a stale query reactivating, a Signal param change) now starts on a microtask instead of after `setTimeout(0)`, which on React Native can wait a frame. Invalidation still waits a task, so a query whose last reader left in the same task is deactivated rather than refetched. - -Docs no longer claim that reading `.value` on a pending or rejected query suspends or throws; it does neither. +- A refetch without `debounce` starts on a microtask instead of after `setTimeout(0)`. +- Reading `.value` on a pending or failed query doesn't suspend or throw. The docs said it did. diff --git a/.changeset/should-retry-hook.md b/.changeset/should-retry-hook.md index 1aa0909..be5b0ab 100644 --- a/.changeset/should-retry-hook.md +++ b/.changeset/should-retry-hook.md @@ -2,4 +2,6 @@ 'fetchium': minor --- -Adds an optional `shouldRetry(error, attempt, status)` hook to `QueryClientConfig` and to `RetryConfig` (per query or mutation, overriding the client's). Return `false` to stop retrying an error you know is permanent, such as a 4xx from a given endpoint. `status` is the attempt's HTTP status when known: the response the adapter received (a REST error response whose body fails validation) or `status`, `statusCode` or `response.status` on the thrown error. Also exports `getErrorStatus`. Without a hook, retries are unchanged: every failed attempt is retried with the existing backoff. `RetryConfig.retries` is now optional and defaults as `retry: true` does. +New optional `shouldRetry(error, attempt, status)` on `QueryClientConfig` and on a query's or mutation's `retry` config. Return `false` to stop retrying an error you know is permanent, such as a 404. + +`status` is the HTTP status when it is known, and `getErrorStatus` is exported. Without the hook, retries work as before. `retry.retries` is now optional. From 2dc709ebcf6181eff244031794ef883e2705b558 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Tue, 6 Oct 2026 23:45:15 -0400 Subject: [PATCH 12/18] chore(fetchium): tighten comments Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/LiveCollection.ts | 5 +- packages/fetchium/src/QueryClient.ts | 74 +++++++++---------- packages/fetchium/src/QueryResult.ts | 53 +++++++------ .../__tests__/identical-redelivery.test.ts | 19 ++--- .../src/__tests__/read-path-latency.test.ts | 11 ++- .../src/__tests__/retry-should-retry.test.ts | 5 +- .../fetchium/src/react/use-suspense-query.ts | 10 +-- packages/fetchium/src/retry.ts | 9 ++- 8 files changed, 87 insertions(+), 99 deletions(-) diff --git a/packages/fetchium/src/LiveCollection.ts b/packages/fetchium/src/LiveCollection.ts index 673c54a..5e46539 100644 --- a/packages/fetchium/src/LiveCollection.ts +++ b/packages/fetchium/src/LiveCollection.ts @@ -450,9 +450,8 @@ export class LiveValueInstance { this._value = this._onDelete(this._value, entity ?? entityData); break; } - // Notify after every reducer run, even one that returns the value it was - // given: a reducer may mutate the value in place (push onto an array and - // return it), and a reference check can't tell that from a no-op. + // Notify even when the reducer returns the same reference: it may have + // mutated the value in place. this._notifier.notify(); } diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index 82dc8b0..de92617 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -67,9 +67,8 @@ export interface QueryClientConfig { * Decides whether a failed query attempt (or a mutation attempt, when the * mutation enables retries) is retried. Receives the error, the attempt index * (starting at 0) and the attempt's HTTP status when known. A query's or - * mutation's own `retry.shouldRetry` overrides it. Optional: without it, - * every failed attempt is retried, whatever its status. Use it to stop - * retrying errors you know are permanent, such as a 4xx from an endpoint. + * mutation's own `retry.shouldRetry` overrides it. Without it, every failed + * attempt is retried. Use it to stop retrying permanent errors such as a 4xx. */ shouldRetry?: ShouldRetry; /** @@ -183,13 +182,13 @@ export class QueryClient { /** Release functions of outstanding `retain()` / `prefetch()` leases. */ private leases = new Set<() => void>(); /** - * While a lease's callback first runs: the queries it reached that left - * their first fetch (or a zero-delay refetch) for a microtask. The lease - * starts them before returning. See `retain()`. + * Set during a lease's first run: the queries it reached whose first fetch + * (or zero-delay refetch) is queued on a microtask. The lease starts them + * before returning. See `retain()`. */ private leaseStarts: Set> | undefined = undefined; /** - * Leases taken by `retain()` calls inside a reactive computation, by the + * Leases taken by `retain()` inside a reactive computation, keyed by the * computation, with the run that took them. See `retain()`. */ private reactiveLeases = new WeakMap void> }>(); @@ -402,8 +401,8 @@ export class QueryClient { this.queryInstances.set(queryKey, queryInstance as QueryInstance); } - // Already active with its start still queued (a reader activated it earlier - // in this task): the lease starts it too. + // A reader may have activated it earlier in this task with its start still + // queued. The lease starts it too. this.leaseStarts?.add(queryInstance); return queryInstance.relay; @@ -436,16 +435,16 @@ export class QueryClient { * the query deactivates and its `gcTime` starts as usual. `release` is * idempotent. * - * The fetches the first run of `fn` needs start before `retain` returns: a - * request goes out inside the call, ahead of anything already queued on the - * microtask queue (such as a render React scheduled for the same tap). A - * query that depends on another's result (`fn` reads an id, then fetches by - * it) starts once that result arrives, as with any reader. Call `retain` - * from an event handler or effect, not from inside a reactive computation: - * starting a fetch runs adapter code, and a computation that reruns would - * take a new lease on every run. Called from one anyway, it warns in - * development, starts its fetches on their usual microtask, and releases the - * leases the computation's previous run took. + * The first run's fetches start before `retain` returns, ahead of anything + * already on the microtask queue (such as a render React scheduled for the + * same tap). A query that depends on another's result starts once that + * result arrives, as with any reader. + * + * Call `retain` from an event handler or effect, not a reactive computation: + * starting a fetch runs adapter code, and a rerunning computation would take + * a new lease each run. Called from one anyway, it warns in development, + * starts its fetches on their usual microtask, and releases the leases the + * computation's previous run took. */ retain(fn: () => unknown, options?: RetainOptions): () => void { const owner = currentReactiveOwner(); @@ -522,17 +521,16 @@ export class QueryClient { /** * Starts `QueryClass` with `params` now and keeps it active for `ttl` * milliseconds (default {@link DEFAULT_PREFETCH_TTL}), or until the returned - * `release` is called. The `ttl` is an upper bound: the lease goes when it - * runs out even if the fetch is still in flight (offline, or a topic that is - * never fulfilled), which aborts that fetch unless a reader has joined it. - * Meant for the moment a user commits to a navigation - * (a tap): the destination's reader, mounting within the window, reuses the - * in-flight or finished fetch instead of starting its own, and renders the - * data on its first render if it has arrived. + * `release` is called. Meant for the tap that commits to a navigation: the + * destination's reader, mounting within the window, reuses the in-flight or + * finished fetch and renders the data on its first render if it has arrived. + * + * The `ttl` is an upper bound. The lease ends even if the fetch is still in + * flight (offline, or a topic never fulfilled), which aborts it unless a + * reader has joined. * - * Cached data counts: with a synchronous store a cached, fresh result is - * applied without a request. A stale one is shown and refetched, as on any - * activation. + * With a synchronous store a fresh cached result is applied without a + * request. A stale one is shown and refetched, as on any activation. */ prefetch( QueryClass: new () => T, @@ -550,14 +548,14 @@ export class QueryClient { * never produced a value (a cold miss), or `undefined` when it has one to * render (in memory, or hydrated now from a synchronous store). * - * A cold miss takes a hold that keeps the query active while the render is - * suspended, since React discards a suspended render without subscribing. - * The reader's commit releases it (`releaseSuspenseHold`); otherwise it - * releases itself `SUSPENSE_HOLD_TTL` after the fetch settles. A query whose - * last fetch failed is refetched once per hold: when that attempt fails too, - * `error` is set and the hold is dropped, so the caller can throw it. A - * failure no render claims within `UNCLAIMED_FAILURE_TTL` (the suspended - * tree was abandoned) drops the hold, so a later mount makes a new attempt. + * A cold miss takes a hold that keeps the query active while suspended, + * since React discards a suspended render without subscribing. The reader's + * commit releases it (`releaseSuspenseHold`), or it releases itself + * `SUSPENSE_HOLD_TTL` after the fetch settles. A query whose last fetch + * failed is refetched once per hold. If that attempt fails too, `error` is + * returned for the caller to throw and the hold is dropped. A failure no + * render claims within `UNCLAIMED_FAILURE_TTL` also drops the hold, so a + * later mount makes a new attempt. * * @internal */ @@ -818,7 +816,7 @@ export class QueryClient { /** * Called by a query that queued its start (or a zero-delay refetch) on a - * microtask. During a lease's first run, the lease starts it before returning. + * microtask, so a lease in its first run can start it before returning. * * @internal */ diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 91a77f2..08884c9 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -212,9 +212,9 @@ export class QueryInstance { // If the relay shows pending but the abort controller is gone, the // previous fetch was aborted during deactivation (or never started). - // runDebounced() would bail out because isPending is still true from - // the doomed promise, which may also never settle (a topic query's - // send() waits on a subscription that is gone). + // runDebounced() would bail out on the doomed promise's isPending, + // and that promise may never settle (a topic query's send() waits on + // a subscription that is gone). if (this.relayState.isPending && this._abortController === undefined) { this.restartAbortedFetch(); } else { @@ -311,9 +311,9 @@ export class QueryInstance { * With an asynchronous store (AsyncQueryStore), the cache resolves on a later * tick, outside the activating read, and everything runs from there. * - * A lease (`retain()` / `prefetch()`) doesn't wait for the microtask: it - * calls startPendingNow() once its activating read has returned, so the - * request goes out inside the call, ahead of any render already queued. + * A lease (`retain()` / `prefetch()`) skips the microtask: it calls + * startPendingNow() after its activating read, so the request goes out ahead + * of any render already queued. */ private initialize(): void { this.initialized = true; @@ -353,9 +353,10 @@ export class QueryInstance { }; /** - * Runs, now, the start initialize() or a zero-delay runDebounced() left for - * a microtask, if it hasn't run yet. The microtask then does nothing. Called - * by a lease after its activating read, outside any reactive computation. + * Runs now any start that initialize(), a zero-delay runDebounced() or + * restartAbortedFetch() queued on a microtask, which then does nothing. + * Called by a lease after its activating read, outside any reactive + * computation. * * @internal */ @@ -477,8 +478,8 @@ export class QueryInstance { } const ctx = this.getOrCreateExecutionContext(); - // Put back a subscription a deactivation or pause tore down before - // sending: a topic query's send() waits for data its subscription brings. + // Restore a subscription that a deactivation or pause tore down: a topic + // query's send() waits for data its subscription brings. this.reconcileSubscription(); const adapter = this.queryClient.getAdapter(def.statics.adapterClass); const signal = this._abortController?.signal ?? new AbortController().signal; @@ -517,9 +518,9 @@ export class QueryInstance { /** * Replaces a fetch that a deactivation aborted, from inside the activating - * read. The relay takes the new promise now, so the doomed one can no longer - * settle it, but the subscription and the request start on a microtask (or - * when a lease starts them), outside the read: both run adapter code. + * read. The relay takes the new promise now so the doomed one can't settle + * it. The subscription and request run adapter code, so they start outside + * the read, on a microtask or when a lease starts them. */ private restartAbortedFetch(): void { this.fetchStarts++; @@ -564,16 +565,15 @@ export class QueryInstance { * Starts a refetch after the query's `debounce` (plus `extraDelay`). Calls * made before it starts are coalesced into one fetch. * - * With no delay the fetch starts on a microtask rather than a timer. That - * still runs outside the reactive computation that asked for it (a relay - * update) and still coalesces every call made in the same task, but doesn't - * wait for the next macrotask. On React Native a zero timer goes through the - * native timing module and can wait up to a frame. + * With no delay the fetch starts on a microtask, not a timer. It still runs + * outside the reactive computation that asked for it and still coalesces + * calls from the same task, without waiting a macrotask (on React Native a + * zero timer can wait up to a frame). * * `nextTask` keeps the zero-delay fetch on a timer. Invalidation uses it: a - * query invalidated in the same task its last watcher left is still active - * until Signalium's deactivation flush, and the timer lets that flush cancel - * the fetch rather than start and abort it. + * query invalidated in the task its last watcher left stays active until + * Signalium's deactivation flush, and the timer lets that flush cancel the + * fetch instead of starting and aborting it. */ private runDebounced(extraDelay: number = 0, nextTask: boolean = false): void { if (this.relayState.isPending) return; @@ -739,11 +739,10 @@ export class QueryInstance { } /** - * Retry options that report the HTTP status of a failed attempt. Adapters - * that expose their response as `ctx.response` (RESTQueryAdapter does) set - * it before the body is parsed and validated, so an error response whose - * body fails validation surfaces as a schema error, not an HTTP error. A - * response assigned during the failed attempt supplies its status. + * Retry options that report a failed attempt's HTTP status. RESTQueryAdapter + * sets `ctx.response` before parsing and validating the body, so an error + * response with an invalid body throws a schema error that carries no + * status. Only a response assigned during the failed attempt counts. */ private attemptStatusTracker(ctx: Query): { start: () => void; options: WithRetryOptions } { const holder = ctx as unknown as { response?: unknown }; diff --git a/packages/fetchium/src/__tests__/identical-redelivery.test.ts b/packages/fetchium/src/__tests__/identical-redelivery.test.ts index 0afa3d9..6349ef2 100644 --- a/packages/fetchium/src/__tests__/identical-redelivery.test.ts +++ b/packages/fetchium/src/__tests__/identical-redelivery.test.ts @@ -14,13 +14,10 @@ import type { MutationEvent, QueryPromise } from '../types.js'; import { testWithClient, setupTestClient } from './utils.js'; /** - * Identical Re-delivery - * - * A stream reconnect re-snapshots every topic, so every entity on screen is - * re-delivered with the data the store already holds — as a query result, as a - * mutation event, or as a topic event. None of those may notify an entity, bump - * its version, write it, or notify a live collection. A real change still - * notifies exactly once. + * A stream reconnect re-delivers every on-screen entity with the data the + * store already holds, as a query result, mutation event or topic event. None + * may notify an entity, bump its version, write it, or notify a live + * collection. A real change notifies exactly once. */ /** Records `typename:id` for every entity notify. */ @@ -212,14 +209,12 @@ describe('Identical re-delivery', () => { expect(versions(client, keys)).toEqual(before); expect(saveEntity).not.toHaveBeenCalled(); expect(live.tokens).not.toHaveBeenCalled(); - // The live value's onUpdate reducer runs for the Token event and the - // value notifies: a reducer may mutate in place, so a returned identical - // reference doesn't prove nothing changed. + // The live value's reducer still runs and notifies, since it may mutate + // in place. expect(live.count).toHaveBeenCalledTimes(1); // A `create` for an entity the store already holds is the same no-op for - // the entity and the live array. (The live value's `onCreate` reducer - // still runs: it is told about a create, not about a change.) + // the entity and the live array. The live value's `onCreate` still runs. client.applyMutationEvent({ type: 'create', typename: 'Token', data: token(0) }); client.applyMutationEvent({ type: 'create', typename: 'Owner', data: { id: 'o-1', name: 'Ann' } }); diff --git a/packages/fetchium/src/__tests__/read-path-latency.test.ts b/packages/fetchium/src/__tests__/read-path-latency.test.ts index 20b6878..e6f92aa 100644 --- a/packages/fetchium/src/__tests__/read-path-latency.test.ts +++ b/packages/fetchium/src/__tests__/read-path-latency.test.ts @@ -164,8 +164,8 @@ describe('read-path latency (macrotask turns)', () => { console.log( `[latency] param change -> fetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`, ); - // A watched relay reruns its update in Signalium's flush (one timer); the - // fetch then starts on a microtask of that flush rather than another timer. + // The relay update reruns in Signalium's flush (one timer), and the fetch + // starts on a microtask of that flush. expect(turns).toBe(1); expect(mockFetch.calls[1].url).toContain('/users/2'); unsub(); @@ -197,10 +197,9 @@ describe('read-path latency (macrotask turns)', () => { unsub(); // Before the response (30 ms). await sleep(10); - // Signalium deactivates on its next flush (a timer), after the microtask - // that starts the refetch, so the refetch starts and deactivation cancels - // it. A mount and unmount within one task is the cost of not waiting a - // timer on every reactivation. + // Signalium deactivates on its next flush (a timer), after the refetch's + // microtask has started it. This wasted request is the trade-off for not + // waiting a timer on every reactivation. expect(mockFetch.calls).toHaveLength(1); expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); await sleep(30); diff --git a/packages/fetchium/src/__tests__/retry-should-retry.test.ts b/packages/fetchium/src/__tests__/retry-should-retry.test.ts index 9ded5a9..f0fb2d5 100644 --- a/packages/fetchium/src/__tests__/retry-should-retry.test.ts +++ b/packages/fetchium/src/__tests__/retry-should-retry.test.ts @@ -262,8 +262,7 @@ describe('shouldRetry', () => { }); it('does not pass an earlier response status for a later network error', async () => { - // Attempt 0 gets a 404 response; the later attempts fail with network - // errors, which must report no status rather than the earlier 404. + // Attempt 0 gets a 404, every later attempt a network error. const statuses: Array = []; client.destroy(); client = createClient((_error, _attempt, status) => { @@ -280,8 +279,6 @@ describe('shouldRetry', () => { }); it('does not pass the status of an earlier fetch for a refetch network error', async () => { - // Fetch 1 succeeds; the refetch fails with a network error, which must - // not inherit a status from the response assigned by fetch 1. const statuses: Array = []; client.destroy(); client = createClient((_error, _attempt, status) => { diff --git a/packages/fetchium/src/react/use-suspense-query.ts b/packages/fetchium/src/react/use-suspense-query.ts index 9ce80d9..26c03a3 100644 --- a/packages/fetchium/src/react/use-suspense-query.ts +++ b/packages/fetchium/src/react/use-suspense-query.ts @@ -20,8 +20,8 @@ import { useQuery } from './use-query.js'; * Wrap the change in `startTransition` to keep showing the previous result. * * Call it from a plain function component. Like `useQuery`, it can't run - * inside a Signalium `component()` (that is a reactive context), where - * `fetchQuery` is the way to read queries. + * inside a Signalium `component()` (a reactive context); use `fetchQuery` + * there. */ export function useSuspenseQuery( QueryClass: new () => T, @@ -35,9 +35,9 @@ export function useSuspenseQuery( throw new Error('QueryClient not found'); } - // Decide before useQuery: a render that suspends is discarded without a - // commit, so a subscription taken by it would never be cleaned up. The - // client holds the query active while suspended instead. + // Decide before useQuery: a suspended render is discarded without a commit, + // so a subscription it took would never be cleaned up. The client holds the + // query active while suspended instead. const { promise, failed, error, key } = client.suspendOnColdMiss( QueryDefinition.for(QueryClass), args[0] as QueryParams | undefined, diff --git a/packages/fetchium/src/retry.ts b/packages/fetchium/src/retry.ts index 37877ed..cbd375d 100644 --- a/packages/fetchium/src/retry.ts +++ b/packages/fetchium/src/retry.ts @@ -6,10 +6,11 @@ import type { ResolvedRetryConfig } from './query.js'; * * @param error What the attempt threw. * @param attempt Index of the failed attempt, starting at 0 (the first request). - * @param status HTTP status of the failed attempt, when known: the status the - * adapter received (`ctx.response.status` for a REST query whose response - * body failed to parse or validate), or else `status`, `statusCode` or - * `response.status` on the error. Undefined for network errors. + * @param status HTTP status of the failed attempt, when known: `status`, + * `statusCode` or `response.status` on the error, or else the status of the + * response the adapter received (`ctx.response.status`, for a REST error + * response whose body failed to parse or validate). Undefined for network + * errors. */ export type ShouldRetry = (error: unknown, attempt: number, status: number | undefined) => boolean; From aa417f33fcf5ca3ed737fab75f4c18a3faff5ad2 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Tue, 6 Oct 2026 23:57:02 -0400 Subject: [PATCH 13/18] fix(fetchium): release a retain() lease when starting one of its queries throws Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryClient.ts | 7 ++++++- .../src/__tests__/retain-prefetch.test.ts | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index bb49e9e..9e3c127 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -505,7 +505,12 @@ export class QueryClient { // Outside the watcher's computation now, so adapter code may run. if (starts !== undefined) { - for (const instance of starts) instance.startPendingNow(); + try { + for (const instance of starts) instance.startPendingNow(); + } catch (error) { + release(); + throw error; + } } const ttl = options?.ttl; diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts index 8d19757..0118709 100644 --- a/packages/fetchium/src/__tests__/retain-prefetch.test.ts +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -6,6 +6,7 @@ import { QueryClient, QueryClientContext, DEFAULT_PREFETCH_TTL } from '../QueryC import { RESTQuery, RESTQueryAdapter } from '../rest/index.js'; import { t } from '../typeDefs.js'; import { fetchQuery, QueryDefinition } from '../query.js'; +import { QueryInstance } from '../QueryResult.js'; import type { MutationEvent } from '../types.js'; import { GcManager, type GcKeyType } from '../GcManager.js'; import { NetworkManager } from '../NetworkManager.js'; @@ -577,4 +578,23 @@ describe('client.retain()', () => { }), ).toThrow('boom'); }); + + it('releases and rethrows when starting a query throws', async () => { + const { client } = setup(); + const start = QueryInstance.prototype.startPendingNow; + let calls = 0; + const spy = vi.spyOn(QueryInstance.prototype, 'startPendingNow').mockImplementation(function ( + this: QueryInstance, + ) { + if (++calls === 2) throw new Error('boom'); + start.call(this); + }); + + expect(() => client.retain(() => [fetchQuery(GetStreamed), fetchQuery(GetItem)])).toThrow('boom'); + spy.mockRestore(); + + await sleep(10); + expect(subscribed).toBe(1); + expect(unsubscribed).toBe(1); + }); }); From 10958e9d250be75384024db9b936761c2a6f34a7 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Wed, 7 Oct 2026 00:16:59 -0400 Subject: [PATCH 14/18] chore(fetchium): cut and shorten comments Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/GcManager.ts | 1 - packages/fetchium/src/LiveCollection.ts | 3 +- packages/fetchium/src/QueryClient.ts | 163 +++++------------- packages/fetchium/src/QueryResult.ts | 73 ++------ .../__tests__/identical-redelivery.test.ts | 33 +--- .../src/__tests__/read-path-latency.test.ts | 25 +-- .../src/__tests__/retain-prefetch.test.ts | 16 +- .../src/__tests__/retry-should-retry.test.ts | 2 - .../identical-redelivery-react.test.tsx | 10 +- .../__tests__/prefetch-suspense.test.tsx | 10 +- .../fetchium/src/react/use-suspense-query.ts | 22 +-- packages/fetchium/src/retry.ts | 24 +-- packages/fetchium/src/types.ts | 6 +- 13 files changed, 89 insertions(+), 299 deletions(-) diff --git a/packages/fetchium/src/GcManager.ts b/packages/fetchium/src/GcManager.ts index ba786b6..30196d2 100644 --- a/packages/fetchium/src/GcManager.ts +++ b/packages/fetchium/src/GcManager.ts @@ -62,7 +62,6 @@ export class GcManager { private _nextTickScheduled = false; private _onEvict: (key: number, type: GcKeyType) => void; private _multiplier: number; - /** Set by destroy(). Queries that deactivate afterwards schedule nothing. */ private _destroyed = false; constructor(onEvict: (key: number, type: GcKeyType) => void, multiplier: number = 1) { diff --git a/packages/fetchium/src/LiveCollection.ts b/packages/fetchium/src/LiveCollection.ts index 3634ad2..2dba6ea 100644 --- a/packages/fetchium/src/LiveCollection.ts +++ b/packages/fetchium/src/LiveCollection.ts @@ -450,8 +450,7 @@ export class LiveValueInstance { this._value = this._onDelete(this._value, entity ?? entityData); break; } - // Notify even when the reducer returns the same reference: it may have - // mutated the value in place. + // Notify even for the same reference: the reducer may mutate in place. this._notifier.notify(); } diff --git a/packages/fetchium/src/QueryClient.ts b/packages/fetchium/src/QueryClient.ts index 9e3c127..f1dae3c 100644 --- a/packages/fetchium/src/QueryClient.ts +++ b/packages/fetchium/src/QueryClient.ts @@ -63,11 +63,9 @@ export interface QueryClientConfig { */ reactivationGraceMs?: number; /** - * Decides whether a failed query attempt (or a mutation attempt, when the - * mutation enables retries) is retried. Receives the error, the attempt index - * (starting at 0) and the attempt's HTTP status when known. A query's or - * mutation's own `retry.shouldRetry` overrides it. Without it, every failed - * attempt is retried. Use it to stop retrying permanent errors such as a 4xx. + * Decides whether a failed query or mutation attempt is retried, for example + * to stop on a 4xx. A query's or mutation's own `retry.shouldRetry` overrides + * it. Default: every failed attempt is retried. */ shouldRetry?: ShouldRetry; /** @@ -94,45 +92,29 @@ export interface QueryClientConfig { } export interface RetainOptions { - /** - * Milliseconds after which the lease releases itself. Omit to hold it until - * the returned `release` is called. - */ + /** Milliseconds until the lease releases itself. Omit to hold until `release` is called. */ ttl?: number; } export interface PrefetchOptions { - /** - * Milliseconds to keep the query active if nothing else reads it. Default: - * `DEFAULT_PREFETCH_TTL` (10 s). - */ + /** Milliseconds to keep the query active. Default: `DEFAULT_PREFETCH_TTL` (10 s). */ ttl?: number; } -/** How long `prefetch()` keeps a query active when no `ttl` is given. */ export const DEFAULT_PREFETCH_TTL = 10_000; -/** - * How long a suspense hold outlives its fetch settling when no reader commits - * to claim it (the suspended tree was abandoned). - */ +/** How long a hold outlives its fetch when no reader commits (abandoned tree). */ const SUSPENSE_HOLD_TTL = 10_000; -/** - * How long a failed cold fetch's error waits for a render to claim it. React - * retries a suspended render within a task or two of its promise settling; an - * error nobody claimed by then belongs to an abandoned tree, and a later mount - * makes a fresh attempt instead of inheriting it. - */ +/** React retries a suspended render within a task or two. An error unclaimed by then is dropped. */ const UNCLAIMED_FAILURE_TTL = 50; interface SuspenseHold { release: () => void; - /** Resolves (never rejects) once the cold fetch settles. */ + /** Never rejects. */ settled: Promise | undefined; - /** The fetch this hold suspended on has settled. */ done: boolean; - /** That fetch failed and its error has been handed to a render. */ + /** The error has been handed to a render. */ failed: boolean; timer: ReturnType | undefined; } @@ -177,21 +159,13 @@ export class QueryClient { private staggerQueue = new Set>(); private staggerTimer: ReturnType | undefined = undefined; - /** Release functions of outstanding `retain()` / `prefetch()` leases. */ private leases = new Set<() => void>(); - /** - * Set during a lease's first run: the queries it reached whose first fetch - * (or zero-delay refetch) is queued on a microtask. The lease starts them - * before returning. See `retain()`. - */ + /** During a lease's first run: queries with a start queued on a microtask, for the lease to run now. */ private leaseStarts: Set> | undefined = undefined; - /** - * Leases taken by `retain()` inside a reactive computation, keyed by the - * computation, with the run that took them. See `retain()`. - */ + /** Leases `retain()` took inside a reactive computation, by computation and run. */ private reactiveLeases = new WeakMap void> }>(); private warnedReactiveRetain = false; - /** Leases taken by `useSuspenseQuery` for cold misses, by query instance key. */ + /** `useSuspenseQuery` cold-miss leases, by query key. */ private suspenseHolds = new Map(); private context!: QueryContext; @@ -399,8 +373,7 @@ export class QueryClient { this.queryInstances.set(queryKey, queryInstance as QueryInstance); } - // A reader may have activated it earlier in this task with its start still - // queued. The lease starts it too. + // An earlier reader may have activated it with its start still queued. this.leaseStarts?.add(queryInstance); return queryInstance.relay; @@ -411,38 +384,15 @@ export class QueryClient { // ====================================================== /** - * Keeps the queries `fn` reads active (fetched, subscribed, and exempt from - * GC) until the returned `release` is called, or for `ttl` milliseconds, - * whichever comes first. Use it to keep a hidden surface's queries warm, or - * to start the queries a likely next screen needs. - * - * `fn` runs immediately with this client as the `QueryClientContext`, and - * again whenever what it reads changes (for example a Signal param). A query - * is held when `fn` reads one of its fields (`isReady`, `value`, ...), or - * when `fn` returns its promise or an array of them: + * Keeps the queries `fn` reads or returns active (fetched, subscribed, exempt + * from GC) until `release` is called or `ttl` elapses. `fn` reruns when what + * it reads changes. First-run fetches start before `retain` returns. Call it + * from an event handler or effect, not a reactive computation. * * ```ts * const release = client.retain(() => [fetchQuery(GetTokens), fetchQuery(GetPrices, { ids })]); - * // later, when the surface is gone for good: * release(); * ``` - * - * A reader that mounts while the lease is held joins the already active - * query: no new request, and the data (if it has arrived) on its first - * render. Releasing never interrupts other readers; when the last one goes, - * the query deactivates and its `gcTime` starts as usual. `release` is - * idempotent. - * - * The first run's fetches start before `retain` returns, ahead of anything - * already on the microtask queue (such as a render React scheduled for the - * same tap). A query that depends on another's result starts once that - * result arrives, as with any reader. - * - * Call `retain` from an event handler or effect, not a reactive computation: - * starting a fetch runs adapter code, and a rerunning computation would take - * a new lease each run. Called from one anyway, it warns in development, - * starts its fetches on their usual microtask, and releases the leases the - * computation's previous run took. */ retain(fn: () => unknown, options?: RetainOptions): () => void { const owner = currentReactiveOwner(); @@ -467,11 +417,7 @@ export class QueryClient { return release; } - /** - * `retain()`, with `startNow` choosing whether the first run's fetches start - * before returning or on their usual microtask. `useSuspenseQuery` leases - * from inside a render, where adapter code must not run, so it passes false. - */ + /** `startNow: false` leaves first-run fetches on their microtask, for callers inside a render. */ private lease(fn: () => unknown, options: RetainOptions | undefined, startNow: boolean): () => void { const w = withContexts([[QueryClientContext, this]], () => watcher(() => holdReturned(fn()))); const unsubscribe = w.addListener(noop); @@ -493,8 +439,7 @@ export class QueryClient { const starts = startNow ? new Set>() : undefined; this.leaseStarts = starts; try { - // Run now, so the queries activate (and a synchronous store hydrates - // them) before this returns, not on Signalium's next flush. + // Activate now, not on Signalium's next flush. void w.value; } catch (error) { release(); @@ -522,18 +467,10 @@ export class QueryClient { } /** - * Starts `QueryClass` with `params` now and keeps it active for `ttl` - * milliseconds (default {@link DEFAULT_PREFETCH_TTL}), or until the returned - * `release` is called. Meant for the tap that commits to a navigation: the - * destination's reader, mounting within the window, reuses the in-flight or - * finished fetch and renders the data on its first render if it has arrived. - * - * The `ttl` is an upper bound. The lease ends even if the fetch is still in - * flight (offline, or a topic never fulfilled), which aborts it unless a - * reader has joined. - * - * With a synchronous store a fresh cached result is applied without a - * request. A stale one is shown and refetched, as on any activation. + * Starts `QueryClass` now and keeps it active for `ttl` ms (default + * {@link DEFAULT_PREFETCH_TTL}) or until `release` is called, so a reader + * mounting within that window reuses the fetch. The lease ends at `ttl` even + * if the fetch is in flight, which aborts it unless a reader has joined. */ prefetch( QueryClass: new () => T, @@ -547,19 +484,8 @@ export class QueryClient { } /** - * For `useSuspenseQuery`. Returns a promise to suspend on when the query has - * never produced a value (a cold miss), or `undefined` when it has one to - * render (in memory, or hydrated now from a synchronous store). - * - * A cold miss takes a hold that keeps the query active while suspended, - * since React discards a suspended render without subscribing. The reader's - * commit releases it (`releaseSuspenseHold`), or it releases itself - * `SUSPENSE_HOLD_TTL` after the fetch settles. A query whose last fetch - * failed is refetched once per hold. If that attempt fails too, `error` is - * returned for the caller to throw and the hold is dropped. A failure no - * render claims within `UNCLAIMED_FAILURE_TTL` also drops the hold, so a - * later mount makes a new attempt. - * + * On a cold miss, returns a promise to suspend on and holds the query active + * until the reader commits. A failed cold fetch returns `error` instead. * @internal */ suspendOnColdMiss( @@ -578,15 +504,14 @@ export class QueryClient { } if (relay.isReady) { - // Hydrated by the activation: render it, and keep the hold until the reader commits. + // Hydrated by the activation. Keep the hold until the reader commits. this.expireSuspenseHold(key, hold); return { promise: undefined, key }; } if (relay.isPending && hold.done) { - // The fetch this hold waited for settled, and another has started since - // (a refetch from elsewhere). Suspend on that one: the settled promise - // has already resolved, and throwing it again would re-render at once. + // Another fetch started after ours settled. Rethrowing the resolved + // promise would re-render at once. clearTimeout(hold.timer); hold.timer = undefined; hold.done = false; @@ -596,10 +521,8 @@ export class QueryClient { if (!relay.isPending) { if (hold.done) { - // The fetch this hold waited for failed. Keep the hold until the next - // task: React re-renders a throwing component once more before giving - // up, and that render must throw the same error, not refetch. A later - // mount (an error boundary reset) gets a fresh hold and a new attempt. + // Keep the hold one task: React re-renders a throwing component once + // more, and that render must throw the same error, not refetch. const error = relay.error; if (!hold.failed) { hold.failed = true; @@ -610,7 +533,7 @@ export class QueryClient { } return { promise: undefined, failed: true, error, key }; } - // A failed or not-yet-started fetch: start it now rather than after the activation's refetch hop. + // Start now, not after the activation's refetch hop. this.queryInstances.get(key)?.refetch(); } @@ -818,9 +741,7 @@ export class QueryClient { // ====================================================== /** - * Called by a query that queued its start (or a zero-delay refetch) on a - * microtask, so a lease in its first run can start it before returning. - * + * A query queued a start on a microtask. A lease in its first run starts it now. * @internal */ noteDeferredStart(instance: QueryInstance): void { @@ -828,10 +749,10 @@ export class QueryClient { } /** - * Queues a reactivation refetch. Everything queued in the same task is - * started from one flush, spread evenly across `reactivationStaggerMs` in - * the order the queries reactivated, so the first one starts right away. - * Queries whose adapter `coalescesRequests` all start right away instead. + * Queues a reactivation refetch. One flush per task spreads the queue evenly + * across `reactivationStaggerMs` in reactivation order, the first starting + * immediately. Queries of an adapter that `coalescesRequests` all start + * immediately. */ scheduleReactivationRefetch(instance: QueryInstance): void { this.staggerQueue.add(instance); @@ -969,10 +890,8 @@ export const QueryClientContext: Context = context {}; /** - * The reactive computation (and its run) currently executing, if any. Signalium - * has no public API for this: a signal read records its consumer, so read a - * throwaway signal and look at what it recorded. Undefined when the shape - * isn't the expected one. + * The running reactive computation and its run, if any. Signalium has no public + * API for this, so read a throwaway signal and inspect the consumer it recorded. */ function currentReactiveOwner(): { ref: object; run: number } | undefined { try { @@ -987,11 +906,7 @@ function currentReactiveOwner(): { ref: object; run: number } | undefined { } } -/** - * Reads the query promises `retain`'s callback returned, so the lease's - * watcher depends on them and keeps them active. `isReady` changes once per - * query, so the watcher reruns at most once for each. - */ +/** Reads `isReady` on returned query promises so the lease's watcher keeps them active. */ function holdReturned(returned: unknown): void { if (Array.isArray(returned)) { for (const item of returned) holdOne(item); diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 4cd9471..14c4d5e 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -54,16 +54,10 @@ export class QueryInstance { private reconnectsAtDeactivate: number = 0; private currentParams: QueryParams | undefined = undefined; private debounceTimer: ReturnType | undefined = undefined; - /** - * Bumped to schedule or cancel a zero-delay refetch, which runs on a - * microtask and so can't be cleared like a timer. - */ + /** Bumped to cancel a zero-delay refetch, which runs on a microtask that can't be cleared. */ private debounceGeneration: number = 0; - /** The zero-delay refetch runDebounced() queued, until it runs or is cancelled. */ private pendingDebouncedRun: (() => void) | undefined = undefined; - /** initialize() queued startSubscriptionAndFetch() on a microtask that hasn't run yet. */ private startPending: boolean = false; - /** The fetch restartAbortedFetch() queued on a microtask, until it runs. */ private pendingRestart: (() => void) | undefined = undefined; /** The last fetch ended in an error (not an abort). Disables the reactivation grace. */ private lastFetchFailed: boolean = false; @@ -208,11 +202,8 @@ export class QueryInstance { this.reconcileSubscription(); } - // If the relay shows pending but the abort controller is gone, the - // previous fetch was aborted during deactivation (or never started). - // runDebounced() would bail out on the doomed promise's isPending, - // and that promise may never settle (a topic query's send() waits on - // a subscription that is gone). + // Pending with no abort controller: the fetch was aborted or never started, and its + // promise may never settle. runDebounced() would bail on that isPending. if (this.relayState.isPending && this._abortController === undefined) { this.restartAbortedFetch(); } else { @@ -299,19 +290,11 @@ export class QueryInstance { * Runs once, from the relay's first activation, inside the read that watched * the relay (for React, during render). * - * When the store answers synchronously (SyncQueryStore), the cached value is - * applied right here, so the activating read, and the first render, already - * see it. Subscribing and fetching wait for the next microtask: both call - * into adapter and app code that may read or write signals, which must not - * run while the relay's computation is the current consumer. A microtask - * rather than a timer keeps the first fetch off the macrotask queue. - * - * With an asynchronous store (AsyncQueryStore), the cache resolves on a later - * tick, outside the activating read, and everything runs from there. - * - * A lease (`retain()` / `prefetch()`) skips the microtask: it calls - * startPendingNow() after its activating read, so the request goes out ahead - * of any render already queued. + * A synchronous store's cached value is applied here, so the activating read + * already sees it. Subscribing and fetching wait a microtask, not a timer: + * they call adapter and app code that must not touch signals while the + * relay's computation is the current consumer. With an async store, the + * cache resolves on a later tick and everything runs from there. */ private initialize(): void { this.initialized = true; @@ -351,11 +334,7 @@ export class QueryInstance { }; /** - * Runs now any start that initialize(), a zero-delay runDebounced() or - * restartAbortedFetch() queued on a microtask, which then does nothing. - * Called by a lease after its activating read, outside any reactive - * computation. - * + * Runs any microtask-queued start now. Called by a lease outside its reactive read. * @internal */ startPendingNow(): void { @@ -474,8 +453,7 @@ export class QueryInstance { } const ctx = this.getOrCreateExecutionContext(); - // Restore a subscription that a deactivation or pause tore down: a topic - // query's send() waits for data its subscription brings. + // A topic query's send() waits on its subscription, which deactivation may have torn down. this.reconcileSubscription(); const adapter = this.queryClient.getAdapter(def.statics.adapterClass); const signal = this._abortController?.signal ?? new AbortController().signal; @@ -513,10 +491,8 @@ export class QueryInstance { } /** - * Replaces a fetch that a deactivation aborted, from inside the activating - * read. The relay takes the new promise now so the doomed one can't settle - * it. The subscription and request run adapter code, so they start outside - * the read, on a microtask or when a lease starts them. + * Sets the new promise now so the aborted one can't settle the relay. The + * request runs adapter code, so it starts outside the read, on a microtask. */ private restartAbortedFetch(): void { this.fetchStarts++; @@ -558,18 +534,9 @@ export class QueryInstance { } /** - * Starts a refetch after the query's `debounce` (plus `extraDelay`). Calls - * made before it starts are coalesced into one fetch. - * - * With no delay the fetch starts on a microtask, not a timer. It still runs - * outside the reactive computation that asked for it and still coalesces - * calls from the same task, without waiting a macrotask (on React Native a - * zero timer can wait up to a frame). - * - * `nextTask` keeps the zero-delay fetch on a timer. Invalidation uses it: a - * query invalidated in the task its last watcher left stays active until - * Signalium's deactivation flush, and the timer lets that flush cancel the - * fetch instead of starting and aborting it. + * With no delay the fetch starts on a microtask (a zero timer can wait a frame on + * React Native). `nextTask` forces a timer so a deactivation flush in the same + * task can cancel the fetch instead of starting and aborting it. */ private runDebounced(extraDelay: number = 0, nextTask: boolean = false): void { if (this.relayState.isPending) return; @@ -591,7 +558,7 @@ export class QueryInstance { if (generation !== this.debounceGeneration) return; this.debounceGeneration++; this.pendingDebouncedRun = undefined; - // Another path started a fetch in the meantime (refetch(), activation). + // Another path started a fetch in the meantime. if (this.relayState.isPending && this._abortController !== undefined) return; this.runQueryImmediately(); }; @@ -600,7 +567,6 @@ export class QueryInstance { queueMicrotask(run); } - /** Cancels a refetch scheduled by runDebounced(). */ private cancelDebounced(): void { this.debounceGeneration++; this.pendingDebouncedRun = undefined; @@ -622,7 +588,6 @@ export class QueryInstance { const totalDelay = (this.config?.debounce ?? 0) + delay; if (totalDelay === 0) { - // Within the flush task, on runDebounced()'s microtask. this.runDebounced(); return; } @@ -734,10 +699,8 @@ export class QueryInstance { } /** - * Retry options that report a failed attempt's HTTP status. RESTQueryAdapter - * sets `ctx.response` before parsing and validating the body, so an error - * response with an invalid body throws a schema error that carries no - * status. Only a response assigned during the failed attempt counts. + * Reports the status of `ctx.response` when the failed attempt set it, for a + * REST error response whose body failed validation and so carries no status. */ private attemptStatusTracker(ctx: Query): { start: () => void; options: WithRetryOptions } { const holder = ctx as unknown as { response?: unknown }; diff --git a/packages/fetchium/src/__tests__/identical-redelivery.test.ts b/packages/fetchium/src/__tests__/identical-redelivery.test.ts index 6349ef2..5cb4b7a 100644 --- a/packages/fetchium/src/__tests__/identical-redelivery.test.ts +++ b/packages/fetchium/src/__tests__/identical-redelivery.test.ts @@ -13,14 +13,9 @@ import { TopicQueryAdapter } from '../topic/TopicQueryAdapter.js'; import type { MutationEvent, QueryPromise } from '../types.js'; import { testWithClient, setupTestClient } from './utils.js'; -/** - * A stream reconnect re-delivers every on-screen entity with the data the - * store already holds, as a query result, mutation event or topic event. None - * may notify an entity, bump its version, write it, or notify a live - * collection. A real change notifies exactly once. - */ - -/** Records `typename:id` for every entity notify. */ +// Re-delivering data the store already holds must not notify, bump versions, +// write, or notify live collections. A real change notifies exactly once. + function recordNotifies() { const notified: string[] = []; const original = EntityInstance.prototype.notify; @@ -96,7 +91,6 @@ function versions(client: QueryClient, keys: number[]) { return keys.map(k => client.entityMap.getEntity(k)!.version); } -/** Spies on the notifiers of a wallet's live array and live value. */ function liveNotifies(client: QueryClient) { const data = client.entityMap.getEntity(walletKey)!.data; const tokens = data.tokens as LiveCollectionBinding; @@ -107,10 +101,6 @@ function liveNotifies(client: QueryClient) { }; } -// ====================================================== -// Query results -// ====================================================== - class GetWallet extends RESTQuery { path = '/wallet'; result = { wallet: t.entity(Wallet) }; @@ -187,10 +177,6 @@ describe('Identical re-delivery', () => { }); }); - // ====================================================== - // Mutation events - // ====================================================== - describe('(b) mutation event', () => { it('is a no-op for a full update carrying identical data, nested values included', async () => { const { client, store } = await loadWallet(2); @@ -209,12 +195,10 @@ describe('Identical re-delivery', () => { expect(versions(client, keys)).toEqual(before); expect(saveEntity).not.toHaveBeenCalled(); expect(live.tokens).not.toHaveBeenCalled(); - // The live value's reducer still runs and notifies, since it may mutate - // in place. + // The live value's reducer may mutate in place, so it still notifies. expect(live.count).toHaveBeenCalledTimes(1); - // A `create` for an entity the store already holds is the same no-op for - // the entity and the live array. The live value's `onCreate` still runs. + // A `create` for an entity the store already holds is the same no-op. client.applyMutationEvent({ type: 'create', typename: 'Token', data: token(0) }); client.applyMutationEvent({ type: 'create', typename: 'Owner', data: { id: 'o-1', name: 'Ann' } }); @@ -222,7 +206,6 @@ describe('Identical re-delivery', () => { expect(versions(client, keys)).toEqual(before); expect(saveEntity).not.toHaveBeenCalled(); expect(live.tokens).not.toHaveBeenCalled(); - // The unchanged nested object keeps its identity. expect(client.entityMap.getEntity(tokenKey(0))!.data.metadata).toBe(metadata); }); @@ -259,10 +242,6 @@ describe('Identical re-delivery', () => { }); }); -// ====================================================== -// Topic events -// ====================================================== - class MockTopicAdapter extends TopicQueryAdapter { snapshots = new Map(); @@ -326,7 +305,7 @@ describe('(c) topic events', () => { expect(versions(client, keys)).toEqual(before); expect(saveEntity).not.toHaveBeenCalled(); expect(live.tokens).not.toHaveBeenCalled(); - // Each Token event runs the live value's reducer, which notifies (see (b)). + // The live value's reducer notifies once per Token event. expect(live.count).toHaveBeenCalledTimes(3); }); diff --git a/packages/fetchium/src/__tests__/read-path-latency.test.ts b/packages/fetchium/src/__tests__/read-path-latency.test.ts index e6f92aa..f92713a 100644 --- a/packages/fetchium/src/__tests__/read-path-latency.test.ts +++ b/packages/fetchium/src/__tests__/read-path-latency.test.ts @@ -8,15 +8,8 @@ import { t } from '../typeDefs.js'; import { fetchQuery } from '../query.js'; import { createMockFetch, sleep } from './utils.js'; -/** - * Read-path latency, counted in macrotask turns. Each turn drains the - * microtask queue, checks the condition, then yields one `setTimeout(0)`. - * Turn 0 means the condition held before any timer ran. - * - * On React Native a zero timer goes through the native timing module and can - * wait up to a frame, so a turn saved here is up to ~16 ms on device. The - * logged `ms` values are Node timers (about 1 ms per turn). - */ +// Latency in macrotask turns: each turn drains microtasks, checks, then yields one setTimeout(0). +// Turn 0 means no timer ran. On React Native a zero timer can wait a frame. async function flushMicrotasks(count = 30): Promise { for (let i = 0; i < count; i++) await Promise.resolve(); @@ -70,7 +63,6 @@ function setup() { return { client, mockFetch }; } -/** Watches `read` and runs it synchronously, so activation happens now. */ function watchNow(client: QueryClient, read: () => T): { w: { value: T }; unsub: () => void } { const w = withContexts([[QueryClientContext, client]], () => watcher(read)); const unsub = w.addListener(() => {}); @@ -103,7 +95,6 @@ describe('read-path latency (macrotask turns)', () => { expect(mockFetch.calls).toHaveLength(1); const { w, unsub } = watchNow(client, () => fetchQuery(GetFreshItem).value); - // Same synchronous read that activated the query: no microtask, no timer. expect(w.value).toMatchObject({ n: 1 }); await sleep(10); expect(mockFetch.calls).toHaveLength(1); @@ -117,7 +108,6 @@ describe('read-path latency (macrotask turns)', () => { const started = performance.now(); const { w, unsub } = watchNow(client, () => fetchQuery(GetItem).value); - // The cached value shows immediately while the refetch runs. expect(w.value).toMatchObject({ n: 1 }); const turns = await turnsUntil(() => mockFetch.calls.length === 2); console.log( @@ -145,7 +135,7 @@ describe('read-path latency (macrotask turns)', () => { console.log( `[latency] staggered reactivation -> first refetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`, ); - // One timer for the stagger flush, which groups the task's reactivations. + // One timer for the stagger flush. expect(turns).toBe(1); unsub(); }); @@ -164,8 +154,7 @@ describe('read-path latency (macrotask turns)', () => { console.log( `[latency] param change -> fetch start: ${turns} turns, ${(performance.now() - started).toFixed(2)} ms`, ); - // The relay update reruns in Signalium's flush (one timer), and the fetch - // starts on a microtask of that flush. + // One timer for Signalium's flush. The fetch starts on its microtask. expect(turns).toBe(1); expect(mockFetch.calls[1].url).toContain('/users/2'); unsub(); @@ -195,11 +184,9 @@ describe('read-path latency (macrotask turns)', () => { mockFetch.get('/item', { n: 2 }, { delay: 30 }); const { unsub } = watchNow(client, () => fetchQuery(GetItem).value); unsub(); - // Before the response (30 ms). await sleep(10); - // Signalium deactivates on its next flush (a timer), after the refetch's - // microtask has started it. This wasted request is the trade-off for not - // waiting a timer on every reactivation. + // Deactivation lands a timer after the refetch's microtask started it. + // Accepted to avoid a timer on every reactivation. expect(mockFetch.calls).toHaveLength(1); expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); await sleep(30); diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts index 0118709..70572b0 100644 --- a/packages/fetchium/src/__tests__/retain-prefetch.test.ts +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -12,12 +12,6 @@ import { GcManager, type GcKeyType } from '../GcManager.js'; import { NetworkManager } from '../NetworkManager.js'; import { createMockFetch, sleep } from './utils.js'; -/** - * client.retain() and client.prefetch(): app-level leases that keep queries - * active without a reader, so a reader that mounts inside the lease joins the - * running query instead of starting its own. - */ - async function flushMicrotasks(count = 30): Promise { for (let i = 0; i < count; i++) await Promise.resolve(); } @@ -102,7 +96,6 @@ function setup( return { client, mockFetch, kv }; } -/** Mounts a reader: watches `read` and runs it now, like a React render. */ function mountReader(client: QueryClient, read: () => T): { w: { value: T }; unsub: () => void } { const w = withContexts([[QueryClientContext, client]], () => watcher(read)); const unsub = w.addListener(() => {}); @@ -201,7 +194,7 @@ describe('client.prefetch()', () => { await sleep(10); expect(mockFetch.calls).toHaveLength(1); - // staleTime 0, yet the query is still active, so mounting is not a reactivation. + // Still active, so mounting is not a reactivation. const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); expect(w.value).toMatchObject({ n: 1 }); await sleep(10); @@ -248,7 +241,6 @@ describe('client.prefetch()', () => { const { client, mockFetch } = setup(kv); client.prefetch(GetFreshUser, { id: 1 }); - // Hydrated by the activation inside prefetch(), before it returned. expect(client.getQuery(QueryDefinition.for(GetFreshUser), { id: 1 }).value).toMatchObject({ n: 1 }); expect(mockFetch.calls).toHaveLength(0); await sleep(10); @@ -261,7 +253,6 @@ describe('client.prefetch()', () => { await sleep(60); expect(mockFetch.calls).toHaveLength(1); - // staleTime 0 and no grace: reactivating refetches. const { w, unsub } = mountReader(client, () => fetchQuery(GetItem).value); expect(w.value).toMatchObject({ n: 1 }); await sleep(10); @@ -275,7 +266,6 @@ describe('client.prefetch()', () => { mockFetch.get('/streamed', { n: 9 }, { delay: 40 }); client.prefetch(GetStreamed, undefined, { ttl: 10 }); await sleep(25); - // The ttl is an upper bound: past it, before the response, the lease is gone. expect(unsubscribed).toBe(1); expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); await sleep(40); @@ -361,7 +351,6 @@ describe('client.prefetch()', () => { const release = client.prefetch(GetItem); await sleep(5); release(); - // Still before the response (30 ms): the request has been cancelled. await sleep(10); expect(mockFetch.calls).toHaveLength(1); expect(mockFetch.calls[0].options.signal?.aborted).toBe(true); @@ -441,7 +430,6 @@ describe('client.retain()', () => { await sleep(10); expect(subscribed).toBe(1); - // A reader coming and going doesn't restart it. const reader = mountReader(client, () => fetchQuery(GetStreamed).value); reader.unsub(); await sleep(10); @@ -509,7 +497,7 @@ describe('client.retain()', () => { try { const { client } = setup(); - // A real GcManager: the tests' window-less environment gets the no-op one. + // A window-less environment gets the no-op GcManager. (client as unknown as { gcManager: GcManager }).gcManager = new GcManager( (client as unknown as { handleEviction: (key: number, type: GcKeyType) => void }).handleEviction, 1, diff --git a/packages/fetchium/src/__tests__/retry-should-retry.test.ts b/packages/fetchium/src/__tests__/retry-should-retry.test.ts index f0fb2d5..35bdb3d 100644 --- a/packages/fetchium/src/__tests__/retry-should-retry.test.ts +++ b/packages/fetchium/src/__tests__/retry-should-retry.test.ts @@ -12,7 +12,6 @@ import { t } from '../typeDefs.js'; const user = { id: '1', name: 'Alice' }; -/** An error shaped like a fetch wrapper's HTTP error (e.g. axios / request builders). */ function httpError(status: number): Error { return Object.assign(new Error(`HTTP ${status}`), { response: { status } }); } @@ -147,7 +146,6 @@ describe('shouldRetry', () => { }); describe('custom', () => { - /** Opts out of retrying client errors other than 408 and 429. */ const skipClientErrors: ShouldRetry = (_error, _attempt, status) => status === undefined || status < 400 || status >= 500 || status === 408 || status === 429; diff --git a/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx b/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx index 46a4efe..f0ed25b 100644 --- a/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx +++ b/packages/fetchium/src/react/__tests__/identical-redelivery-react.test.tsx @@ -13,12 +13,6 @@ import { TopicQueryAdapter } from '../../topic/TopicQueryAdapter.js'; import type { MutationEvent } from '../../types.js'; import { createRenderCounter } from './utils.js'; -/** - * A stream reconnect re-delivers every on-screen entity with the data it - * already holds. Readers of those entities must not re-render for it; a real - * change re-renders them once. - */ - class MockTopicAdapter extends TopicQueryAdapter { snapshots = new Map(); @@ -125,7 +119,6 @@ describe('React readers on identical re-delivery', () => { const rowRendersBefore = Row.renderCount; const listRendersBefore = ListCounter.renderCount; - // Reconnect: every entity re-delivered with the data the store holds. for (let pass = 0; pass < 3; pass++) { adapter.emit({ type: 'update', typename: 'Wallet', data: { id: 'w-1' } }); adapter.emit({ type: 'update', typename: 'Token', data: token(0) }); @@ -137,8 +130,7 @@ describe('React readers on identical re-delivery', () => { expect(Row.renderCount).toBe(rowRendersBefore); expect(ListCounter.renderCount).toBe(listRendersBefore); - // A real change re-renders the row that reads it, once. (The list re-renders - // too: its constrained live array re-filters on any member change.) + // The list re-renders too: its constrained live array re-filters on any member change. adapter.emit({ type: 'update', typename: 'Token', data: token(1, 99) }); await expect.element(getByTestId('row-tok-1')).toHaveTextContent('99:logo-1.png:Ann'); await sleep(50); diff --git a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx index 09160e9..5680fc1 100644 --- a/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx +++ b/packages/fetchium/src/react/__tests__/prefetch-suspense.test.tsx @@ -82,9 +82,7 @@ describe('client.prefetch() with React', () => { mockFetch.get('/item', { name: 'prefetched' }, { delay: 20 }); const client = makeClient(mockFetch); - // The tap. client.prefetch(GetItem, undefined, { ttl: 5_000 }); - // The navigation animation. await sleep(60); const renders: Array = []; @@ -166,7 +164,6 @@ describe('useSuspenseQuery', () => { mockFetch.get('/item', () => ({ name: `v${++n}` }), { delay: 20 }); const client = makeClient(mockFetch); - // Warm it, then let it go inactive. const release = client.prefetch(GetItem); await sleep(40); release(); @@ -193,7 +190,7 @@ describe('useSuspenseQuery', () => { , ); - // staleTime 0: the reactivation refetches, and the refetch must not suspend. + // The stale reactivation refetches without suspending. await expect.element(getByText('v2')).toBeInTheDocument(); expect(renders[0]).toBe('v1'); expect(fallbacks).toBe(0); @@ -317,8 +314,7 @@ describe('useSuspenseQuery', () => { client.prefetch(GetNoRetry); const relay = withContexts([[QueryClientContext, client]], () => fetchQuery(GetNoRetry)); - // Something else (another reader's retry, a reconnect) refetches as soon - // as the first attempt fails. + // Another refetch starts as soon as the first attempt fails. relay.then( () => {}, () => { @@ -375,7 +371,7 @@ describe('useSuspenseQuery', () => { const screen = render(tree(true)); await expect.element(screen.getByText('Suspended')).toBeInTheDocument(); - // Navigated away while suspended; the fetch fails with no reader. + // Unmount while suspended, so the fetch fails with no reader. screen.rerender(tree(false)); await sleep(150); expect(mockFetch.calls).toHaveLength(1); diff --git a/packages/fetchium/src/react/use-suspense-query.ts b/packages/fetchium/src/react/use-suspense-query.ts index 26c03a3..09b6e56 100644 --- a/packages/fetchium/src/react/use-suspense-query.ts +++ b/packages/fetchium/src/react/use-suspense-query.ts @@ -8,20 +8,10 @@ import { HasRequiredKeys, Optionalize, Signalize } from '../type-utils.js'; import { useQuery } from './use-query.js'; /** - * `useQuery` for components rendered under a React `` boundary. - * - * Suspends only on a cold miss: the query has never produced a value, neither - * in memory nor (with a synchronous store) in the persisted cache. Anything - * else renders immediately, including a stale value whose refetch is in - * flight; refetches never suspend. A cold fetch that fails is thrown to the - * nearest error boundary; resetting the boundary retries it. - * - * Changing params to ones with no value yet is a cold miss, so it suspends. - * Wrap the change in `startTransition` to keep showing the previous result. - * - * Call it from a plain function component. Like `useQuery`, it can't run - * inside a Signalium `component()` (a reactive context); use `fetchQuery` - * there. + * `useQuery` under a `` boundary. Suspends only on a cold miss (no + * value in memory or a sync store), never on refetch. A failed cold fetch is + * thrown to the error boundary. Wrap param changes in `startTransition` to keep + * the previous result. Not usable inside a Signalium `component()`. */ export function useSuspenseQuery( QueryClass: new () => T, @@ -35,9 +25,7 @@ export function useSuspenseQuery( throw new Error('QueryClient not found'); } - // Decide before useQuery: a suspended render is discarded without a commit, - // so a subscription it took would never be cleaned up. The client holds the - // query active while suspended instead. + // Before useQuery: a suspended render never commits, so its subscription would leak. const { promise, failed, error, key } = client.suspendOnColdMiss( QueryDefinition.for(QueryClass), args[0] as QueryParams | undefined, diff --git a/packages/fetchium/src/retry.ts b/packages/fetchium/src/retry.ts index cbd375d..ed8a828 100644 --- a/packages/fetchium/src/retry.ts +++ b/packages/fetchium/src/retry.ts @@ -2,15 +2,9 @@ import type { ResolvedRetryConfig } from './query.js'; /** * Decides whether a failed attempt is retried. Consulted only while retries - * remain. Without one, every failed attempt is retried. - * - * @param error What the attempt threw. - * @param attempt Index of the failed attempt, starting at 0 (the first request). - * @param status HTTP status of the failed attempt, when known: `status`, - * `statusCode` or `response.status` on the error, or else the status of the - * response the adapter received (`ctx.response.status`, for a REST error - * response whose body failed to parse or validate). Undefined for network - * errors. + * remain. `attempt` starts at 0. `status` comes from the error (`status`, + * `statusCode`, `response.status`) or the adapter's `ctx.response`, and is + * undefined for network errors. */ export type ShouldRetry = (error: unknown, attempt: number, status: number | undefined) => boolean; @@ -18,10 +12,7 @@ function asHttpStatus(value: unknown): number | undefined { return typeof value === 'number' && value >= 100 && value <= 599 ? value : undefined; } -/** - * The HTTP status an error carries, read from `status`, `statusCode`, or - * `response.status`. Undefined when none holds an HTTP status code. - */ +/** The HTTP status in an error's `status`, `statusCode` or `response.status`, if any. */ export function getErrorStatus(error: unknown): number | undefined { if (typeof error !== 'object' || error === null) return undefined; const e = error as { status?: unknown; statusCode?: unknown; response?: { status?: unknown } | null }; @@ -36,12 +27,9 @@ export function getFailedResponseStatus(response: unknown): number | undefined { } export interface WithRetryOptions { - /** Used when the retry config has no `shouldRetry`. Without either, every failed attempt is retried. */ + /** Fallback when the retry config has no `shouldRetry`. */ shouldRetry?: ShouldRetry; - /** - * Called after an attempt fails. Returns the HTTP status the attempt - * received, for errors that don't carry one themselves. - */ + /** Status of the failed attempt, for errors that carry none. */ getAttemptStatus?: () => number | undefined; } diff --git a/packages/fetchium/src/types.ts b/packages/fetchium/src/types.ts index 9b873bb..e19fda3 100644 --- a/packages/fetchium/src/types.ts +++ b/packages/fetchium/src/types.ts @@ -68,10 +68,8 @@ export interface RetryConfig { */ retryDelay?: (attemptIndex: number) => number; /** - * Decides whether a failed attempt is retried, from the error, the attempt - * index (starting at 0) and the attempt's HTTP status when known. Overrides - * `QueryClientConfig.shouldRetry`. Without either, every failed attempt is - * retried. + * Decides whether a failed attempt is retried. Overrides + * `QueryClientConfig.shouldRetry`. Default: every failed attempt is retried. */ shouldRetry?: (error: unknown, attempt: number, status: number | undefined) => boolean; } From 8cccf201f0d13a51c3b687254d73e50a22beace7 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Wed, 7 Oct 2026 12:49:02 -0400 Subject: [PATCH 15/18] fix(fetchium): keep a throwing getConfig() from stalling Signalium's scheduler Options resolution now catches a getConfig() throw and returns default options plus the error, so it never throws inside a Signalium flush. The relay rejects with that error instead, and the start path skips fetching while it is set. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryResult.ts | 27 +++++- .../src/__tests__/retain-prefetch.test.ts | 95 +++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 7b366a7..4fff71e 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -73,7 +73,22 @@ export class QueryInstance { private _resolvedOptions: ReadonlySignal<{ config: QueryConfigOptions | undefined; retryConfig: ResolvedRetryConfig; - }> = reactiveSignal(() => this.def.resolveOptions(this._executionCtx!)); + error?: unknown; + }> = reactiveSignal(() => this.resolveOptionsSafely()); + + // A throw here would escape Signalium's flush and stall its scheduler. + private resolveOptionsSafely() { + try { + return this.def.resolveOptions(this._executionCtx!); + } catch (error) { + return { config: undefined, retryConfig: resolveRetryConfig(undefined), error }; + } + } + + private get configError(): unknown { + if (this._executionCtx === undefined) return undefined; + return this._resolvedOptions.value.error; + } get config(): QueryConfigOptions | undefined { if (this._executionCtx === undefined) return undefined; @@ -184,6 +199,12 @@ export class QueryInstance { this.getOrCreateExecutionContext(); + const configError = this.configError; + if (configError !== undefined) { + this.relayState.setError(configError as Error); + return; + } + if (!this.initialized) { this.queryClient.activateQuery(this); this.initialize(); @@ -349,7 +370,7 @@ export class QueryInstance { private startSubscriptionAndFetch(): void { // If deactivated meanwhile, update() fetches on reactivation. - if (!this._isActive || this.isPaused) { + if (!this._isActive || this.isPaused || this.configError !== undefined) { return; } @@ -418,7 +439,7 @@ export class QueryInstance { this.def.statics.adapterClass, ); - this._resolvedOptions = reactiveSignal(() => this.def.resolveOptions(this._executionCtx!)); + this._resolvedOptions = reactiveSignal(() => this.resolveOptionsSafely()); } return this._executionCtx; diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts index 70572b0..9a1a89a 100644 --- a/packages/fetchium/src/__tests__/retain-prefetch.test.ts +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -586,3 +586,98 @@ describe('client.retain()', () => { expect(unsubscribed).toBe(1); }); }); + +describe('a getConfig() that throws after a signal change', () => { + const configBroken = signal(false); + + class GetFragile extends RESTQuery { + path = '/item'; + result = { n: t.number }; + getConfig() { + if (configBroken.value) throw new Error('bad config'); + return { staleTime: 0 }; + } + } + + function trackUnhandled(): { errors: unknown[]; stop: () => void } { + const errors: unknown[] = []; + const onError = (error: unknown) => errors.push(error); + process.on('uncaughtException', onError); + process.on('unhandledRejection', onError); + return { + errors, + stop: () => { + process.off('uncaughtException', onError); + process.off('unhandledRejection', onError); + }, + }; + } + + async function expectSchedulerRunning(): Promise { + const s = signal(0); + const w = watcher(() => s.value); + let fired = 0; + const unsub = w.addListener(() => fired++); + await sleep(10); + const before = fired; + s.value = 1; + await sleep(10); + expect(fired).toBeGreaterThan(before); + unsub(); + } + + afterEach(() => { + configBroken.value = false; + }); + + it('a retained request that breaks a later query getConfig still releases at ttl, and the later query rejects', async () => { + const mockFetch = createMockFetch(); + mockFetch.get('/streamed', { n: 1 }); + mockFetch.get('/item', { n: 2 }); + const fetchBreakingConfig = (url: string, init?: RequestInit) => { + if (url.endsWith('/streamed')) configBroken.value = true; + return mockFetch(url, init); + }; + const client = new QueryClient({ + store: new SyncQueryStore(new MemoryPersistentStore()), + adapters: [new RESTQueryAdapter({ fetch: fetchBreakingConfig as any, baseUrl: 'http://localhost' })], + }); + clients.push(client); + const unhandled = trackUnhandled(); + + let fragile: ReturnType> | undefined; + const release = client.retain(() => [fetchQuery(GetStreamed), (fragile = fetchQuery(GetFragile))], { ttl: 30 }); + expect(release).toBeTypeOf('function'); + expect(subscribed).toBe(1); + + await sleep(10); + expect(fragile!.isRejected).toBe(true); + expect((fragile!.error as Error).message).toBe('bad config'); + + await sleep(40); + expect(unsubscribed).toBe(1); + + await expectSchedulerRunning(); + unhandled.stop(); + expect(unhandled.errors).toEqual([]); + }); + + it('a reader gets the error on its relay, and other watchers keep running', async () => { + const { client } = setup(); + const unhandled = trackUnhandled(); + let fragile: ReturnType> | undefined; + const reader = mountReader(client, () => (fragile = fetchQuery(GetFragile)).value); + await sleep(10); + expect(fragile!.isResolved).toBe(true); + + configBroken.value = true; + await sleep(10); + expect(fragile!.isRejected).toBe(true); + expect((fragile!.error as Error).message).toBe('bad config'); + + await expectSchedulerRunning(); + reader.unsub(); + unhandled.stop(); + expect(unhandled.errors).toEqual([]); + }); +}); From cb300b483ed66dc3c60b1f032e36ebb3b63270aa Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Wed, 7 Oct 2026 12:52:35 -0400 Subject: [PATCH 16/18] fix(fetchium): recover a query once its getConfig() stops throwing A config error now stops the query's running subscription before rejecting. When the error clears, the query refetches and resubscribes on its own. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryResult.ts | 10 ++++ .../src/__tests__/retain-prefetch.test.ts | 53 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 4fff71e..61e3d49 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -60,6 +60,7 @@ export class QueryInstance { private pendingRestart: (() => void) | undefined = undefined; /** The last fetch ended in an error (not an abort). Disables the reactivation grace. */ private lastFetchFailed: boolean = false; + private rejectedByConfig: boolean = false; /** Not set by poll(): it delivers by refetching, which moves `updatedAt`. */ private lastPushAt: number | undefined = undefined; /** Lets a queued reactivation refetch detect that another fetch overtook it. */ @@ -201,13 +202,22 @@ export class QueryInstance { const configError = this.configError; if (configError !== undefined) { + this.stopSubscription(); + this.rejectedByConfig = true; this.relayState.setError(configError as Error); return; } + const recovering = this.rejectedByConfig; + this.rejectedByConfig = false; + if (!this.initialized) { this.queryClient.activateQuery(this); this.initialize(); + } else if (recovering) { + if (wasPaused || activating) this.queryClient.activateQuery(this); + this.reconcileSubscription(); + this.runDebounced(); } else if (wasPaused || activating) { this.queryClient.activateQuery(this); diff --git a/packages/fetchium/src/__tests__/retain-prefetch.test.ts b/packages/fetchium/src/__tests__/retain-prefetch.test.ts index 9a1a89a..be52a9c 100644 --- a/packages/fetchium/src/__tests__/retain-prefetch.test.ts +++ b/packages/fetchium/src/__tests__/retain-prefetch.test.ts @@ -599,6 +599,23 @@ describe('a getConfig() that throws after a signal change', () => { } } + class GetFragileStreamed extends RESTQuery { + path = '/streamed'; + result = { n: t.number }; + getConfig() { + if (configBroken.value) throw new Error('bad config'); + return { + staleTime: 0, + subscribe: () => { + subscribed++; + return () => { + unsubscribed++; + }; + }, + }; + } + } + function trackUnhandled(): { errors: unknown[]; stop: () => void } { const errors: unknown[] = []; const onError = (error: unknown) => errors.push(error); @@ -680,4 +697,40 @@ describe('a getConfig() that throws after a signal change', () => { unhandled.stop(); expect(unhandled.errors).toEqual([]); }); + + it('stops a running subscription when getConfig starts throwing', async () => { + const { client } = setup(); + let fragile: ReturnType> | undefined; + const reader = mountReader(client, () => (fragile = fetchQuery(GetFragileStreamed)).value); + await sleep(10); + expect(subscribed).toBe(1); + + configBroken.value = true; + await sleep(10); + expect(fragile!.isRejected).toBe(true); + expect(unsubscribed).toBe(1); + reader.unsub(); + // Let the reader tear down before afterEach clears the error. + await sleep(10); + }); + + it('recovers when getConfig stops throwing: refetches and resubscribes', async () => { + const { client, mockFetch } = setup(); + let fragile: ReturnType> | undefined; + const reader = mountReader(client, () => (fragile = fetchQuery(GetFragileStreamed)).value); + await sleep(10); + expect(mockFetch.calls).toHaveLength(1); + + configBroken.value = true; + await sleep(10); + expect(fragile!.isRejected).toBe(true); + + configBroken.value = false; + await sleep(10); + expect(fragile!.isResolved).toBe(true); + expect(fragile!.value).toMatchObject({ n: 2 }); + expect(mockFetch.calls).toHaveLength(2); + expect(subscribed).toBe(2); + reader.unsub(); + }); }); From 27ea7d8e7d965f78e16b7e63800aecc922276d66 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Wed, 7 Oct 2026 18:38:45 -0400 Subject: [PATCH 17/18] fix(fetchium): cancel any queued debounced run when starting a fetch immediately Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/QueryResult.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 21de0f6..113b109 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -535,8 +535,7 @@ export class QueryInstance { private runQueryImmediately(): void { this.fetchStarts++; - clearTimeout(this.debounceTimer); - this.debounceTimer = undefined; + this.cancelDebounced(); this._abortController?.abort(); this._abortController = new AbortController(); this._fetchNextAbort?.abort(); From a0f62081cd3548bcd910c70956acf31dbf6f5403 Mon Sep 17 00:00:00 2001 From: Adam Azbel-Bedell Date: Wed, 7 Oct 2026 18:42:06 -0400 Subject: [PATCH 18/18] fix(fetchium): pass a mutation response's status to shouldRetry Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fetchium/src/MutationResult.ts | 13 ++++++++++--- .../src/__tests__/retry-should-retry.test.ts | 18 ++++++++++++++++++ packages/fetchium/src/rest/RESTMutation.ts | 2 ++ packages/fetchium/src/rest/RESTQueryAdapter.ts | 2 ++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/fetchium/src/MutationResult.ts b/packages/fetchium/src/MutationResult.ts index 3410413..78041fa 100644 --- a/packages/fetchium/src/MutationResult.ts +++ b/packages/fetchium/src/MutationResult.ts @@ -2,12 +2,12 @@ import { task, type ReactiveTask } from 'signalium'; import { ComplexTypeDef, MutationEffects, EntityClassOrTypename } from './types.js'; import { ValidatorDef, getEntityDef } from './typeDefs.js'; import { type QueryClient } from './QueryClient.js'; -import { MutationDefinition } from './mutation.js'; +import { MutationDefinition, type Mutation } from './mutation.js'; import { resolveRetryConfig } from './query.js'; import { parseEntities, ParseContext } from './parseEntities.js'; import { createExecutionContext, reifyValue } from './fieldRef.js'; import { Entity } from './proxy.js'; -import { withRetry } from './retry.js'; +import { getFailedResponseStatus, withRetry } from './retry.js'; /** * Internal mutation manager. Consumers interact with the public `task` property, @@ -112,20 +112,27 @@ export class MutationResultImpl { ); } + let attemptCtx: Mutation | undefined; return withRetry( async () => { + attemptCtx = undefined; const abortController = new AbortController(); const ctx = createExecutionContext( this.def.captured, (request ?? {}) as Record, this.queryClient.getContext(), ); + attemptCtx = ctx; return (await adapter.sendMutation!(ctx, abortController.signal)) as Result; }, retryConfig, undefined, - { shouldRetry: this.queryClient.shouldRetry }, + { + shouldRetry: this.queryClient.shouldRetry, + // Each attempt has a fresh ctx, so a response here is this attempt's. + getAttemptStatus: () => getFailedResponseStatus((attemptCtx as { response?: unknown } | undefined)?.response), + }, ); } } diff --git a/packages/fetchium/src/__tests__/retry-should-retry.test.ts b/packages/fetchium/src/__tests__/retry-should-retry.test.ts index 35bdb3d..40e1432 100644 --- a/packages/fetchium/src/__tests__/retry-should-retry.test.ts +++ b/packages/fetchium/src/__tests__/retry-should-retry.test.ts @@ -327,6 +327,24 @@ describe('shouldRetry', () => { expect(mockFetch.calls).toHaveLength(1); }); + + it('passes the status of a mutation 4xx whose body is not JSON', async () => { + const statuses: Array = []; + client.destroy(); + client = createClient((error, attempt, status) => { + statuses.push(status); + return skipClientErrors(error, attempt, status); + }); + mockFetch.post('/users', null, { status: 422, jsonError: new SyntaxError('Unexpected token <') }); + + await testWithClient(client, async () => { + const mut = getMutation(CreateUser); + await expect(mut.run({ name: 'Test' })).rejects.toBeInstanceOf(SyntaxError); + }); + + expect(mockFetch.calls).toHaveLength(1); + expect(statuses).toEqual([422]); + }); }); }); diff --git a/packages/fetchium/src/rest/RESTMutation.ts b/packages/fetchium/src/rest/RESTMutation.ts index a1e2e1a..38d0228 100644 --- a/packages/fetchium/src/rest/RESTMutation.ts +++ b/packages/fetchium/src/rest/RESTMutation.ts @@ -12,6 +12,8 @@ export abstract class RESTMutation extends Mutation { headers?: HeadersInit; requestOptions?: QueryRequestOptions; + declare response: Response | undefined; + getIdentityKey(): string { return `${this.method ?? 'POST'}:${this.path ?? ''}`; } diff --git a/packages/fetchium/src/rest/RESTQueryAdapter.ts b/packages/fetchium/src/rest/RESTQueryAdapter.ts index c41901c..bba1acd 100644 --- a/packages/fetchium/src/rest/RESTQueryAdapter.ts +++ b/packages/fetchium/src/rest/RESTQueryAdapter.ts @@ -216,6 +216,8 @@ export class RESTQueryAdapter extends QueryAdapter { ...fetchOptions, }); + restCtx.response = fetchResponse as unknown as Response; + return fetchResponse.json(); } }