FetchQueries and FetchInfiniteQueries#89
Conversation
📝 WalkthroughWalkthroughAdds reactive ChangesFetch query support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FetchQuery
participant QueryClient
participant makeFetchRequest
participant Fetch
FetchQuery->>QueryClient: resolve fetchQueries defaults
FetchQuery->>makeFetchRequest: pass fetch params and signal
makeFetchRequest->>Fetch: issue HTTP request
Fetch-->>makeFetchRequest: return response
makeFetchRequest-->>FetchQuery: return parsed or transformed data
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
src/preset/create-fetch-query.ts (1)
31-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting shared factory logic to reduce duplication.
The implementation of
createFetchQuery(lines 31-50) is nearly identical tocreateFetchInfiniteQuery(lines 80-99) — same args-length dispatch, same function-vs-object wrapping, samemountQueryClientOnce(getQueryClient(...))pattern. A shared helper could eliminate this duplication and ensure both factories stay in sync as the pattern evolves.♻️ Optional: shared factory helper
+// src/preset/create-query-factory.ts +import { + type AnyQueryClient, + getQueryClient, + mountQueryClientOnce, +} from 'mobx-tanstack-query'; +import { queryClient } from './query-client.js'; + +export function createQueryFactory<T extends new (...args: any[]) => any>( + QueryClass: T, +): (...args: [any, any?]) => InstanceType<T> { + return (...args) => { + let instance: InstanceType<T>; + if (args.length === 2) { + instance = new QueryClass( + args[0], + typeof args[1] === 'function' ? args[1] : () => args[1], + ); + } else { + const options = args[0]; + instance = new QueryClass({ + ...options, + queryClient: options.queryClient ?? queryClient, + }); + } + mountQueryClientOnce(getQueryClient(instance)); + return instance; + }; +}Then each factory becomes a one-liner delegation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/preset/create-fetch-query.ts` around lines 31 - 50, Extract the shared argument-dispatch and query-client mounting logic from createFetchQuery and createFetchInfiniteQuery into a reusable factory helper. Preserve each factory’s existing FetchQuery/FetchInfiniteQuery construction behavior, including function-versus-object wrapping and default queryClient handling, then delegate both factories through the helper so the mountQueryClientOnce(getQueryClient(...)) flow is centralized.src/preset/create-fetch-query.test.ts (1)
64-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication between "returns FetchQuery instance from options overload" and "uses preset queryClient when not provided".
Lines 64-72 and 85-94 are functionally identical — both create a query without an explicit queryClient, assert
instanceof FetchQuery, and calldestroy(). Consider consolidating or making the second test verify the preset client is actually used rather than just checking the instance type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/preset/create-fetch-query.test.ts` around lines 64 - 107, Remove the redundant “uses preset queryClient when not provided” coverage or change it to assert that the preset queryClient is actually used, while retaining the instance and cleanup assertions in “returns FetchQuery instance from options overload.”src/preset/create-fetch-infinite-query.test.ts (1)
96-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication between "returns FetchInfiniteQuery instance from options overload" and "uses preset queryClient when not provided".
Lines 96-106 and 121-131 both create a query without an explicit queryClient, assert
instanceof FetchInfiniteQuery, and calldestroy(). They are functionally identical. Consider consolidating or differentiating the second test to verify the preset client is actually used (e.g., by checkingquery.options.queryClientor similar).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/preset/create-fetch-infinite-query.test.ts` around lines 96 - 146, Remove the duplicate coverage between “returns FetchInfiniteQuery instance from options overload” and “uses preset queryClient when not provided” by changing the latter to assert that the preset query client is actually selected. Keep the instance assertion in the overload test, and use the query’s exposed options or equivalent client reference in the preset-client test before destroying the query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/version-or-publish.yml:
- Around line 49-61: Add NODE_AUTH_TOKEN to the env configuration of the
changesets/action step identified by id pub-or-release-pr, sourcing it from the
repository’s npm authentication secret. Preserve the existing GITHUB_TOKEN and
publishing configuration so pnpm pub can authenticate with the npm registry.
In `@docs/api/FetchQuery.md`:
- Around line 79-80: Update the FetchQuery documentation’s headers option to use
the complete Record type arguments, changing it to Record<string, string>;
retain Record<string, unknown> for object request bodies.
- Line 34: Update the petId observable declaration to initialize the MobX box
with a value, using undefined with a string-or-undefined type or an appropriate
concrete pet ID. Preserve the existing petId usage and documentation context.
In `@docs/api/QueryClient.md`:
- Line 101: Update the headers option type in the QueryClient API documentation
from Record<string> to Record<string, string>, keeping the existing description
and behavior unchanged.
In `@docs/errors/2.md`:
- Around line 37-40: Update the params callback to serialize the cursor in the
request, using the existing cursor value as a query parameter or interpolating
it into the path; do not return it as pageParam, which is only the callback
argument. Preserve the null return when no cursor is available.
In `@src/fetch-infinite-query.ts`:
- Around line 36-70: Unify the page-data type in FetchInfiniteQueryConfig and
the related fetchInfiniteQuery flow: make the pagination callbacks,
InfiniteQuery configuration, and select input use the actual makeFetchRequest
result type, TOutputData | null, instead of the independent TQueryFnData type.
Remove the incompatible output generic or propagate the unified type through the
parent query configuration, including the implementations referenced around the
additional affected ranges.
- Around line 73-93: Update FetchInfiniteQueryPositionalConfig to redeclare
initialPageParam, getNextPageParam, and getPreviousPageParam after omitting them
from InfiniteQueryConfig, using the same types and optionality as the
corresponding pagination options. Keep the existing params and select
definitions unchanged so positional overloads accept pagination configuration.
In `@src/fetch-query.ts`:
- Around line 38-47: Include throwOnError in FetchQueryKeyMeta and update
parseFetchQueryKey() to reconstruct it from the query key, preserving the
query-specified value instead of falling back to the client default. Apply the
same round-trip change to the infinite-query key handling identified by the
related ranges.
- Around line 31-34: Update the select/transform callback contract in
fetch-query.ts so its input type includes null when notFoundAsNull is enabled,
matching makeFetchRequest()’s reachable result. Propagate this nullable type
through the option definition and the related callback invocation/type
declarations at the referenced sections, while preserving non-null typing when
the option is disabled.
- Around line 121-140: Update buildFetchQueryKey and buildFetchInfiniteQueryKey
to avoid placing raw BodyInit values in query metadata. Derive and store a
stable bodyKey by serializing supported body types explicitly, ensuring distinct
FormData, Blob, ReadableStream, URLSearchParams, ArrayBuffer, and similar
payloads produce distinct TanStack cache keys; preserve the existing key
structure and request behavior.
In `@src/utils/make-fetch-request.ts`:
- Around line 84-90: Update the non-OK response handling in makeFetchRequest so
throwOnError raises the advertised exported DefaultError (or a suitable exported
Error subclass) instead of throwing the raw Response, while preserving the
response status/details on the error for callers that need them. Keep the
notFoundAsNull 404 behavior unchanged and ensure the public error type matches
the thrown value.
- Around line 60-68: Update the timeout handling around
abortControllerWithTimeout so an already-aborted signal is propagated
immediately before issuing the request. Check the incoming signal state and
abort the combined controller when necessary, while preserving the existing
abort listeners and timeout behavior for signals that are not yet aborted.
---
Nitpick comments:
In `@src/preset/create-fetch-infinite-query.test.ts`:
- Around line 96-146: Remove the duplicate coverage between “returns
FetchInfiniteQuery instance from options overload” and “uses preset queryClient
when not provided” by changing the latter to assert that the preset query client
is actually selected. Keep the instance assertion in the overload test, and use
the query’s exposed options or equivalent client reference in the preset-client
test before destroying the query.
In `@src/preset/create-fetch-query.test.ts`:
- Around line 64-107: Remove the redundant “uses preset queryClient when not
provided” coverage or change it to assert that the preset queryClient is
actually used, while retaining the instance and cleanup assertions in “returns
FetchQuery instance from options overload.”
In `@src/preset/create-fetch-query.ts`:
- Around line 31-50: Extract the shared argument-dispatch and query-client
mounting logic from createFetchQuery and createFetchInfiniteQuery into a
reusable factory helper. Preserve each factory’s existing
FetchQuery/FetchInfiniteQuery construction behavior, including
function-versus-object wrapping and default queryClient handling, then delegate
both factories through the helper so the
mountQueryClientOnce(getQueryClient(...)) flow is centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d23dff83-a471-489f-9a40-2bcabf265ffb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
.changeset/wet-turtles-relax.md.github/workflows/version-or-publish.ymldocs/.vitepress/config.mtsdocs/api/FetchInfiniteQuery.mddocs/api/FetchQuery.mddocs/api/QueryClient.mddocs/api/other.mddocs/errors/1.mddocs/errors/2.mddocs/preset/createFetchInfiniteQuery.mddocs/preset/createFetchQuery.mddocs/preset/index.mdpackage.jsonscripts/prepare-dist.tssrc/fetch-infinite-query.tssrc/fetch-query.test.tssrc/fetch-query.tssrc/index.tssrc/inifinite-query.types.tssrc/preset/create-fetch-infinite-query.test.tssrc/preset/create-fetch-infinite-query.tssrc/preset/create-fetch-query.test.tssrc/preset/create-fetch-query.tssrc/preset/index.tssrc/query-client.tssrc/query-client.types.tssrc/utils/make-fetch-request.tstsconfig.jsontsconfig.test.json
| - run: npm install -g npm@latest | ||
|
|
||
| - name: Create Release Pull Request | ||
| uses: changesets/action@v1 | ||
| continue-on-error: false | ||
| id: pub-or-release-pr | ||
| with: | ||
| version: pnpm changeset version | ||
| publish: pnpm pub | ||
| createGithubReleases: false | ||
| env: | ||
| CI: true | ||
| GITHUB_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,140p' .github/workflows/version-or-publish.yml && printf '\n---\n' && rg -n "NODE_AUTH_TOKEN|NPM_TOKEN|registry-url|changesets/action|pnpm pub" .github/workflows . -g '!**/node_modules/**'Repository: js2me/mobx-tanstack-query
Length of output: 2586
🏁 Script executed:
sed -n '1,220p' package.json && printf '\n---\n' && fd -a 'config.json' .changeset .github . && printf '\n---\n' && rg -n "\"pub\"|changesets|publishConfig|npmrc|NODE_AUTH_TOKEN|NPM_TOKEN" package.json .changeset .github -g '!**/node_modules/**'Repository: js2me/mobx-tanstack-query
Length of output: 3817
🌐 Web query:
actions/setup-node registry-url NODE_AUTH_TOKEN authenticated npm publish documentation
💡 Result:
To authenticate and publish an npm package using GitHub Actions, you use the actions/setup-node action to configure a local.npmrc file with your registry information and authentication token [1][2]. For classic token-based authentication: 1. Create a repository secret (e.g., NPM_TOKEN) in your GitHub repository containing your npm authentication token [1][2]. 2. In your workflow YAML, use actions/setup-node with the registry-url input set to the registry (e.g., https://registry.npmjs.org/) [1][2]. 3. Provide the secret to the action via the NODE_AUTH_TOKEN environment variable in the step that runs npm publish [1][2]. Example workflow configuration: - uses: actions/setup-node@v4 with: node-version: '20.x' registry-url: 'https://registry.npmjs.org' - run: npm publish env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} When the registry-url is set, actions/setup-node automatically creates a temporary.npmrc file that references the NODE_AUTH_TOKEN variable, allowing npm to authenticate during the publish process [1][2]. Note on Trusted Publishing (OIDC): Modern workflows can use npm's Trusted Publisher (OIDC) feature to publish without long-lived tokens [3]. If you are using OIDC, be aware that setting registry-url in actions/setup-node historically adds an _authToken placeholder to the.npmrc file [4]. While this is the standard setup for token-based auth, some users have reported issues with this configuration in OIDC-only environments [4]. Always ensure your workflow permissions include id-token: write when using OIDC [1][2].
Citations:
- 1: https://github.com/github/docs/blob/main/content/actions/tutorials/publish-packages/publish-nodejs-packages.md
- 2: https://docs.github.com/en/enterprise-cloud@latest/actions/tutorials/publish-packages/publish-nodejs-packages
- 3: https://docs.npmjs.com/trusted-publishers/
- 4: registry-url writes _authToken line that breaks npm Trusted Publisher OIDC when no NODE_AUTH_TOKEN is set actions/setup-node#1551
Add NODE_AUTH_TOKEN to the publish step. actions/setup-node expects it for npm auth when registry-url is set, but this job only passes GITHUB_TOKEN. pnpm pub will fail to publish unless the npm token is added back.
🔧 Proposed fix
env:
CI: true
GITHUB_TOKEN: ${{ github.token }}
+ NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}📝 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.
| - run: npm install -g npm@latest | |
| - name: Create Release Pull Request | |
| uses: changesets/action@v1 | |
| continue-on-error: false | |
| id: pub-or-release-pr | |
| with: | |
| version: pnpm changeset version | |
| publish: pnpm pub | |
| createGithubReleases: false | |
| env: | |
| CI: true | |
| GITHUB_TOKEN: ${{ github.token }} | |
| - run: npm install -g npm@latest | |
| - name: Create Release Pull Request | |
| uses: changesets/action@v1 | |
| continue-on-error: false | |
| id: pub-or-release-pr | |
| with: | |
| version: pnpm changeset version | |
| publish: pnpm pub | |
| createGithubReleases: false | |
| env: | |
| CI: true | |
| GITHUB_TOKEN: ${{ github.token }} | |
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 49-49: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/version-or-publish.yml around lines 49 - 61, Add
NODE_AUTH_TOKEN to the env configuration of the changesets/action step
identified by id pub-or-release-pr, sourcing it from the repository’s npm
authentication secret. Preserve the existing GITHUB_TOKEN and publishing
configuration so pnpm pub can authenticate with the npm registry.
Source: Linters/SAST tools
| `params` can be a function for dynamic queries. When `params` returns a falsy value, the query is automatically disabled. | ||
|
|
||
| ```ts | ||
| const petId = observable.box<string>(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- docs/api/FetchQuery.md (around the cited lines) ---\n'
sed -n '1,140p' docs/api/FetchQuery.md | cat -n
printf '\n--- package references to mobx ---\n'
rg -n '"mobx"|observable\.box|FetchQuery' package.json . -g 'package.json' -g '*.md' -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Repository: js2me/mobx-tanstack-query
Length of output: 254
🌐 Web query:
MobX observable.box requires initial value TypeScript API
💡 Result:
In MobX, observable.box(value, options?) requires an initial value to be provided [1][2]. While earlier versions or certain type definitions may have permitted omitting the argument, providing an initial value is standard practice to ensure type safety and prevent runtime errors [3]. If you attempt to use observable.box without an argument, it may lead to TypeScript errors or runtime issues when you subsequently call.get on the empty box [3]. To correctly initialize a box with no specific value, you should explicitly pass undefined or null: const myBox = observable.box(undefined); // Correct initialization [3] myBox.set("new value"); // Safely update later The observable.box API is used to create an observable reference to a value, primarily for primitive values that are not owned by an object [2]. Once created, you interact with the value using the.get and.set(newValue) methods [1][2].
Citations:
- 1: https://mobx.js.org/api.html
- 2: https://github.com/mobxjs/mobx/blob/main/docs/api.md
- 3: Remove observable.box type inconsistancy mobxjs/mobx#3409
Initialize the MobX box with a value. observable.box<string>() needs an initial value, so use observable.box<string | undefined>(undefined) or a concrete pet ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api/FetchQuery.md` at line 34, Update the petId observable declaration
to initialize the MobX box with a value, using undefined with a
string-or-undefined type or an appropriate concrete pet ID. Preserve the
existing petId usage and documentation context.
| - `headers?: Record<string>` — Request headers. Merged with global headers from `QueryClient.fetchQueries` | ||
| - `body?: BodyInit | Record<string, unknown>` — Request body. Objects are JSON-serialized with `Content-Type: application/json`; native `BodyInit` (Blob, FormData, etc.) is passed through as-is |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section with line numbers and a bit of surrounding context.
sed -n '60,95p' docs/api/FetchQuery.md | cat -n
# Also locate any similar Record typings elsewhere in the docs to compare conventions.
rg -n "Record<string" docs -SRepository: js2me/mobx-tanstack-query
Length of output: 3140
Use complete Record type arguments. Record<string> is invalid TypeScript; document headers as Record<string, string> and keep object bodies as Record<string, unknown>.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api/FetchQuery.md` around lines 79 - 80, Update the FetchQuery
documentation’s headers option to use the complete Record type arguments,
changing it to Record<string, string>; retain Record<string, unknown> for object
request bodies.
| Global configuration for [`FetchQuery`](/api/FetchQuery) and [`FetchInfiniteQuery`](/api/FetchInfiniteQuery). These defaults are merged with per-request `params` in every fetch query. | ||
|
|
||
| - `baseUrl?: string` — Base URL prepended to every `path` | ||
| - `headers?: Record<string>` — Default headers merged with per-request headers |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the documented line and nearby context
sed -n '85,120p' docs/api/QueryClient.md
# Search for the same header type pattern elsewhere in docs and source
rg -n "Record<string>" docs src . --glob '!**/node_modules/**' || true
# Find the runtime/query option contract that this doc refers to
rg -n "headers\\?:|Default headers|per-request headers|headers" docs src --glob '!**/node_modules/**' || trueRepository: js2me/mobx-tanstack-query
Length of output: 7341
Use a complete header type here.
Record<string> is incomplete; use Record<string, string> so the header value type is explicit and matches the contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api/QueryClient.md` at line 101, Update the headers option type in the
QueryClient API documentation from Record<string> to Record<string, string>,
keeping the existing description and behavior unchanged.
| params: () => { | ||
| if (!cursor) return null; // ✅ disable when no cursor | ||
| return { path: '/api/users', pageParam: cursor }; | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Serialize cursor into the request.
pageParam is the callback argument, not a request field. This example returns the cursor without using it in the URL, so pagination can repeatedly fetch the same page. Use query: { cursor } or interpolate cursor into path, as shown in createFetchInfiniteQuery.md.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/errors/2.md` around lines 37 - 40, Update the params callback to
serialize the cursor in the request, using the existing cursor value as a query
parameter or interpolating it into the path; do not return it as pageParam,
which is only the callback argument. Preserve the null return when no cursor is
available.
| notFoundAsNull?: boolean; | ||
| responseType?: ResponseType; | ||
| transformResponse?: TransformResponseFn<TOutputData>; | ||
| throwOnError?: boolean; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include the reachable null result in the select contract.
With notFoundAsNull, makeFetchRequest() returns null, but select is typed as receiving only TOutputData. A valid 404 can therefore crash a callback that TypeScript considered safe.
- select?: (data: TOutputData) => TData;
+ select?: (data: TOutputData | null) => TData;Also applies to: 56-64, 234-239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fetch-query.ts` around lines 31 - 34, Update the select/transform
callback contract in fetch-query.ts so its input type includes null when
notFoundAsNull is enabled, matching makeFetchRequest()’s reachable result.
Propagate this nullable type through the option definition and the related
callback invocation/type declarations at the referenced sections, while
preserving non-null typing when the option is disabled.
| export type FetchQueryKeyMeta = Pick< | ||
| FetchQueryParams, | ||
| | 'baseUrl' | ||
| | 'method' | ||
| | 'headers' | ||
| | 'body' | ||
| | 'notFoundAsNull' | ||
| | 'responseType' | ||
| | 'request' | ||
| >; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve throwOnError through the query-key round trip.
parseFetchQueryKey() never reconstructs this option, so every request falls back to the client default even when the query specifies throwOnError.
Proposed fix
export type FetchQueryKeyMeta = Pick<
FetchQueryParams,
| 'baseUrl'
| 'method'
| 'headers'
| 'body'
| 'notFoundAsNull'
| 'responseType'
| 'request'
+ | 'throwOnError'
>;
// buildFetchQueryKey
+throwOnError: fetchParams.throwOnError,
// parseFetchQueryKey
+throwOnError: meta.throwOnError,Apply the same round-trip fix to the infinite-query key.
Also applies to: 131-139, 156-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fetch-query.ts` around lines 38 - 47, Include throwOnError in
FetchQueryKeyMeta and update parseFetchQueryKey() to reconstruct it from the
query key, preserving the query-specified value instead of falling back to the
client default. Apply the same round-trip change to the infinite-query key
handling identified by the related ranges.
| export const buildFetchQueryKey = ( | ||
| fetchParams: FetchQueryParams | null, | ||
| ): FetchQueryKey => { | ||
| if (!fetchParams) { | ||
| return [null]; | ||
| } | ||
|
|
||
| return [ | ||
| ...fetchParams.path.split('/'), | ||
| serializeFetchQuery(fetchParams.query) as any, | ||
| { | ||
| baseUrl: fetchParams.baseUrl, | ||
| method: fetchParams.method, | ||
| headers: fetchParams.headers, | ||
| body: fetchParams.body as any, | ||
| notFoundAsNull: fetchParams.notFoundAsNull, | ||
| responseType: fetchParams.responseType, | ||
| request: fetchParams.request, | ||
| } satisfies FetchQueryKeyMeta, | ||
| ]; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
tarball="$(
curl -fsSL https://registry.npmjs.org/@tanstack/query-core/5.90.2 |
jq -r '.dist.tarball'
)"
curl -fsSL "$tarball" | tar -xz -C "$tmp"
rg -n -A12 -B3 'function hashKey|JSON\.stringify' "$tmp/package"Repository: js2me/mobx-tanstack-query
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg '^src/(fetch-query|.*fetch.*)\.ts$|^src/.*\.ts$'Repository: js2me/mobx-tanstack-query
Length of output: 1200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- src/fetch-query.ts outline ---\n'
ast-grep outline src/fetch-query.ts --view expanded
printf '\n--- relevant buildFetch* definitions ---\n'
rg -n -A30 -B10 'buildFetch(Query|InfiniteQuery)Key|body: fetchParams\.body|serializeFetchQuery|BodyInit' src/fetch-query.ts src -g '!**/*.map'Repository: js2me/mobx-tanstack-query
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
cases = [
("FormData", "new FormData()"),
("Blob", "new Blob(['x'])"),
("ReadableStream", "new ReadableStream({ start(c) { c.close() } })"),
("URLSearchParams", "new URLSearchParams('a=1')"),
]
for name, expr in cases:
# Use node for actual evaluation of JSON.stringify behavior
pass
PY
node - <<'JS'
const cases = [
['FormData', 'new FormData()'],
['Blob', "new Blob(['x'])"],
['ReadableStream', 'new ReadableStream({ start(c) { c.close() } })'],
['URLSearchParams', "new URLSearchParams('a=1')"],
['ArrayBuffer', 'new ArrayBuffer(8)'],
['Uint8Array', 'new Uint8Array([1,2,3])'],
];
for (const [name, expr] of cases) {
let v;
try { v = eval(expr); } catch (e) { v = e.toString(); }
let json;
try { json = JSON.stringify(v); } catch (e) { json = `THREW: ${e}`; }
console.log(name + ' => ' + json);
}
JSRepository: js2me/mobx-tanstack-query
Length of output: 284
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# inspect body typing and key builders
sed -n '1,240p' src/fetch-query.ts | cat -n
printf '\n--- body/key usages ---\n'
rg -n -A12 -B6 'buildFetchQueryKey|buildFetchInfiniteQueryKey|body:' src/fetch-query.ts src -g '!**/*.map'Repository: js2me/mobx-tanstack-query
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const cases = [
['FormData', 'new FormData()'],
['Blob', "new Blob(['x'])"],
['ReadableStream', 'new ReadableStream({ start(c) { c.close() } })'],
['URLSearchParams', "new URLSearchParams('a=1')"],
['ArrayBuffer', 'new ArrayBuffer(8)'],
['Uint8Array', 'new Uint8Array([1,2,3])'],
['Headers', "new Headers({a:'b'})"],
];
for (const [name, expr] of cases) {
let v;
try { v = eval(expr); } catch (e) { v = e.toString(); }
let json;
try { json = JSON.stringify(v); } catch (e) { json = `THREW: ${e}`; }
console.log(`${name}: ${json}`);
}
JSRepository: js2me/mobx-tanstack-query
Length of output: 284
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/fetch-query.ts (selected lines) ---'
sed -n '1,240p' src/fetch-query.ts | cat -n | sed -n '1,240p'
echo
echo '--- body/key references ---'
rg -n -A8 -B4 'buildFetch(Query|InfiniteQuery)Key|body: fetchParams\.body|serializeFetchQuery|type FetchQueryParams|interface FetchQueryParams|BodyInit' src/fetch-query.ts src -g '!**/*.map'
echo
echo '--- JSON.stringify behavior for candidate body types ---'
node - <<'JS'
const cases = [
['FormData', () => new FormData()],
['Blob', () => new Blob(['x'])],
['ReadableStream', () => new ReadableStream({ start(c) { c.close(); } })],
['URLSearchParams', () => new URLSearchParams('a=1')],
['ArrayBuffer', () => new ArrayBuffer(8)],
['Uint8Array', () => new Uint8Array([1,2,3])],
['Headers', () => new Headers({a:'b'})],
];
for (const [name, make] of cases) {
let v, json;
try { v = make(); } catch (e) { console.log(`${name}: construct threw ${e}`); continue; }
try { json = JSON.stringify(v); } catch (e) { json = `THREW: ${e}`; }
console.log(`${name}: ${json}`);
}
JSRepository: js2me/mobx-tanstack-query
Length of output: 31312
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A20 -B6 'type FetchQueryParams|interface FetchQueryParams|request:|body:|buildFetchInfiniteQueryKey|buildFetchQueryKey' src/fetch-query.tsRepository: js2me/mobx-tanstack-query
Length of output: 4677
Avoid raw BodyInit values in the query key. buildFetchQueryKey() and buildFetchInfiniteQueryKey() store body verbatim, but TanStack hashes keys with JSON.stringify; FormData, Blob, ReadableStream, URLSearchParams, ArrayBuffer, and similar values collapse to {} and can make distinct payloads share cache entries. Use a stable bodyKey or serialize supported body types explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fetch-query.ts` around lines 121 - 140, Update buildFetchQueryKey and
buildFetchInfiniteQueryKey to avoid placing raw BodyInit values in query
metadata. Derive and store a stable bodyKey by serializing supported body types
explicitly, ensuring distinct FormData, Blob, ReadableStream, URLSearchParams,
ArrayBuffer, and similar payloads produce distinct TanStack cache keys; preserve
the existing key structure and request behavior.
| if (clientFetchQueries.timeout) { | ||
| const abortControllerWithTimeout = new AbortController(); | ||
| const abortHandler = () => abortControllerWithTimeout.abort(); | ||
|
|
||
| signal.addEventListener('abort', abortHandler, { once: true }); | ||
|
|
||
| const timeoutSignal = AbortSignal.timeout(clientFetchQueries.timeout); | ||
| timeoutSignal.addEventListener('abort', abortHandler, { once: true }); | ||
| abortSignal = abortControllerWithTimeout.signal; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Propagate an already-aborted query signal.
Registering an abort listener does not fire when signal was aborted beforehand. With a timeout configured, such a cancelled query can still issue its request.
- signal.addEventListener('abort', abortHandler, { once: true });
+ if (signal.aborted) {
+ abortHandler();
+ } else {
+ signal.addEventListener('abort', abortHandler, { once: true });
+ }📝 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.
| if (clientFetchQueries.timeout) { | |
| const abortControllerWithTimeout = new AbortController(); | |
| const abortHandler = () => abortControllerWithTimeout.abort(); | |
| signal.addEventListener('abort', abortHandler, { once: true }); | |
| const timeoutSignal = AbortSignal.timeout(clientFetchQueries.timeout); | |
| timeoutSignal.addEventListener('abort', abortHandler, { once: true }); | |
| abortSignal = abortControllerWithTimeout.signal; | |
| if (clientFetchQueries.timeout) { | |
| const abortControllerWithTimeout = new AbortController(); | |
| const abortHandler = () => abortControllerWithTimeout.abort(); | |
| if (signal.aborted) { | |
| abortHandler(); | |
| } else { | |
| signal.addEventListener('abort', abortHandler, { once: true }); | |
| } | |
| const timeoutSignal = AbortSignal.timeout(clientFetchQueries.timeout); | |
| timeoutSignal.addEventListener('abort', abortHandler, { once: true }); | |
| abortSignal = abortControllerWithTimeout.signal; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/make-fetch-request.ts` around lines 60 - 68, Update the timeout
handling around abortControllerWithTimeout so an already-aborted signal is
propagated immediately before issuing the request. Check the incoming signal
state and abort the combined controller when necessary, while preserving the
existing abort listeners and timeout behavior for signals that are not yet
aborted.
| if (!response.ok) { | ||
| if (fetchParams.notFoundAsNull && response.status === 404) { | ||
| return null; | ||
| } | ||
|
|
||
| if (throwOnError) { | ||
| throw response; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align HTTP errors with the advertised error type.
The wrappers expose DefaultError, but this branch throws a Response, which has no Error.message or stack. Either throw an exported Error subclass carrying the response, or change the public error types to include Response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/make-fetch-request.ts` around lines 84 - 90, Update the non-OK
response handling in makeFetchRequest so throwOnError raises the advertised
exported DefaultError (or a suitable exported Error subclass) instead of
throwing the raw Response, while preserving the response status/details on the
error for callers that need them. Keep the notFoundAsNull 404 behavior unchanged
and ensure the public error type matches the thrown value.
There was a problem hiding this comment.
25 issues found across 30 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/fetch-query.ts">
<violation number="1" location="src/fetch-query.ts:38">
P1: Per-query `throwOnError` from `FetchQueryParams` is silently ignored because it's not stored in the query key and not captured via closure.
`FetchQueryKeyMeta` picks `baseUrl`, `method`, `headers`, `body`, `notFoundAsNull`, `responseType`, and `request` — but not `throwOnError`. When `queryFn` runs, `parseFetchQueryKey` reconstructs params from the key, so `throwOnError` is always `undefined`. Then `makeFetchRequest` resolves it as `undefined ?? clientFetchQueries.throwOnError ?? true`, always defaulting to `true` (or the client default).
This means setting `throwOnError: false` on a specific query's params has no effect — non-ok responses will always throw.
**Fix option A (simpler, matching existing `transformResponse` pattern):** Capture `throwOnError` via closure in the `FetchQuery` constructor alongside `transformResponse`, then pass it through to `makeFetchRequest`:
```ts
let throwOnError: boolean | undefined;
options: () => {
const fetchParams = callFunction(queryFullConfig.params) || null;
throwOnError = fetchParams?.throwOnError;
// ...
},
queryFn: async ({ signal, queryKey }) => {
// ...
return makeFetchRequest({
fetchParams: { ...fetchParams, throwOnError },
transformResponse,
queryClient: queryFullConfig.queryClient,
signal,
});
},
Fix option B: Add throwOnError to FetchQueryKeyMeta so it roundtrips through the query key alongside the other fields.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| meta?: QueryMeta; | ||
| }; | ||
|
|
||
| export type FetchQueryKeyMeta = Pick< |
There was a problem hiding this comment.
P1: Per-query throwOnError from FetchQueryParams is silently ignored because it's not stored in the query key and not captured via closure.
FetchQueryKeyMeta picks baseUrl, method, headers, body, notFoundAsNull, responseType, and request — but not throwOnError. When queryFn runs, parseFetchQueryKey reconstructs params from the key, so throwOnError is always undefined. Then makeFetchRequest resolves it as undefined ?? clientFetchQueries.throwOnError ?? true, always defaulting to true (or the client default).
This means setting throwOnError: false on a specific query's params has no effect — non-ok responses will always throw.
Fix option A (simpler, matching existing transformResponse pattern): Capture throwOnError via closure in the FetchQuery constructor alongside transformResponse, then pass it through to makeFetchRequest:
let throwOnError: boolean | undefined;
options: () => {
const fetchParams = callFunction(queryFullConfig.params) || null;
throwOnError = fetchParams?.throwOnError;
// ...
},
queryFn: async ({ signal, queryKey }) => {
// ...
return makeFetchRequest({
fetchParams: { ...fetchParams, throwOnError },
transformResponse,
queryClient: queryFullConfig.queryClient,
signal,
});
},Fix option B: Add throwOnError to FetchQueryKeyMeta so it roundtrips through the query key alongside the other fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/fetch-query.ts, line 38:
<comment>Per-query `throwOnError` from `FetchQueryParams` is silently ignored because it's not stored in the query key and not captured via closure.
`FetchQueryKeyMeta` picks `baseUrl`, `method`, `headers`, `body`, `notFoundAsNull`, `responseType`, and `request` — but not `throwOnError`. When `queryFn` runs, `parseFetchQueryKey` reconstructs params from the key, so `throwOnError` is always `undefined`. Then `makeFetchRequest` resolves it as `undefined ?? clientFetchQueries.throwOnError ?? true`, always defaulting to `true` (or the client default).
This means setting `throwOnError: false` on a specific query's params has no effect — non-ok responses will always throw.
**Fix option A (simpler, matching existing `transformResponse` pattern):** Capture `throwOnError` via closure in the `FetchQuery` constructor alongside `transformResponse`, then pass it through to `makeFetchRequest`:
```ts
let throwOnError: boolean | undefined;
options: () => {
const fetchParams = callFunction(queryFullConfig.params) || null;
throwOnError = fetchParams?.throwOnError;
// ...
},
queryFn: async ({ signal, queryKey }) => {
// ...
return makeFetchRequest({
fetchParams: { ...fetchParams, throwOnError },
transformResponse,
queryClient: queryFullConfig.queryClient,
signal,
});
},
Fix option B: Add throwOnError to FetchQueryKeyMeta so it roundtrips through the query key alongside the other fields.
| with: | ||
| version: pnpm changeset version | ||
| publish: pnpm pub | ||
| createGithubReleases: false |
There was a problem hiding this comment.
P1: The NPM_TOKEN and NPM_AUTH_TOKEN environment variables were removed from the changesets/action step, but no alternative npm authentication was provided. The setup-node step adds registry-url but does not set NODE_AUTH_TOKEN, so no auth token is written to .npmrc. The changesets/action relies on the NPM_TOKEN env var to automatically configure npm authentication for publishing. Without it, the publish step will fail when attempting to publish to the npm registry. Either restore the NPM_TOKEN secret as an env var on the action step, or set NODE_AUTH_TOKEN with ${{ secrets.NPM_TOKEN }} if relying on setup-node auth.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/version-or-publish.yml, line 58:
<comment>The `NPM_TOKEN` and `NPM_AUTH_TOKEN` environment variables were removed from the `changesets/action` step, but no alternative npm authentication was provided. The `setup-node` step adds `registry-url` but does not set `NODE_AUTH_TOKEN`, so no auth token is written to `.npmrc`. The `changesets/action` relies on the `NPM_TOKEN` env var to automatically configure npm authentication for publishing. Without it, the publish step will fail when attempting to publish to the npm registry. Either restore the `NPM_TOKEN` secret as an env var on the action step, or set `NODE_AUTH_TOKEN` with `${{ secrets.NPM_TOKEN }}` if relying on `setup-node` auth.</comment>
<file context>
@@ -43,15 +46,16 @@ jobs:
with:
version: pnpm changeset version
publish: pnpm pub
+ createGithubReleases: false
env:
CI: true
</file context>
| baseUrl: fetchParams.baseUrl, | ||
| method: fetchParams.method, | ||
| headers: fetchParams.headers, | ||
| body: fetchParams.body as any, |
There was a problem hiding this comment.
P1: Requests with different FormData, Blob, or URLSearchParams bodies can share a query cache entry and return the first payload's data. Use a stable, payload-specific serializable key for native bodies, or reject bodies that cannot produce one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/fetch-query.ts, line 135:
<comment>Requests with different `FormData`, `Blob`, or `URLSearchParams` bodies can share a query cache entry and return the first payload's data. Use a stable, payload-specific serializable key for native bodies, or reject bodies that cannot produce one.</comment>
<file context>
@@ -0,0 +1,243 @@
+ baseUrl: fetchParams.baseUrl,
+ method: fetchParams.method,
+ headers: fetchParams.headers,
+ body: fetchParams.body as any,
+ notFoundAsNull: fetchParams.notFoundAsNull,
+ responseType: fetchParams.responseType,
</file context>
| if (args.length === 2) { | ||
| fetchInfiniteQuery = new FetchInfiniteQuery( | ||
| args[0], | ||
| typeof args[1] === 'function' ? args[1] : () => args[1], |
There was a problem hiding this comment.
P1: The queryClient + (() => config) overload drops every returned option, leaving this query unconfigured and disabled. Evaluate the options callback before passing it to FetchInfiniteQuery (or remove this unsupported overload).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/preset/create-fetch-infinite-query.ts, line 86:
<comment>The `queryClient + (() => config)` overload drops every returned option, leaving this query unconfigured and disabled. Evaluate the options callback before passing it to `FetchInfiniteQuery` (or remove this unsupported overload).</comment>
<file context>
@@ -0,0 +1,99 @@
+ if (args.length === 2) {
+ fetchInfiniteQuery = new FetchInfiniteQuery(
+ args[0],
+ typeof args[1] === 'function' ? args[1] : () => args[1],
+ );
+ } else {
</file context>
| ``` | ||
|
|
||
| ## `QueryConfigFromFn`, `MutationConfigFromFn`, `InfiniteQueryConfigFromFn` | ||
| ## `QueryConfigFromFn`, `MutationConfigFromFn`, `InfiniteQueryConfigFromFn`, `FetchQueryConfigFromFn`, `FetchInfiniteQueryConfigFromFn` |
There was a problem hiding this comment.
P1: The types FetchQueryConfigFromFn and FetchInfiniteQueryConfigFromFn are documented in the heading but don't exist in the source code. Users trying to import them will get TS errors. Add the missing type definitions (analogous to QueryConfigFromFn/InfiniteQueryConfigFromFn) or remove the docs references.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/api/other.md, line 15:
<comment>The types `FetchQueryConfigFromFn` and `FetchInfiniteQueryConfigFromFn` are documented in the heading but don't exist in the source code. Users trying to import them will get TS errors. Add the missing type definitions (analogous to `QueryConfigFromFn`/`InfiniteQueryConfigFromFn`) or remove the docs references.</comment>
<file context>
@@ -1,16 +1,18 @@
-## QueryConfigFromFn, MutationConfigFromFn, InfiniteQueryConfigFromFn
+## QueryConfigFromFn, MutationConfigFromFn, InfiniteQueryConfigFromFn, FetchQueryConfigFromFn, FetchInfiniteQueryConfigFromFn
This types are needed to create configuration types from your functions of your http client
</file context>
</details>
| - `baseUrl?: string` — Base URL prepended to `path`. Can also be set globally via [`QueryClient.fetchQueries`](/api/QueryClient#fetchqueries) | ||
| - `query?: Record<string, string | number | boolean | null | undefined>` — Query parameters to be serialized and appended to the URL | ||
| - `method?: string` — HTTP method. Defaults to `"GET"` (or `"POST"` when `body` is provided) | ||
| - `headers?: Record<string>` — Request headers. Merged with global headers from `QueryClient.fetchQueries` |
There was a problem hiding this comment.
P2: Record<string> is not valid TypeScript — the Record utility type requires two type arguments (Record<Keys, Type>). This should be Record<string, string> to match the actual type used in the source code (Dict<string>).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/api/FetchQuery.md, line 79:
<comment>`Record<string>` is not valid TypeScript — the `Record` utility type requires two type arguments (`Record<Keys, Type>`). This should be `Record<string, string>` to match the actual type used in the source code (`Dict<string>`).</comment>
<file context>
@@ -0,0 +1,118 @@
+- `baseUrl?: string` — Base URL prepended to `path`. Can also be set globally via [`QueryClient.fetchQueries`](/api/QueryClient#fetchqueries)
+- `query?: Record<string, string | number | boolean | null | undefined>` — Query parameters to be serialized and appended to the URL
+- `method?: string` — HTTP method. Defaults to `"GET"` (or `"POST"` when `body` is provided)
+- `headers?: Record<string>` — Request headers. Merged with global headers from `QueryClient.fetchQueries`
+- `body?: BodyInit | Record<string, unknown>` — Request body. Objects are JSON-serialized with `Content-Type: application/json`; native `BodyInit` (Blob, FormData, etc.) is passed through as-is
+- `request?: Partial<RequestInit>` — Additional `RequestInit` options (except `signal`, `method`, `headers`, `body`)
</file context>
| Global configuration for [`FetchQuery`](/api/FetchQuery) and [`FetchInfiniteQuery`](/api/FetchInfiniteQuery). These defaults are merged with per-request `params` in every fetch query. | ||
|
|
||
| - `baseUrl?: string` — Base URL prepended to every `path` | ||
| - `headers?: Record<string>` — Default headers merged with per-request headers |
There was a problem hiding this comment.
P2: Record<string> is invalid TypeScript — it requires two type arguments. Use Record<string, string> to correctly document the headers type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/api/QueryClient.md, line 101:
<comment>`Record<string>` is invalid TypeScript — it requires two type arguments. Use `Record<string, string>` to correctly document the headers type.</comment>
<file context>
@@ -92,6 +93,38 @@ Entity lifecycle events. Available hooks:
+Global configuration for [`FetchQuery`](/api/FetchQuery) and [`FetchInfiniteQuery`](/api/FetchInfiniteQuery). These defaults are merged with per-request `params` in every fetch query.
+
+- `baseUrl?: string` — Base URL prepended to every `path`
+- `headers?: Record<string>` — Default headers merged with per-request headers
+- `customFetch?: typeof globalThis.fetch` — Custom fetch implementation (useful for testing or non-browser environments)
+- `transformResponse?: TransformResponseFn` — Global response transformation function
</file context>
| - `headers?: Record<string>` — Default headers merged with per-request headers | |
| - `headers?: Record<string, string>` — Default headers merged with per-request headers |
| @@ -0,0 +1,38 @@ | |||
| # Error `#1`: Fetch query is not configured | |||
|
|
|||
| This happened because the `params` option for [`FetchQuery`](/api/Query#fetchquery) returned a falsy value and the query key could not be parsed. | |||
There was a problem hiding this comment.
P3: Link to /api/Query#fetchquery references an anchor that likely doesn't exist on the Query page. FetchQuery has its own dedicated documentation page at /api/FetchQuery, which should be linked directly instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/errors/1.md, line 3:
<comment>Link to `/api/Query#fetchquery` references an anchor that likely doesn't exist on the Query page. FetchQuery has its own dedicated documentation page at `/api/FetchQuery`, which should be linked directly instead.</comment>
<file context>
@@ -0,0 +1,38 @@
+# Error `#1`: Fetch query is not configured
+
+This happened because the `params` option for [`FetchQuery`](/api/Query#fetchquery) returned a falsy value and the query key could not be parsed.
+
+## Explanation:
</file context>
| This happened because the `params` option for [`FetchQuery`](/api/Query#fetchquery) returned a falsy value and the query key could not be parsed. | |
| This happened because the `params` option for [`FetchQuery`](/api/FetchQuery) returned a falsy value and the query key could not be parsed. |
| query.destroy(); | ||
| }); | ||
|
|
||
| it('uses explicit queryClient in single options overload', () => { |
There was a problem hiding this comment.
P3: Test 'uses explicit queryClient in single options overload' adds queryClient: customClient to the options and checks the instance type, but never actually verifies that the custom client was used. Missing expect(getQueryClient(query)).toBe(customClient) assertion. Compare with create-infinite-query.test.ts which does expect(getQueryClient(query)).toBe(customClient) in its equivalent test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/preset/create-fetch-infinite-query.test.ts, line 133:
<comment>Test 'uses explicit queryClient in single options overload' adds `queryClient: customClient` to the options and checks the instance type, but never actually verifies that the custom client was used. Missing `expect(getQueryClient(query)).toBe(customClient)` assertion. Compare with `create-infinite-query.test.ts` which does `expect(getQueryClient(query)).toBe(customClient)` in its equivalent test.</comment>
<file context>
@@ -0,0 +1,147 @@
+ query.destroy();
+ });
+
+ it('uses explicit queryClient in single options overload', () => {
+ const customClient = new QueryClient();
+
</file context>
| ## API Signature | ||
|
|
||
| ```ts | ||
| createFetchInfiniteQuery(queryClient, options) |
There was a problem hiding this comment.
P3: The API Signature block omits the lazy-initialization overload. The two-argument variant also accepts a function returning config (for deferred computation), but only createFetchInfiniteQuery(queryClient, options) is listed. Consider documenting it explicitly (e.g., createFetchInfiniteQuery(queryClient, () => options)) to match the convention used in createQuery.md and reflect the actual overloads.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/preset/createFetchInfiniteQuery.md, line 8:
<comment>The API Signature block omits the lazy-initialization overload. The two-argument variant also accepts a function returning config (for deferred computation), but only `createFetchInfiniteQuery(queryClient, options)` is listed. Consider documenting it explicitly (e.g., `createFetchInfiniteQuery(queryClient, () => options)`) to match the convention used in `createQuery.md` and reflect the actual overloads.</comment>
<file context>
@@ -0,0 +1,35 @@
+## API Signature
+
+```ts
+createFetchInfiniteQuery(queryClient, options)
+createFetchInfiniteQuery(options)
+```
</file context>
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/fetch-query.ts">
<violation number="1" location="src/fetch-query.ts:276">
P2: Manual `start({ params })` loses request metadata and any per-start query options because it forwards only `queryKey` and `enabled`. Preserve the remaining start options and set `meta` from `fetchParams` (with the client fallback), matching constructor behavior.</violation>
</file>
<file name="src/fetch-infinite-query.ts">
<violation number="1" location="src/fetch-infinite-query.ts:328">
P1: Manual infinite queries cannot encode later page parameters: `fetchNextPage()` repeats the initial URL/query because this static params object has no page-param mapping. Accept a page-aware params callback for `start`, or merge each context `pageParam` into the request before `makeFetchRequest`.</violation>
<violation number="2" location="src/fetch-infinite-query.ts:331">
P2: `start({ params, cancelRefetch: false })` silently ignores `cancelRefetch` and every other inherited start/update option. Preserve `startParams` when building the override, then replace only `queryKey` and `enabled`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (startParams?.params) { | ||
| const fetchParams: FetchInfiniteQueryParams<TOutputData, TPageParam> = | ||
| startParams.params; | ||
| this._holders.params = fetchParams; |
There was a problem hiding this comment.
P1: Manual infinite queries cannot encode later page parameters: fetchNextPage() repeats the initial URL/query because this static params object has no page-param mapping. Accept a page-aware params callback for start, or merge each context pageParam into the request before makeFetchRequest.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/fetch-infinite-query.ts, line 328:
<comment>Manual infinite queries cannot encode later page parameters: `fetchNextPage()` repeats the initial URL/query because this static params object has no page-param mapping. Accept a page-aware params callback for `start`, or merge each context `pageParam` into the request before `makeFetchRequest`.</comment>
<file context>
@@ -270,11 +299,41 @@ export class FetchInfiniteQuery<
+ if (startParams?.params) {
+ const fetchParams: FetchInfiniteQueryParams<TOutputData, TPageParam> =
+ startParams.params;
+ this._holders.params = fetchParams;
+ this._holders.transformResponse = fetchParams.transformResponse;
+
</file context>
| this._holders.params = fetchParams; | ||
| this._holders.transformResponse = fetchParams.transformResponse; | ||
|
|
||
| return super.start({ |
There was a problem hiding this comment.
P2: Manual start({ params }) loses request metadata and any per-start query options because it forwards only queryKey and enabled. Preserve the remaining start options and set meta from fetchParams (with the client fallback), matching constructor behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/fetch-query.ts, line 276:
<comment>Manual `start({ params })` loses request metadata and any per-start query options because it forwards only `queryKey` and `enabled`. Preserve the remaining start options and set `meta` from `fetchParams` (with the client fallback), matching constructor behavior.</comment>
<file context>
@@ -233,11 +251,34 @@ export class FetchQuery<TData = unknown, TOutputData = any> extends Query<
+ this._holders.params = fetchParams;
+ this._holders.transformResponse = fetchParams.transformResponse;
+
+ return super.start({
+ queryKey: buildFetchQueryKey(fetchParams),
+ enabled: true,
</file context>
| this._holders.params = fetchParams; | ||
| this._holders.transformResponse = fetchParams.transformResponse; | ||
|
|
||
| return super.start({ |
There was a problem hiding this comment.
P2: start({ params, cancelRefetch: false }) silently ignores cancelRefetch and every other inherited start/update option. Preserve startParams when building the override, then replace only queryKey and enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/fetch-infinite-query.ts, line 331:
<comment>`start({ params, cancelRefetch: false })` silently ignores `cancelRefetch` and every other inherited start/update option. Preserve `startParams` when building the override, then replace only `queryKey` and `enabled`.</comment>
<file context>
@@ -270,11 +299,41 @@ export class FetchInfiniteQuery<
+ this._holders.params = fetchParams;
+ this._holders.transformResponse = fetchParams.transformResponse;
+
+ return super.start({
+ queryKey: buildFetchInfiniteQueryKey(fetchParams),
+ enabled: true,
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
docs/api/FetchQuery.md (2)
28-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInitialize the MobX box with a value.
observable.box<string>()needs an initial value — useobservable.box<string | undefined>(undefined).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/FetchQuery.md` around lines 28 - 56, Update the Dynamic params example’s petId declaration to initialize the MobX box with undefined, using the string-or-undefined type so the existing falsy-value query disabling behavior remains valid.
58-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse complete
Recordtype arguments.Record<string>on line 93 is invalid TypeScript; document headers asRecord<string, string>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/api/FetchQuery.md` around lines 58 - 101, Update the headers field in the FetchQueryParams documentation to use the complete Record type, specifying string keys and string values as Record<string, string>.src/fetch-infinite-query.ts (2)
104-107: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
FetchInfiniteQueryPositionalConfigstill omits pagination options.This interface omits
initialPageParam,getNextPageParam, andgetPreviousPageParam(lines 100–102) without redeclaring them. The positional constructor overload (new FetchInfiniteQuery(queryClient, config)) therefore rejects the pagination configuration required for infinite queries. The past review marked this as addressed in commit f14fce0, but the current code retains the same omission.Proposed fix
export interface FetchInfiniteQueryPositionalConfig< TQueryFnData = unknown, TError = DefaultError, TPageParam = unknown, TData = InfiniteData<TQueryFnData, TPageParam>, TOutputData = any, -> extends Omit< - Partial<InfiniteQueryConfig<TQueryFnData, TError, TPageParam, TData, any>>, - | 'queryFn' - | 'queryKey' - | 'options' - | 'select' - | 'initialPageParam' - | 'getNextPageParam' - | 'getPreviousPageParam' - > { - params?: MaybeFn< - MaybeFalsy<FetchInfiniteQueryParams<TOutputData, TPageParam>> - >; - select?: (data: InfiniteData<TQueryFnData, TPageParam>) => TData; -} +> extends Omit< + FetchInfiniteQueryConfig< + TQueryFnData, + TError, + TPageParam, + TData, + TOutputData + >, + 'queryClient' + > {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fetch-infinite-query.ts` around lines 104 - 107, Update FetchInfiniteQueryPositionalConfig to include the infinite-query pagination options initialPageParam, getNextPageParam, and getPreviousPageParam, matching the corresponding configuration interface. Ensure the positional constructor overload accepts these options while preserving the existing params and select properties.
82-85: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winType inconsistency between
TOutputDataandTQueryFnDatapersists.
select(line 85) and pagination callbacks (lines 70–75) useTQueryFnData, butqueryFn(line 300) returnsTOutputData | nullfrommakeFetchRequest<TOutputData>. When callers specify differentTQueryFnDataandTOutputDatagenerics, the compiler accepts incompatible types and runtime values contradict the type signatures. This was previously flagged and remains unresolved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fetch-infinite-query.ts` around lines 82 - 85, Align the generic data types throughout fetchInfiniteQuery so the queryFn result from makeFetchRequest<TOutputData> matches the type used by select and the pagination callbacks. Update the related InfiniteData and callback signatures to use the same output type, or enforce the generic relationship so incompatible TQueryFnData and TOutputData combinations are rejected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/api/FetchInfiniteQuery.md`:
- Around line 30-67: Update the “How it works” description for
`FetchInfiniteQuery` so `data` is documented as an `InfiniteData` object
containing `pages` and `pageParams`, with fetched results accessed through
`data.pages` rather than directly as an array.
---
Duplicate comments:
In `@docs/api/FetchQuery.md`:
- Around line 28-56: Update the Dynamic params example’s petId declaration to
initialize the MobX box with undefined, using the string-or-undefined type so
the existing falsy-value query disabling behavior remains valid.
- Around line 58-101: Update the headers field in the FetchQueryParams
documentation to use the complete Record type, specifying string keys and string
values as Record<string, string>.
In `@src/fetch-infinite-query.ts`:
- Around line 104-107: Update FetchInfiniteQueryPositionalConfig to include the
infinite-query pagination options initialPageParam, getNextPageParam, and
getPreviousPageParam, matching the corresponding configuration interface. Ensure
the positional constructor overload accepts these options while preserving the
existing params and select properties.
- Around line 82-85: Align the generic data types throughout fetchInfiniteQuery
so the queryFn result from makeFetchRequest<TOutputData> matches the type used
by select and the pagination callbacks. Update the related InfiniteData and
callback signatures to use the same output type, or enforce the generic
relationship so incompatible TQueryFnData and TOutputData combinations are
rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31b775b9-00ed-4e39-86e6-eb7f2a383866
📒 Files selected for processing (7)
docs/api/FetchInfiniteQuery.mddocs/api/FetchQuery.mdpackage.jsonsrc/fetch-infinite-query.tssrc/fetch-query.tssrc/preset/create-fetch-query.test.tssrc/preset/create-fetch-query.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/preset/create-fetch-query.ts
- package.json
- src/fetch-query.ts
| ### Manual control with `start()` | ||
|
|
||
| Create a `FetchInfiniteQuery` without `params` (disabled by default), then fetch manually: | ||
|
|
||
| ```ts | ||
| const starsQuery = new FetchInfiniteQuery({ | ||
| queryClient, | ||
| initialPageParam: 1, | ||
| getNextPageParam: (lastPage, _, lastPageParam) => { | ||
| return lastPage.length ? lastPageParam + 1 : null; | ||
| }, | ||
| }); | ||
|
|
||
| const result = await starsQuery.start({ | ||
| params: { path: '/stars', query: { count: 20 } }, | ||
| }); | ||
| ``` | ||
|
|
||
| ### How it works | ||
|
|
||
| 1. `params` is a function that receives `{ pageParam }` — the current page parameter (starts at `initialPageParam`) | ||
| 2. Each page request uses the same `params` logic with a different `pageParam` | ||
| 3. After each page loads, `getNextPageParam` determines the next page parameter — return `null` to indicate no more pages | ||
| 4. `data` is an array of all fetched pages, and `fetchNextPage()` triggers the next request | ||
|
|
||
| ### Example: cursor-based pagination | ||
|
|
||
| ```ts | ||
| const commentsQuery = new FetchInfiniteQuery({ | ||
| queryClient, | ||
| params: ({ pageParam }) => ({ | ||
| path: '/comments', | ||
| query: { cursor: pageParam }, | ||
| }), | ||
| initialPageParam: undefined as string | undefined, | ||
| getNextPageParam: (lastPage) => lastPage.nextCursor ?? null, | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
data is not a plain array — it's an InfiniteData object.
Line 53 states "data is an array of all fetched pages," but FetchInfiniteQuery uses InfiniteData<TQueryFnData, TPageParam> (per src/fetch-infinite-query.ts), which is { pages: TQueryFnData[], pageParams: TPageParam[] }. The aggregated pages live in data.pages, not data directly.
📝 Proposed fix
-4. `data` is an array of all fetched pages, and `fetchNextPage()` triggers the next request
+4. `data` is an `InfiniteData` object whose `.pages` array holds all fetched pages, and `fetchNextPage()` triggers the next request📝 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.
| ### Manual control with `start()` | |
| Create a `FetchInfiniteQuery` without `params` (disabled by default), then fetch manually: | |
| ```ts | |
| const starsQuery = new FetchInfiniteQuery({ | |
| queryClient, | |
| initialPageParam: 1, | |
| getNextPageParam: (lastPage, _, lastPageParam) => { | |
| return lastPage.length ? lastPageParam + 1 : null; | |
| }, | |
| }); | |
| const result = await starsQuery.start({ | |
| params: { path: '/stars', query: { count: 20 } }, | |
| }); | |
| ``` | |
| ### How it works | |
| 1. `params` is a function that receives `{ pageParam }` — the current page parameter (starts at `initialPageParam`) | |
| 2. Each page request uses the same `params` logic with a different `pageParam` | |
| 3. After each page loads, `getNextPageParam` determines the next page parameter — return `null` to indicate no more pages | |
| 4. `data` is an array of all fetched pages, and `fetchNextPage()` triggers the next request | |
| ### Example: cursor-based pagination | |
| ```ts | |
| const commentsQuery = new FetchInfiniteQuery({ | |
| queryClient, | |
| params: ({ pageParam }) => ({ | |
| path: '/comments', | |
| query: { cursor: pageParam }, | |
| }), | |
| initialPageParam: undefined as string | undefined, | |
| getNextPageParam: (lastPage) => lastPage.nextCursor ?? null, | |
| }); | |
| ``` | |
| ### Manual control with `start()` | |
| Create a `FetchInfiniteQuery` without `params` (disabled by default), then fetch manually: | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/api/FetchInfiniteQuery.md` around lines 30 - 67, Update the “How it
works” description for `FetchInfiniteQuery` so `data` is documented as an
`InfiniteData` object containing `pages` and `pageParams`, with fetched results
accessed through `data.pages` rather than directly as an array.
Summary by cubic
Add
FetchQueryandFetchInfiniteQueryfor first-class HTTP queries with MobX reactivity, plus presets and globalQueryClient.fetchQueriesdefaults. Also adds error docs and improves types/exports for better DX.New Features
FetchQuery/FetchInfiniteQuerywith declarativeparams(path, baseUrl, query, method, headers, body, request, responseType, notFoundAsNull, transformResponse); reactiveparams;start({ params }).queryKeyfromparams;makeFetchRequestbuilds URLs, merges headers, serializes JSON/native bodies, supports 404→null,responseType, abort/timeout.QueryClient.fetchQueries(baseUrl, headers, customFetch, transformResponse, throwOnError, timeout, credentials, mode, meta).createFetchQuery/createFetchInfiniteQuerythat auto-mount the client once.Meta#1, [Feature Request] AddqueryOptionsfunction #2). New types (InferFetchQuery,InferFetchInfiniteQuery,FetchQueryConfigFromFn,FetchInfiniteQueryConfigFromFn), exports, and tests.Bug Fixes
InfiniteQueryObserverResultre-exports); exclude tests from build intsconfig.Written for commit 91ac7db. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation