useInfiniteQuery stale data when updated with setQueryData while another page is fetching

[lgenzelis]

Describe the bug

I found a bug with useInfiniteQuery. If an update is performed to an old page (using setQueryData) while a newer page is being fetched, the update is lost.

This is a basic reproduction of the bug

Context (if anyone's wondering why I need this! xD): I'm writing a chat application, and noticed that new messages which arrive (via websocket) while an old batch of messages is being fetched are lost as soon as the old batch finishes fetching.

Your minimal, reproducible example

https://codesandbox.io/s/useinfinitequery-bug-rux7pr?file=/src/App.js

Steps to reproduce

1. Go to the MWE
2. Click "fetch next page" a couple times (each "request" takes 3 seconds)
3. Click "update first page" a few times, observe how the result for the first page is updated.
4. Click "fetch next page". Note that, after the request finishes, nothing changes in the first result (as expected).
5. Now comes the weird part. Click "fetch next page", and then click "update first page" as many times as you want. However: a. As soon as you click "update first page", the "loading" indicator disappears (i.e., isFetching is no longer true). This is not the bug I'm interested in, but I thought it's worth mentioning it. b. As soon as the "request" finishes, the data for the first page reverts back to what it was before clicking "fetch next page". This is the important one 🐛🐛🐛

Expected behavior

As a user, I expected the updates to one page of the infinite query performed while another page was fetching to persist.

How often does this bug happen?

Every time

Screenshots or Videos

https://user-images.githubusercontent.com/4582440/166328686-cf98c9d3-c441-4569-be1f-79bafd84a614.mov

Platform

• OS: macos
• browser: chrome
• version: 100

react-query version

3.38.1 (also happens with 4.0.0-beta.7)

TypeScript version

No response

Additional context

No response

[hirbod]

You need to make sure to call cancelQueries before mutating data, that's also like this in the examples.

https://react-query.tanstack.com/guides/optimistic-updates

So when you receive a message via Websocket, you will need to trigger cancelQueries (make sure to use abort signaling) on that key before you mutate. After you're done, invalidate the key so it can refetch.

The downside of this approach is that it will refetch all the pages of your infinite query, so you might need to add some logic to only refetch a couple of pages etc

[lgenzelis]

Thanks @hirbod ! But I don't fell that applies in the context of infinite queries. I don't want to cancel a request for page 7 if I just need to mutate the content of the 1st page.

[TkDodo]

I think it does apply. For an infinite query, we only have one cache entry. The fact that we chunk it up into multiple network requests doesn't matter. Once all pages are done fetching, we write all pages to the one cache entry we have. We couldn't even reconcile each page and keep the ones that were updated later, because there is only one dataUpdatedAt for the whole cache entry. So if you write to the cache while you are refetching, you'd need to call cancelQueries to stop your manual update from being overwritten when the fetch finishes.

[TkDodo]

a. As soon as you click "update first page", the "loading" indicator disappears (i.e., isFetching is no longer true). This is not the bug I'm interested in, but I thought it's worth mentioning it.

I also think this is interesting. What happens is that queryClient.setQueryData puts the query into a success state, which is the same "event" we get when the fetch succeeds. In that case, we set isFetching to false. I bet we could also reproduce this with useQuery. A potential fix might be to mark updates coming from setQueryData as manual, in which case we wouldn't touch the isFetching state. This is also important for v4, where queries can be in paused state, so we really shouldn't touch that. Only the status, data and dataUpdatedAt fields should be set. Can you create a reproduction and a separate issue for this?

[lgenzelis]

To your first comment, @TkDodo , I'd argue this is a bug. I've looked into onFetch in infiniteQueryBehavior.ts and it seems that what react-query is doing is:

1. Get current pages
2. Fetch new page
3. Append new page to the "current pages" (got in 1.). Since 2. is async, someone may change the value of some of the "current pages" while 2. is running, and that change would be overwritten here.

I feel this issue would be solved simply by flipping 1. and 2. So, we fetch the new page first, and only then we get the current pages and append the new page to them. However, the piece of code I'm referring to (onFetch) also handles a bunch of other scenarios I don't get that well. So, it's very likely that fixing this isn't as simple as I'm saying 😅

To your second comment, alright, I've just created #3594 :)

[TkDodo]

Thanks for creating the separate issue 🙏

Regarding your scenario:

Let's assume I have 3 pages, each has 5 items of strings:

- page 0: [A, B, C, D, E]
- page 1: [F, G, H, I, J]
- page 2: [K, L, M, N, O]

keep in mind that this is one cache entry. Now, a refetch starts due to some invalidation, and we trigger the refetches. Each item has a number appended now. After we fetch page0, we update page2:

- page 0: [A1, B1, C1, D1, E1]

page 0 is updated and page 1 is currently refetching, when we do:

queryClient.setQueryData(prev => ({ ...prev, pages: prev.pages.map((page, index) => index === 2 ? ['FOO'] : page))

this is the code to update the last page to the the Array ['FOO']. However, because we have only one cache entry, we have to "set" the whole cache entry. After that, our cache entry looks like:

- page 0: [A, B, C, D, E]
- page 1: [F, G, H, I, J]
- page 2: [FOO]

the background refetches are continuing in the meantime, so we'll eventually get this from the backend:

- page 0: [A1, B1, C1, D1, E1]
- page 1: [F1, G1, H1, I1, J1]
- page 2: [K1, L1, M1, N1, O1]

and then, after all the background refetches are finished, that data is put into the one cache entry again, thus overriding your manual update.

---

Please correct me if that example is wrong, but where do you see the bug here, and how would you solve it differently? I don't think that we can just merge the two concurrent updates together.

I think you do need to call queryClient.cancelQueries() if you do a manual update. That's also what the docs on optimistic updates suggest.

[lgenzelis]

a refetch starts due to some invalidation

In that scenario, yes, I agree with you, and I don't see a way around it.

But what if there is no invalidation? You're just trying to fetch page 3 (in your example), using fetchNextPage. While page 3 is being fetched you go to the cache and make the change to

- page 0: [A, B, C, D, E]
- page 1: [F, G, H, I, J]
- page 2: [FOO]

When you get the result for page 3, you read the current data from the cache, and append the new page to it.

[TkDodo]

Aaah yes that makes sense. I understand now. Let me see if we can fix that. If you want to help out , I'd usually start by making a failing test :)

[TkDodo]

okay, this would be the failing test case I believe, but it's harder to fix than I thought.

first of all, we are not reading the pages per se, we are getting them passed via the context:

https://github.com/tannerlinsley/react-query/blob/b84681fdc7d37645cd149f40e98cb18fd8bc393c/src/core/infiniteQueryBehavior.ts#L25

next, we are already passing these old pages to getNextPageParam, so it would be weird if we would calculate the next page param based on a version of previous pages that are then not the ones that wind up in the cache.

  it('should not set stale data when updating query while fetching next page', async () => {
    const key = queryKey()

    function Page() {
      const { data, fetchNextPage } = useInfiniteQuery(
        key,
        async ({ pageParam = 1 }) => {
          await sleep(10)
          return Number(pageParam)
        },
        {
          getNextPageParam: lastPage => lastPage + 1,
        }
      )

      return (
        <div>
          data: {JSON.stringify(data?.pages)}
          <button
            onClick={async () => {
              void fetchNextPage()
              await sleep(2)
              queryClient.setQueryData<InfiniteData<number>>(key, prev =>
                prev
                  ? {
                      ...prev,
                      pages: prev.pages.map((page, index) =>
                        index === 0 ? 100 : page
                      ),
                    }
                  : undefined
              )
            }}
          >
            fetch next page
          </button>
        </div>
      )
    }

    const rendered = renderWithClient(queryClient, <Page />)

    await rendered.findByText('data: [1]')
    fireEvent.click(rendered.getByRole('button', { name: 'fetch next page' }))
    await rendered.findByText('data: [100,2]')
  })

[lgenzelis]

Thanks a lot for looking into this @TkDodo ! I tried tweaking some stuff in the code, but you're right, it's not an easy task. I feel intimidated by the amount of different cases that onFetch in infiniteQueryBehavior.ts is handling. It's not that I can just modify the behavior of fetchNextPage on itself... :/ I wouldn't really know where to begin.

[lgenzelis]

I've been thinking of ways to get around this issue. Is there any way to get a callback or a promise that resolves once the query finish fetching? Currently I can tell whether some page of the infinite query is being fetched or not using

const infiniteQueryIsFetching = queryClient.getQueryState(['my-infinite-query'])?.isFetching

Let's say infiniteQueryIsFetching is true. Does queryClient expose a way to await for it to be false? (or something equivalent)

[donysukardi]

Let's say infiniteQueryIsFetching is true. Does queryClient expose a way to await for it to be false? (or something equivalent)

Actually there's promise inside query that you could await on. Though it's private, but we're talking about JavaScript 🙈

[lgenzelis]

Thanks @donysukardi ! I'm hoping there's a way to do it with the public API though 🙏

In the same file that you linked, I see an addObserver method that looks promising. But I don't see any reference to that method in the official docs.

[TkDodo]

Let's say infiniteQueryIsFetching is true. Does queryClient expose a way to await for it to be false? (or something equivalent)

you can spawn a useEffect with that count as a dependency and triggers something when it gets back to zero?

[lgenzelis]

Thanks @TkDodo , but no, I can't do that :( , because the place where I need to await for the infinite query to finish is not inside a react component. It's a callback for websocket events, which triggers when I get a new item to add to the first page of the infinite query, but I need to make sure not to add it while the query is loading... otherwise it'll get lost as soon as the loading finishes

[lgenzelis]

I'd like to share my workaround, in case anyone else ever runs into this issue:

const queryKey = ['my-infinite-query'];
const isSomePageFetching = queryClient.getQueryState(queryKey)?.isFetching;
if (isSomePageFetching) {
  await new Promise<void>((resolve) => {
    const observer = new QueryObserver(queryClient, { queryKey });
    const unsubscribe = observer.subscribe(({ isFetching }) => {
      // this is going to run when there's any update to the query
      if (!isFetching) {
        unsubscribe();
        resolve();
      }
    });
  });
}

// we can now safely read the cache (queryClient.getQueryData(queryKey)) 
// and update it (queryClient.setQueryData(queryKey, updatedData))

[huyhomie66]

some thing wrong , data update but data props in hook not update

[bujardeari]

I'd like to share my workaround, in case anyone else ever runs into this issue:

const queryKey = ['my-infinite-query'];
const isSomePageFetching = queryClient.getQueryState(queryKey)?.isFetching;
if (isSomePageFetching) {
  await new Promise<void>((resolve) => {
    const observer = new QueryObserver(queryClient, { queryKey });
    const unsubscribe = observer.subscribe(({ isFetching }) => {
      // this is going to run when there's any update to the query
      if (!isFetching) {
        unsubscribe();
        resolve();
      }
    });
  });
}

// we can now safely read the cache (queryClient.getQueryData(queryKey)) 
// and update it (queryClient.setQueryData(queryKey, updatedData))

Where do you put this?

[lgenzelis]

@bujardeari here's the same example which I used to show the bug, but adding that workaround (lines 26 to 38).

If you now click "update first page" while another page is fetching, you'll notice two things:

• you'll still see "loading..." in the first button
• nothing will change in the first bullet until the page fetch finishes. At this point, you'll see the whole update reflected correctly (so, if you clicked "update first page" 5 times, you'll see the count in the first bullet point incremented in 5)

[TkDodo]

I'm gonna close this issue since there's no traction for quite some time, and there is a workaround to address this in userland.

[rickvian]

i bumped to this issue

i have Table with list of trips. It has infinite scroll.

in table, i can perform mutation actions (like depart a trip), and change the trip status (via queryClient.setQueryData)

but next page fetching may be still running in background, upon resolved next page, it will overwrite the trip status update and reverted it..

i modified @lgenzelis code a bit to reflect my case reproduction using dummy repros:

1. Try to fetch 3 page
2. when fetching 4th page, when not complete, try to update page 0 content

when the 4th page is finished, the page 0 content will be reverted, this is issue for me, is there any workaround for this?

code sandbox reproduction

[TkDodo]

fetching one page will always set the whole cached data for all pages, because there is only one cache entry. It's a classic "concurrent modification" scenario - two actors write to the same cache location at the same time, one overwriting the other.

I think we could fix this by reading the latest state again after we've finished fetching a page, but I fear this will only work in limited scenarios. I would just not allow updating page 0 while page 4 is fetching.

[rickvian]

@TkDodo thanks sir, thats also the option i'm considering, looks like i will disable any action that possibily update data while page are fetching...