Uh oh!
There was an error while loading. Please reload this page.
docs(vue-query): explain why reactive() is needed for result narrowing - #10580
docs(vue-query): explain why reactive() is needed for result narrowing#10580ousamabenyounes wants to merge 1 commit into
Conversation
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe changes document TypeScript narrowing for Vue Query results and add regression tests for non- ChangesVue Query type narrowing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to This PR clarifies Vue Query type-narrowing behavior and adds type tests without changing runtime behavior, so no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks 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 2163cc3
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue-query/src/useBaseQuery.ts`:
- Around line 38-49: UseBaseQueryReturnType currently hardcodes the suspense
return to Promise<QueryObserverResult<TData, TError>> which loses precision for
parameterized TResult (e.g., DefinedQueryObserverResult,
InfiniteQueryObserverResult); update the type so that the mapped properties
remain distributive over TResult but the suspense property is parameterized by
TResult (i.e., make suspense: () => Promise<TResult extends unknown ? TResult :
never> or lift suspense outside the distributive conditional) so awaiting
suspense() returns the actual TResult shape (preserving
fetchNextPage/hasNextPage/data non-undefined), and ensure this aligns with the
runtime behavior of
observer.getOptimisticResult/fetchOptimistic/getCurrentResult so no other
signatures need changing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bd7d5bb-ab5a-4222-9ec2-4d0fe0de7e76
📒 Files selected for processing (4)
.changeset/vue-query-narrow-result-type.mddocs/framework/vue/typescript.mdpackages/vue-query/src/__tests__/useQuery.test-d.tspackages/vue-query/src/useBaseQuery.ts
Uh oh!
There was an error while loading. Please reload this page.
ousamabenyounes
commented
Apr 25, 2026
Thanks @coderabbitai — addressed in |
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Adds a regression test asserting that `useQuery(...).suspense()` resolves to `Promise<QueryObserverResult<TData, TError>>` (parameterized by `TResult` on `UseBaseQueryReturnType`) and that awaiting it preserves the discriminated union narrowing on `isSuccess` / `isError`. Closes the verification gap noted on TanStack#10580 — the parameterization was previously only validated via tsc build, not by an explicit `expectTypeOf` assertion.
Adds a regression test asserting that `useQuery(...).suspense()` resolves to `Promise<QueryObserverResult<TData, TError>>` (parameterized by `TResult` on `UseBaseQueryReturnType`) and that awaiting it preserves the discriminated union narrowing on `isSuccess` / `isError`. Closes the verification gap noted on TanStack#10580 — the parameterization was previously only validated via tsc build, not by an explicit `expectTypeOf` assertion.
2163cc3 to
dccf5b6CompareThe type narrowing example wraps useQuery() in reactive() without saying why, so destructuring the result directly and then checking isSuccess looks like it should work (TanStack#9244). Spell out that reactive() is what flattens the refs into one discriminated union, and show the data.value !== undefined check as the alternative when reactive() is not an option. Pin both shapes with type tests: the non-reactive result exposes Ref<T> | Ref<undefined>, and narrowing on data.value refines the ref itself.
dccf5b6 to
b56dd6cCompareousamabenyounes
commented
Aug 20, 2026
Rebased onto The What is left is docs plus two type tests: the narrowing section now says why |
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Summary
Follow-up to the discussion in #9244 (does not close it).
docs/framework/vue/typescript.mdshows the type-narrowing example wrapped inreactive()but never says why, so the natural reading is that destructuringuseQuery()directly and then checkingisSuccessnarrowsdataas well. It does not — destructuring yields independent refs, and TypeScript cannot carry a narrowing fromisSuccesstodata.This PR says that explicitly and shows the alternative when
reactive()is not an option (data.value !== undefined), plus two type tests pinning both shapes:Ref<T> | Ref<undefined>(the shape already documented just above the example);data.valuerefines the ref itself toRef<T>.Docs and tests only, no runtime or type change, so no changeset.
Note on the previous revision
This branch previously carried a change to
UseBaseQueryReturnTypethat made the mapped type explicitly distributive. That was a no-op:{ [K in keyof TResult]: ... }is homomorphic and already distributes over the union, and the two formulations are mutually assignable forQueryObserverResult,DefinedQueryObserverResultandInfiniteQueryObserverResult. The type tests added here pass with or without it, so I dropped it instead of shipping a diff with no effect.The underlying request in #9244 needs the core result union to carry
Refin its signature, as noted in that thread. That is breaking for every adapter and out of scope here.Summary by CodeRabbit
Documentation
reactive()withuseQuery()for improved TypeScript narrowing.Tests
reactive().