Inconsistency in query invalidation as part of a dom event callback

[priyajeet]

Describe the bug
Query invalidation (or removal) doesn't seem to happen consistently when done as part of DOM event callback where in ReactDom.unstable_batchedUpdates() gets involved.

To Reproduce
Steps to reproduce the behavior:

1. Go to https://codesandbox.io/s/silly-davinci-6rjwr?file=/src/index.js
   • Have the debugger console open for log statements
2. Click on the Reload button a few times
   • Notice the api call / rendering log statements (API call doesn't happen more than once)
3. Now instead type and execute reload() from within the debugger console (its exposed on the window object)
   • Notice the api call / rendering log statements (API call happens on every call)

Expected behavior
Behavior of invalidating the query while calling the reload() function directly should be roughly the same as when calling it as part of a DOM event callback.

Desktop (please complete the following information):

• OS: any
• Browser: any
• Version: any

[TkDodo]

do you expect disabled queries to not refetch if you pass refetchActive: false to invalidateQueries? Because disabled and inactive are not the same thing.

In fact, if I remove the enabled thingy completely, both calls behave the same: https://codesandbox.io/s/rough-glade-rovcf?file=/src/index.js

So I think this is more about how react batches updates together in event callbacks, but not outside of it: You setting enabled to false and then true in the same event handler might lead react to discard that setState altogether, because it happens in the same event handler.

On the other hand, when called from outside, react doesn't do that. Now it disables the query, calls refetch (which doesn't do anything because of refetchActive: false, and then it enabled the query. The query enabling is then what triggers the fetch imo. You can also observe that if you log the enabled flag in addition when rendering: https://codesandbox.io/s/compassionate-agnesi-svup4?file=/src/index.js

• the first reload click calls the api, and then enabled is true.
• any further click will only render once with enabled: true, because the enabled:false setting is discarded
• called from the window, you'll see an additional render in between with enabled: false, and then it goes back to enabled: true.

further, if you await invalidateQueries and make your function async, react will also not be able to batch updates, and you can see that the re-renders are happening in between as well.

---

all in all, this looks like the expected behaviour of how react batches updates, and I don't think there's anything react-query can do about it ...

[TkDodo]

FYI, React 18 will bring some changes to automatic batching, in which case it should work out of the box in more situations: reactwg/react-18#21

The discussion also explains quite well how it works in current v17, and also shows that your two setStates to setEnabled will be batched in synchronous functions.

I don't think it would affect this situation though, because you have one setState before the async action, and then another one after it. So they can't be batched together, because an async action can take quite some time to complete :)

[priyajeet]

I am purposely disabling the query in the example, and also making refetchActive as false to control when the new request goes out - onclick of the button for this particular example. Essentially trying to trigger a refetch (without using the refetch of useQuery) every time the component renders after a reload button push.

Initially it is disabled to start off with. It will get enabled after the 1st time you hit reload. The request goes out as expected.

Reloading after that, still triggers a re-render of the component (irrespective of being batched or not, just the amount of re-renders change) as you can see from the logs. However this time the request doesn't go out again despite the fact that there is invalidateQueries in there.

If react batched the updates and determined nothing really changed it would not trigger a re-render, which you can try by just commenting out the 1st setEnabled(false). Then the component doesn't re-render.

So it is coming down to the queryClient.invalidateQueries() line is not doing anything at all. Like the behavior of it being in there or not being in there is the same when using the reload button.

And yeah since React18 will have batching outside of event handlers, wont this be an issue then?

[priyajeet]

BTW same issue if I use queryClient.removeQueries() instead of invalidate. I would have expected that if the query is enabled but invalidated or non existent after being removed, it would trigger a auto-refetch on the next render pass. I may be wrong if this is not the intended behavior.

In my real example, I am setting refetchActive to false, only because I don't want the request to go out till the next render pass, because I have updated the state in that click handler with some params/data that the query should use before it re-fetches. Setting refetchActive to true would have triggered a refetch before the next render pass and with stale data.

[priyajeet]

Another example https://codesandbox.io/s/silent-firefly-tcn4m?file=/src/index.js

• Not rendering anything in this, no reload button, just console logs.
• Exposed queryClient and a setState to the window object to be executed manually.

After the 1st load/render, the API call goes out as expected.

Now using the console if I do

1. queryClient.invalidateQueries("whatever", { refetchActive: false }) // to invalidate the query
2. setSomething(2) // To trigger a manual re-render of the component, like forceUpdate()

The API call still doesn't go out

[TkDodo]

I would have expected that if the query is enabled but invalidated or non existent after being removed, it would trigger a auto-refetch on the next render pass. I may be wrong if this is not the intended behavior.

no, that's not what invalidation is doing. react-query will never re-fetch just because you re-render. staleTime defaults to zero, so all queries are instantly "stale" per default.

invalidation marks a query as stale, and it automatically re-fetches active queries (an active query is a query that is currently used by useQuery) right away (not in the next render cycle or so). But since you set refetchActive: false, all that the invalidation is doing is setting the query to stale, which means it will be refetched the next time react-query does a smart refetch, e.g. when a new observer mounts or when you focus the window. That is clearly not what you want to achieve.

This also explains the last example. Invalidating the query and triggering a re-render will not get you a refetch, because again, that's not what invalidation means.

I think it's time you take a step back and tell us about your use-case: What do you want to achieve instead of what code you already have that isn't working. If all you want to do is have a disabled query and then refetch when a button is clicked, there are other ways. If you want a lazy query, there are also other ways...

Meanwhile, I'm converting this to a discussion because I'm convinced that there is no issue in react-query per se.

[priyajeet]

Thanks for the explanation, clears up a few misconceptions I had.

Essentially what I am wanting is reset the query back to its original state (a state that is used to generate the query key) and refetch when all such states for the component have been updated as part of that reload function (that can be called via on-click, or a non-dom-event callback).

I use the same query, but with with different query-keys to fetch against the same API with different params. My query-key is obviously an array that includes those params. Roughly they are like

['fubar', ''] // Always initial
['fubar', 'param=something'] // Subsequent and optional
['fubar', 'param=somethingElse'] // Subsequent and optional
...

So currently I have it working like this

  function reload() {
    queryClient.invalidateQueries('fubar', { refetchActive: false });
    // bunch of setStates, React can choose to batch them

    if (param === "") {
      refetch({ throwOnError: true }); // setParam("") will be a no-op as that is the current state, hence refetch
    } else {
      setParam(""); // set the param back to "", thereby changing the query key to its original state which triggers a auto-refetch
    }
  }

Is there a better way for this, or is this the recommended approach

[priyajeet]

btw I am no longer enabling or disabling the query as none of the setStates other than the setParam changes the query key

[priyajeet]

And maybe I should be using removeQueries() here instead of invalidateQueries()