Skip to content

Conversation

markerikson
Copy link
Collaborator

@markerikson markerikson commented Dec 1, 2024

This PR:

Note that the "Pagination" example was originally a normal useQuery with a page arg. I started porting it and then ran into issues figuring out how to track the current page. Per Dominik, this is actually a bad use case for an infinite query in the first place ( https://bsky.app/profile/tkdodo.eu/post/3lcbjhjvlpk24 ):

The idea is to only use an infinite query for when you want to display all pages at the same time. A normal query where the currentPage is managed in useState or e.g. a url param is better suited for that case.
The drawback is shown right in your example: fetching is no longer declarative - you need to keep page in-sync with fetchNextPage. And if you are on page 3 and go back to two, then forward to 3 again it falls apart as fetchNextPage would fetch page 4 instead of refetching page 3

I'm keeping it in here for now just because I've already got it written.

In the process I found a couple more features we're missing:

  • we're not exporting auto-generated hooks at the root API object level for infinite queries (at least at the types level, and maybe not for runtime either)
  • definitely not calculating hasNext/PrevPage either

Copy link

codesandbox bot commented Dec 1, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@markerikson markerikson force-pushed the feature/infinite-query-examples-1 branch from cd4e48c to ee81aae Compare December 1, 2024 22:13
Copy link

codesandbox-ci bot commented Dec 1, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 242ac2a:

Sandbox Source
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration
rtk-esm-cra Configuration

@markerikson markerikson merged commit 0f1db3d into feature/infinite-query-integration Dec 1, 2024
69 checks passed
markerikson added a commit that referenced this pull request Dec 14, 2024
* Add blank RTK app

* Nuke initial examples

* Add react-router

* Basic routing

* Add msw

* Configure MSW

* Set up MSW

* Use RTK CSB CI build

* Add baseApi

* Add pagination example

* Add react-intersection-observer

* Add infinite scroll example

* Add max-pages example

* Drop local example lockfile

* Drop back to Vite 5 to fix TS issue

* Align Vite versions to fix test TS error
markerikson added a commit that referenced this pull request Dec 30, 2024
* Add blank RTK app

* Nuke initial examples

* Add react-router

* Basic routing

* Add msw

* Configure MSW

* Set up MSW

* Use RTK CSB CI build

* Add baseApi

* Add pagination example

* Add react-intersection-observer

* Add infinite scroll example

* Add max-pages example

* Drop local example lockfile

* Drop back to Vite 5 to fix TS issue

* Align Vite versions to fix test TS error
@aryaemami59 aryaemami59 deleted the feature/infinite-query-examples-1 branch January 3, 2025 07:28
markerikson added a commit that referenced this pull request Jan 12, 2025
* Add blank RTK app

* Nuke initial examples

* Add react-router

* Basic routing

* Add msw

* Configure MSW

* Set up MSW

* Use RTK CSB CI build

* Add baseApi

* Add pagination example

* Add react-intersection-observer

* Add infinite scroll example

* Add max-pages example

* Drop local example lockfile

* Drop back to Vite 5 to fix TS issue

* Align Vite versions to fix test TS error
@agusterodin
Copy link

agusterodin commented Jan 22, 2025

Continued messing around with my RTKQ + Tanstack Virtual example with filters.

Added filters to both the official Tanstack Query + Virtual example and my RTKQ "port" of the example.

There are two meaningful differences in behavior that I am noticing. I intentionally added a 2 second artificial delay to the fetches so fetching behavior is easier to observe.

Last Page Fetch When Filter Change
Tanstack: When I check and uncheck the "show pinned" filter, if I immediately scroll to the bottom of the list, the new (last) page gets immediately fetched and I don't have to wait around for all other previously fetched (cached) pages to refetch. Once the new page is fetched, the other will pages refetch. Notice that new last page fetches always take priority over refetching of old (already loaded) pages.

RTKQ: When I check and uncheck the "show pinned" filter, if I immediately scroll to the bottom of the list, all of the cached pages refetch first. Once all of these old pages finish refetching, the new page (one at the bottom of list) will then fetch. This can result in a pretty high wait time to see the new page (2s delay * number of old pages that are getting refetched). See :27 timestamp in RTKQ video.

Side point: It's probably ideal for fetching of new page to always take priority, even in situations where scroll position gets reset to top of a list with a bunch of cached pages and user immediately scrolls down to the very bottom.

Copy of All Pages In Cache Get Appended To End In RTKQ After Filter Change Happens and Fetch New Page Finishes
Hard to explain exactly what is happening and it appears as if the correct offset value is being included in every request, but for some reason I am getting a duplicate of all entries in the list appended to end of the data. The easiest way to see what I mean is at the :40 timestamp in the RTKQ video below.

Video Screenshots

RTKQ.Behavior.mp4
Tanstack.Behavior.mp4

Reproduction Repository

https://github.com/agusterodin/rtkq-infinite-query-playground

Side note: When originally playing around with these examples, I created really nice abstractions to clean things up (eg: move logic out of the component into hooks). I ditched those abstractions for now so that it's easier do a more 1-to-1 comparison between RTKQ and Tanstack behavior. I will most definitely re-improve the abstractions in my example and post to discussion board in the future :)

@markerikson
Copy link
Collaborator Author

markerikson commented Jan 22, 2025

@agusterodin : gotcha, thanks for putting this together!

Just to check, what is supposed to be triggering the refetch? Skimmed the code and didn't see anything obvious.

Hmm. Is it maybe the refetchOnMountOrArgChange? What happens if you remove that option?

@agusterodin
Copy link

agusterodin commented Jan 22, 2025

It is probably easier to understand what is happening in this hook I abstracted out. The only reason I got rid of this hook in my latest commit was so the code was closer to the official example.

https://github.com/agusterodin/rtkq-infinite-query-playground/blob/9ccdcba24471c0d8f95e71ec67b87e183f84e40a/app/components/ReduxToolkitExample/useFetchWhenLoaderRowInView.ts

Basically when the elements that virtualizer renders on DOM change (rowVirtualizer.getVirtualItems()), we check to see if the last item in the virtual list is a "loader row".

Will play around with refetchOnMountOrArgChange right quick.

@agusterodin
Copy link

agusterodin commented Jan 22, 2025

At a glance it appears to behave much more ideally when I set refetchOnMountOrArgChange to false. The new page gets fetched immediately when changing back filters and scrolling to bottom + the "duplicate list appended to end" behavior doesn't occur.

@markerikson
Copy link
Collaborator Author

Okay, good, that's what I was hoping :) Is there a reason you needed to have that turned on?

@agusterodin
Copy link

agusterodin commented Jan 22, 2025

Mostly so that we can keep cache around for a long period of time (high keepUnusedDataFor value) to avoid excessive loading spinners, but still keep data fresh. Eg: user leaves the route that had the infinite query list but comes back later to that route after a few minutes.

We keep the global setting on, but occasionally disable it on a per-hook basis if we're dealing with a situation where we fetch large payloads that don't change. Eg: we have a dashboard that fetches 10mb+ of data from an endpoint and we don't have to worry about the dashboard "results" changing after processing finishes.

@markerikson
Copy link
Collaborator Author

Gotcha.

Yeah, having it "refetch before fetching the next page" in this example sounds like what I would expect - you changed the args, and doing that is configured to trigger a refetch, so it does that immediately and that has to complete before a next page can be fetched. So, sounds like "working as intended" in this case.

The duplicate data does seem like a bug :) I'll have to see if I can repro that.

@agusterodin
Copy link

agusterodin commented Jan 22, 2025

It appears the Tanstack example does perform the refetch on mount / arg change but is always prioritizes the newest page before going back and refetching the old ones.

For the particular use-case of a virtualized list, this seems ideal. Otherwise you could end up waiting a while before finally being able to see the new page. I will dig through Tanstack documentation to see if this is something they let you customize.

Unsure of unintended consequences in Tanstack’s behavior though.

markerikson added a commit that referenced this pull request Feb 20, 2025
* Add blank RTK app

* Nuke initial examples

* Add react-router

* Basic routing

* Add msw

* Configure MSW

* Set up MSW

* Use RTK CSB CI build

* Add baseApi

* Add pagination example

* Add react-intersection-observer

* Add infinite scroll example

* Add max-pages example

* Drop local example lockfile

* Drop back to Vite 5 to fix TS issue

* Align Vite versions to fix test TS error
@markerikson markerikson mentioned this pull request Feb 20, 2025
26 tasks
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.

2 participants