feat(solid-query): type useQuery/useInfiniteQuery data as non-nullable - #11292
Closed
brenelz wants to merge 6 commits into
Closed
feat(solid-query): type useQuery/useInfiniteQuery data as non-nullable#11292brenelz wants to merge 6 commits into
brenelz wants to merge 6 commits into
Conversation
Reading data on a useQuery/useInfiniteQuery result is backed by an async resource that suspends the component into the nearest Loading boundary while the query loads, so by the time data is read during render it has settled. Reflect that in the types: data is TData, never undefined. - Add a distributive NonNullableData wrapper and apply it to UseBaseQueryResult and UseInfiniteQueryResult, keeping each status variant's other discriminants intact. - useQueries results are a plain reactive store with no resource backing (reads never suspend), so keep its data nullable via a local alias. - Update type tests for the new expectations, and widen intentional pending-state observations in runtime tests through a new pendingData test helper, which documents that those reads happen before the value exists at runtime.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit daa9052
☁️ Nx Cloud last updated this comment at |
Rework the test call sites to sit closer to the non-nullable data
contract instead of widening reads back to T | undefined:
- Scalar displays read data naked ({state.data} renders empty while
pending), including the persist-client provider test, which already
read data naked everywhere else. Assertions on the removed 'null'
fallbacks now match the bare label.
- Snapshot captures push state.data directly. No test in the infinite
file uses the reconcile option, so data is replaced rather than
mutated in place and the JSON deep-copies were redundant. The useQuery
reconcile test keeps its snapshot() capture, since in-place mutation
is exactly what it asserts on.
- Reads that dereference data (.pages, indexing) gate on isSuccess. On
the client the proxy returns the raw store value, so a pending read is
undefined at runtime and dereferencing it throws a TypeError that
halts the reactive system - only the isServer branch routes through
the query resource and throws NotReadyError. Reads already guarded by
a Switch with a pending Match need no gate, since the fallback is
never evaluated while pending.
- suspense.test's render effect now tracks its spread in the compute
function rather than reading state in the callback, clearing the
STRICT_READ_UNTRACKED warnings that run emitted.
brenelz
marked this pull request as draft
August 25, 2026 19:26
The non-nullable data type only held on the server. In the result
Proxy, just the isServer branch routed reads through the query
resource (which throws NotReadyError); on the client the Proxy handed
back the raw store value, so a pending data read was undefined and
dereferencing it threw a TypeError that halted the reactive system.
Suspend on the client too: a tracked data read while isLoading throws
NotReadyError to the nearest <Loading> boundary. The state reads in the
check re-subscribe the reader, so it re-runs when the subscriber syncs
the settled result. Only isLoading suspends - a pending-but-idle query
(disabled, or reset with nothing in flight) still yields undefined
rather than parking the boundary forever. Untracked reads pass through,
so event handlers and effect callbacks can peek without suspending.
This also fixes a hydration bug: the server suspended and rendered the
resolved markup while the client rendered a pending pass and appended a
second copy instead of claiming it, duplicating server-rendered lists.
Both sides now suspend alike and hydration claims the markup.
Tests read data naked again, with no status gates:
- Effects that tracked the whole result via { ...state } now track
deep(state), which subscribes to every property without routing data
through the suspending read, so pending states stay observable and
every existing assertion holds unchanged.
- Narrow effects that tracked data now track dataUpdatedAt, keeping the
same re-run points; the one effect that pushes its own computed
record reads data through untrack to keep that record intact.
- Guards on data inside tracking scopes are gone, since a tracked read
can no longer observe undefined.
Suspending a tracked data read while Solid is claiming server-rendered DOM (sharedConfig.hydrating) bails the claim: the server rendered this content from settled data that the streaming hydration channel may not have primed on the client yet, so the throw leaves unclaimed server nodes and crashes the reactive system with 'Potential Infinite Loop Detected'. Reads during the hydration window return the store value instead — exactly the pre-suspense behavior — and the per-query hydration coordinator re-syncs them once their entry lands. Reproduced in a fullstack streaming-SSR testbed (queries still in flight at hydration time): crashes on every affected load without the guard, zero errors across repeated loads with it. 329/329 package tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Superceeded by this pr |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In the Solid 2.0 port, reading
dataon auseQuery/useInfiniteQueryresult is backed by an async resource: while the query is loading, the component suspends into the nearest<Loading>boundary, so by the timedatais actually read during render the value has settled. The types didn't reflect that —datawas stillTData | undefined, forcing guards that the runtime makes unnecessary.This PR makes
datanon-nullable on the public result types:NonNullableData<TResult, TData>wrapper intypes.tsand applies it toUseBaseQueryResult(and thereforeUseQueryResult) andUseInfiniteQueryResult. Each status variant of the result union keeps its other discriminants (status,error, ...); onlydatais narrowed.useQueriesresults are a plain reactive store with no resource backing — reads never suspend — so itsdatastays honestly nullable via a local alias.No runtime changes.
Test updates
useQuery.test-d,useInfiniteQuery.test-d,queryOptions.test-d,infiniteQueryOptions.test-d). The maybe-undefinedinitialDatafunction case keeps| undefined, since there the undefined comes fromTDataitself through the defined-initialData overload (matching React Query).useQueries.test-dnow asserts against the raw query-coreQueryObserverResult, sinceuseQuerieskeeps nullable data.databefore the value exists at runtime; those reads are widened through a newpendingDatatest helper in__tests__/utils.tsx, which documents the mismatch instead of trippingno-unnecessary-conditionor losing runtime-necessary guards.Verification
tsc --buildcleaneslint ./srccleanvitest run: 22 files, 329 tests passed