Skip to content
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.

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

Infinity Query Refetching Not Considering Original Fetch Direction Of First Page #7407

Closed
anatolzak opened this issue May 10, 2024 · 5 comments
Labels
duplicate This issue or pull request already exists

Comments

@anatolzak
Copy link

Describe the bug

The issue I am going to describe occurs when using useInfiniteQuery with bi-directional cursor pagination following the below logic.

The server accepts a cursor and a direction ('forward' | 'backward').
If the direction is forward, the server returns items greater than or equal to the cursor.
If the direction is backward, the server returns items smaller then or equal to the cursor.

Each response contains:

  • 5 items
  • nextId, which is the ID of the first item in the next page
  • previousId, which is the ID of the last item in the previous page

The initialPageParam is 0, so the initial request data is { cursor: 0, direction: 'forward' }, and the response will be:

{
    items: [0, 1, 2, 3, 4],
    previousId: -1,
    nextId: 5,
}

Let's say we fetch the previous page, the request data is { cursor: -1, direction: 'backward' }, and the response will be:

{
    items: [-5, -4, -3, -2, -1],
    previousId: -6,
    nextId: 0,
}

So currently pages will contain:

[
    { items: [-5, -4, -3, -2, -1], previousId: -6, nextId: 0 },
    { items: [0, 1, 2, 3, 4], previousId: -1, nextId: 5 }
]

And pageParams will contain: [-1, 0]. The first pageParam is -1 because we fetched the previous page with a cursor of -1. And the second pageParam is 0 because the initialPageParam was 0.

Now, if React Query refetches the infinite query, each group will be fetched sequentially, starting from the first one. What will happen is that the first group will be fetched with a cursor of -1, however React Query will pass a direction of forward, even though that group was fetched during backwards navigation.

So after refetching, pages will contain the following which does not match the original pages before the refetching occured.

[
    { items: [-1, 0, 1, 2, 3], previousId: 0, nextId: 4 },
    { items: [4, 5, 6, 7, 8], previousId: 3, nextId: 9 }
]

Your minimal, reproducible example

https://stackblitz.com/edit/vitejs-vite-ri6fvw?file=src%2FApp.tsx

Steps to reproduce

  1. Go to the provided stackblitz
  2. Wait for the initial infinite query page to load.
  3. Click Load Older and wait for previous page to load.
  4. Note that the items showing are from -5 to 4
  5. Click the Unmount Component button at the top of the screen
  6. Click the Mount Component button at the top of the screen
  7. Wait for refetching to occur.
  8. Note that the items showing now are from -1 to 8

Expected behavior

During refetching, I would expect react query to pass the original direction of the first page that is being refetched.

How often does this bug happen?

Every time

Screenshots or Videos

Screen.Recording.2024-05-10.at.1.42.57.PM.mp4

Platform

  • OS: macOS
  • Browser: Chrome
  • Version: 124.0.6367.155

Tanstack Query adapter

react-query

TanStack Query version

v5.35.1

TypeScript version

v5.2.2

Additional context

No response

@TkDodo
Copy link
Collaborator

TkDodo commented May 10, 2024

duplicate of #7203

I actually have a PR open for that but kinda forgot about it 🙈

@TkDodo TkDodo closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2024
@TkDodo TkDodo added the duplicate This issue or pull request already exists label May 10, 2024
@TkDodo
Copy link
Collaborator

TkDodo commented May 10, 2024

actually, I'm not sure if I should merge the PR. Maybe the mistake was passing direction into the queryFn in the first place.

What I think should work is to do this yourself within the pageParam.

If pageParam is an object with { id: number, direction: 'forward' | 'backward' }, you could build the value yourself with getNextPageParam and getPreviousPageParam.

If that works, I think marking direction passed to the queryFunctionContext as deprecated would be the better way to go.

here would be a working reproduction:

https://stackblitz.com/edit/vitejs-vite-xmwghr?file=src%2FApp.tsx

what do you think?

@anatolzak
Copy link
Author

anatolzak commented May 10, 2024

@TkDodo I like the idea. With what you are suggesting, when refetching, would every page use the same direction as it was originally? If so, I think there is a problem with this.

We should only use a direction of backward for the first page if the direction was backward. For following pages, we should use direction: 'forward' and build the pageParam out of getNextPageParam.

@TkDodo
Copy link
Collaborator

TkDodo commented May 10, 2024

when refetching, only the first fetch will take the stored pageParams. Then, we call getNextPageParam() on each page to a) see if we still need to continue fetching and b) to get the params for that page.

So yes, after a refetch, the first page has the original param and all others would have the one from getNextPageParam.

@anatolzak
Copy link
Author

anatolzak commented May 10, 2024

@TkDodo in that case, I think what you purposed is definitely the better solution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants