-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
docs(react-query): undefined vs. null in query-functions.md #10008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
📝 WalkthroughWalkthroughDocumentation change: clarifies that query functions must not return undefined (treated as failed query) and recommends using null to represent a successful "nothing" value; adds an example showing a queryFn that returns item lookup results with null fallback when not found. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 339d39f
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/framework/react/guides/query-functions.md (1)
8-9: Minor style improvements for conciseness.Two small refinements from a style perspective:
- Line 8: Three successive sentences begin with "If". Consider rewording one to avoid repetition.
- Line 9: "All of the following" → "All the following" is more concise.
🔎 Proposed style refinements
-If **resolving the data**, the contained value may be anything **except `undefined`**. If your query function resolves to `undefined`, this will be [treated as a failed query](https://tanstack.com/query/latest/docs/framework/react/guides/migrating-to-react-query-4#undefined-is-an-illegal-cache-value-for-successful-queries). If you need to store "nothing" as a successful result in the query cache, resolve `null` instead. +**Resolving the data** with `undefined` is not allowed. When a query function resolves to `undefined`, it will be [treated as a failed query](https://tanstack.com/query/latest/docs/framework/react/guides/migrating-to-react-query-4#undefined-is-an-illegal-cache-value-for-successful-queries). To store "nothing" as a successful result in the query cache, resolve `null` instead. -All of the following are valid query function configurations: +All the following are valid query function configurations:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/framework/react/guides/query-functions.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
docs/framework/react/guides/query-functions.md
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
docs/framework/react/guides/query-functions.md
🪛 LanguageTool
docs/framework/react/guides/query-functions.md
[style] ~8-~8: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...al-cache-value-for-successful-queries). If you need to store "nothing" as a succes...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~9-~9: Consider removing “of” to be more concise
Context: ...e query cache, resolve null instead. All of the following are valid query function conf...
(ALL_OF_THE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/framework/react/guides/query-functions.md (1)
10-10: Minor: Consider removing "of" for conciseness (optional).The static analyzer suggests "All the following" instead of "All of the following" for slightly more concise phrasing. Both are valid; this is purely stylistic.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/framework/react/guides/query-functions.md
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
docs/framework/react/guides/query-functions.md
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
docs/framework/react/guides/query-functions.md
🪛 LanguageTool
docs/framework/react/guides/query-functions.md
[style] ~9-~9: Consider removing “of” to be more concise
Context: ...e query cache, resolve null instead. All of the following are valid query function conf...
(ALL_OF_THE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (2)
docs/framework/react/guides/query-functions.md (2)
8-8: Clear and helpful guidance on undefined vs. null semantics.This addition directly addresses the user confusion mentioned in the PR objectives and provides accurate, actionable guidance. The link to the migration guide is a nice touch for users seeking deeper context.
28-34: Well-crafted example demonstrating the null fallback pattern.This example is practical and directly reinforces the guidance on line 8. The variable naming is consistent (
allTodosByIdused correctly throughout), and the null coalescing operator is an idiomatic way to show the recommended pattern. This is a good real-world use case for returningnullto represent a successful "nothing" value.
|
Failing PR check ends with: I do have "Allow edits by maintainers" set on this PR. Not sure what the "size limit" thing is about. |
|
the docs sentence is fine but the example makes little sense. Why would we fetch all items and then only access one per todo? This is a situation where you’d want the |
🎯 Changes
The docs currently do not specify anything about
nullorundefinedforqueryFns. I.e., whether each value is legal, what the semantics are etc. I found a few cases of people being confused about it, including this discussion. That discussion mentioned a change in behaviour which is documented in v3->v4 migration guide, but I think this should be in the main docs as well.✅ Checklist
I have followed the steps in the Contributing guide.DATABASE_URL. I've reviewed the rendered markdown from my changes though and it looks fine to meI have tested this code locally withpnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.