Skip to content

feat: imperative infinite queries #6

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
wants to merge 2 commits into
base: main
Choose a base branch
from
+170 −17
Open

feat: imperative infinite queries #6

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
>()

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'
Comment on lines +84 to +85
Copy link

Choose a reason for hiding this comment

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

Inconsistent Direction Handling

The code now uses fetchMore object but inconsistently handles the direction property. In some places it checks for direction existence, in others it assumes direction is defined. This inconsistency creates maintenance challenges and potential bugs when modifying direction-related logic.

        if (fetchMore?.direction && oldPages.length) {
          const previous = fetchMore.direction === 'backward'
Commitable Suggestion
Suggested change
if (fetchMore && oldPages.length) {
const previous = fetchMore.direction === 'backward'
if (fetchMore?.direction && oldPages.length) {
const previous = fetchMore.direction === 'backward'
Standards
  • Clean-Code-Consistency
  • SOLID-SRP
  • Maintainability-Quality-Predictability

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

Choose a reason for hiding this comment

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

Missing PageParam Validation

User-provided pageParam is used without validation. Attackers could inject malicious values causing data exposure or application crashes. Especially risky when pageParam flows to database queries.

Suggested change
const param =
fetchMore.pageParam === undefined
? pageParamFn(options, oldData)
: fetchMore.pageParam
const param =
fetchMore.pageParam === undefined
? pageParamFn(options, oldData)
: validatePageParam(fetchMore.pageParam, options)
// Add this function to validate incoming pageParam values
function validatePageParam(pageParam: unknown, options: InfiniteQueryPageParamsOptions<TData>): unknown {
// Implement validation based on expected pageParam type
// Example: type checking, range validation, sanitization
return pageParam; // Return validated/sanitized value
}
Standards
  • CWE-20
  • OWASP-A03

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Missing Null Check

No null check for pageParamFn before invocation. If getNextPageParam/getPreviousPageParam is undefined, this will cause runtime errors when fetchMore.pageParam is undefined.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Refetch Performance Impact

The refetch operation now uses manual pageParams when available, which is beneficial for custom pagination. However, when refetching all pages, this could lead to unnecessary computation if pageParam functions are complex. Consider adding a performance optimization that caches computed pageParams during initial fetch to avoid recalculation during refetches.

Standards
  • ISO-IEC-25010-Performance-Time-Behaviour
  • Algorithm-Opt-Memoization

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam Check

The code assumes pageParamFn always exists, but getNextPageParam is now optional. If getNextPageParam is undefined, pageParamFn will be undefined causing runtime error when called.

Suggested change
const param =
fetchMore.pageParam === undefined
? pageParamFn(options, oldData)
: fetchMore.pageParam
const param =
fetchMore.pageParam === undefined && pageParamFn
? pageParamFn(options, oldData)
: fetchMore.pageParam ?? options.initialPageParam
Standards
  • Logic-Verification-Null-Safety
  • Business-Rule-Optional-Parameters
  • Algorithm-Correctness-Function-Invocation

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Missing getNextPageParam Check

pageParamFn is called unconditionally even when options.getNextPageParam might be undefined. This could cause runtime errors when manual pageParam is not provided and getNextPageParam is missing.

Standards
  • Logic-Verification-Null-Safety
  • Business-Rule-Validation
  • Algorithm-Correctness-Function-Calls

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Inconsistent Parameter Handling

Logic allows manual pageParam to override automatic pageParam calculation without checking if getNextPageParam/getPreviousPageParam exists. This creates inconsistency where automatic pagination can be unexpectedly bypassed, breaking pagination continuity.

Standards
  • Business-Rule-Consistency
  • Logic-Verification-Parameter-Precedence

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Conditional Logic Complexity

Nested conditional logic increases cognitive complexity. The parameter determination logic mixes multiple concerns, making future modifications to parameter handling more error-prone.

Standards
  • Clean-Code-Simplicity
  • Clean-Code-Functions

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Preserve Manual PageParams

The implementation correctly uses manual pageParam when provided, but doesn't store this value for future refetches. When refetching, the code uses oldPageParams which may not contain the manually provided pageParam.

Standards
  • ISO-IEC-25010-Performance-Time-Behaviour
  • Algorithm-Opt-State-Preservation

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Optimize Refetch Logic

The current implementation checks fetchMore.pageParam === undefined to determine whether to use the pageParamFn or the explicit pageParam. This creates unnecessary function calls when pageParam is provided. Consider short-circuit evaluation to avoid calling pageParamFn entirely when pageParam is provided, reducing computational overhead during pagination operations.

Standards
  • ISO-IEC-25010-Performance-Time-Behaviour
  • Algorithm-Opt-Short-Circuit

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Input Validation Missing

Manual pageParam is used without validation. Attackers could inject malicious values causing unexpected behavior or data exposure. Add validation before using external pageParam values.

Standards
  • CWE-20
  • OWASP-A03

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Optional Parameter Type

The pageParam is conditionally used but lacks explicit typing. Adding proper type annotations would improve code clarity and maintainability, especially for future developers working with this API.

Standards
  • Clean-Code-Type-Safety
  • Clean-Code-Explicit-Intent

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Potential Parameter Override

The implementation allows overriding pageParam with user-provided values without validation. This could potentially lead to injection attacks if pageParam is used in sensitive operations like database queries or file operations without proper validation.

Standards
  • CWE-20
  • OWASP-A03

Comment on lines +91 to +94
Copy link

Choose a reason for hiding this comment

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

Conditional Logic Complexity

The nested ternary operator increases cognitive complexity. This pattern could be simplified with a more direct approach like destructuring with defaults or early returns, making the code more readable and maintainable.

Standards
  • Clean-Code-Simplicity
  • Maintainability-Quality-Complexity


result = await fetchPage(oldData, param, previous)
} else {
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)
Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Missing getNextPageParam Check

Logic incorrectly treats missing getNextPageParam as a condition to use oldPageParams instead of calculating next page. This breaks expected behavior for subsequent pages when getNextPageParam is undefined.

Standards
  • Algorithm-Correctness-Conditional
  • Logic-Verification-Flow
  • Business-Rule-Pagination

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Optional GetNextPageParam Check

The check for !options.getNextPageParam is new but will always be false when refetching since getNextPageParam is used later. This creates an unnecessary condition check on every page fetch. Consider simplifying this logic or adding a comment explaining the purpose of this check for better code clarity.

Standards
  • ISO-IEC-25010-Performance-Time-Behaviour
  • Algorithm-Opt-Conditional-Evaluation

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Conditional Operator Inconsistency

The condition adds !options.getNextPageParam but only affects currentPage === 0 case. For subsequent pages, getNextPageParam is still required but now optional in the type definition, creating a logical inconsistency.

Standards
  • Logic-Verification-Conditional-Consistency
  • Business-Rule-Completeness

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam Logic

Making getNextPageParam optional changes core behavior where currentPage > 0 but getNextPageParam is undefined. Logic now falls back to oldPageParams[currentPage] instead of calculating next param, potentially causing inconsistent pagination behavior.

Standards
  • Logic-Verification-Conditional-Flow
  • Business-Rule-Pagination
  • Algorithm-Correctness-Default-Handling

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Inconsistent Direction Parameter

When refetching, all pages use 'forward' direction regardless of original fetch direction. This inconsistency could cause unexpected behavior for direction-sensitive queryFn implementations.

Standards
  • Business-Rule-Consistency
  • Algorithm-Correctness-Parameter-Handling
  • Logic-Verification-State-Consistency

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Incorrect pageParam Condition

Logic incorrectly applies initialPageParam when getNextPageParam is missing for any page. This creates inconsistency where subsequent pages would use oldPageParams but first page might not, causing unexpected parameter values.

Standards
  • Logic-Verification-Conditional-Flow
  • Business-Rule-Parameter-Handling

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Incomplete Optional Handling

Logic added to handle missing getNextPageParam but only in one location. Other code paths still assume getNextPageParam exists, creating inconsistent behavior.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Completeness

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Inconsistent Direction Parameter

When refetching pages, direction is always 'forward' regardless of original fetch direction. This could cause inconsistent behavior when refetching pages originally fetched with backward direction.

Standards
  • Algorithm-Correctness-Control-Flow
  • Logic-Verification-Consistency
  • Business-Rule-Validation

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Inconsistent PageParam Logic

Logic change creates inconsistency in pageParam selection. For currentPage=0, oldPageParams[0] was replaced with oldPageParams[currentPage], but for currentPage>0 with getNextPageParam defined, the logic still uses getNextPageParam without considering oldPageParams[currentPage].

Standards
  • Algorithm-Correctness-Conditional-Logic
  • Logic-Verification-Consistency
  • Business-Rule-State-Management

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

The condition checks for missing getNextPageParam but uses oldPageParams[currentPage] which may be undefined. This creates inconsistent parameter handling that could lead to unexpected behavior when refetching pages.

Standards
  • Algorithm-Correctness-Null-Safety
  • Logic-Verification-Control-Flow
  • Business-Rule-State-Consistency

Comment on lines +103 to +104
Copy link

Choose a reason for hiding this comment

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

Incorrect PageParam Selection

The condition incorrectly uses currentPage === 0 OR !options.getNextPageParam, which means even if getNextPageParam is defined, it will still use oldPageParams for the first page. This breaks the expected behavior where getNextPageParam should be used for all pages except the first one.

              currentPage === 0
                ? (oldPageParams[0] ?? options.initialPageParam)
Commitable Suggestion
Suggested change
currentPage === 0 || !options.getNextPageParam
? (oldPageParams[currentPage] ?? options.initialPageParam)
currentPage === 0
? (oldPageParams[0] ?? options.initialPageParam)
Standards
  • Algorithm-Correctness-Conditional-Logic
  • Business-Rule-State-Consistency
  • Logic-Verification-Data-Flow

: getNextPageParam(options, result)
if (currentPage > 0 && param == null) {
Comment on lines +103 to 106
Copy link

@refacto-test refacto-test bot Sep 19, 2025 •
edited by visz11
Loading

Choose a reason for hiding this comment

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

Missing Null Check

The code attempts to call getNextPageParam() at line 104 even when options.getNextPageParam is undefined (checked at line 103). This inconsistency could lead to runtime errors when getNextPageParam is undefined.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

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?.(
Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam Creates Inconsistent Behavior

Changed getNextPageParam from required to optional with ?. operator, but code logic still depends on it for subsequent pages. Without proper fallback logic, this can cause runtime errors when function is undefined.

Standards
  • Algorithm-Correctness-Null-Safety
  • Logic-Verification-Function-Existence

Copy link

Choose a reason for hiding this comment

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

Nullable Function Call

Changed getNextPageParam from required to optional with ?. operator but still unconditionally used in logic flow. If undefined, will return undefined instead of expected next page parameter, potentially breaking pagination.

Standards
  • Logic-Verification-Nullability
  • Algorithm-Correctness-Function-Calls
  • Business-Rule-Optional-Parameters

Copy link

Choose a reason for hiding this comment

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

Null Check Missing

Optional chaining added for getNextPageParam but similar null checks are missing elsewhere in the code. This inconsistency could cause runtime errors when getNextPageParam is undefined.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

pages[lastIndex],
Comment on lines +142 to 143
Copy link

Choose a reason for hiding this comment

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

Incorrect Optional Chaining

Optional chaining added to getNextPageParam but the function is already checked for existence on line 104. This creates inconsistent logic flow where the function is treated as both optional and required.

Standards
  • Logic-Verification-Consistency
  • Algorithm-Correctness-Nullability
  • Business-Rule-Validation

Comment on lines +142 to 143
Copy link

Choose a reason for hiding this comment

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

Null Check Missing

Optional chaining added but result can be undefined. Missing null check before using returned pageParam could cause runtime errors if getNextPageParam returns undefined.

Suggested change
? options.getNextPageParam?.(
pages[lastIndex],
? options.getNextPageParam?.(
pages[lastIndex],
pages,
pageParams[lastIndex],
pageParams
) ?? null
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +142 to 143
Copy link

Choose a reason for hiding this comment

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

Nullable Function Call

Optional chaining on getNextPageParam could result in undefined function call if getNextPageParam is not provided. This would cause runtime errors when attempting to invoke undefined as a function.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +142 to 143
Copy link

Choose a reason for hiding this comment

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

Missing Null Check

Optional chaining added for getNextPageParam but missing null check before usage. If undefined, subsequent property access will cause runtime error.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

pages,
pageParams[lastIndex],
Comment on lines +142 to 145
Copy link

Choose a reason for hiding this comment

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

Missing Null Check

Optional chaining on getNextPageParam but no fallback if undefined. Could return undefined when called, potentially causing runtime errors in consumers expecting a value.

Suggested change
? options.getNextPageParam?.(
pages[lastIndex],
pages,
pageParams[lastIndex],
? (options.getNextPageParam?.(
pages[lastIndex],
pages,
pageParams[lastIndex],
) ?? null)
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +142 to 145
Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

Making getNextPageParam optional could cause runtime issues if code expects it to be defined. The optional chaining operator allows it to be undefined, but code might still assume a valid function exists.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Comment on lines +142 to 145
Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

The getNextPageParam function is now optional but used without null check at line 105. This could cause runtime errors if getNextPageParam is undefined but the code at line 105 attempts to call it.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

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 134
Copy link

Choose a reason for hiding this comment

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

Missing Parameter Validation

fetchNextPage doesn't validate pageParam before passing it to fetch. When getNextPageParam is defined, pageParam should be rejected as it would override the calculated value, potentially causing inconsistent pagination behavior.

Standards
  • Business-Rule-Validation
  • Logic-Verification-Parameter-Constraints

})
Comment on lines +127 to 135
Copy link

Choose a reason for hiding this comment

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

Missing Type Validation

The pageParam is passed directly to fetchMore without type validation. Since this is a new parameter that could be undefined, null, or any other value, it should be validated before use to prevent potential type-related issues in the pagination logic.

Standards
  • Business-Rule-Input-Validation
  • Logic-Verification-Data-Flow
  • Algorithm-Correctness-Type-Safety

}
Comment on lines +127 to 136
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Good update to support imperative infinite queries

The fetchNextPage method now accepts an optional pageParam property which enables users to explicitly control the pagination, aligning with the PR's objective to reintroduce imperative infinite queries functionality.

A small issue to note: the pipeline shows a TypeScript error (TS2339) indicating that 'pageParam' doesn't exist on the FetchNextPageOptions type. The corresponding type definition should be updated.

Comment on lines +127 to 136
Copy link

Choose a reason for hiding this comment

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

Optional Parameter Type

The pageParam parameter is destructured but its type isn't explicitly defined in the function signature. This could lead to maintainability issues when other developers need to understand the expected type.

Standards
  • Clean-Code-Type-Safety
  • Clean-Code-Function-Signatures

Comment on lines +127 to 136
Copy link

Choose a reason for hiding this comment

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

Optional PageParam Type

The pageParam parameter is passed without type checking. This could lead to type inconsistencies when manual pageParam values don't match expected types in the query function.

Standards
  • Clean-Code-Type-Safety
  • Design-Pattern-Interface


fetchPreviousPage(
options?: FetchPreviousPageOptions,
): Promise<InfiniteQueryObserverResult<TData, TError>> {
fetchPreviousPage({
pageParam,
...options
}: FetchPreviousPageOptions = {}): Promise<
InfiniteQueryObserverResult<TData, TError>
> {
Comment on lines +138 to +143
Copy link

Choose a reason for hiding this comment

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

Missing PageParam Validation

fetchPreviousPage accepts pageParam without validation. If undefined/null is passed, it could cause unexpected behavior when getNextPageParam is missing.

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

Comment on lines +138 to +143
Copy link

Choose a reason for hiding this comment

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

Missing PageParam Validation

fetchPreviousPage accepts pageParam without validation. Invalid pageParam values could cause unexpected behavior or failures during pagination operations.

Standards
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • ISO-IEC-25010-Reliability-Fault-Tolerance

Comment on lines +127 to +143
Copy link

Choose a reason for hiding this comment

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

Missing pageParam Validation

No validation ensures pageParam is provided when getNextPageParam is missing. Without validation, fetchNextPage/fetchPreviousPage could silently pass undefined pageParam when required, causing runtime errors.

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>
> {
fetchNextPage({ pageParam, ...options }: FetchNextPageOptions = {}): Promise<
InfiniteQueryObserverResult<TData, TError>
> {
if (!this.options.getNextPageParam && pageParam === undefined) {
throw new Error('pageParam is required when getNextPageParam is not defined')
}
return this.fetch({
...options,
meta: {
fetchMore: { direction: 'forward', pageParam },
},
})
}
Standards
  • Business-Rule-Validation
  • Logic-Verification-Parameter-Validation
  • Algorithm-Correctness-Input-Validation

Comment on lines +127 to +143
Copy link

Choose a reason for hiding this comment

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

Optional Parameter Handling

Inconsistent parameter destructuring patterns between fetchNextPage and fetchPreviousPage methods. The first uses inline destructuring while the second uses multi-line format. This inconsistency makes the code harder to maintain as developers need to mentally parse different patterns.

Standards
  • Clean-Code-Consistency
  • Maintainability-Quality-Readability

return this.fetch({
...options,
meta: {
fetchMore: { direction: 'backward' },
fetchMore: { direction: 'backward', pageParam },
},
})
}
Comment on lines +138 to 150
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Support for explicit pageParam added to fetchPreviousPage

This change mirrors the update to fetchNextPage, adding support for explicit page parameters to the fetchPreviousPage method. This implementation maintains consistency between both pagination directions.

The same type issue applies here - ensure that FetchPreviousPageOptions type is updated to include the pageParam property.

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

Choose a reason for hiding this comment

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

Type Definition Update

Making getNextPageParam optional changes the contract significantly. This requires comprehensive documentation updates and might affect consumers who expect this function to always exist.

Standards
  • SOLID-LSP
  • Clean-Code-Interface-Design

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam Without Fallback

Making getNextPageParam optional without fallback behavior could cause runtime failures when used without pageParam. This change requires consumers to provide pageParam manually when getNextPageParam is omitted.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Copy link

Choose a reason for hiding this comment

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

Optional GetNextPageParam Function

Making getNextPageParam optional changes a core API contract. This could lead to inconsistent usage patterns where some implementations use automatic pagination while others require manual pageParam specification.

Standards
  • SOLID-OCP
  • Clean-Code-API-Design

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

Making getNextPageParam optional without updating all dependent code could cause runtime errors. Some code may assume this function always exists, leading to potential null reference exceptions.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

Making getNextPageParam optional without default behavior could cause runtime issues when undefined. Without getNextPageParam, pagination logic depends entirely on manual pageParam values.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Copy link

Choose a reason for hiding this comment

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

Type Definition Mismatch

Making getNextPageParam optional without updating all dependent code paths could cause runtime errors. Some code may assume this function always exists.

Standards
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
  • ISO-IEC-25010-Reliability-Fault-Tolerance

Copy link

Choose a reason for hiding this comment

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

Optional GetNextPageParam

Making getNextPageParam optional without updating all dependent code could cause runtime errors. Function is called directly in infiniteQueryBehavior.ts without null checks.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

Making getNextPageParam optional changes API contract without corresponding type adjustments in fetchNextPage/fetchPreviousPage. This creates inconsistent behavior where manual pageParam is required when getNextPageParam is undefined.

Standards
  • Business-Rule-Validation
  • Logic-Verification-Completeness
  • Algorithm-Correctness-API-Contract

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam Logic

Making getNextPageParam optional without updating all dependent logic creates inconsistency. InfiniteQueryBehavior.ts line 142 still calls getNextPageParam directly without null check, risking runtime errors when getNextPageParam is undefined.

Standards
  • Logic-Verification-Null-Safety
  • Algorithm-Correctness-Optional-Handling
  • Business-Rule-API-Contract

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam Without Code Updates

Making getNextPageParam optional without updating all dependent code could cause runtime failures. Some code paths may assume this function exists without checking, leading to potential undefined behavior.

Suggested change
getNextPageParam?: GetNextPageParamFunction<TPageParam, TQueryFnData>
getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>
Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness

Copy link

Choose a reason for hiding this comment

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

Optional getNextPageParam

Making getNextPageParam optional changes the core API contract. This requires careful consideration of backward compatibility and may lead to inconsistent behavior in existing code that expects getNextPageParam to always be defined.

Standards
  • Maintainability-Quality-API-Stability
  • Clean-Code-Interface-Design

}

export type ThrowOnError<
Expand Down
Loading