RTK Query - refetchOnFocus refetches unmounted queries

[saschametz]

We're using the refetchOnFocus option and set keepUnusedDataFor to 5 minutes.

The problem is that when the user navigates through the application, this creates a lot of API requests at once when the page is refocused.

This question already seems to have been up for discussion in the old repo:

• Implement refetchOnFocus, refetchOnReconnect rtk-incubator/rtk-query#90 (comment)
• Make removal/refetch on invalidate/refetchOnX configurable rtk-incubator/rtk-query#99

---

It would be very useful to change the default behaviour of this (refetch mounted queries only on refocus) or have an option to disable the fetching of unmounted queries.

Changing the default behaviour would be the preferable way IMHO. We don't have the problem of stale data for unmounted queries because we're using the refetchOnMountOrArgChange option.

[phryneas]

Hmm, the questions is really what the best - most correct - behaviour would be. If it's not refetching that data, it might grow stale and not everybody uses refetchOnMountOrArgChange.

The "cleanest" way I see for this would be to just remove that stale data from the store, but that would maybe not be desirable for everyone as well 🤔.

Maybe we could make it configurable between

• keep
• refetch
• delete
  ?

[saschametz]

I'm used to react-query where it only fetches the mounted queries on refocus. But I see this may not be the best solution because refetchOnMountOrArgChange is not active by default.

I was actually surprised at the number of queries on refocus. I just spotted it by accident and I think this can happen to others as well, that they don't even know that they may produce a lot of queries by using refetchOnFocus. Would it be an option to change the default value of refetchOnMountOrArgChange to true so that the default behavior of refetchOnFocus refetches only mounted queries?

Having the option between keeping the stale data or refetching it for refetchOnFocus sounds good to me. I'm not sure about the delete option, though, because if you click on another tab for just one second, the cached data is already deleted.

[AlexanderArvidsson]

I am also noticing this problem and after reading #1409 it seems like this logic observed here is contradictory to the arguments made in that issue.

In that issue, it is mentioned that revalidation only occurs if there are active subscribers to the data. However, here, when focusing, it will refetch ALL endpoints regardless if there are any subscribers.

I am using the following options on my API slice (which means ALL queries use this):

  refetchOnMountOrArgChange: 1,
  refetchOnFocus: true,
  refetchOnReconnect: true,

I am seeing LOTS of executeQuery actions for endpoints that clearly have no subscribers. Looking in the state before focusing the window, the majority of endpoints have no subscribers. When refocusing, all endpoints are refetched.

I am noticing, however, that this might actually be a bug.
After leaving the page that initiates certain queries and removing the active subscribers, a part of my state looks like this:

If I refresh the page, it looks like this:

Clearly, the refetchOnFocus option does not check if the subscription is empty, rather it checks if it exists (empty object counts as existing).

Is this incorrect behavior?

---

The offending code is here:

        if (
          !subscriptionSubState ||
          !querySubState ||
          querySubState.status === QueryStatus.uninitialized
        )
          return

This does not check if the subscription sub state is empty, which it should?

How about adding a check Object.keys(subscriptionSubState).length === 0 here?

---

An alternative solution would be to make sure the subscription subtree removes its key rather than setting it to {}.
However, looking at the code here it seems like this should already be handled?
Scratch that, keepUnusedDataFor clears it after a while.
But shouldn't the refetchOnFocus check if there are active subscriptions regardless?

[AlexanderArvidsson]

Any progress on this?

This seems like it's a critical bug as it is refetching queries that have no active subscriptions which is really should not.

[phryneas]

Sorry, when you left your last comment I had Covid and I haven't cought up on everything yet.

Generally, just "not refetching" doesn't really cut it here, as in that case the data has to be completely removed from the cache, so it is re-queried on a remount. Similar logic to invalidation would apply here.
Unfortunately I won't get to it in quite some while as I still have a lot of real life to catch up, but it would be great if you could open a PR on this.

[AlexanderArvidsson]

@phryneas I opened a PR for the simple change of adding the condition, but if that is not enough I would need some more information to fix it completely.

