error query doesn't fire again when keys change

[TkDodo]

Discussed in #2676

^{Originally posted by jdupont22 September 16, 2021}
Hi,

I've a strange situation, i don't know if it's normal!
I try to load some "offers" with useQuery(["offers, deepJson]).

I have a "staleTime=20s" and the default "cacheTime=5m".

useQuery(["offers",{"order":"desc","page":1,"q":null,"size":10,"sort":"name","states":["ACTIVE"]}])

I've the following result, react-query retry 3 times and failed! It's ok.

Now, I click on the tab "Inactive", that change the deepJson states to "states":["INACTIVE"]
I see in the react-query devtool the params change, but the request is not trigger!!!

There is a "disabled" query with this keys, but i would imagine that a query in error will retry the next time? it's based on the cache expiration?

Can you explain to me this current behavior? When the query return 200 OK, switching tab work perfectly!

Thx

thanks for reporting. This seems to have worked correctly up until v3.19.4. I'll create an issue out of this.

Here is a slight variation of your codesandbox without window focus refetching, without retries, with a 1 second fetch delay and with devtools, so that we can better see when fetching transitions are happening: https://codesandbox.io/s/interesting-elbakyan-zz28f?file=/src/App.js

[TkDodo]

@JaeYeopHan could you please have a look at this? It seems that queries that are in error state will not trigger a refetch when the query key changes, due to the changes made in #2556 and #2565. You can compare the above sandbox with v3.19.4 (works OK) and v3.19.5 / v3.19.6 (not OK)

as discussed on the PR, maybe it would've been better to just scope the changes you made for when suspense is enabled? I just don't think we've accounted for all usages, and fixes for suspense should likely only affect suspense users :)

[TkDodo]

to that, I think all we'd need to do is change the line you added:

https://github.com/tannerlinsley/react-query/blob/f9b23fcae9c5d45e3985df4519dd8f78a9fa364e/src/core/queryObserver.ts#L770

and add a !options.suspense || to it.

alternatively, we could also compare query hashes, because maybe this is also a problem in suspense as well ? (I can't really tell because I'm not using it)

[TkDodo]

Here would be a test that doesn't work on the current version, but worked before that change:

  it('should refetch when query key changed when switching between erroneous queries', async () => {
    const consoleMock = mockConsoleError()

    function Page({ id }: { id: boolean }) {
      const { error, isFetching } = useQuery(
        [id],
        async () => {
          await sleep(10)
          return Promise.reject(new Error('Error'))
        },
        {
          retry: false,
          retryOnMount: false,
          refetchOnMount: false,
          refetchOnWindowFocus: false,
        }
      )

      if (isFetching) {
        return <div>status: fetching</div>
      }
      if (error instanceof Error) {
        return <div>error</div>
      }
      return <div>rendered</div>
    }

    function App() {
      const [value, toggle] = React.useReducer(x => !x, true)

      return (
        <div>
          <Page id={value} />
          <button aria-label="change" onClick={toggle}>
            change {value}
          </button>
        </div>
      )
    }

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

    // initial state check
    rendered.getByText('status: fetching')

    // render error state component
    await waitFor(() => rendered.getByText('error'))

    // change to mount second query
    fireEvent.click(rendered.getByLabelText('change'))
    await waitFor(() => rendered.getByText('status: fetching'))
    await waitFor(() => rendered.getByText('error'))

    // change to mount first query again
    fireEvent.click(rendered.getByLabelText('change'))
    await waitFor(() => rendered.getByText('status: fetching'))
    await waitFor(() => rendered.getByText('error'))

    consoleMock.mockRestore()
  })

[TkDodo]

also, adding:

|| prevQuery.queryHash !== query.queryHash

makes that test also work, but then fails the suspense tests that were added to prevent the infinite loop.

adding !options.suspense || passes all tests. @JaeYeopHan would you agree that this is a sensible fix?

[jbee37142]

@TkDodo Thanks for the detailed explanation. I agree with the part you fixed. If you post a PR, i 'll take a look!!

[tannerlinsley]

🎉 This issue has been resolved in version 3.24.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