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

[DRAFT] RTKQ Infinite Query integration #4738

Draft
wants to merge 60 commits into
base: master
Choose a base branch
from

Conversation

markerikson
Copy link
Collaborator

@markerikson markerikson commented Nov 29, 2024

Overview

This is a running integration PR for the upcoming RTK Query support for "infinite / paginated queries".

The overall plan is to reimplement React Query's public API for infinite queries on top of RTK Query's internals.

This work was started by @riqts in [API Concept] - Infinite Query API. I picked up that work, rebased it, and have fleshed out the TS types, tests, and implementation. See that PR for the original discussion and progress. Also, thanks to @TkDodo for the advice on how they designed the infinite query API for React Query!

My current somewhat ambitious goal is to ship a final version of infinite query support for RTKQ by the end of this year. I am absolutely not going to guarantee that :) It's entirely dependent on how much free time I have to dedicate to this effort, how complicated this turns out to be, and how much polish is needed. But in terms of maintenance effort, shipping this is now my main priority!

Status

It currently works sufficiently that it's ready to be tried out in example apps so that we can confirm it works across a variety of use cases, and find bugs or areas that need work. It's not ready for an official alpha yet.

You can try installing the PR preview build using the installation instructions from the "CodeSandbox CI" job listed at the bottom of the PR. Please leave comments and feedback here!

I've got a laundry list of examples and tests that ought to be added - see the "Todos" section below. Contributions are appreciated!

Preview Builds

Open the "CodeSandbox CI" details check:

Click the most recent commit on the left, then copy-paste the install instructions for your package manager from the "Local Install Instructions" section on the right, such as:

# yarn 1
yarn add https://pkg.csb.dev/reduxjs/redux-toolkit/commit/1e5789f1/@reduxjs/toolkit 

# yarn 2, 3
yarn add @reduxjs/toolkit@https://pkg.csb.dev/reduxjs/redux-toolkit/commit/1e5789f1/@reduxjs/toolkit/_pkg.tgz 

# npm
npm i https://pkg.csb.dev/reduxjs/redux-toolkit/commit/1e5789f1/@reduxjs/toolkit 

Todos

Jotting down some todos as I think of them:

Functionality

  • Auto-generate infinite query hooks (types and runtime)
  • Refetching
    • refetching with hooks?
  • Max pages
    • enforce both gN/PPP when maxPages > 0
  • isFetchingNext/PreviousPage flags
  • hasNext/PreviousPage flags
  • Investigate moving pageParams into some new metadata field in the cache entry, so that it's not directly exposed to the user in data (per discussion with Dominik) This probably still makes sense to keep with the data in case of users upserting
  • Possibly some kind of combinePages option, so that you don't have to do selectFromResult: ({data}) => data.pages.flat() (and memoize it) for every endpoint
    • do we wrap this in createSelector by default?
  • See how much of the types and logic can be deduplicated

Tests / Example Use Cases

React Query examples

ref:

Other

  • optimistic updates? (what does this even look like, conceptually and usage-wise?)
  • upsertQueryData / upsertQueryEntries?
  • RN FlatList
  • CRUD edits to pages?
  • Tag invalidation of an infinite endpoint

Copy link

codesandbox bot commented Nov 29, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

codesandbox-ci bot commented Nov 29, 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 db14ff5:

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

Copy link

github-actions bot commented Nov 29, 2024

size-limit report 📦

