Skip to content

Clone feature/imperative infinite queries #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
visz11 wants to merge 2 commits into
base: main
Choose a base branch
from
+170 −17
Open

Clone feature/imperative infinite queries #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01a81ee
feat: imperative infinite queries
TkDodo Feb 14, 2025
644677e
test: add type tests of expected behaviour (currently failing)
TkDodo Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,110 @@ describe('InfiniteQueryBehavior', () => {
unsubscribe()
})

test('InfiniteQueryBehavior should apply pageParam', async () => {
const key = queryKey()

const queryFn = vi.fn().mockImplementation(({ pageParam }) => {
return pageParam
})

const observer = new InfiniteQueryObserver<number>(queryClient, {
queryKey: key,
queryFn,
initialPageParam: 0,
})

let observerResult:
| InfiniteQueryObserverResult<unknown, unknown>
| undefined

const unsubscribe = observer.subscribe((result) => {
observerResult = result
})

// Wait for the first page to be fetched
await waitFor(() =>
expect(observerResult).toMatchObject({
isFetching: false,
data: { pages: [0], pageParams: [0] },
}),
)

queryFn.mockClear()

// Fetch the next page using pageParam
await observer.fetchNextPage({ pageParam: 1 })

expect(queryFn).toHaveBeenNthCalledWith(1, {
queryKey: key,
pageParam: 1,
meta: undefined,
client: queryClient,
direction: 'forward',
signal: expect.anything(),
})

expect(observerResult).toMatchObject({
isFetching: false,
data: { pages: [0, 1], pageParams: [0, 1] },
})

queryFn.mockClear()

// Fetch the previous page using pageParam
await observer.fetchPreviousPage({ pageParam: -1 })

expect(queryFn).toHaveBeenNthCalledWith(1, {
queryKey: key,
pageParam: -1,
meta: undefined,
client: queryClient,
direction: 'backward',
signal: expect.anything(),
})

expect(observerResult).toMatchObject({
isFetching: false,
data: { pages: [-1, 0, 1], pageParams: [-1, 0, 1] },
})

queryFn.mockClear()

// Refetch pages: old manual page params should be used
await observer.refetch()

expect(queryFn).toHaveBeenCalledTimes(3)

expect(queryFn).toHaveBeenNthCalledWith(1, {
queryKey: key,
pageParam: -1,
meta: undefined,
client: queryClient,
direction: 'forward',
signal: expect.anything(),
})

expect(queryFn).toHaveBeenNthCalledWith(2, {
queryKey: key,
pageParam: 0,
meta: undefined,
client: queryClient,
direction: 'forward',
signal: expect.anything(),
})

expect(queryFn).toHaveBeenNthCalledWith(3, {
queryKey: key,
pageParam: 1,
meta: undefined,
client: queryClient,
direction: 'forward',
signal: expect.anything(),
})

unsubscribe()
})

