useQuery().promise is not restored when staleTime is set

[ali-idrizi]

Describe the bug

I was trying out the new useQuery().promise implementation with the React.use() hook, and noticed two issues with it.

For the reproduction I have a very simple setup similar to the docs. There is an input that updates the q state onChange, and the following query:

useQuery({
  queryKey: [q],
  queryFn: () => getData(q),
  staleTime: 60000,
});

There's also a Data component wrapped in a Suspense, which take the promise of the query above as a prop and calls use on it. The query function takes the q, delays 1s and returns the same:

const getData = async (q) => {
  await new Promise((resolve) => setTimeout(resolve, 1000));
  return { q };
};

---

The first issue occurs when staleTime is set, and it still has fresh data. In this case it seems that the promise in not being restored, and incorrect data is shown. Here's a screen recording:

2024-10-02.15-23-47.mp4

When typing extra characters, it correctly updated the data. However, when I remove characters (in which case it already had fresh data cached) it doesn't seem to restore the correct promise.

The devtools is showing the correct query as active, and the Data component is re-rendering. but from the console logs it seems to be receiving an incorrect promise.

<Search> requested query: "1234".  received promise results:  {q: '12345'}
<Search> requested query: "123".  received promise results:  {q: '12345'}
<Search> requested query: "12".  received promise results:  {q: '12345'}
<Search> requested query: "1".  received promise results:  {q: '12345'}

This issue does not occur when the queryFn is ran, e.g. when providing a query that is not cached or when staleTime: 0.

---

And the second one, the suspense is only triggering once, and not when the query changes. In the screen recording you can notice as I type "12345", it no longer triggers the suspense (which has "suspending..." as fallback), but it provides a stale promise, until it's done fetching the data for the new query.

I was expecting the suspense to trigger when the query key changes and it's about to fetch data. I thought the current behavior (i.e. show stale data until the new render is done) would have to be achieved through useDeferredValue instead.

I am not sure if this is the intended behavior, so I am did not create a new issue for it, but please let me know if I should do that.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/awesome-williamson-k2269m

Steps to reproduce

On the first issue, simply type anything in the input and notice the data under it updating. Now remove characters and notice that incorrect data is shown.

On the second issue, notice the suspense triggering when reloading the page, but not when changing the query (in which case query.isFetching is indeed true).

Expected behavior

Already explained above.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

• OS: Windows 11
• Browser: Chrome
• Version: 129.0

Tanstack Query adapter

react-query

TanStack Query version

5.59.0

TypeScript version

N/A

Additional context

No response

[TkDodo]

Thanks for reporting 🙏

The first one with staleTime is definitely wrong, we need to look into this.

As for the second one, that’s also an issue 😂 . When we go to a new Query with a key that has no data, we need to suspend again. You’re absolutely right that you would need deferred values, transitions or our placeholderData: keepPreviousData feature to keep “old data” from a different key on the screen.

@KATT FYI.

[TkDodo]

here is a failing test case for the first one:

it('should show correct data when switching between cache entries without re-fetches', async () => {
      const key = queryKey()

      function MyComponent(props: { promise: Promise<string> }) {
        const data = React.use(props.promise)

        return <>{data}</>
      }

      function Loading() {
        return <>loading..</>
      }
      function Page() {
        const [count, setCount] = React.useState(0)
        const query = useQuery({
          queryKey: [key, count],
          queryFn: async () => {
            await sleep(10)
            return 'test' + count
          },
          staleTime: Infinity,
        })

        return (
          <div>
            <React.Suspense fallback={<Loading />}>
              <MyComponent promise={query.promise} />
            </React.Suspense>
            <button onClick={() => setCount(count + 1)}>inc</button>
            <button onClick={() => setCount(count - 1)}>dec</button>
          </div>
        )
      }

      const rendered = renderWithClient(queryClient, <Page />)
      await waitFor(() => rendered.getByText('loading..'))
      await waitFor(() => rendered.getByText('test0'))

      fireEvent.click(rendered.getByText('inc'))

      await waitFor(() => rendered.getByText('test1'))

      fireEvent.click(rendered.getByText('dec'))

      await waitFor(() => rendered.getByText('test0'))
    })

It’s also interesting / wrong that we don’t suspend in between queries. When going from test0 to test1, it’s a new cache entry, so we should see the suspense boundary again. That is, unless we would use placeholderData or useTransition.

But as it stands, if I add this as an assertion, it will also fail:

  fireEvent.click(rendered.getByText('inc'))
+ await waitFor(() => rendered.getByText('loading..'))
  await waitFor(() => rendered.getByText('test1'))

[TkDodo]

I think #8169 should fix both reported cases - please give it a try @ali-idrizi. I’ve also added two test cases for your two reported findings

[ali-idrizi]