Path Size
1. entry point: @reduxjs/toolkit/query (modern.mjs) 3.7 KB (+0.4% 🔺)
1. entry point: @reduxjs/toolkit/query/react (modern.mjs) 14.47 KB (+6.79% 🔺)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (modern.mjs) 56 B (-22.23% 🔽)
1. entry point: @reduxjs/toolkit (cjs, production.min.cjs) 14.14 KB (+0.03% 🔺)
1. entry point: @reduxjs/toolkit/react (cjs, production.min.cjs) 14.29 KB (+0.07% 🔺)
1. entry point: @reduxjs/toolkit/query (cjs, production.min.cjs) 23.32 KB (+3.97% 🔺)
1. entry point: @reduxjs/toolkit/query/react (cjs, production.min.cjs) 25.73 KB (+5.01% 🔺)
2. entry point: @reduxjs/toolkit (without dependencies) (cjs, production.min.cjs) 7.5 KB (-0.06% 🔽)
2. entry point: @reduxjs/toolkit/query (without dependencies) (cjs, production.min.cjs) 10.12 KB (+9.76% 🔺)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (cjs, production.min.cjs) 3.12 KB (+8.56% 🔺)
3. createSlice (.modern.mjs) 4.2 KB (-0.03% 🔽)
3. buildCreateSlice and asyncThunkCreator (.modern.mjs) 5.03 KB (+0.04% 🔺)
3. createEntityAdapter (.modern.mjs) 4.92 KB (+0.26% 🔺)
3. createListenerMiddleware (.modern.mjs) 1.98 KB (-0.05% 🔽)
3. createApi (.modern.mjs) 14.88 KB (+6.68% 🔺)
3. createApi (react) (.modern.mjs) 17 KB (+8.44% 🔺)
3. fetchBaseQuery (.modern.mjs) 4.64 KB (+0.19% 🔺)

Copy link

netlify bot commented Nov 29, 2024

Deploy Preview for redux-starter-kit-docs ready!

Name Link
🔨 Latest commit db14ff5
🔍 Latest deploy log https://app.netlify.com/sites/redux-starter-kit-docs/deploys/67747cd9afe8d500080af3cc
😎 Deploy Preview https://deploy-preview-4738--redux-starter-kit-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@markerikson
Copy link
Collaborator Author

@remus-selea you reported over in #4393 (comment) that:

I've noticed that upsertQueryData no longer behaves as it used to. I verified with the Redux DevTools extension and it now removes the data field entirely for the query.

I just tried reproducing that and I'm not seeing it happen with the current PR. It's entirely possible my test doesn't match what you were doing. Could you give me a repro?

@markerikson
Copy link
Collaborator Author

Knocked out maxPages and refetching. Good progress for the day!

@markerikson
Copy link
Collaborator Author

markerikson commented Dec 1, 2024