test('InfiniteQueryBehavior should support query cancellation', async () => {
const key = queryKey()
let abortSignal: AbortSignal | null = null
Expand Down
43 changes: 43 additions & 0 deletions packages/query-core/src/__tests__/infiniteQueryObserver.test-d.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,47 @@ describe('InfiniteQueryObserver', () => {
expectTypeOf(result.status).toEqualTypeOf<'success'>()
}
})

it('should not allow pageParam on fetchNextPage / fetchPreviousPage if getNextPageParam is defined', async () => {
const observer = new InfiniteQueryObserver(queryClient, {
queryKey: queryKey(),
queryFn: ({ pageParam }) => String(pageParam),
initialPageParam: 1,
getNextPageParam: (page) => Number(page) + 1,
})

expectTypeOf<typeof observer.fetchNextPage>()
.parameter(0)
.toEqualTypeOf<
{ cancelRefetch?: boolean; throwOnError?: boolean } | undefined
>()

expectTypeOf<typeof observer.fetchPreviousPage>()
.parameter(0)
.toEqualTypeOf<
{ cancelRefetch?: boolean; throwOnError?: boolean } | undefined
>()
})

it('should require pageParam on fetchNextPage / fetchPreviousPage if getNextPageParam is missing', async () => {
const observer = new InfiniteQueryObserver(queryClient, {
queryKey: queryKey(),
queryFn: ({ pageParam }) => String(pageParam),
initialPageParam: 1,
})

expectTypeOf<typeof observer.fetchNextPage>()
.parameter(0)
.toEqualTypeOf<
| { pageParam: number; cancelRefetch?: boolean; throwOnError?: boolean }
| undefined
>()
Comment on lines +104 to +109
Copy link

@refacto-visz refacto-visz bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect Type Test Allows Undefined Arguments

The type test asserts that the options argument for fetchNextPage can be undefined, which contradicts the test's stated goal to 'require pageParam'. This lenient type allows calling fetchNextPage() without arguments when getNextPageParam is missing, which can lead to a silent runtime failure where the queryFn receives pageParam: undefined. The type should enforce that the options object containing pageParam is mandatory.

Standards
  • Type Safety & API Contracts


expectTypeOf<typeof observer.fetchPreviousPage>()
.parameter(0)
.toEqualTypeOf<
| { pageParam: number; cancelRefetch?: boolean; throwOnError?: boolean }
| undefined
>()
})
})
17 changes: 10 additions & 7 deletions packages/query-core/src/infiniteQueryBehavior.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function infiniteQueryBehavior<TQueryFnData, TError, TData, TPageParam>(
return {
onFetch: (context, query) => {
const options = context.options as InfiniteQueryPageParamsOptions<TData>
const direction = context.fetchOptions?.meta?.fetchMore?.direction
const fetchMore = context.fetchOptions?.meta?.fetchMore
const oldPages = context.state.data?.pages || []
const oldPageParams = context.state.data?.pageParams || []
let result: InfiniteData<unknown> = { pages: [], pageParams: [] }
Expand Down Expand Up @@ -81,14 +81,17 @@ export function infiniteQueryBehavior<TQueryFnData, TError, TData, TPageParam>(
}

// fetch next / previous page?
if (direction && oldPages.length) {
const previous = direction === 'backward'
if (fetchMore && oldPages.length) {
const previous = fetchMore.direction === 'backward'
const pageParamFn = previous ? getPreviousPageParam : getNextPageParam
const oldData = {
pages: oldPages,
pageParams: oldPageParams,
}
const param = pageParamFn(options, oldData)
const param =
fetchMore.pageParam === undefined
? pageParamFn(options, oldData)
: fetchMore.pageParam
Comment on lines +91 to +94
Copy link

@refacto-visz refacto-visz bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type Definition and Runtime Behavior Mismatch

The runtime logic allows pageParam to override the result of getNextPageParam if both are provided. However, the accompanying type definitions explicitly forbid providing pageParam when getNextPageParam is defined. This discrepancy between the typed API contract and the actual runtime behavior can lead to confusion and unexpected behavior, undermining the reliability of the type system.

Standards
  • ISO-IEC-25010-Functional-Correctness-Appropriateness


result = await fetchPage(oldData, param, previous)
} else {
Comment on lines +85 to 97
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Explicit pageParam is dropped on the very first fetch
If the consumer starts an infinite query with no cached pages (e.g. enabled: false) and then calls fetchNextPage({ pageParam }), fetchMore is populated but oldPages.length is 0. Because of the if (fetchMore && oldPages.length) guard we fall into the else branch, completely ignoring the caller-supplied pageParam and defaulting back to initialPageParam. That makes it impossible to bootstrap a query from a runtime-provided cursor—the main use case this feature is supposed to unlock.

Please let the fetchMore branch run even when there are no cached pages and treat an explicit pageParam override as the source of truth. Something like the diff below preserves the old behavior for existing pages while honoring manual cursors for the first fetch:

-        if (fetchMore && oldPages.length) {
-          const previous = fetchMore.direction === 'backward'
-          const pageParamFn = previous ? getPreviousPageParam : getNextPageParam
-          const oldData = {
-            pages: oldPages,
-            pageParams: oldPageParams,
-          }
-          const param =
-            fetchMore.pageParam === undefined
-              ? pageParamFn(options, oldData)
-              : fetchMore.pageParam
-
-          result = await fetchPage(oldData, param, previous)
+        if (fetchMore) {
+          const previous = fetchMore.direction === 'backward'
+          const pageParamFn = previous ? getPreviousPageParam : getNextPageParam
+          const baseData = oldPages.length
+            ? { pages: oldPages, pageParams: oldPageParams }
+            : { pages: [], pageParams: [] }
+          const param =
+            fetchMore.pageParam !== undefined
+              ? fetchMore.pageParam
+              : oldPages.length
+                ? pageParamFn(options, baseData)
+                : options.initialPageParam
+
+          result = await fetchPage(baseData, param, previous)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/query-core/src/infiniteQueryBehavior.ts around lines 85 to 97, the
current guard skips the fetchMore branch when there are no cached pages causing
an explicit fetchMore.pageParam to be ignored; change the condition so fetchMore
is processed even if oldPages.length === 0 when the caller provided a pageParam.
Specifically, always enter the fetchMore logic when fetchMore exists, compute
previous and pageParamFn as before, and derive param by preferring
fetchMore.pageParam when it is defined (use that value even if oldPages is
empty); only fall back to calling pageParamFn(options, oldData) when
fetchMore.pageParam is undefined (and for existing pages preserve current
behavior).

Expand All @@ -97,8 +100,8 @@ export function infiniteQueryBehavior<TQueryFnData, TError, TData, TPageParam>(
// Fetch all pages
do {
const param =
currentPage === 0
? (oldPageParams[0] ?? options.initialPageParam)
currentPage === 0 || !options.getNextPageParam
? (oldPageParams[currentPage] ?? options.initialPageParam)
: getNextPageParam(options, result)
Comment on lines 102 to 105
Copy link

@gemini-code-assist gemini-code-assist bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The logic for determining the param for refetching seems to have a bug when getNextPageParam is not provided. For subsequent pages (currentPage > 0), if oldPageParams[currentPage] is undefined or null, it incorrectly falls back to options.initialPageParam. This would cause a refetch with the wrong page parameter. For example, if pageParams were [1, undefined], a refetch would request pages for 1 and then for initialPageParam, instead of 1 and undefined.

The fallback to initialPageParam should only happen for the very first page (currentPage === 0) when there are no previous page params.

            const param =
              currentPage === 0
                ? oldPageParams[0] ?? options.initialPageParam
                : !options.getNextPageParam
                ? oldPageParams[currentPage]
                : getNextPageParam(options, result)

if (currentPage > 0 && param == null) {
break
Expand Down Expand Up @@ -136,7 +139,7 @@ function getNextPageParam(
): unknown | undefined {
const lastIndex = pages.length - 1
return pages.length > 0
? options.getNextPageParam(
? options.getNextPageParam?.(
pages[lastIndex],
pages,
pageParams[lastIndex],
Expand Down
19 changes: 11 additions & 8 deletions packages/query-core/src/infiniteQueryObserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,24 +124,27 @@ export class InfiniteQueryObserver<
>
}

fetchNextPage(
options?: FetchNextPageOptions,
): Promise<InfiniteQueryObserverResult<TData, TError>> {
fetchNextPage({ pageParam, ...options }: FetchNextPageOptions = {}): Promise<
InfiniteQueryObserverResult<TData, TError>
> {
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'forward' },
fetchMore: { direction: 'forward', pageParam },
},
})
}
Comment on lines +127 to 136
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Add pageParam to FetchNextPageOptions type definition.

The pipeline is failing because pageParam is being destructured from FetchNextPageOptions, but the type definition doesn't include this property. The relevant code snippets show that FetchNextPageOptions in types.ts (lines 585-595) only includes cancelRefetch.

To fix this, update the FetchNextPageOptions interface in packages/query-core/src/types.ts:

 export interface FetchNextPageOptions extends ResultOptions {
+  /**
+   * Optional page parameter to use when fetching the next page.
+   * If provided, this will override the value computed from getNextPageParam.
+   */
+  pageParam?: unknown
   /**
    * If set to `true`, calling `fetchNextPage` repeatedly will invoke `queryFn` every time,
    * whether the previous invocation has resolved or not. Also, the result from previous invocations will be ignored.
    *
    * If set to `false`, calling `fetchNextPage` repeatedly won't have any effect until the first invocation has resolved.
    *
    * Defaults to `true`.
    */
   cancelRefetch?: boolean
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/query-core/src/types.ts around lines 585 to 595, the
FetchNextPageOptions interface is missing the pageParam property but code in
infiniteQueryObserver.ts destructures pageParam from FetchNextPageOptions;
update the FetchNextPageOptions type to include an optional pageParam (e.g.
pageParam?: unknown or a properly generic page param type used across the
package), so the compiler knows this property exists and type-checks correctly.


fetchPreviousPage(
options?: FetchPreviousPageOptions,
): Promise<InfiniteQueryObserverResult<TData, TError>> {
fetchPreviousPage({
pageParam,
...options
}: FetchPreviousPageOptions = {}): Promise<
InfiniteQueryObserverResult<TData, TError>
> {
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'backward' },
fetchMore: { direction: 'backward', pageParam },
},
})
}
Comment on lines +127 to 150
Copy link

@refacto-visz refacto-visz bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New Feature Lacks Required Feature Flag

This change introduces a significant new feature (imperative page parameters) without being gated by a feature flag. This violates the organization guideline for new functionality, which is crucial for controlled rollouts and ensuring a clear rollback plan. The new API surface should be conditionally enabled via a feature flag.

  fetchNextPage(
    options?: FetchNextPageOptions,
  ): Promise<InfiniteQueryObserverResult<TData, TError>> {
    return this.fetch({
      ...options,
      meta: {
        fetchMore: { direction: 'forward' },
      },
    })
  }

  fetchPreviousPage(
    options?: FetchPreviousPageOptions,
  ): Promise<InfiniteQueryObserverResult<TData, TError>> {
    return this.fetch({
      ...options,
      meta: {
        fetchMore: { direction: 'backward' },
      },
    })
  }
Commitable Suggestion
Suggested change
fetchNextPage({ pageParam, ...options }: FetchNextPageOptions = {}): Promise<
InfiniteQueryObserverResult<TData, TError>
> {
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'forward' },
fetchMore: { direction: 'forward', pageParam },
},
})
}
fetchPreviousPage(
options?: FetchPreviousPageOptions,
): Promise<InfiniteQueryObserverResult<TData, TError>> {
fetchPreviousPage({
pageParam,
...options
}: FetchPreviousPageOptions = {}): Promise<
InfiniteQueryObserverResult<TData, TError>
> {
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'backward' },
fetchMore: { direction: 'backward', pageParam },
},
})
}
fetchNextPage(
options?: FetchNextPageOptions,
): Promise<InfiniteQueryObserverResult<TData, TError>> {
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'forward' },
},
})
}
fetchPreviousPage(
options?: FetchPreviousPageOptions,
): Promise<InfiniteQueryObserverResult<TData, TError>> {
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'backward' },
},
})
}
Standards
  • Org-Guideline-Use feature flags for new functionality and include a clear rollback plan.