Hey @TkDodo, thanks for the update. I just checked this out and it's working much better! However, I still noticed an issue with incorrect data being shown. Here's a screen recording:

2024-10-12.09-48-10.mp4

I have pushed that example in a repo: https://github.com/ali-idrizi/rq-stale-invalid-promise-reproduction.

This seems to occur only when typing faster in the input, and multiple promises are pending. I not super familiar with the inner workings of everything here, but to me it looks like this is an issue with React itself? Looking at the logs:

<Search> requested query: "1".  received promise:  pending undefined
<Search> requested query: "12".  received promise:  pending undefined
<Search> requested query: "123".  received promise:  pending undefined
<Search> requested query: "1234".  received promise:  pending undefined
<Search> requested query: "12345".  received promise:  pending undefined
<Search> requested query: "123456".  received promise:  pending undefined
<Search> requested query: "123456".  received promise:  fulfilled {q: '123456'}

The correct promise was received for every single query. However, in the recording the <Data> component was rendered multiple times for an incorrect q, even though the parent <Search> component received a fulfilled promise only once.

I tested further by instead having the <Data> component get the promise directly from useQuery instead of the props (like it's shown in the docs), and in that case it works perfectly fine. So in the repo:

-  // const response = use(useData(q).promise);
-  const response = use(query.promise);
+  const response = use(useData(q).promise);
+  // const response = use(query.promise);

[TkDodo]

@ali-idrizi thanks for trying out this feature and reporting the feedback - I really appreciate it ❤

I think I have a fix for what you are reporting here:

• fix(query-core): don't finalizeThenable of different queries #8171

Basically, we were resolving with data from a previous query once it resolves, which explains the intermediate data flashes. I think this was introduced with my latest refactoring, because createResult is called way more often than updateResult.

I still need to write a test for it, but maybe you can try out the preview build from that PR in the meantime and let me know if that fixes it? All other tests are still passing so I hope there is no regression (at least of paths that we are aware about):

pnpm add https://pkg.pr.new/@tanstack/react-query@8171

[ali-idrizi]

@TkDodo You're welcome! I'm glad to help out. I tested the new change, and it works much better when typing now.

The only potential regression that I noticed, is that it's now suspending briefly when it has cached data. I suppose in this case a fulfilled promise should be provided instead of a pending one which resolves later (that was the case previously)?

Here's a screen recording where you can see that as I remove characters from the input:

2024-10-12.10-48-40.mp4

[TkDodo]

yeah I’ll look into that too

[TkDodo]

ok, got it; this was introduced by my latest attempts to improve things 😂

I pushed another commit and more tests. can you try again?

[TkDodo]

latest commit is: https://pkg.pr.new/@tanstack/react-query@a4a2cf7

[ali-idrizi]

Perfect 🚀. I'll try it out on a larger repo next week, and report if I find any other issues.

[TkDodo]

alright, shipped it so that you don’t have to use preview builds for that :) enjoy 🍻

[ali-idrizi]

@TkDodo I looked a bit more into this, and perhaps there's one more issue. I am not sure whether what I am doing is the intended usage, so this is more of a question.

The docs show the query is being passed as a prop to the child component, and then the promise is extracted from it. That still works well! But, in a larger repo, the data might be needed quite deep in the tree or by many components, so calling the useQuery again to get the promise would be favored.

What I did in the <Search> component is simply call useData(q), so it can start prefetching (not doing anything with the results, so nothing being passed to <Data>). In the <Data> component, I am calling the same useQuery again, and getting the promise from there:

Search.js

function Search() {
  const [q, setQ] = useState("");

  useData(q); // `useData` simply calls `useQuery` and returns everything

  return (
    <>
      <input value={q} onChange={(e) => setQ(e.target.value)} />
      <Suspense fallback={<div>suspending...</div>}>
        <Data q={q} />
      </Suspense>
    </>
  );
}

Data.js

function Data({ q }) {
  const query = useData(q);
  const data = use(query.promise);

  return ...;
}

However, that works well only for the first ever fetch, if I change the q it gets stuck suspending:

2024-10-12.16-22-16.mp4

If you look at the devtools you can notice that [""] for some reason still has one observer, and ["5"] also has one instead of two. I have no idea what's happening there.

If this could work it would make getting the promise much easier, but I understand if this is not be the intended usage. Just let me know either way :)

[TkDodo]

I'll look into this as well, probably on Monday.

[TkDodo]

took a bit longer but I found it:

• fix(react-query): make sure promises are always finalized #8211

[ali-idrizi]

Thanks for the update. I tried out the latest release and that is working great now!

I came across another case that suspends indefinitely. It happens consistently when the promise and the suspense are colocated, but the promise fails. I modified one of the existing test, it currently fails waiting for the error boundary. I have filed a new issue for better tracking: #8219.