docs(solid-query): convert guide docs to ref-based with Solid-style code#10146
Merged
TkDodo merged 9 commits intoTanStack:mainfrom Feb 26, 2026
Merged
docs(solid-query): convert guide docs to ref-based with Solid-style code#10146TkDodo merged 9 commits intoTanStack:mainfrom
TkDodo merged 9 commits intoTanStack:mainfrom
Conversation
|
github-actions
bot
added
the
documentation
|
View your CI Pipeline Execution ↗ for commit 77a6218
☁️ Nx Cloud last updated this comment at |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Solid offline example (project files and README) and registers it in docs/config.json; extensively expands Solid framework guides with concrete Solid TSX examples and simplifies prior React→Solid replacement mappings; inserts numerous documentation anchors and minor wording/formatting edits across React guides. No runtime API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant UI as "App UI (Solid)"
participant QC as "QueryClient / Persistor"
participant API as "MSW Worker / API"
participant LS as "localStorage (persister)"
User->>UI: submit mutation (update comment)
UI->>QC: call mutate (optimistic update)
QC->>API: POST /movies/:id
API-->>QC: respond (success / error)
QC->>UI: update mutation state (isPending / isSuccess / isError)
QC->>LS: persist mutation state / queue
Note over LS,QC: On reload, QC restores persisted state
QC->>API: resume paused mutations
QC->>UI: reflect final query data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (4)
docs/framework/solid/guides/placeholder-query-data.md (1)
22-22: Heading level skip is acceptable in ref-based docs.The h3 heading appears without a preceding h2 in this file, but since this is a ref-based document (line 4), the parent h2 sections are inherited from the React guide. This is likely intentional structure, though it causes markdownlint to flag a violation when analyzing the file in isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/placeholder-query-data.md` at line 22, Add an explicit inline comment above the "Placeholder Data Memoization" h3 to indicate the heading-level skip is intentional because this is a ref-based document whose parent h2 sections are inherited from the React guide; this comment should reference the h3 ("Placeholder Data Memoization") so reviewers and linters understand the skip (or alternatively add a markdownlint disable directive at the top of the file for the specific heading-level rule) to prevent the isolated markdownlint violation.docs/framework/solid/guides/disabling-queries.md (1)
23-38: Consider reorderingMatchconditions for clearer state handling.The current order checks
todosQuery.databeforeisErrorandisLoading. If data exists from a previous fetch but a refetch fails, the error state won't be displayed. This might be intentional for stale-while-revalidate UX, but typically error/loading states are checked first.Suggested reordering for explicit state prioritization
<Switch> - <Match when={todosQuery.data}> - <ul> - <For each={todosQuery.data}>{(todo) => <li>{todo.title}</li>}</For> - </ul> - </Match> - <Match when={todosQuery.isError}> - <span>Error: {todosQuery.error.message}</span> - </Match> <Match when={todosQuery.isLoading}> <span>Loading...</span> </Match> + <Match when={todosQuery.isError}> + <span>Error: {todosQuery.error.message}</span> + </Match> + <Match when={todosQuery.data}> + <ul> + <For each={todosQuery.data}>{(todo) => <li>{todo.title}</li>}</For> + </ul> + </Match> <Match when={true}> <span>Not ready ...</span> </Match> </Switch>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/disabling-queries.md` around lines 23 - 38, The Switch/Match block currently checks todosQuery.data before todosQuery.isError and todosQuery.isLoading, which can hide error/loading states when stale data exists; reorder the Match checks in the Switch so that todosQuery.isError and todosQuery.isLoading are evaluated before todosQuery.data (i.e., check todosQuery.isError, then todosQuery.isLoading, then todosQuery.data, finally the fallback Match when={true}) to ensure explicit prioritization of error/loading states in the rendering logic for the todosQuery component.examples/solid/offline/src/api.ts (1)
17-42: Consider adding response status checks.The fetch functions don't verify
response.okbefore parsing JSON. If the server returns an error status (e.g., 404 from line 59), callingresponse.json()on non-JSON error bodies could throw or produce unexpected results.Example fix for fetchMovie
export const fetchMovie = async ( id: string, ): Promise<{ ts: number movie: { comment: string; id: string; title: string } }> => { const response = await fetch(`/movies/${id}`) + if (!response.ok) { + throw new Error(`Failed to fetch movie: ${response.status}`) + } return await response.json() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/solid/offline/src/api.ts` around lines 17 - 42, The three API helpers (fetchMovie, fetchMovies, updateMovie) don't check response.ok before calling response.json(), so update each function to verify response.ok after fetch; if not ok, read the response body (preferably text or safe JSON parse), and throw an Error that includes the HTTP status and returned message/body so callers can handle failures. Specifically, in fetchMovie, fetchMovies, and updateMovie add a conditional like "if (!response.ok) { const errBody = await response.text(); throw new Error(`Fetch <functionName> failed ${response.status}: ${errBody}`) }" (replace <functionName> with the real function names) before returning response.json().examples/solid/offline/src/movies.ts (1)
43-45: Type the error handler parameters properly.Using
anytypes loses type safety. Consider using proper types or at minimum a more descriptive approach.Proposed fix
- onError: (_: any, __: any, context: any) => { + onError: (_error, _variables, context) => { queryClient.setQueryData(movieKeys.detail(movieId), context.previousData) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/solid/offline/src/movies.ts` around lines 43 - 45, The onError handler currently uses any for all params; change it to use meaningful types (avoid any) such as (error: unknown, variables: { movieId: string } | undefined, context: { previousData?: Movie | null } | undefined) => { queryClient.setQueryData(movieKeys.detail(movieId), context?.previousData) }; import or reference the Movie type used by your fetcher and/or define a small MutationContext type for context to improve type-safety while keeping the call to queryClient.setQueryData(movieKeys.detail(movieId), context?.previousData) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/framework/react/guides/request-waterfalls.md`:
- Line 187: Summary: Hyphenate compound modifiers like "real world" to
"real-world" for grammar consistency. Fix: update the prose that mentions
"<Comments>" and "<Article>" to use "real-world applications" and any other
compound modifiers (e.g., "child might be nested far below the parent" if used
as a modifier) to be hyphenated; search for occurrences around the discussion of
hoisting the comments query to the parent and at the alternate locations noted
(also applies to lines referencing 368-369) and replace plain compounds with
hyphenated forms to maintain consistent compound-adjective styling.
- Around line 272-275: The fenced code block showing the waterfall steps (the
lines containing "getFeed()" and "getGraphDataById()") is missing a language
tag; update that triple-backtick block to include a language specifier (e.g.,
"text") so Markdownlint stops warning and the snippet is properly annotated.
In `@docs/framework/solid/guides/mutations.md`:
- Line 275: The heading "Persisting Offline mutations" is an h3 (###) that skips
from the document's h1; change it to h2 (## Persisting Offline mutations) so
heading levels increment correctly per markdownlint MD001 and maintain document
structure; update the heading token for "Persisting Offline mutations" to use
two hashes and ensure nearby headings remain properly nested.
- Around line 173-190: The onSuccess callbacks in the Example7 snippet use an
incorrect four-parameter signature (data, variables, onMutateResult, context);
update both the global useMutation onSuccess and the per-call mutate onSuccess
to the correct signature used by this library (e.g., onSuccess: (data,
variables, context) => { ... }) and adjust any references to the removed
parameter (onMutateResult) accordingly; specifically edit the useMutation block
and the mutate(...) call that reference addTodo, useMutation, and mutate to use
(data, variables, context) so the docs match the real API.
- Around line 143-168: The example callbacks for useMutation and mutate use an
incorrect fourth parameter name "onMutateResult"; update all
onSuccess/onError/onSettled callback signatures in the useMutation options and
in the mutate call to match the correct mutation callback shape (three params):
onSuccess(data, variables, context), onError(error, variables, context),
onSettled(dataOrUndefined, errorOrUndefined, variables, context) — or if your
API expects three params for settled, use (data, error, context); replace any
"onMutateResult" occurrences with "context" and remove the extra parameter so
the callbacks in useMutation and mutate reference the correct symbols and
ordering.
- Around line 221-255: The callbacks provided to queryClient.setMutationDefaults
use the wrong signatures—onMutate currently expects (variables, context) and
returns a context named onMutateResult, while onSuccess/onError are declared
with an extra parameter; fix by making onMutate async (variables) => { await
queryClient.cancelQueries(['todos']); const optimisticTodo = { id: uuid(),
title: variables.title }; queryClient.setQueryData(['todos'], old => [...old,
optimisticTodo]); return { optimisticTodo } } and update onSuccess and onError
to use the proper signature (data, variables, context) => {
queryClient.setQueryData(['todos'], old => old.map/todo filtering using
context.optimisticTodo.id) } so they read context.optimisticTodo (replace uses
of onMutateResult and context.client with the returned context and the outer
queryClient respectively).
In `@docs/framework/solid/guides/prefetching.md`:
- Around line 216-253: The example uses the old constructor-style API; update it
to the v1 functional API by replacing RouterContext/createRootRoute/new Route
with createRootRoute (or createRootRouteWithContext<MyContext>() for typed
context), createRoute for individual routes, build the tree with
rootRoute.addChildren([ ...childRoutes... ]), and instantiate the router via
createRouter({ routeTree: rootRoute }); also update any usages of RouterContext
methods to the new route context hooks (e.g., useRouteContext) and ensure
loader/beforeLoad signatures match the v1 createRoute API.
In `@docs/framework/solid/guides/queries.md`:
- Around line 7-95: The MD042 warnings are caused by the inline marker comments
like [//]: # 'SubscribeDescription' and [//]: # 'Example' in the markdown; wrap
the marker blocks with markdownlint disable/enable comments (e.g. insert <!--
markdownlint-disable MD042 --> before the sequence of [//]: # '...' markers and
<!-- markdownlint-enable MD042 --> after) or convert those marker lines to a
neutral HTML comment format so MD042 is suppressed for the markers (update the
marker blocks surrounding useQuery examples and the
SubscribeDescription/Example/Example2/Example3/Example4 markers accordingly).
In `@docs/framework/solid/guides/query-cancellation.md`:
- Around line 111-120: The queryFn passed to useQuery (in the todosQuery
example) does not return the Promise from client.request so the query resolves
to undefined; update the queryFn to return the client.request call (i.e., ensure
the function passed as queryFn returns the result of client.request({ document:
query, signal })) so useQuery receives the fetched data—look for GraphQLClient,
client.request, useQuery, queryFn and todosQuery in the snippet and add the
missing return.
In `@examples/solid/offline/README.md`:
- Around line 5-6: The README references running `npm run start` but
package.json only defines the `dev` script, causing the command to fail; update
the README.md instructions to use `npm run dev` (or add a `start` script to
package.json that maps to `dev`) so the command matches the actual script
name—look for the script entries in package.json and the command lines in
README.md and make them consistent.
In `@examples/solid/offline/src/App.tsx`:
- Around line 146-153: submitForm can pass an undefined comment into
updateMovie.mutate which triggers a TypeError in api.ts when
comment.toUpperCase() is called; change submitForm to ensure a defined string is
sent (e.g., compute const safeComment = comment() ??
movieQuery.data?.movie.comment ?? "" before calling updateMovie.mutate) or
validate and coerce comment() to a string and pass that into updateMovie.mutate
so api.ts always receives a string value; update the submitForm function (and
any hook return usage of comment()) to guarantee non-undefined input to
updateMovie.mutate.
In `@examples/solid/offline/src/index.tsx`:
- Around line 6-14: worker.start() is asynchronous and currently races with
initial renders; await the worker startup before calling render so MSW is ready
to intercept requests — change the flow so you await the Promise returned by
worker.start() (referencing worker.start()) and only then call render(...) to
mount <App /> (referencing render and App) to eliminate the race condition.
In `@examples/solid/offline/src/movies.ts`:
- Around line 22-42: In updateMovie's onMutate handler capture the current
comment value into a local variable before calling setComment(undefined) so the
optimistic update uses the preserved value; specifically, inside the onMutate
for updateMovie (which uses movieKeys.detail(movieId)) read comment() into e.g.
previousComment, then call setComment(undefined), and pass previousComment into
queryClient.setQueryData(...) when setting movie.comment and return
previousComment in the mutation context so rollback can restore it if needed.
---
Duplicate comments:
In `@docs/framework/solid/guides/optimistic-updates.md`:
- Around line 101-129: The mutation callbacks use a `context.client` API that
may not match Solid Query's actual context shape; verify and update
useMutation's callbacks (onMutate, onError, onSettled) to use the correct
context object and method names (e.g., confirm whether cancelQueries,
getQueryData, setQueryData, invalidateQueries are accessed via context.client or
directly via context or a queryClient provided by useMutation), then adjust
references to newTodo/onMutateResult accordingly so onMutate returns the proper
snapshot shape and onError/onSettled read that shape correctly (ensure functions
like cancelQueries(['todos', newTodo.id]) and setQueryData/getQueryData use the
actual API signature and key format used by Solid Query).
---
Nitpick comments:
In `@docs/framework/solid/guides/disabling-queries.md`:
- Around line 23-38: The Switch/Match block currently checks todosQuery.data
before todosQuery.isError and todosQuery.isLoading, which can hide error/loading
states when stale data exists; reorder the Match checks in the Switch so that
todosQuery.isError and todosQuery.isLoading are evaluated before todosQuery.data
(i.e., check todosQuery.isError, then todosQuery.isLoading, then
todosQuery.data, finally the fallback Match when={true}) to ensure explicit
prioritization of error/loading states in the rendering logic for the todosQuery
component.
In `@docs/framework/solid/guides/placeholder-query-data.md`:
- Line 22: Add an explicit inline comment above the "Placeholder Data
Memoization" h3 to indicate the heading-level skip is intentional because this
is a ref-based document whose parent h2 sections are inherited from the React
guide; this comment should reference the h3 ("Placeholder Data Memoization") so
reviewers and linters understand the skip (or alternatively add a markdownlint
disable directive at the top of the file for the specific heading-level rule) to
prevent the isolated markdownlint violation.
In `@examples/solid/offline/src/api.ts`:
- Around line 17-42: The three API helpers (fetchMovie, fetchMovies,
updateMovie) don't check response.ok before calling response.json(), so update
each function to verify response.ok after fetch; if not ok, read the response
body (preferably text or safe JSON parse), and throw an Error that includes the
HTTP status and returned message/body so callers can handle failures.
Specifically, in fetchMovie, fetchMovies, and updateMovie add a conditional like
"if (!response.ok) { const errBody = await response.text(); throw new
Error(`Fetch <functionName> failed ${response.status}: ${errBody}`) }" (replace
<functionName> with the real function names) before returning response.json().
In `@examples/solid/offline/src/movies.ts`:
- Around line 43-45: The onError handler currently uses any for all params;
change it to use meaningful types (avoid any) such as (error: unknown,
variables: { movieId: string } | undefined, context: { previousData?: Movie |
null } | undefined) => { queryClient.setQueryData(movieKeys.detail(movieId),
context?.previousData) }; import or reference the Movie type used by your
fetcher and/or define a small MutationContext type for context to improve
type-safety while keeping the call to
queryClient.setQueryData(movieKeys.detail(movieId), context?.previousData)
intact.
|
|
||
| [//]: # 'NestedExample' | ||
|
|
||
| Note that while `<Comments>` takes a prop `id` from the parent, that id is already available when the `<Article>` renders so there is no reason we could not fetch the comments at the same time as the article. In real world applications, the child might be nested far below the parent and these kinds of waterfalls are often trickier to spot and fix, but for our example, one way to flatten the waterfall would be to hoist the comments query to the parent instead: |
Contributor
There was a problem hiding this comment.
Hyphenate compound modifiers for grammar consistency.
These read as compound adjectives and should be hyphenated.
✍️ Suggested edits
- In real world applications, the child might be nested far below the parent
+ In real-world applications, the child might be nested far below the parent
- - Include all data fetching code in the main bundle, even if we seldom use it
- - Put the data fetching code in the code split bundle, but with a request waterfall
+ - Include all data-fetching code in the main bundle, even if we seldom use it
+ - Put the data-fetching code in the code-split bundle, but with a request-waterfallAlso applies to: 368-369
🧰 Tools
🪛 LanguageTool
[grammar] ~187-~187: Use a hyphen to join words.
Context: ...at the same time as the article. In real world applications, the child might be n...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/react/guides/request-waterfalls.md` at line 187, Summary:
Hyphenate compound modifiers like "real world" to "real-world" for grammar
consistency. Fix: update the prose that mentions "<Comments>" and "<Article>" to
use "real-world applications" and any other compound modifiers (e.g., "child
might be nested far below the parent" if used as a modifier) to be hyphenated;
search for occurrences around the discussion of hoisting the comments query to
the parent and at the alternate locations noted (also applies to lines
referencing 368-369) and replace plain compounds with hyphenated forms to
maintain consistent compound-adjective styling.
Comment on lines
272
to
275
| ``` | ||
| 1. |> getFeed() | ||
| 2. |> getGraphDataById() | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the fenced code block.
Markdownlint warns about missing language specifier here. Consider marking it as plain text.
💡 Suggested change
-```
+```text
1. |> getFeed()
2. |> getGraphDataById()</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 272-272: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/react/guides/request-waterfalls.md` around lines 272 - 275,
The fenced code block showing the waterfall steps (the lines containing
"getFeed()" and "getGraphDataById()") is missing a language tag; update that
triple-backtick block to include a language specifier (e.g., "text") so
Markdownlint stops warning and the snippet is properly annotated.
Comment on lines
+143
to
+168
| ```tsx | ||
| useMutation(() => ({ | ||
| mutationFn: addTodo, | ||
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| // I will fire first | ||
| }, | ||
| onError: (error, variables, onMutateResult, context) => { | ||
| // I will fire first | ||
| }, | ||
| onSettled: (data, error, variables, onMutateResult, context) => { | ||
| // I will fire first | ||
| }, | ||
| })) | ||
|
|
||
| mutate(todo, { | ||
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| // I will fire second! | ||
| }, | ||
| onError: (error, variables, onMutateResult, context) => { | ||
| // I will fire second! | ||
| }, | ||
| onSettled: (data, error, variables, onMutateResult, context) => { | ||
| // I will fire second! | ||
| }, | ||
| }) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Same callback signature issue in Example6.
The callbacks here also use the incorrect onMutateResult parameter pattern.
Proposed fix
useMutation(() => ({
mutationFn: addTodo,
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// I will fire first
},
- onError: (error, variables, onMutateResult, context) => {
+ onError: (error, variables, context) => {
// I will fire first
},
- onSettled: (data, error, variables, onMutateResult, context) => {
+ onSettled: (data, error, variables, context) => {
// I will fire first
},
}))
mutate(todo, {
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// I will fire second!
},
- onError: (error, variables, onMutateResult, context) => {
+ onError: (error, variables, context) => {
// I will fire second!
},
- onSettled: (data, error, variables, onMutateResult, context) => {
+ onSettled: (data, error, variables, context) => {
// I will fire second!
},
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/solid/guides/mutations.md` around lines 143 - 168, The example
callbacks for useMutation and mutate use an incorrect fourth parameter name
"onMutateResult"; update all onSuccess/onError/onSettled callback signatures in
the useMutation options and in the mutate call to match the correct mutation
callback shape (three params): onSuccess(data, variables, context),
onError(error, variables, context), onSettled(dataOrUndefined, errorOrUndefined,
variables, context) — or if your API expects three params for settled, use
(data, error, context); replace any "onMutateResult" occurrences with "context"
and remove the extra parameter so the callbacks in useMutation and mutate
reference the correct symbols and ordering.
Comment on lines
+173
to
+190
| ```tsx | ||
| useMutation(() => ({ | ||
| mutationFn: addTodo, | ||
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| // Will be called 3 times | ||
| }, | ||
| })) | ||
|
|
||
| const todos = ['Todo 1', 'Todo 2', 'Todo 3'] | ||
| todos.forEach((todo) => { | ||
| mutate(todo, { | ||
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| // Will execute only once, for the last mutation (Todo 3), | ||
| // regardless which mutation resolves first | ||
| }, | ||
| }) | ||
| }) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Same callback signature issue in Example7.
Proposed fix
useMutation(() => ({
mutationFn: addTodo,
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// Will be called 3 times
},
}))
const todos = ['Todo 1', 'Todo 2', 'Todo 3']
todos.forEach((todo) => {
mutate(todo, {
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// Will execute only once, for the last mutation (Todo 3),
// regardless which mutation resolves first
},
})
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/solid/guides/mutations.md` around lines 173 - 190, The
onSuccess callbacks in the Example7 snippet use an incorrect four-parameter
signature (data, variables, onMutateResult, context); update both the global
useMutation onSuccess and the per-call mutate onSuccess to the correct signature
used by this library (e.g., onSuccess: (data, variables, context) => { ... })
and adjust any references to the removed parameter (onMutateResult) accordingly;
specifically edit the useMutation block and the mutate(...) call that reference
addTodo, useMutation, and mutate to use (data, variables, context) so the docs
match the real API.
Comment on lines
+5
to
+6
| - `npm install` | ||
| - `npm run start` |
Contributor
There was a problem hiding this comment.
Incorrect script name in instructions.
The README references npm run start, but the package.json defines a dev script instead. This will cause the command to fail.
📝 Proposed fix
- `npm install`
-- `npm run start`
+- `npm run dev`📝 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.
Suggested change
| - `npm install` | |
| - `npm run start` | |
| - `npm install` | |
| - `npm run dev` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/offline/README.md` around lines 5 - 6, The README references
running `npm run start` but package.json only defines the `dev` script, causing
the command to fail; update the README.md instructions to use `npm run dev` (or
add a `start` script to package.json that maps to `dev`) so the command matches
the actual script name—look for the script entries in package.json and the
command lines in README.md and make them consistent.
Comment on lines
+146
to
+153
| function submitForm(event: SubmitEvent) { | ||
| event.preventDefault() | ||
|
|
||
| updateMovie.mutate({ | ||
| id: props.movieId, | ||
| comment: comment(), | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Potential undefined comment when submitting.
comment() could return undefined if no local edits were made (it falls back to movieQuery.data?.movie.comment in the hook return). When mutating, if comment is undefined, the API call in api.ts line 75 will call comment.toUpperCase() which throws a TypeError.
Proposed fix
function submitForm(event: SubmitEvent) {
event.preventDefault()
+ const currentComment = comment()
+ if (currentComment === undefined) return
+
updateMovie.mutate({
id: props.movieId,
- comment: comment(),
+ comment: currentComment,
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/offline/src/App.tsx` around lines 146 - 153, submitForm can
pass an undefined comment into updateMovie.mutate which triggers a TypeError in
api.ts when comment.toUpperCase() is called; change submitForm to ensure a
defined string is sent (e.g., compute const safeComment = comment() ??
movieQuery.data?.movie.comment ?? "" before calling updateMovie.mutate) or
validate and coerce comment() to a string and pass that into updateMovie.mutate
so api.ts always receives a string value; update the submitForm function (and
any hook return usage of comment()) to guarantee non-undefined input to
updateMovie.mutate.
Comment on lines
+6
to
+14
| worker.start() | ||
|
|
||
| render( | ||
| () => ( | ||
| <div style={{ padding: '16px' }}> | ||
| <App /> | ||
| </div> | ||
| ), | ||
| document.getElementById('root')!, |
Contributor
There was a problem hiding this comment.
Race condition: worker.start() should be awaited.
worker.start() returns a Promise. Rendering the app immediately without waiting could cause initial fetch requests to bypass MSW interception if they fire before the worker is ready.
Proposed fix
-worker.start()
-
-render(
- () => (
- <div style={{ padding: '16px' }}>
- <App />
- </div>
- ),
- document.getElementById('root')!,
-)
+worker.start().then(() => {
+ render(
+ () => (
+ <div style={{ padding: '16px' }}>
+ <App />
+ </div>
+ ),
+ document.getElementById('root')!,
+ )
+})📝 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.
Suggested change
| worker.start() | |
| render( | |
| () => ( | |
| <div style={{ padding: '16px' }}> | |
| <App /> | |
| </div> | |
| ), | |
| document.getElementById('root')!, | |
| worker.start().then(() => { | |
| render( | |
| () => ( | |
| <div style={{ padding: '16px' }}> | |
| <App /> | |
| </div> | |
| ), | |
| document.getElementById('root')!, | |
| ) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/offline/src/index.tsx` around lines 6 - 14, worker.start() is
asynchronous and currently races with initial renders; await the worker startup
before calling render so MSW is ready to intercept requests — change the flow so
you await the Promise returned by worker.start() (referencing worker.start())
and only then call render(...) to mount <App /> (referencing render and App) to
eliminate the race condition.
Comment on lines
+22
to
+42
| const updateMovie = useMutation(() => ({ | ||
| mutationKey: movieKeys.detail(movieId), | ||
| onMutate: async () => { | ||
| await queryClient.cancelQueries({ queryKey: movieKeys.detail(movieId) }) | ||
| const previousData = queryClient.getQueryData< | ||
| Awaited<ReturnType<typeof api.fetchMovie>> | ||
| >(movieKeys.detail(movieId)) | ||
|
|
||
| // remove local state so that server state is taken instead | ||
| setComment(undefined) | ||
|
|
||
| queryClient.setQueryData(movieKeys.detail(movieId), { | ||
| ...previousData, | ||
| movie: { | ||
| ...previousData?.movie, | ||
| comment: comment(), | ||
| }, | ||
| }) | ||
|
|
||
| return { previousData } | ||
| }, |
Contributor
There was a problem hiding this comment.
Bug: comment() returns undefined after clearing the signal.
On line 31, setComment(undefined) clears the local comment state. Then on line 37, comment() is called to set the optimistic cache value, but it will return undefined since the signal was just cleared. The previous comment value should be captured before clearing.
Proposed fix
onMutate: async () => {
await queryClient.cancelQueries({ queryKey: movieKeys.detail(movieId) })
const previousData = queryClient.getQueryData<
Awaited<ReturnType<typeof api.fetchMovie>>
>(movieKeys.detail(movieId))
+ // Capture the current comment before clearing
+ const newComment = comment()
+
// remove local state so that server state is taken instead
setComment(undefined)
queryClient.setQueryData(movieKeys.detail(movieId), {
...previousData,
movie: {
...previousData?.movie,
- comment: comment(),
+ comment: newComment,
},
})
return { previousData }
},📝 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.
Suggested change
| const updateMovie = useMutation(() => ({ | |
| mutationKey: movieKeys.detail(movieId), | |
| onMutate: async () => { | |
| await queryClient.cancelQueries({ queryKey: movieKeys.detail(movieId) }) | |
| const previousData = queryClient.getQueryData< | |
| Awaited<ReturnType<typeof api.fetchMovie>> | |
| >(movieKeys.detail(movieId)) | |
| // remove local state so that server state is taken instead | |
| setComment(undefined) | |
| queryClient.setQueryData(movieKeys.detail(movieId), { | |
| ...previousData, | |
| movie: { | |
| ...previousData?.movie, | |
| comment: comment(), | |
| }, | |
| }) | |
| return { previousData } | |
| }, | |
| const updateMovie = useMutation(() => ({ | |
| mutationKey: movieKeys.detail(movieId), | |
| onMutate: async () => { | |
| await queryClient.cancelQueries({ queryKey: movieKeys.detail(movieId) }) | |
| const previousData = queryClient.getQueryData< | |
| Awaited<ReturnType<typeof api.fetchMovie>> | |
| >(movieKeys.detail(movieId)) | |
| // Capture the current comment before clearing | |
| const newComment = comment() | |
| // remove local state so that server state is taken instead | |
| setComment(undefined) | |
| queryClient.setQueryData(movieKeys.detail(movieId), { | |
| ...previousData, | |
| movie: { | |
| ...previousData?.movie, | |
| comment: newComment, | |
| }, | |
| }) | |
| return { previousData } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/solid/offline/src/movies.ts` around lines 22 - 42, In updateMovie's
onMutate handler capture the current comment value into a local variable before
calling setComment(undefined) so the optimistic update uses the preserved value;
specifically, inside the onMutate for updateMovie (which uses
movieKeys.detail(movieId)) read comment() into e.g. previousComment, then call
setComment(undefined), and pass previousComment into
queryClient.setQueryData(...) when setting movie.comment and return
previousComment in the mutation context so rollback can restore it if needed.
TkDodo
reviewed
Feb 25, 2026
sukvvon
force-pushed
the
docs/solid-query-guides
branch
from
35ca6d1 to
cd3b683
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
docs/framework/solid/guides/query-cancellation.md (1)
111-120:⚠️ Potential issue | 🔴 CriticalMissing
returnstatement inqueryFn.The
queryFndoesn't return the promise fromclient.request(), so the query will resolve withundefinedinstead of the actual data.🐛 Proposed fix
const todosQuery = useQuery(() => ({ queryKey: ['todos'], queryFn: ({ signal }) => { - client.request({ document: query, signal }) + return client.request({ document: query, signal }) }, }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/query-cancellation.md` around lines 111 - 120, The queryFn passed to useQuery (in todosQuery) forgets to return the promise from client.request, so the query resolves to undefined; update the queryFn to return client.request({ document: query, signal }) (i.e., ensure the function body of queryFn returns the client.request promise) so useQuery receives the fetched data from GraphQLClient.request.docs/framework/solid/guides/mutations.md (5)
275-275:⚠️ Potential issue | 🟡 MinorHeading level skips from h1 to h3.
Per markdownlint MD001, heading levels should increment by one. This
###(h3) heading appears without a preceding##(h2) heading.📝 Proposed fix
-### Persisting Offline mutations +## Persisting Offline mutations🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/mutations.md` at line 275, The heading "Persisting Offline mutations" is written as an h3 (###) but there is no preceding h2; change the header to an h2 (## Persisting Offline mutations) or insert an appropriate h2 above it so heading levels increment correctly per MD001; update the header token for the "Persisting Offline mutations" line in docs/framework/solid/guides/mutations.md (look for the exact heading text) to restore proper heading hierarchy.
106-122:⚠️ Potential issue | 🟠 MajorIncorrect callback signatures with extra
onMutateResultparameter.The
onError,onSuccess, andonSettledcallbacks use an incorrect four-parameter signature. The correct signatures are:
onError: (error, variables, context) => {}onSuccess: (data, variables, context) => {}onSettled: (data, error, variables, context) => {}The
contextparameter already contains the result returned fromonMutate, so there's no separateonMutateResultparameter.🔧 Proposed fix
useMutation(() => ({ mutationFn: addTodo, - onMutate: (variables, context) => { + onMutate: (variables) => { // A mutation is about to happen! // Optionally return a result containing data to use when for example rolling back return { id: 1 } }, - onError: (error, variables, onMutateResult, context) => { + onError: (error, variables, context) => { // An error happened! - console.log(`rolling back optimistic update with id ${onMutateResult.id}`) + console.log(`rolling back optimistic update with id ${context.id}`) }, - onSuccess: (data, variables, onMutateResult, context) => { + onSuccess: (data, variables, context) => { // Boom baby! }, - onSettled: (data, error, variables, onMutateResult, context) => { + onSettled: (data, error, variables, context) => { // Error or success... doesn't matter! }, }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/mutations.md` around lines 106 - 122, The callback signatures in the mutation example are incorrect: update the handlers so they match the correct Solid Query signatures — keep onMutate: (variables, context) => { return { id: 1 } }, change onError to onError: (error, variables, context) => { console.log(`rolling back optimistic update with id ${context?.id}`) }, change onSuccess to onSuccess: (data, variables, context) => { /* ... */ }, and change onSettled to onSettled: (data, error, variables, context) => { /* ... */ }; remove the extra onMutateResult parameter and read the onMutate result from context in onError/onSuccess/onSettled.
173-190:⚠️ Potential issue | 🟠 MajorSame callback signature issue in Example7.
🔧 Proposed fix
useMutation(() => ({ mutationFn: addTodo, - onSuccess: (data, variables, onMutateResult, context) => { + onSuccess: (data, variables, context) => { // Will be called 3 times }, })) const todos = ['Todo 1', 'Todo 2', 'Todo 3'] todos.forEach((todo) => { mutate(todo, { - onSuccess: (data, variables, onMutateResult, context) => { + onSuccess: (data, variables, context) => { // Will execute only once, for the last mutation (Todo 3), // regardless which mutation resolves first }, }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/mutations.md` around lines 173 - 190, Example7 uses the wrong onSuccess callback signature (it lists four args: data, variables, onMutateResult, context); update both the useMutation onSuccess and the per-call mutate onSuccess to the correct signature used by the library (data, variables, context) so the parameters match the actual implementation for useMutation, mutationFn, onSuccess and mutate.
143-168:⚠️ Potential issue | 🟠 MajorSame callback signature issue in Example6.
The callbacks here also use the incorrect
onMutateResultparameter pattern.🔧 Proposed fix
useMutation(() => ({ mutationFn: addTodo, - onSuccess: (data, variables, onMutateResult, context) => { + onSuccess: (data, variables, context) => { // I will fire first }, - onError: (error, variables, onMutateResult, context) => { + onError: (error, variables, context) => { // I will fire first }, - onSettled: (data, error, variables, onMutateResult, context) => { + onSettled: (data, error, variables, context) => { // I will fire first }, })) mutate(todo, { - onSuccess: (data, variables, onMutateResult, context) => { + onSuccess: (data, variables, context) => { // I will fire second! }, - onError: (error, variables, onMutateResult, context) => { + onError: (error, variables, context) => { // I will fire second! }, - onSettled: (data, error, variables, onMutateResult, context) => { + onSettled: (data, error, variables, context) => { // I will fire second! }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/mutations.md` around lines 143 - 168, The callback signatures in the useMutation and mutate examples are wrong — they include a spurious onMutateResult parameter; update the handlers for useMutation(() => ({ mutationFn: addTodo, onSuccess, onError, onSettled })) and the mutate(todo, { onSuccess, onError, onSettled }) call to use the correct parameter ordering: onSuccess should be (data, variables, context), onError should be (error, variables, context), and onSettled should be (data, error, variables, context), removing any onMutateResult parameter from all callback definitions.
221-255:⚠️ Potential issue | 🟠 MajorIncorrect callback signatures and context usage in Example10.
The
onMutatecallback has an incorrect signature (shouldn't acceptcontextparameter), and the other callbacks use the wrong parameter names. Additionally,context.clientshould be replaced with the outerqueryClientreference.🔧 Proposed fix
const queryClient = new QueryClient() // Define the "addTodo" mutation queryClient.setMutationDefaults(['addTodo'], { mutationFn: addTodo, - onMutate: async (variables, context) => { + onMutate: async (variables) => { // Cancel current queries for the todos list - await context.client.cancelQueries({ queryKey: ['todos'] }) + await queryClient.cancelQueries({ queryKey: ['todos'] }) // Create optimistic todo const optimisticTodo = { id: uuid(), title: variables.title } // Add optimistic todo to todos list - context.client.setQueryData(['todos'], (old) => [...old, optimisticTodo]) + queryClient.setQueryData(['todos'], (old) => [...old, optimisticTodo]) // Return a result with the optimistic todo return { optimisticTodo } }, - onSuccess: (result, variables, onMutateResult, context) => { + onSuccess: (result, variables, context) => { // Replace optimistic todo in the todos list with the result - context.client.setQueryData(['todos'], (old) => + queryClient.setQueryData(['todos'], (old) => old.map((todo) => - todo.id === onMutateResult.optimisticTodo.id ? result : todo, + todo.id === context.optimisticTodo.id ? result : todo, ), ) }, - onError: (error, variables, onMutateResult, context) => { + onError: (error, variables, context) => { // Remove optimistic todo from the todos list - context.client.setQueryData(['todos'], (old) => - old.filter((todo) => todo.id !== onMutateResult.optimisticTodo.id), + queryClient.setQueryData(['todos'], (old) => + old.filter((todo) => todo.id !== context.optimisticTodo.id), ) }, retry: 3, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/mutations.md` around lines 221 - 255, The example uses incorrect callback signatures and wrongly references context.client; fix setMutationDefaults on the QueryClient so onMutate has the signature onMutate = async (variables) => { await queryClient.cancelQueries({ queryKey: ['todos'] }); const optimisticTodo = { id: uuid(), title: variables.title }; queryClient.setQueryData(['todos'], (old) => [...old, optimisticTodo]); return { optimisticTodo }; } and update onSuccess and onError to use the correct parameters (onSuccess = (result, variables, context) => { queryClient.setQueryData(['todos'], (old) => old.map((todo) => todo.id === context.optimisticTodo.id ? result : todo)); } and onError = (error, variables, context) => { queryClient.setQueryData(['todos'], (old) => old.filter((todo) => todo.id !== context.optimisticTodo.id)); }); also keep mutationFn: addTodo and retry: 3 as before.docs/framework/solid/guides/prefetching.md (1)
216-253:⚠️ Potential issue | 🟠 MajorOutdated TanStack Router API - uses constructor-style instead of v1 functional API.
The example uses deprecated patterns (
new RouterContext(),routerContext.createRootRoute(),new Route()). TanStack Router v1 for Solid uses functional APIs:
- Use
createRootRoute()directly (orcreateRootRouteWithContext<MyContext>()for typed context)- Use
createRoute()for child routes- Build the tree with
rootRoute.addChildren([...])- Create router with
createRouter({ routeTree })TanStack Router v1 Solid createRoute createRootRoute API syntax🔧 Proposed fix
-const queryClient = new QueryClient() -const routerContext = new RouterContext() -const rootRoute = routerContext.createRootRoute({ - component: () => { ... } -}) - -const articleRoute = new Route({ - getParentRoute: () => rootRoute, - path: 'article', +const rootRoute = createRootRoute({ + component: () => <Outlet />, +}) + +const articleRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/article', beforeLoad: () => { return { articleQueryOptions: { queryKey: ['article'], queryFn: fetchArticle }, commentsQueryOptions: { queryKey: ['comments'], queryFn: fetchComments }, } }, loader: async ({ context: { queryClient }, routeContext: { articleQueryOptions, commentsQueryOptions }, }) => { // Fetch comments asap, but don't block queryClient.prefetchQuery(commentsQueryOptions) // Don't render the route at all until article has been fetched await queryClient.prefetchQuery(articleQueryOptions) }, component: ({ useRouteContext }) => { const { articleQueryOptions, commentsQueryOptions } = useRouteContext() const articleQuery = useQuery(() => articleQueryOptions) const commentsQuery = useQuery(() => commentsQueryOptions) return ( ... ) }, errorComponent: () => 'Oh crap!', }) + +const routeTree = rootRoute.addChildren([articleRoute]) +const router = createRouter({ routeTree })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/prefetching.md` around lines 216 - 253, Update the example to use TanStack Router v1 functional APIs instead of the deprecated constructors: replace RouterContext/new RouterContext and routerContext.createRootRoute() with createRootRoute() or createRootRouteWithContext<...>(), replace new Route({...}) with createRoute({...}), assemble children via rootRoute.addChildren([...]) and create the router with createRouter({ routeTree: rootRoute }) (or the v1 equivalent), while keeping the existing query-related symbols (queryClient.prefetchQuery, beforeLoad, loader, useRouteContext, articleQueryOptions, commentsQueryOptions) intact so the prefetching logic and component useQuery calls work the same under the new API.
🧹 Nitpick comments (1)
docs/framework/solid/guides/optimistic-updates.md (1)
67-67: Remove or use thequeryClientvariable.The
queryClientvariable is declared but never used in the example. The mutation callbacks access the client viacontext.clientinstead. Consider removing this line or clarifying its purpose if it's meant to demonstrate an alternative pattern.🧹 Proposed cleanup
-const queryClient = useQueryClient() - useMutation(() => ({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/solid/guides/optimistic-updates.md` at line 67, Remove the unused local variable by deleting the line that assigns queryClient via useQueryClient(), or if you intended to show the alternate pattern, update the mutation callbacks to use the local queryClient instead of context.client and add a brief comment explaining the alternative; specifically address the const queryClient = useQueryClient() declaration and either remove it or replace usages of context.client inside the mutation callback functions with queryClient so the example is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/framework/react/guides/important-defaults.md`:
- Line 41: Update the documentation text to hyphenate the compound modifier;
replace the phrase "non-JSON compatible values" with "non-JSON-compatible
values" in the paragraph that references structural sharing and the
config.structuralSharing option so the wording reads correctly (retain
surrounding context about providing a custom function for
config.structuralSharing).
In `@docs/framework/solid/guides/dependent-queries.md`:
- Line 23: The current enabled flag uses a truthy coercion (!!userId()) which
will disable the query when userId() returns 0; change the guard to a nullish
check so numeric IDs like 0 are allowed — e.g., replace the coercion with a
nullish comparison such as userId() != null (or userId() !== null && userId()
!== undefined) for the enabled property that references userId(). Ensure you
update the enabled assignment where it's defined to use this nullish check.
In `@docs/framework/solid/guides/placeholder-query-data.md`:
- Line 22: The heading "Placeholder Data Memoization" is at h3 (###) but should
be incremented from an h2; change the heading token from "### Placeholder Data
Memoization" to "## Placeholder Data Memoization" so the document uses
sequential heading levels and complies with Markdown conventions.
---
Duplicate comments:
In `@docs/framework/solid/guides/mutations.md`:
- Line 275: The heading "Persisting Offline mutations" is written as an h3 (###)
but there is no preceding h2; change the header to an h2 (## Persisting Offline
mutations) or insert an appropriate h2 above it so heading levels increment
correctly per MD001; update the header token for the "Persisting Offline
mutations" line in docs/framework/solid/guides/mutations.md (look for the exact
heading text) to restore proper heading hierarchy.
- Around line 106-122: The callback signatures in the mutation example are
incorrect: update the handlers so they match the correct Solid Query signatures
— keep onMutate: (variables, context) => { return { id: 1 } }, change onError to
onError: (error, variables, context) => { console.log(`rolling back optimistic
update with id ${context?.id}`) }, change onSuccess to onSuccess: (data,
variables, context) => { /* ... */ }, and change onSettled to onSettled: (data,
error, variables, context) => { /* ... */ }; remove the extra onMutateResult
parameter and read the onMutate result from context in
onError/onSuccess/onSettled.
- Around line 173-190: Example7 uses the wrong onSuccess callback signature (it
lists four args: data, variables, onMutateResult, context); update both the
useMutation onSuccess and the per-call mutate onSuccess to the correct signature
used by the library (data, variables, context) so the parameters match the
actual implementation for useMutation, mutationFn, onSuccess and mutate.
- Around line 143-168: The callback signatures in the useMutation and mutate
examples are wrong — they include a spurious onMutateResult parameter; update
the handlers for useMutation(() => ({ mutationFn: addTodo, onSuccess, onError,
onSettled })) and the mutate(todo, { onSuccess, onError, onSettled }) call to
use the correct parameter ordering: onSuccess should be (data, variables,
context), onError should be (error, variables, context), and onSettled should be
(data, error, variables, context), removing any onMutateResult parameter from
all callback definitions.
- Around line 221-255: The example uses incorrect callback signatures and
wrongly references context.client; fix setMutationDefaults on the QueryClient so
onMutate has the signature onMutate = async (variables) => { await
queryClient.cancelQueries({ queryKey: ['todos'] }); const optimisticTodo = { id:
uuid(), title: variables.title }; queryClient.setQueryData(['todos'], (old) =>
[...old, optimisticTodo]); return { optimisticTodo }; } and update onSuccess and
onError to use the correct parameters (onSuccess = (result, variables, context)
=> { queryClient.setQueryData(['todos'], (old) => old.map((todo) => todo.id ===
context.optimisticTodo.id ? result : todo)); } and onError = (error, variables,
context) => { queryClient.setQueryData(['todos'], (old) => old.filter((todo) =>
todo.id !== context.optimisticTodo.id)); }); also keep mutationFn: addTodo and
retry: 3 as before.
In `@docs/framework/solid/guides/prefetching.md`:
- Around line 216-253: Update the example to use TanStack Router v1 functional
APIs instead of the deprecated constructors: replace RouterContext/new
RouterContext and routerContext.createRootRoute() with createRootRoute() or
createRootRouteWithContext<...>(), replace new Route({...}) with
createRoute({...}), assemble children via rootRoute.addChildren([...]) and
create the router with createRouter({ routeTree: rootRoute }) (or the v1
equivalent), while keeping the existing query-related symbols
(queryClient.prefetchQuery, beforeLoad, loader, useRouteContext,
articleQueryOptions, commentsQueryOptions) intact so the prefetching logic and
component useQuery calls work the same under the new API.
In `@docs/framework/solid/guides/query-cancellation.md`:
- Around line 111-120: The queryFn passed to useQuery (in todosQuery) forgets to
return the promise from client.request, so the query resolves to undefined;
update the queryFn to return client.request({ document: query, signal }) (i.e.,
ensure the function body of queryFn returns the client.request promise) so
useQuery receives the fetched data from GraphQLClient.request.
---
Nitpick comments:
In `@docs/framework/solid/guides/optimistic-updates.md`:
- Line 67: Remove the unused local variable by deleting the line that assigns
queryClient via useQueryClient(), or if you intended to show the alternate
pattern, update the mutation callbacks to use the local queryClient instead of
context.client and add a brief comment explaining the alternative; specifically
address the const queryClient = useQueryClient() declaration and either remove
it or replace usages of context.client inside the mutation callback functions
with queryClient so the example is consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/solid/offline/src/assets/favicon.icois excluded by!**/*.icopnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
docs/config.jsondocs/framework/react/guides/caching.mddocs/framework/react/guides/important-defaults.mddocs/framework/react/guides/mutations.mddocs/framework/react/guides/parallel-queries.mddocs/framework/react/guides/queries.mddocs/framework/react/guides/query-cancellation.mddocs/framework/react/guides/query-options.mddocs/framework/react/guides/request-waterfalls.mddocs/framework/solid/guides/background-fetching-indicators.mddocs/framework/solid/guides/caching.mddocs/framework/solid/guides/default-query-function.mddocs/framework/solid/guides/dependent-queries.mddocs/framework/solid/guides/disabling-queries.mddocs/framework/solid/guides/important-defaults.mddocs/framework/solid/guides/infinite-queries.mddocs/framework/solid/guides/initial-query-data.mddocs/framework/solid/guides/invalidations-from-mutations.mddocs/framework/solid/guides/mutations.mddocs/framework/solid/guides/optimistic-updates.mddocs/framework/solid/guides/paginated-queries.mddocs/framework/solid/guides/parallel-queries.mddocs/framework/solid/guides/placeholder-query-data.mddocs/framework/solid/guides/prefetching.mddocs/framework/solid/guides/queries.mddocs/framework/solid/guides/query-cancellation.mddocs/framework/solid/guides/query-functions.mddocs/framework/solid/guides/query-invalidation.mddocs/framework/solid/guides/query-keys.mddocs/framework/solid/guides/query-options.mddocs/framework/solid/guides/request-waterfalls.mddocs/framework/solid/guides/scroll-restoration.mddocs/framework/solid/guides/suspense.mddocs/framework/solid/guides/updates-from-mutation-responses.mddocs/framework/solid/guides/window-focus-refetching.mdexamples/solid/offline/.eslintrc.cjsexamples/solid/offline/README.mdexamples/solid/offline/index.htmlexamples/solid/offline/package.jsonexamples/solid/offline/src/App.tsxexamples/solid/offline/src/api.tsexamples/solid/offline/src/index.tsxexamples/solid/offline/src/movies.tsexamples/solid/offline/tsconfig.jsonexamples/solid/offline/vite.config.ts
✅ Files skipped from review due to trivial changes (3)
- examples/solid/offline/index.html
- docs/framework/react/guides/caching.md
- docs/framework/solid/guides/parallel-queries.md
🚧 Files skipped from review as they are similar to previous changes (19)
- docs/framework/solid/guides/background-fetching-indicators.md
- examples/solid/offline/package.json
- examples/solid/offline/src/index.tsx
- docs/framework/solid/guides/initial-query-data.md
- docs/framework/react/guides/query-cancellation.md
- docs/framework/solid/guides/query-options.md
- docs/framework/solid/guides/window-focus-refetching.md
- docs/framework/solid/guides/default-query-function.md
- docs/framework/solid/guides/invalidations-from-mutations.md
- examples/solid/offline/src/api.ts
- docs/framework/solid/guides/query-invalidation.md
- examples/solid/offline/README.md
- docs/config.json
- examples/solid/offline/vite.config.ts
- examples/solid/offline/src/App.tsx
- examples/solid/offline/.eslintrc.cjs
- docs/framework/react/guides/parallel-queries.md
- docs/framework/solid/guides/suspense.md
- docs/framework/solid/guides/paginated-queries.md
| > Structural sharing only works with JSON-compatible values, any other value types will always be considered as changed. If you are seeing performance issues because of large responses for example, you can disable this feature with the `config.structuralSharing` flag. If you are dealing with non-JSON compatible values in your query responses and still want to detect if data has changed or not, you can provide your own custom function as `config.structuralSharing` to compute a value from the old and new responses, retaining references as required. | ||
| [//]: # 'StructuralSharing' | ||
|
|
||
| > Structural sharing only works with JSON-compatible values, any other value types will always be considered as changed. If you are seeing performance issues because of large responses for example, you can disable this feature with the `config.structuralSharing` flag. If you are dealing with non-JSON compatible values in your query responses and still want to detect if data has changed or not, you can provide your own custom function as `config.structuralSharing` to compute a value from the old and new responses, retaining references as required. |
Contributor
There was a problem hiding this comment.
Hyphenate compound modifier in docs text.
On Line 41, “non-JSON compatible values” should be “non-JSON-compatible values” for correct grammar and consistency.
✏️ Proposed wording tweak
-> Structural sharing only works with JSON-compatible values, any other value types will always be considered as changed. If you are seeing performance issues because of large responses for example, you can disable this feature with the `config.structuralSharing` flag. If you are dealing with non-JSON compatible values in your query responses and still want to detect if data has changed or not, you can provide your own custom function as `config.structuralSharing` to compute a value from the old and new responses, retaining references as required.
+> Structural sharing only works with JSON-compatible values, any other value types will always be considered as changed. If you are seeing performance issues because of large responses for example, you can disable this feature with the `config.structuralSharing` flag. If you are dealing with non-JSON-compatible values in your query responses and still want to detect if data has changed or not, you can provide your own custom function as `config.structuralSharing` to compute a value from the old and new responses, retaining references as required.📝 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.
Suggested change
| > Structural sharing only works with JSON-compatible values, any other value types will always be considered as changed. If you are seeing performance issues because of large responses for example, you can disable this feature with the `config.structuralSharing` flag. If you are dealing with non-JSON compatible values in your query responses and still want to detect if data has changed or not, you can provide your own custom function as `config.structuralSharing` to compute a value from the old and new responses, retaining references as required. | |
| > Structural sharing only works with JSON-compatible values, any other value types will always be considered as changed. If you are seeing performance issues because of large responses for example, you can disable this feature with the `config.structuralSharing` flag. If you are dealing with non-JSON-compatible values in your query responses and still want to detect if data has changed or not, you can provide your own custom function as `config.structuralSharing` to compute a value from the old and new responses, retaining references as required. |
🧰 Tools
🪛 LanguageTool
[grammar] ~41-~41: Use a hyphen to join words.
Context: ...` flag. If you are dealing with non-JSON compatible values in your query response...
(QB_NEW_EN_HYPHEN)
[style] ~41-~41: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...uery responses and still want to detect if data has changed or not, you can provid...
(IF_WHETHER)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/react/guides/important-defaults.md` at line 41, Update the
documentation text to hyphenate the compound modifier; replace the phrase
"non-JSON compatible values" with "non-JSON-compatible values" in the paragraph
that references structural sharing and the config.structuralSharing option so
the wording reads correctly (retain surrounding context about providing a custom
function for config.structuralSharing).
| queryKey: ['projects', userId()], | ||
| queryFn: getProjectsByUser, | ||
| // The query will not execute until the userId exists | ||
| enabled: !!userId(), |
Contributor
There was a problem hiding this comment.
Use a nullish check for enabled instead of truthiness.
enabled: !!userId() will incorrectly block execution when a valid ID is 0. Prefer a nullish guard.
Suggested fix
- enabled: !!userId(),
+ enabled: userId() != null,📝 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.
Suggested change
| enabled: !!userId(), | |
| enabled: userId() != null, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/solid/guides/dependent-queries.md` at line 23, The current
enabled flag uses a truthy coercion (!!userId()) which will disable the query
when userId() returns 0; change the guard to a nullish check so numeric IDs like
0 are allowed — e.g., replace the coercion with a nullish comparison such as
userId() != null (or userId() !== null && userId() !== undefined) for the
enabled property that references userId(). Ensure you update the enabled
assignment where it's defined to use this nullish check.
| [//]: # 'ExampleValue' | ||
| [//]: # 'Memoization' | ||
|
|
||
| ### Placeholder Data Memoization |
Contributor
There was a problem hiding this comment.
Fix heading level to comply with markdown conventions.
The heading jumps directly to h3 (###) without a preceding h2 (##). Markdown headings should increment by one level at a time.
📝 Proposed fix
-### Placeholder Data Memoization
+## Placeholder Data Memoization📝 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.
Suggested change
| ### Placeholder Data Memoization | |
| ## Placeholder Data Memoization |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 22-22: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/framework/solid/guides/placeholder-query-data.md` at line 22, The
heading "Placeholder Data Memoization" is at h3 (###) but should be incremented
from an h2; change the heading token from "### Placeholder Data Memoization" to
"## Placeholder Data Memoization" so the document uses sequential heading levels
and complies with Markdown conventions.
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
2 participants
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.
🎯 Changes
ref-based withreplacerules and marker overridescreateSignal,Show/For/Switch, no destructuring)✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Documentation
New Examples
Chore