Comment on lines +138 to 150
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Add pageParam to FetchPreviousPageOptions type definition.

The same issue applies here—pageParam is being destructured from FetchPreviousPageOptions, but the type definition doesn't include this property.

Update the FetchPreviousPageOptions interface in packages/query-core/src/types.ts:

 export interface FetchPreviousPageOptions extends ResultOptions {
+  /**
+   * Optional page parameter to use when fetching the previous page.
+   * If provided, this will override the value computed from getPreviousPageParam.
+   */
+  pageParam?: unknown
   /**
    * If set to `true`, calling `fetchPreviousPage` repeatedly will invoke `queryFn` every time,
    * whether the previous invocation has resolved or not. Also, the result from previous invocations will be ignored.
    *
    * If set to `false`, calling `fetchPreviousPage` repeatedly won't have any effect until the first invocation has resolved.
    *
    * Defaults to `true`.
    */
   cancelRefetch?: boolean
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/query-core/src/infiniteQueryObserver.ts around lines 138 to 150,
fetchPreviousPage destructures pageParam from FetchPreviousPageOptions but the
type definition lacks pageParam; update packages/query-core/src/types.ts to add
an optional pageParam?: unknown (or generic TPageParam) to the
FetchPreviousPageOptions interface (and any related
FetchNextPageOptions/FetchPageOptions types if present) so the destructured
property is correctly typed and preserves proper generics/optional usage.

Expand Down
2 changes: 1 addition & 1 deletion packages/query-core/src/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export interface QueryBehavior<
export type FetchDirection = 'forward' | 'backward'

export interface FetchMeta {
fetchMore?: { direction: FetchDirection }
fetchMore?: { direction: FetchDirection; pageParam?: unknown }
}

export interface FetchOptions<TData = unknown> {
Expand Down
2 changes: 1 addition & 1 deletion packages/query-core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export interface InfiniteQueryPageParamsOptions<
* This function can be set to automatically get the next cursor for infinite queries.
* The result will also be used to determine the value of `hasNextPage`.
*/
getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>
getNextPageParam?: GetNextPageParamFunction<TPageParam, TQueryFnData>
Copy link

@refacto-visz refacto-visz bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API Contract Changed Without Feature Flag

A significant new feature allowing imperative page fetching has been introduced by making getNextPageParam optional. This changes a core API contract without being placed behind a feature flag, violating company policy. This makes rollbacks difficult and increases risk if unforeseen issues arise from the new behavior.

  getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>
Commitable Suggestion
Suggested change
getNextPageParam?: GetNextPageParamFunction<TPageParam, TQueryFnData>
getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>
Standards
  • Org-Guideline-Use feature flags for new functionality and include a clear rollback plan.

}

export type ThrowOnError<
Expand Down
Loading