I have not dove too far into the code for RTK Query, but enough to see roughly how it works.
So if I understand you correctly, in order for the data to be re-queried when mounting the component, the data has to be removed from the cache, else it will not refetch and it will contain stale data? This feels very off to me and not at all what I have observed (if this was the case, this would make every single query have stale data if you enable keepUnusedDataFor, because the cache wouldn't get cleared until the time has passed, then it would refetch the data according to what you said)

However, wouldn't the subscription state become non-empty once you mount your component again, resulting in the if statement not passing and thus refetching the data correctly?
Unless I am missing something, the logic already does this when introducing the change that I have in the PR, it is refetching correctly on mount and it does not refetch if there are no subscriptions.

[phryneas]

It would only refetch if refetchOnMountOrArgChange is set - if that is not set it will reuse the existing cache value on mount. So if all other data was refreshed, but this one cache value was not, a newly mounting component will get a cache value that is potentially outdated.

Essentially the same as with invalidation goes: if it is mounted, it needs a refetch, if it is not mounted, it needs to be cleaned out so it is fetched on a later mount in the future.

redux-toolkit/packages/toolkit/src/query/core/buildMiddleware/invalidationByTags.ts

Lines 75 to 82 in d4e95ca

	if (Object.keys(subscriptionSubState).length === 0) {
	mwApi.dispatch(
	removeQueryResult({
	queryCacheKey: queryCacheKey as QueryCacheKey,
	})
	)
	} else if (querySubState.status !== QueryStatus.uninitialized) {
	mwApi.dispatch(refetchQuery(querySubState, queryCacheKey))

[AlexanderArvidsson]

@phryneas Isn't this already handled by the handleUnsubscribe though?

redux-toolkit/packages/toolkit/src/query/core/buildMiddleware/cacheCollection.ts

Lines 79 to 103 in d5b0eb5

	function handleUnsubscribe(
	queryCacheKey: QueryCacheKey,
	endpointName: string | undefined,
	api: SubMiddlewareApi,
	config: ConfigState<string>
	) {
	const endpointDefinition = context.endpointDefinitions[
	endpointName!
	] as QueryDefinition<any, any, any, any>
	const keepUnusedDataFor =
	endpointDefinition?.keepUnusedDataFor ?? config.keepUnusedDataFor
	const currentTimeout = currentRemovalTimeouts[queryCacheKey]
	if (currentTimeout) {
	clearTimeout(currentTimeout)
	}
	currentRemovalTimeouts[queryCacheKey] = setTimeout(() => {
	const subscriptions =
	api.getState()[reducerPath].subscriptions[queryCacheKey]
	if (!subscriptions || Object.keys(subscriptions).length === 0) {
	api.dispatch(removeQueryResult({ queryCacheKey }))
	}
	delete currentRemovalTimeouts![queryCacheKey]
	}, keepUnusedDataFor * 1000)
	}

We have 4 cases:

1. keepUnusedDataFor = x and refetchOnMountOrArgChange = true: Mounting will always refetch regardless if the data is in the query, because mounting results in a new subscription.

2. keepUnusedDataFor = undefined and refetchOnMountOrArgChange = true: Unmounting the last component (i.e. subscription sub state is empty) will remove the data, and every mount afterwards will query the data again because of refetchOnMountOrArgChange.

3. keepUnusedDataFor = undefined and refetchOnMountOrArgChange = false: Similar to the last, unmounting the last component will clean the query data. The next mount (i.e. first mount) will query the data, but mounting more than one component will use the cached result (as per documentation).

4. keepUnusedDataFor = x and refetchOnMountOrArgChange = false: Unmounting last component and waiting x seconds will clean up the query data, at which point a mount will query the data. If you try to mount within x time, it will not trigger a query, so the data will be stale (but that is the intended behavior, because if you do not want to refetch on mount then you will have stale data as per the documentation 'This setting allows you to control whether if a cached result is already available RTK Query will only serve a cached result,')

In all these cases, the fix in my PR works as intended; do not refetch if there are no active subscribers. Have I missed something?
Removing the query data when you're attempting to refetch (like invalidation does, which is not the same) will break keepUnusedDataFor.

[phryneas]

Case 4 is the default case. keepUnusedDataFor defaults to 60 seconds and refetchOnMountOrArgChange defaults to false. It will probably be the 90% use case.

"serving a cached result" is different from "serving something we know to be out of sync with all other cache results". If you use refetchOnFocus you expect all data to be up-to-date after refocus. You would not expect data to stay around for longer and be older than the last refocus when a component mounts with a value that was available in cache.

Thus, that data has to be removed from cache in that case.

PS: I'd go as far as saying that the current behaviour of "refetch everything" would be more correct than "refetch everything visible and leave the rest potentially outdated".

[AlexanderArvidsson]

In my use case, I am using refetchOnFocus (throttled, though, custom handler, might make a PR to support this using signature refetchOnFocus?: boolean | number), refetchOnMountOrArgChange and keepUnusedDataFor = 60 (default).

If I initiate a query and unmount the component, then blur the window and refocus it, it will re-query all data again for the component that was unmounted (no component is listening on the data).

If the reasoning is that refetchOnFocus is meant to refetch all data, including unused data, then I understand the current logic.

However, my question is then, should we ignore keepUnusedDataFor and remove the query data if there are no subscribers when trying to refetch? Should refetching only remove the data if it is older than keepUnusedDataFor?

[phryneas]

However, my question is then, should we ignore keepUnusedDataFor and remove the query data if there are no subscribers when trying to refetch?

Yes, it is the same we do for invalidate with keepUnusedDataFor. At that point we have to assume the data in the cache is outdated - or at least not "in sync" with other stuff we are displaying.
So we can either refetch it, which can be costly for stuff we maybe never display again. Or we remove it and it will be refetched if it is actually required. Either way, we cannot keep around a potentially outdated version of that data.

So I guess that the current behaviour might be overfetching a lot, so at this point I'm leaning towards removing it. We could also think about making that configurable and a user choice, but that might be going too far.

PS: "data older than keepUnusedDataFor" will never happen. That data will be removed once it hits that point.

PPS: I'm going to bed now, so no more answers from me for today, just so you're not irritated I'm not answering any more ;)

[AlexanderArvidsson]

PS: "data older than keepUnusedDataFor" will never happen. That data will be removed once it hits that point.

Yes, of course, I was not referring to it as actually existing, it is removed exactly on time due to the setTimeout.

I am beginning to understand exactly what you are saying and I agree, it really does not make sense to know that the cache could include stale data relative to other cache data. I have implemented the same logic from invalidateTags and it works fine for my use case.

To be exact with my use case, I am calling the query without hooks (as per documentation) and instantly unsubscribing (I only want the data, I do not want to subscribe). This is because I am storing the results in an entity adaptor that I want to manage manually (experimenting with infinite pagination). The problem then arose where it started refetching my data even though I unsubscribed and that was unnecessary (we have real-time long polling that I will integrate into the entity adaptor to keep in sync). Removing the data like invalidateTags works very well in this case.

I will update my PR with these changes, but first I will also go to bed.
Good talk and appreciate the fast responses!

[phryneas]

Released in https://github.com/reduxjs/redux-toolkit/releases/tag/v1.7.2