docs(react-query): recommend defaultError = unknown instead of AxiosError
#9575
Conversation
…rror Signed-off-by: leesb971204 <[email protected]>
WalkthroughDocs-only updates: examples in React, Solid, Angular, and Vue TypeScript guides change Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/framework/react/typescript.md (2)
158-168: ImportisAxiosErroror qualify it to prevent an undefined identifier in the snippetAs written,
isAxiosErrorwill be undefined in this snippet. Either import it or qualify viaaxios.isAxiosError.Apply one of the following diffs:
Option A — import the named helper:
```tsx +import { isAxiosError } from 'axios' const { error } = useQuery(options) if (isAxiosError(error)) { // handle Axios-specific details } else if (error instanceof Error) { // handle generic errors } else { // handle non-Error throwables if needed }Option B — use the default export qualifier (and import it): ```diff ```tsx +import axios from 'axios' const { error } = useQuery(options) -if (isAxiosError(error)) { +if (axios.isAxiosError(error)) { // handle Axios-specific details } else if (error instanceof Error) { // handle generic errors } else { // handle non-Error throwables if needed }--- `170-187`: **Define `normalizeToDomainError` and strengthen `DomainError` subclassing for clarity and correctness** Readers may look for `normalizeToDomainError`. Adding a minimal example helps. Also, setting `name` and prototype improves Error subclassing robustness. ```diff ```tsx +import { isAxiosError } from 'axios' class DomainError extends Error { constructor(public kind: 'Network' | 'Auth' | 'Unknown', message?: string) { super(message) + this.name = 'DomainError' + // Ensures instanceof works reliably across transpilation targets + Object.setPrototypeOf(this, new.target.prototype) } } +function normalizeToDomainError(e: unknown): DomainError { + if (isAxiosError(e)) { + // map Axios/network concerns + return new DomainError('Network', e.message) + } + if (e instanceof Error) { + // map other Errors + return new DomainError('Unknown', e.message) + } + // handle non-Error throwables + return new DomainError('Unknown', String(e)) +} + const queryFn = async () => { try { const res = await api.get('/user') return res.data } catch (e) { throw normalizeToDomainError(e) // convert AxiosError or others → DomainError } }</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between b6516bd25edcc67dfaced09412f52c9660386a9b and 8b85d83aa5ea1191bc376995b2baa915c4103545. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/framework/react/typescript.md` (1 hunks) </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>docs/framework/react/typescript.md (2)</summary> `133-150`: **Solid rationale and correct module augmentation for `defaultError: unknown`** The shift to `unknown` avoids misleading global assumptions and keeps inference intact for the rest of the generics. The module augmentation pattern is correct and results in `error: unknown | null` at call sites as intended. --- `189-189`: **Concise wrap-up reads well** The closing sentence clearly reinforces the motivation without overpromising. LGTM. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
docs/framework/react/typescript.md
Outdated
| @@ -130,24 +130,63 @@ if (axios.isAxiosError(error)) { | |||
|
|
|||
| ### Registering a global Error | |||
|
|
|||
| TanStack Query v5 allows for a way to set a global Error type for everything, without having to specify generics on call-sides, by amending the `Register` interface. This will make sure inference still works, but the error field will be of the specified type: | |||
| TanStack Query v5 allows for a way to set a global Error type for everything, without having to specify generics on call-sides, by amending the `Register` interface. This will make sure inference still works, but the error field will be of the specified type. We recommend `unknown` to ensure safe, explicit narrowing at call sites, by amending the `Register` interface: | |||
There was a problem hiding this comment.
| TanStack Query v5 allows for a way to set a global Error type for everything, without having to specify generics on call-sides, by amending the `Register` interface. This will make sure inference still works, but the error field will be of the specified type. We recommend `unknown` to ensure safe, explicit narrowing at call sites, by amending the `Register` interface: | |
| TanStack Query v5 allows for a way to set a global Error type for everything, without having to specify generics on call-sides, by amending the `Register` interface. This will make sure inference still works, but the error field will be of the specified type. If you want to enforce that call-sides must do explicit type-narrowing, set `defaultError` to `unknown`: |
docs/framework/react/typescript.md
Outdated
|
|
||
| [//]: # 'RegisterErrorType' | ||
|
|
||
| ```tsx | ||
| import '@tanstack/react-query' | ||
|
|
||
| // ✅ Recommended |
There was a problem hiding this comment.
If this were “recommended”, we’d have kept it as unknown per default instead of Error. Error will work for 99.99% of cases, so going back to unknown is something you can do if you want the safest possible variant because you explicitly throw things that aren’t errors (which also isn’t recommended!), but let’s please not document that this is “recommended”.
docs/framework/react/typescript.md
Outdated
| ``` | ||
|
|
||
| [//]: # 'RegisterErrorType' | ||
|
|
||
| #### Why `unknown` instead of `AxiosError`? |
There was a problem hiding this comment.
Please remove this whole paragraph. The reference to AxiosError makes no sense anymore since it was removed from the previous paragraph, and it’s just confusing, and the pattern with try/catch in the queryFn is not something I would recommend. Instead, the API layer should likely already throw the type of error you want, which is also why axios or ky are a good idea. But that’s all speculative, people can do whatever they want inside the queryFn, doesn’t even have to be data fetching. So let’s remove this please
… registration Signed-off-by: leesb971204 <[email protected]>
|
Thank you for the feedback! I’ve applied your suggestions. |
|
View your CI Pipeline Execution ↗ for commit c66177a
☁️ Nx Cloud last updated this comment at |
|
yes please, you probably only need to change the examples |
…rror Signed-off-by: leesb971204 <[email protected]>
…sError Signed-off-by: leesb971204 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
docs/framework/solid/typescript.md (1)
155-159: Broken self-link: “Registering a global error” anchor is not defined in this pageThis section links to
#registering-a-global-error, but no such header exists in this file. Add the heading near the new augmentation snippet to avoid a dead intra-page link.Apply this diff to introduce the missing section header and a short rationale just before the snippet marker:
- -[//]: # 'RegisterErrorType' +## Registering a global error + +It’s often preferable to register a global default error type of `unknown`. This makes error handling explicit at call sites and avoids implying that all errors come from a specific client (e.g., Axios). Errors can also originate from `select`, memoized callbacks, or other userland code. + +[//]: # 'RegisterErrorType'
🧹 Nitpick comments (1)
docs/framework/solid/typescript.md (1)
134-151: Usetsfence (nottsx) for module augmentation snippetThere’s no JSX here. Using
tshelps syntax highlighters and linters.-```tsx +```ts import '@tanstack/solid-query' declare module '@tanstack/solid-query' { interface Register { // Use unknown so call sites must narrow explicitly. defaultError: unknown } } const query = useQuery(() => ({ queryKey: ['groups'], queryFn: fetchGroups, })) query.error // ^? (property) error: unknown | null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/framework/solid/typescript.md(2 hunks)
🔇 Additional comments (3)
docs/framework/solid/typescript.md (3)
139-141: Good call:defaultError: unknownremoves misleading Axios coupling and enforces explicit narrowingThis aligns with the PR goal and is the safer recommendation.
150-150: LGTM:query.errorbecomesunknown | nullas expectedAccurately reflects the global registration change and communicates the need for narrowing at call sites.
132-132: Snippet marker consistency verified across frameworksI’ve checked all occurrences in the repo (Angular, Vue, React, Solid) and confirmed they all use the exact same single-quoted
[//]: # 'RegisterErrorType'syntax with no deviations or alternate spellings. No changes are needed here.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/framework/angular/typescript.md (1)
166-170: Consider adding a tiny narrowing example to demonstrate handlingunknown.A short follow-up snippet helps readers see how to safely narrow
unknownat call sites without biasing towards any specific client:computed(() => { const error = query.error() // error: unknown | null if (error) { if (error instanceof Error) { console.error(error.message) // safe access } else { // handle non-Error throwables (strings, objects, etc.) console.error(error) } } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/framework/angular/typescript.md(2 hunks)
🔇 Additional comments (2)
docs/framework/angular/typescript.md (2)
156-158: Good call: preferunknownforRegister.defaultError.This aligns with the guidance that errors can originate from multiple places (not just the
queryFn) and avoids misleading consumers into assuming Axios-specific errors globally.
151-160: Docs consistency check passed: No lingeringdefaultError: AxiosErrorusages
Verified across Angular, React, and Solid documentation—defaultErroris uniformly set tounknown.
Signed-off-by: leesb971204 <[email protected]>
|
Solid didn’t have the all done! 🫡 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/framework/vue/typescript.md(1 hunks)
🔇 Additional comments (1)
docs/framework/vue/typescript.md (1)
96-101: LGTM: Register augmentation todefaultError: unknownis correct and desirableThis matches TanStack Query’s global register pattern and avoids misleading framework-wide coupling to Axios. Good callout in the comment about forcing explicit narrowing at call sites.
| const { error } = useQuery({ queryKey: ['groups'], queryFn: fetchGroups }) | ||
| // ^? const error: unknown | null | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Fix Vue type hint: error is a Ref — show Ref<unknown | null> instead of unknown | null
In @tanstack/vue-query, fields returned from useQuery are Refs. The current hint drops the Ref wrapper and can confuse readers. It should be Ref<unknown | null>.
Apply this diff:
-// ^? const error: unknown | null
+// ^? const error: Ref<unknown | null>Follow-up: Earlier in this page, the “TypingError” example shows Ref<unknown> (without | null). For consistency and accuracy, consider updating that hint to Ref<unknown | null> as well.
Fix Vue Query useQuery error type hint to use Ref
- In docs/framework/vue/typescript.md at line 103, update the inferred type comment for
error:-// ^? const error: unknown | null +// ^? const error: Ref<unknown | null>
- Follow-up: earlier “TypingError” example currently shows
Ref<unknown>—consider changing it toRef<unknown | null>for consistency and accuracy.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { error } = useQuery({ queryKey: ['groups'], queryFn: fetchGroups }) | |
| // ^? const error: unknown | null | |
| ``` | |
| const { error } = useQuery({ queryKey: ['groups'], queryFn: fetchGroups }) | |
| // ^? const error: Ref<unknown | null> |
🤖 Prompt for AI Agents
In docs/framework/vue/typescript.md around lines 103 to 105, the inline type
comment for the `error` returned from useQuery is incorrect: change the hint
from `const error: unknown | null` to `const error: Ref<unknown | null>` so it
reflects Vue's ref wrapper, and update the earlier “TypingError” example to use
`Ref<unknown | null>` instead of `Ref<unknown>` for consistency and accuracy.
This PR updates the TypeScript docs for Registering a global error:
defaultError = unknowninstead ofAxiosError.Error.unknown.queryFn; they may also come fromselect, memoized callbacks, or other user-land code.AxiosErrorglobally can mislead developers into thinking all errors originate from Axios, which is incorrect.discussions in here
Summary by CodeRabbit