Skip to content

Conversation

minchodang
Copy link

@minchodang minchodang commented Sep 23, 2025

Changes

What does this PR change? Link to any related issue(s).
close #2458

How to Review

This PR closes a type-safety hole in useInfiniteQuery: required pagination options are now enforced at compile time and the pageParam type is correctly inferred from initialPageParam.

What changed (at a glance)
• Introduced InferPageParamType and threaded it into UseInfiniteQueryMethod so pageParam is inferred from options.initialPageParam.
• Required initialPageParam in the useInfiniteQuery options we expose.
• Plumbed initialPageParam through to the actual useInfiniteQuery call and removed the implicit pageParam = 0 default.
• Goal: make “forgetting getNextPageParam/initialPageParam” a TypeScript error, and restore proper inference for pageParam.

Files to open first
• Types where UseInfiniteQueryMethod is declared (look for InferPageParamType usage).
• createClient(...) implementation (look for useInfiniteQuery(...) and the extraction of initialPageParam).

Verification steps

  1. Type-level checks (compile-time)

Use any endpoint type; the shape here is illustrative.

type Page = { items: string[]; next?: string };

declare const api: ReturnType;

// ❌ Should FAIL: missing getNextPageParam
api.examples.list.useInfiniteQuery(
{ limit: 10 },
{
initialPageParam: undefined,
// @ts-expect-error getNextPageParam is required for infinite queries
}
);

// ❌ Should FAIL: wrong initialPageParam type
api.examples.list.useInfiniteQuery(
{ limit: 10 },
{
// @ts-expect-error initialPageParam must match pageParam type
initialPageParam: 0,
getNextPageParam: (last: Page) => last.next,
}
);

// ✅ Should PASS: both required options provided; pageParam is inferred as string | undefined
api.examples.list.useInfiniteQuery(
{ limit: 10 },
{
initialPageParam: undefined,
getNextPageParam: (last: Page) => last.next,
select: (data) => data, // keep an eye on select inference too
}
);

If the repo uses tsd, add two assertions:

// tsd
expectError(
useInfiniteQuery({ /* ... /, initialPageParam: undefined }) // missing getNextPageParam
);
expectAssignable(
useInfiniteQuery({ /
... */, initialPageParam: undefined, getNextPageParam: (p: Page) => p.next })
);

What to keep in mind
• pageParam in the queryFn context should now be inferred from initialPageParam.
• Omitting getNextPageParam should fail type-check.
• select should continue to infer correctly for InfiniteData.

  1. Runtime sanity (manual)

    1. Wire a trivial endpoint returning { items, next }.
    2. Call the generated useInfiniteQuery with:
      • initialPageParam: undefined
      • getNextPageParam: (last) => last.next
    3. Click a “Load more” button calling fetchNextPage().
      Expect: pagination works as before. No behavior regression from removing the pageParam = 0 default.
  2. Backward compatibility
    • Existing callers that already provided both initialPageParam and getNextPageParam continue to compile and run.
    • Callers relying on the old implicit pageParam = 0 default will need to set an explicit initialPageParam—this is an intentional tightening for safety.
    • Non-infinite useQuery paths are unaffected.

  3. Code reading checklist
    • InferPageParamType does not widen to unknown in common cases.
    • useInfiniteQuery options intersection requires initialPageParam.
    • The option set for useInfiniteQuery effectively requires getNextPageParam (no recursive type loosening).
    • select and error/data generics remain intact (no any leakage).
    • No runtime dead paths introduced; initialPageParam is passed through to TanStack.

  4. Edge cases to spot-check
    • Custom pageParamName still works with the stricter types.
    • Non-cursor pagination (numeric pages) with initialPageParam: 1 and getNextPageParam: (last, pages) => pages.length + 1.
    • previousPageParam flows (where applicable) still type-check when used.

Risks & mitigations
• Risk: Slight source break for callers who omitted initialPageParam/getNextPageParam.
Mitigation: compile-time error with clear fix; matches TanStack’s intended contract for infinite queries.

Reviewer tips
• Try intentionally removing getNextPageParam in a local example: it should fail the build.
• Hover pageParam inside the queryFn: it should be the exact type implied by initialPageParam (e.g., string | undefined, not unknown).
• Confirm no change to emitted JS other than threading initialPageParam.

Author note
I have this patch running locally. If there’s appetite, I can add tsd tests in this PR to lock the constraints in CI.

Checklist

  • Unit tests updated
  • docs/ updated (if necessary)
  • pnpm run update:examples run (only applicable for openapi-typescript)

@minchodang minchodang requested a review from a team as a code owner September 23, 2025 12:37
Copy link

changeset-bot bot commented Sep 23, 2025

⚠️ No Changeset found

Latest commit: cd3e971

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented Sep 23, 2025

Deploy Preview for openapi-ts failed.

Name Link
🔨 Latest commit cd3e971
🔍 Latest deploy log https://app.netlify.com/projects/openapi-ts/deploys/68d294a0f21a790008102d1c

@minchodang
Copy link
Author

@duncanbeevers @martyndavies @garyposter
Hello.
I want to review my code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useInfiniteQuery types are unsafe: required pagination options (e.g. getNextPageParam) not enforced due to recursive types
1 participant