Added an example app that ports over the main React Query sandboxes for infinite queries. (Technically "pagination" didn't use an infinite query, and in fact Dominik says you shouldn't use an infinite query for pagination, but I'd already gotten something working so I kept it.)

In the process I found out that:

  • 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

@remus-selea
Copy link

remus-selea commented Dec 8, 2024

@remus-selea you reported over in #4393 (comment) that:

I've noticed that upsertQueryData no longer behaves as it used to. I verified with the Redux DevTools extension and it now removes the data field entirely for the query.

I just tried reproducing that and I'm not seeing it happen with the current PR. It's entirely possible my test doesn't match what you were doing. Could you give me a repro?

As I worked on creating a demo of the bug I discovered more details. The bug seems to only happen for queries that take no arguments.

Codesandbox where the issue occurs: https://codesandbox.io/p/sandbox/spring-star-dtc2gy
Another codesandbox with identical code except for the Redux Toolkit version and the removal of the infinte query endpoint, where the issue doesn't occur: https://codesandbox.io/p/sandbox/rtk-2-4-upserquerydata-44fd46

@guaizi149
Copy link

hi, I have been using rtk-query, but now I need this function urgently. Is there a timeline for releasing the beta version for infinite query?

@markerikson
Copy link
Collaborator Author

@guaizi149 : no timeline, but the code in this PR works enough that we're asking people to try it out, let us know if they see bugs, and offer feedback on the API design.

See the top comment for installation instructions (and be sure to actually look in the CodeSandbox CI job for the actual latest commit installation command).

@agusterodin
Copy link

agusterodin commented Dec 11, 2024

Is there any way to access the input parameter provided to the infinite query hook inside of the endpoint definition's query callback function? I can't access the filters I passed in in order to include them in request.

Test.tsx

import { queryEndpoints } from 'api/query'

export default function InfinteQueryTest() {
  const filters = {
    includeOtherUsers: true,
    orderBy: 'id'
  }

  const { } = queryEndpoints.getQueriesCursor.useInfiniteQuery(filters)

  return <div>Chuck</div>
}

query.ts

import { QueryListResponse } from 'datamodels'
import { xplorerApi } from './index'

const queryApiSlice = xplorerApi.injectEndpoints({
  endpoints: builder => ({
    // Generic: <Response type, input type (filters), type that we're using use to keep track of pages>
    getQueriesCursor: builder.infiniteQuery<QueryListResponse, string, number>({
      // We can access variable that keeps track of page, but are unable to access filters because there is no second parameter in callback
      query: (offset, filters) => ({
        url: `queries?offset=${offset}`,
        params: filters
      }),
      infiniteQueryOptions: {
        initialPageParam: 0,
        getPreviousPageParam: (firstPage, allPages, firstPageParam) => {
          if (firstPageParam - 50 > 0) {
            return firstPageParam - 50
          }
        },
        getNextPageParam: (lastPage, allPages, lastPageParam) => {
          if (lastPage.totalResultcount < lastPageParam + 50) {
            return lastPageParam + 50
          }
        }
      }
    })
  })
})

export const { endpoints: queryEndpoints, util: queryUtil } = queryApiSlice

@markerikson
Copy link
Collaborator Author

@agusterodin : you'd have to change the page param type to be {offset: string, page: number}, then update the getNextPageParam callback to actually return that object.

@agusterodin
Copy link

agusterodin commented Dec 12, 2024

@agusterodin : you'd have to change the page param type to be {offset: string, page: number}, then update the getNextPageParam callback to actually return that object.

Oops, my example was screwed up. I meant to have the endpoint generics set up like this:

interface QueryListFilters {
  includeOtherUsers: boolean
  orderBy: 'id' | 'name'
}

const queryApiSlice = xplorerApi.injectEndpoints({
  endpoints: builder => ({
    // Generic: <Response type, input type (filters), type that we're using use to keep track of pages>
    getQueriesCursor: builder.infiniteQuery<QueryListResponse, QueryListFilters, number>({
...

Are you suggesting to do this? I must misunderstand how infinite queries work because this feels like a workaround.

import { QueryListResponse } from 'datamodels'
import { xplorerApi } from './index'

interface QueryListFilters {
  includeOtherUsers?: boolean
  orderBy?: 'id' | 'name'
}

const queryApiSlice = xplorerApi.injectEndpoints({
  endpoints: builder => ({
    getQueriesCursor: builder.infiniteQuery<QueryListResponse, QueryListFilters, { offset: number } & QueryListFilters>({
      query: ({ offset, ...filters }) => ({
        url: `query?offset=${offset}`,
        params: filters
      }),
      infiniteQueryOptions: {
        initialPageParam: { offset: 0 },
        getPreviousPageParam: (firstPage, allPages, firstPageParam) => {
          const { offset, ...filters } = firstPageParam
          if (firstPageParam.offset - 50 > 0) {
            return { offset: offset - 50, filters }
          }
        },
        getNextPageParam: (lastPage, allPages, lastPageParam) => {
          const { offset, ...filters } = lastPageParam
          if (lastPage.totalResultCount < offset + 50) {
            return { offset: offset + 50, ...filters }
          }
        }
      }
    })
  })
})

export const { endpoints: queryEndpoints, util: queryUtil } = queryApiSlice

@markerikson
Copy link
Collaborator Author

@agusterodin : I think that looks right, yeah. Think of it this way: pageParam is essentially what the query arg is for a normal query endpoint, but you need to update that for each different page.

One other nuance here:

There ends up being a separation between the "cache key" value, which is still what you pass to the query hook, and the "initial page param" value. In your case, I think they end up being related - ie, you probably do want to use the filters as the cache key ("all the pages are results for this set of filters"), and then also pass them as the override initial hook value:

// The overall cache entry containing _all_ the pages
// will be keyed by serializing `filters`
const { data } = useMyInfiniteQuery(filters, {
  infiniteQueryOptions: {
    // in order to fetch any given page, we need both `filters` and `offset`
    initialPageParam: {filters, offset: 0}
  }
})

But yeah, questions like this are also why we're asking people to try this out :)

@agusterodin
Copy link

agusterodin commented Dec 13, 2024

Attempting to "port" this Tanstack Query/Virtual infinite scroll example to use RTKQ. Assuming this would be a relatively common use-case.

I got stuck at hasNextPage always being false (looks like this is something on your to-do list).

Not sure if it is helpful, but leaving this here to show how i'm attempting to use the infinite query implementation. Will continue testing against future versions and leave feedback!

InfiniteQueryTest.tsx

import { RefObject, useEffect, useRef } from 'react'
import { useVirtualizer } from '@tanstack/react-virtual'
import { infiniteQueryEndpoints, QueryListFilters } from 'api/infiniteQuery'
import LoadingSpinner from 'common/components/LoadingSpinner'

const filters: QueryListFilters = {
  all: true,
  orderby: 'id|DESC' as const
}

export default function InfiniteQueryTest() {
  const parentRef = useRef<HTMLDivElement>(null)

  const { allQueries, infiniteQuery, rowVirtualizer } = useInfiniteVirtualizedQueryList(parentRef)
  const { isLoading, isError, hasNextPage } = infiniteQuery

  if (isLoading) {
    return <div>Loading...</div>
  }

  if (isError) {
    return <div>Could not load data.</div>
  }

  return (
    <div className="p-4">
      <div ref={parentRef} className="h-96 w-full max-w-full overflow-auto rounded-lg border border-gray-400 bg-white">
        <div
          className="relative w-full"
          style={{
            height: `${rowVirtualizer.getTotalSize()}px`
          }}
        >
          {rowVirtualizer.getVirtualItems().map(virtualRow => {
            const isLoaderRow = virtualRow.index > allQueries.length - 1
            const query = allQueries[virtualRow.index]

            return (
              <div
                key={virtualRow.index}
                className="absolute left-0 top-0 w-full"
                style={{
                  height: `${virtualRow.size}px`,
                  transform: `translateY(${virtualRow.start}px)`
                }}
              >
                {isLoaderRow && hasNextPage ? <LoadingSpinner className="h-7 w-7 text-teal-500" /> : <div className="px-2">{query.name}</div>}
              </div>
            )
          })}
        </div>
      </div>
    </div>
  )
}

function useInfiniteVirtualizedQueryList<T extends HTMLElement>(parentRef: RefObject<T>) {
  const infiniteQuery = infiniteQueryEndpoints.getQueriesCursor.useInfiniteQuery(filters, {
    pollingInterval: 100000,
    initialPageParam: { offset: 0, ...filters }
  })

  const { hasNextPage, data, fetchNextPage, isFetchingNextPage } = infiniteQuery

  const allQueries = data ? data.pages.flatMap(d => d.queries) : []

  const rowVirtualizer = useVirtualizer({
    count: hasNextPage ? allQueries.length + 1 : allQueries.length,
    getScrollElement: () => parentRef.current,
    estimateSize: () => 23,
    overscan: 5
  })

  useEffect(() => {
    const [lastItem] = [...rowVirtualizer.getVirtualItems()].reverse()

    if (!lastItem) return

    if (lastItem.index >= allQueries.length - 1 && hasNextPage && !isFetchingNextPage) {
      fetchNextPage()
    }
  }, [hasNextPage, fetchNextPage, allQueries.length, isFetchingNextPage, rowVirtualizer.getVirtualItems()])

  return {
    allQueries,
    infiniteQuery,
    rowVirtualizer
  }
}

@markerikson
Copy link
Collaborator Author

markerikson commented Dec 13, 2024

@agusterodin yeah, if you look at the PR I actually did port that example already :) scratch that, didn't realize you were pointing to TanStack Virtual there - I ported the similar-but-different TanStack Query example.

and yes, we don't have hasNextPage implemented built-in yet, so I had to calculate it while rendering.

@markerikson markerikson force-pushed the feature/infinite-query-integration branch from 0f1db3d to a28de8f Compare December 14, 2024 21:17
@markerikson
Copy link
Collaborator Author

markerikson commented Dec 15, 2024

@agusterodin I just put up a PR implementing all of the infinite query status flags over in #4771 - want to give that a shot?

went ahead and merged in the PR - it also fixed a types error after I rebased this branch, and I feel good about the implementation.

@agusterodin
Copy link

agusterodin commented Dec 19, 2024

Just tried commit 33e30af. Sorry for delayed response.

For some reason the network requests being made to get the "next page" are always the same as the initial request (first page, offset of 0).

Note that the code below is based off of the example you provided under the examples directory (not using Tanstack Virtual).

InfiniteQueryTest.tsx (barely modified from original example). The only noteworthy difference is that I provide initialPageParam to the infinite query hook.filters is intentionally just a hardcoded constant for the time being.

import React from 'react'
import { useInView } from 'react-intersection-observer'
import { infiniteQueryEndpoints, QueryListFilters } from 'api/infiniteQueryApi'

const filters: QueryListFilters = {
  all: true,
  orderby: 'id|DESC' as const
}

export default function InfiniteQueryTest() {
  const { data, error, fetchNextPage, fetchPreviousPage, hasNextPage, isFetchingNextPage, isFetching, isError } =
    infiniteQueryEndpoints.getQueriesCursor.useInfiniteQuery(filters, { initialPageParam: { offset: 0, ...filters } })

  const { ref, inView } = useInView()

  React.useEffect(() => {
    if (inView) {
      console.log('Fetching next page')
      fetchNextPage()
    }
  }, [fetchNextPage, inView])

  return (
    <div className="overflow-auto">
      <h2>Infinite Scroll Example</h2>
      {isFetching ? <p>Loading...</p> : isError ? <span>Error: {error.message}</span> : null}
      {
        <>
          <div>
            <button
              onClick={() => fetchPreviousPage()}
              // disabled={!hasPreviousPage || isFetchingPreviousPage}
            >
              {/* {isFetchingPreviousPage
                ? "Loading more..."
                : hasPreviousPage
                  ? "Load Older"
                  : "Nothing more to load"} */}
            </button>
          </div>
          {data?.pages.map(page => (
            <React.Fragment key={page.queries[0].id}>
              {page.queries.map(query => (
                <p
                  style={{
                    border: '1px solid gray',
                    borderRadius: '5px',
                    padding: '1rem 1rem',
                    background: 'red'
                  }}
                  key={query.id}
                >
                  {query.name}
                </p>
              ))}
            </React.Fragment>
          ))}
          <div>
            <button ref={ref} onClick={() => {
              console.log('Clicked "load newer" button')
              fetchNextPage()
            }} disabled={!hasNextPage || isFetchingNextPage}>
              {isFetchingNextPage ? 'Loading more...' : hasNextPage ? 'Load Newer' : 'Nothing more to load'}
            </button>
          </div>
        </>
      }
    </div>
  )
}

infiniteQueryApi.ts. Our backend doesn't use cursors, we have an offset (starting index of items to return) and limit (amount of items to return).

import { QueryListResponse } from 'datamodels'
import { xplorerApi } from './index'

export interface QueryListFilters {
  all?: boolean
  orderby?: 'id|DESC'
}

const PAGE_SIZE = 50

const queryApiSlice = xplorerApi.injectEndpoints({
  endpoints: builder => ({
    // Generic: <Response type, input type (filters), cache key (page + filters)>
    getQueriesCursor: builder.infiniteQuery<QueryListResponse, QueryListFilters, { offset: number } & QueryListFilters>({
      query: filtersAndOffset => {
        console.log('filters and offset being sent with request', filtersAndOffset)
        return {
          url: 'query/',
          params: { ...filtersAndOffset, limit: PAGE_SIZE }
        }
      },
      infiniteQueryOptions: {
        initialPageParam: { offset: 0 },
        getPreviousPageParam: (firstPage, allPages, firstPageParam) => {
          const { offset, ...filters } = firstPageParam
          if (offset - PAGE_SIZE >= 0) {
            return { ...filters, offset: offset - PAGE_SIZE }
          }
        },
        getNextPageParam: (lastPage, allPages, lastPageParam) => {
          const { offset, ...filters } = lastPageParam
          if (lastPage.count >= offset + PAGE_SIZE) {
            console.log('There is a next page and its offset is: ', offset + PAGE_SIZE)
            return { ...filters, offset: offset + PAGE_SIZE }
          }
        }
      }
    })
  }),
})

export const { endpoints: infiniteQueryEndpoints } = queryApiSlice

Notice from the below screenshot that the getNextPageParam function is getting called prior to when I scroll to the bottom of the page (inView becomes true) or when I manually click the "load newer" button at the bottom of the page. It appears that the offset for the second page is computed correctly inside of my logic ("there is a next page and its offset is: 50" is printed to console), but when the request is made and the query function is called, the value of offset is always 0.

image

Also, another general API question: I have two initialPageParams defined (one in hook and another in endpoint definition). Will this cause conflict? initialPageParams in the endpoint definition was a required field, so I had to put something there despite the fact that I don't think it will ever be used.

PS: sorry that all I have is a shitty screenshot and code snippets. I was contemplating setting up a minimal Vite project, but I wouldn't be able to have it send requests to our backend because it is hitting an authenticated endpoint which makes the setup hard to recreate. I would have attached a video screenshot at the bare minimum, but the MacOS video screenshot tool is apparently broken in the current version of MacOS. If what I provided doesn't just contain a trivial error and isn't adequate to help you identify the issue, I will set something better up.

@markerikson
Copy link
Collaborator Author

@agusterodin yeah, if you could give me a project that shows the inconsistent next page behavior, that would definitely help!

I have two initialPageParams defined (one in hook and another in endpoint definition). Will this cause conflict?

As with the other hook options, providing an option in the hook overrides the default value supplied in the endpoint. Think of it as the general equivalent of a default value for a function argument or destructured object field - if you don't pass it in the hook, we have to have something to fall back to.

@agusterodin
Copy link

Identified issue. If you have the global refetchOnMountOrArgChange: true option set on your API, fetching the next page in infinite queries won't work.

Uncomment line refetchOnMountOrArgChange: true line in lib/notesApi.ts in below repository and the bug will occur.

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

markerikson and others added 21 commits December 28, 2024 12:31
# Conflicts:
#	packages/toolkit/src/query/core/buildThunks.ts
#	packages/toolkit/src/query/index.ts
#	packages/toolkit/src/query/react/buildHooks.ts
* 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
* Extract InfiniteQueryDirection

* Export page param functions

* Fix useRefs with React 19

* Fix infinite query selector arg type

* Implement infinite query status flags

* Fix types and flags for infinite query hooks

* Add new error messages
@markerikson markerikson force-pushed the feature/infinite-query-integration branch from 87270fe to 79f6ff8 Compare December 30, 2024 19:35
…4795)

* Consolidate test assertions

* Add failing tests for infinite queries vs refetching

* Tweak infinite query forced check
@markerikson
Copy link
Collaborator Author

@agusterodin Thanks for the repro, that made it pretty easy to identify the cause.

The internal logic had this sequence:

        // Start by looking up the existing InfiniteData value from state,
        // falling back to an empty value if it doesn't exist yet
        const blankData = { pages: [], pageParams: [] }
        const cachedData = getState()[reducerPath].queries[arg.queryCacheKey]
          ?.data as InfiniteData<unknown, unknown> | undefined
        const existingData = (
          isForcedQuery(arg, getState()) || !cachedData ? blankData : cachedData
        ) as InfiniteData<unknown, unknown>

Problem is that isForcedQuery() includes a check against api.refetchOnMountOrArgChange too:

function isForcedQuery(
    arg: QueryThunkArg,
    state: RootState<any, string, ReducerPath>,
  ) {
    const requestState = state[reducerPath]?.queries?.[arg.queryCacheKey]
    const baseFetchOnMountOrArgChange =
      state[reducerPath]?.config.refetchOnMountOrArgChange

    const fulfilledVal = requestState?.fulfilledTimeStamp
    const refetchVal =
      arg.forceRefetch ?? (arg.subscribe && baseFetchOnMountOrArgChange)

    if (refetchVal) {
      // Return if it's true or compare the dates because it must be a number
      return (
        refetchVal === true ||
        (Number(new Date()) - Number(fulfilledVal)) / 1000 >= refetchVal
      )
    }
    return false
  }

So, it was always opting to use blankData instead of the cache data, therefore it it didn't have any existing page params to use, and it never fetches the next page.

I spent a while staring at it, and eventually changed the line to use a different flag we set internally when you actually call refetch(). I'm still not convinced that change is right, but I wrote some new tests that exhibited this broken behavior, and it at least lets those tests pass.

(also I figured out that we have two different refetchOnMountOrArgChange flag definitions. If you define it on the API, it gets used here. If you pass it in to the hooks, it gets used inside of the hook itself, at the React level. Interestingly, we don't currently support overriding that at the endpoint level, which seems like an omission to me, especially since some other config options can be done per-endpoint.)

* Add bidirectional cursor infinite scroll example

* Add example using limit and offset
- Add example using page and size
- add an intersection observer callback ref hook

* Bump infinite query RTK version

---------

Co-authored-by: Mark Erikson <[email protected]>
@markerikson
Copy link
Collaborator Author

@remus-selea added several more examples:

  • cursor-based
  • limit and offset
  • page and size infinite scrolling

* Fix updateQueryData for infinite queries

* Fix upsertQueryData for infinite queries

* Fix upsertQueryEntries for infinite queries

* Fix types in lifecycle middleware
…le (#4798)

* Bump infinite query RTKQ build

* Add rn-web and force infinite query example lockfile

* Bump MSW worker

* Add rn-web types

* Expose refetch from infinite query hooks

* Add RN FlatList example

* Update root lockfile
@markerikson
Copy link
Collaborator Author

Pretty significant progress today!

  • Fixed types and behavior for the various cache util methods with infinite queries
  • Added types for the auto-generated useSomeInfiniteQuery hooks
  • Exposed refresh from query hooks
  • Added an example with RN FlatList

Also did some checks on bundle size. Looks like the new functionality adds 7K min to all RTKQ usages (but only about 1.5K min+gz).

The bad news is that's non-negotiable and the increase applies even if you never use an infinite query endpoint, due to the changes currently being deeply embedded inside all of the RTKQ core logic.

That said, this PR started with a lot of intentional copy-paste duplication in order to get things working at all. I think I can manage some deduplication and maybe shave off about 2K min of that.

FWIW, it looks like React Query's implementation of infinite queries is about 3K min, so this isn't that far off. Theirs is partially shakeable.

My general intent at this point is that we'll end up shipping roughly the architectural approach that's in this PR, minus whatever deduplication and byteshaving I can do as cleanup before I decide it's ready. I'm always sensitive about bundle size increases, so I certainly wish I had an easy way to make the bundle size cost opt-in and only if you actually have some infinite query endpoints. At the moment, and with the way this PR is currently architected, I don't have a good idea for how to do that.

Longer-term, I've been vaguely tossing around some ideas for seeing if we can pull out some of our functionality based on RTKQ's existing "modules" concept. Currently we just have the "core" and "React" modules. I don't know if it's feasible to make this more shakeable or opt-in, but it's worth investigating. That would happen as part of a future RTK 3.0 effort, no ETA.

All that said, given the amount of actual functionality involved here, 5K min seems like a plausibly reasonable cost to pay to add the feature.

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.

6 participants