-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix loader function for posts route example #5016
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
WalkthroughAdds a QueryClient instantiation to a TanStack Query example in a React Router docs page so the loader and hooks reference a concrete client instance. No other code or APIs are changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant Router as Router Loader
participant QC as QueryClient
participant API as Posts API
participant UI as React Component
User->>Router: Navigate to /posts
Router->>QC: ensureQueryData(postsQueryOptions)
alt Cache miss
QC->>API: fetch posts
API-->>QC: posts data
QC-->>Router: cache filled
else Cache hit
QC-->>Router: return cached data
end
Router-->>UI: render route
UI->>QC: useSuspenseQuery(postsQueryOptions)
QC-->>UI: serve cached posts
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
✨ 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 (
|
View your CI Pipeline Execution ↗ for commit 4ea18c1
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/router/framework/react/guide/external-data-loading.md (1)
109-136
: Broken snippet: queryClient is undefined in this second example.The “Error handling” example calls
queryClient.ensureQueryData(...)
but never defines or retrievesqueryClient
in this snippet. Readers copying this block will hit a ReferenceError.Prefer pulling the client from the router context (consistent with the SSR section below) instead of creating a new singleton here:
export const Route = createFileRoute('/')({ - loader: () => queryClient.ensureQueryData(postsQueryOptions), + loader: ({ context: { queryClient } }) => + queryClient.ensureQueryData(postsQueryOptions), errorComponent: ({ error, reset }) => {If you want the bare-minimal fix (client-only), alternatively add a local instance above the loader:
export const Route = createFileRoute('/')({ + // For client-only examples; see SSR section below for per-request setup + const queryClient = new QueryClient() loader: () => queryClient.ensureQueryData(postsQueryOptions),(First option recommended for consistency across docs.)
🧹 Nitpick comments (3)
docs/router/framework/react/guide/external-data-loading.md (3)
78-79
: Clarify provider context (or show it) to avoid runtime errors with useSuspenseQuery.
useSuspenseQuery
requires aQueryClientProvider
higher in the tree. Consider adding a brief comment or a one-line provider example link here to make it clear that the app must be wrapped, otherwise this snippet will throw at runtime.You can inline a minimal hint without cluttering the snippet:
-const queryClient = new QueryClient() +// Ensure your app is wrapped in <QueryClientProvider> at the root. +const queryClient = new QueryClient()
86-93
: Verify returned data shape for posts.The component destructures
data: { posts }
, which impliesfetchPosts()
returns{ posts: Post[] }
. Many examples returnPost[]
directly. IffetchPosts()
returns an array, this destructure will throw. Either adjust the destructure or clarify the expected return shape in the snippet/comment.Possible change if
fetchPosts()
returns an array:-const { - data: { posts }, -} = useSuspenseQuery(postsQueryOptions) +const { data: posts } = useSuspenseQuery(postsQueryOptions)
78-88
: Optional: avoid module-scoped singletons to stay SSR-safe and consistent with the SSR section.Since the “SSR Dehydration/Hydration” section correctly shows per-request
QueryClient
creation and passing via router context, you can mirror that pattern here too. This avoids teaching a pattern that can leak state across requests.Example adjustment:
-export const Route = createFileRoute('/posts')({ - // Use the `loader` option to ensure that the data is loaded - loader: () => queryClient.ensureQueryData(postsQueryOptions), +export const Route = createFileRoute('/posts')({ + // Use the `loader` option to ensure that the data is loaded + loader: ({ context: { queryClient } }) => + queryClient.ensureQueryData(postsQueryOptions),And you can add a short note above that the
QueryClient
is provided via router context (as demonstrated in the SSR section below).
📜 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/router/framework/react/guide/external-data-loading.md
(1 hunks)
🔇 Additional comments (1)
docs/router/framework/react/guide/external-data-loading.md (1)
78-79
: Good fix: define a concrete QueryClient for the loader to use.This resolves the undefined
queryClient
in the first example’s loader and aligns the cache preloading with TanStack Query’s API.
@@ -75,6 +75,8 @@ Let's take a look at a more realistic example using TanStack Query. | |||
|
|||
```tsx | |||
// src/routes/posts.tsx | |||
const queryClient = new QueryClient() |
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.
you wouldnt create the queryClient instance in a specific route
Yes, good point. I opened this because I was unclear how to access the |
typically you would add the queryClient to the router context, see for example here https://github.com/TanStack/router/blob/main/examples/react/basic-react-query-file-based/src/main.tsx#L14 |
Summary by CodeRabbit