RTKQ: Data stays as old value and loading immediately jumps back to false when fetching fresh data from different endpoint + query param combination

[agusterodin]

Hey, I am experiencing an issue when using the useQuery auto-generated hook. If I change query parameters, the data remains the same (I expect it to be undefined so I can show loading placeholder) and loading jumps from "true" immediately back to "false" despite the data for that given combination not being loaded yet. I have keepUnusedDataFor set to 0 btw (not sure if relevant or not).

The docs claim:

isLoading refers to a query being in flight for the first time for the given endpoint + query param combination. No data will be available at this time.

This is really hard to reproduce in a Codepen since it relies on the server having some sort of delay. I wasn't aware of this bug until I used an endpoint that is extremely slow (4+ sec response time).

Hopefully this attached video, code snippet, and console log demonstrate the issue effectively. Please let me know if there are any other details I can provide!!

Notice that despite the argument changing from "monthly" to "weekly" it is still providing the "monthly" data instead of data being undefined while still loading.

Component

import { useSelector } from 'react-redux'
import MetricGraphs from './MetricGraphs'
import MetricStats from './MetricStats'
import { getOverviewMetricTimeframe } from 'state/campaigns/overview/overview'
import { useGetAggregateCampaignMetricsQuery } from 'api/campaign'
import MetricTimeframeSelector from './MetricTimeframeSelector'

export default function Metrics() {
  const currentlySelectedTimeframe = useSelector(getOverviewMetricTimeframe)
  const { data, isLoading, originalArgs } = useGetAggregateCampaignMetricsQuery(currentlySelectedTimeframe)
  console.table({ currentlySelectedTimeframe, originalArgs, isLoading, data })

  return (
    <>
      <div className="relative">
        <MetricTimeframeSelector />
      </div>
      <div className="relative grid grid-cols-1 gap-5 sm:grid-cols-2 lg:grid-cols-4 mt-5">
        <MetricStats metricResults={data} />
        <MetricGraphs metricResults={data} />
      </div>
    </>
  )
}

Recreation In Browser
https://user-images.githubusercontent.com/10248395/126681813-9656f46e-46a8-4a2e-b316-9229291699b5.mov

[ghost]

Have you tried using isFetching for this?

See: https://redux-toolkit.js.org/rtk-query/api/created-api/hooks#usequery

  isFetching: boolean // Query is currently fetching, but might have data from an earlier request.

[agusterodin]

isFetching wouldn't be the right tool for the job. If I am loading data from an entirely different endpoint + query param combination I would want the data to be undefined so that I can show loading placeholders since no data for that query param combination has been loaded.

I can't just set loading placeholders to go away if isFetching is true since that would mean anytime polling happens (user just sits on "monthly" screen with polling running for example), the data would disappear from the user while fetching the updated data.

[agusterodin]

A temporary workaround I am using is just to set the key of the component that contains the hook to the value of the query parameter. This causes React to create an entirely new component from scratch anytime that query parameter changes. This workaround is definitely not ideal though.

[phryneas]

Have you also taken a look at the rest of that query result? The most obvious would be isError in case you have some kind of error for some reason.

Also, take a look in the Redux Devtools and take a look what you see there. You could also export those actions and post them here.

Also: isLoading is only for the first fetch of a hook. If you are changing query args, it will never go back to true again. I'm not entirely sure what behaviour you would want there.

[agusterodin]

Just checked and don't see any errors in the browser console logs or in Redux DevTools.

To me this language in the docs is misleading:

isLoading refers to a query being in flight for the first time for the given endpoint + query param combination. No data will be available at this time.

To me based on the way this is worded I would expect isLoading to remain true if query param changes and the new, unrelated data is being loaded. This is also, in my opinion, desirable behavior. If the user selects "weekly", it would be inaccurate to show any data at all since that would be misleading. Typically this would go unnoticed, but the endpoint we use is obnoxiously slow AF.

I attached a video of my desired behavior: data goes away entirely when the tab changes. I achieved this by doing the aforementioned component key workaround.

Redux DevTools capture (sorry for zip, GitHub makes you do it for json files)
RTKQ Issue Reproduction.json.zip

Desired Behavior
https://user-images.githubusercontent.com/10248395/126702827-8eec90c6-1d3e-441f-8ed3-47426d884e2c.mov

[phryneas]

Yeah, that is a wrong wording in the docs.

See it from a few other perspective:

• Paginated data. On first load, you don't have data. Show a spinner. Changing to page 2 grays out the data (isFetching), but data from page 1 is still displayed to prevent the UI jumping unnecessarily.
• A potentially filtered list. On first load, you don't have data. Show a spinner. On filter change, data is grayed out, but still displayed until the server-side filter finishes with new data.

There are plenty cases where it just makes a better user experience to keep the original data around.

Even in your case, I would personally probably prefer everything going "gray" for a second instead of going UI -> nothing -> same UI with different data. If that data loads quickly, that could be some hard flickering.

But of course, your key hack works very well here.

We might also add an option to change this behaviour in the future, but for now it's more a reason for a docs correction ;)

[phryneas]

Can you give #1340 a short read-over and see if you think it explains it good for you?

[agusterodin]

Agreed, the desired behavior definitely depends on use case.

Another scenario I could imagine "reset data and isLoading when query params change" being desirable is in an e-commerce scenario. Imagine navigating from one store item page to another store item page by clicking a link in a "related items" sidebar. Technically the same component would be mounted since the router continues to show the same component. In this scenario I would expect to the old data from the old item to disappear and replaced with a loading skeleton while the new item contents load.

You are definitely right about violent loading spinner/skeleton flashes being a potential issue with this particular type of behavior. To prevent this phenomenon I created a hook that returns a boolean saying whether a provided "minimum loading time threshold" has elapsed. If the minimum time hasn't elapsed, I continue to pass the components undefined instead of the fetched data so that they continue to show placeholders. Here is my example hook for that: #1276 (comment). I am very strongly convinced that other big-tech applications implement a "minimum loading time threshold" when using skeletons. For example: I have never seen the loading skeletons on the Headspace Android app flicker violently regardless of how fast my connection is.

Would love to see an option you can pass the hook to control this behavior in the future.

Also looked over the doc change. Much clearer.

Much appreciated!

[agusterodin]

Another point that I realized about why in certain use cases it makes sense not to show data from old args and instead make it explicitly clear that data is loading:

Imagine someone using your product on an extremely low quality connection (2G or third world internet). Imagine it is bad to the point where every request takes 20 secs. Now imagine that someone clicks away from the "monthly" the "weekly" tab. Imagine that they look away for 8 secs and come back to their screen (or a colleague walks up to the screen). They will likely be under the full impression that what they are seeing in that moment is information for "weekly" despite actually being the data for "monthly".

This scenario sounds comical but there is definitely something to be said about considering low quality connection scenarios and potentially misleading UI.

It is personally hard for me to imagine experiencing the web with a low quality connection but there are definitely people out there that have this issue. I'd say creating a user experience that neglects poor connections or potentially misleads people with poor connection (in a general sense, not just my product scenario) is detrimental to accessability of information on the web.

[phryneas]

This scenario is why you should use the information isFetching to display to the user that actually, data is being fetched right now. Like I already said: gray it out, for example.

[phryneas]

Oh, by the way: if you want to circumvent this hook-specific behaviour:

const selector = useMemo(() => api.endpoints.myEndpoint.select(arg), [arg])
const dataWithIsLoadingJustForThisCacheEntry = useSelector(selector)

[agusterodin]

Yes using isFetching is definitely a solution. It would be really nice to differentiate initial content load from situations like polling though.

Based on the situation you may want the visuals of initial content load to be different than that of polling (full loading skeleton for initial load vs tiny spinner at top right of page when polling for example).

Update: saw your previous post, seems like a good temporary solution, thanks!

[phryneas]

I'll add an "enhancement" label on this one. Maybe we can add a switch to change behaviour or add a isLoadingFirstTimeForArg flag on top there.

[pserrer1]

@phryneas Would this isLoadingFirstTimeForArg also solve my issue described here? https://stackoverflow.com/questions/68576509/redux-toolkit-query-invalid-isloading-state

[phryneas]

@phryneas Would this isLoadingFirstTimeForArg also solve my issue described here? stackoverflow.com/questions/68576509/redux-toolkit-query-invalid-isloading-state

I just wanted to link this thread over there... 😅
yes, that is what you are looking for - for now you'll have to work with the selector approach described above

[agusterodin]

I think the approach that React Query uses is to have an option in hook called "keepPreviousData" (https://react-query.tanstack.com/reference/useQuery)

[HansBrende]

@phryneas @agusterodin I am in exactly the same boat and it is causing subtle problems with my UI. Say, for instance, I've built a card that displays the information for a particular item.

There are 2 different use-cases desirable here:

(1) refetching cases where the item's id stays the same (e.g., polling, forced refetching, etc.): expected behavior here should be the old data stays present to avoid a jarring effect as the item's info is refreshed.

(2) refetching cases where the item's id changes (e.g., when react props change): expected behavior here should be the old data gets reset to undefined because now all of that data is definitely incorrect/stale/wrong (e.g. it might be showing the wrong item that the user has NOT selected), creating a very buggy UI experience!!!

Examining the isFetching flag is inadequate here, because it is unknown whether we are refetching because of polling/forced refetch etc. OR because of an arg change.

The workaround I TRIED to do:

function ItemCardView({itemId}) {
    const {data: _possiblyWrongData, originalArgs} = useGetItemQuery({itemId});
    const correctData = originalArgs?.itemId === itemId ? _possiblyWrongData : undefined;
    ...
}

Basically I am trying to manually overwrite data back to undefined if it is associated with stale query args (i.e., the query args have changed since data was fetched).

Unfortunately it seems like originalArgs is tied to the latest args passed in, not the original args that _possiblyWrongData was queried from?? I could be wrong, but I can't find any documentation on this.

[Not that I should have to do this hacky workaround in the first place! Default library behavior, IMHO, should be the behavior that is least likely to lead to a buggy UI. Bugs should be opt-in, not opt-out. The whole point of using this library was to reduce boilerplate for common use-cases... but if even the most common of use-cases is not supported in the normal idiomatic style without integrity bugs, I'd rather migrate to a more mature library such as React Query which apparently supports this use-case.]

[HansBrende]

FYI... I am having to do the same hack I (attempted to) do above for the skip condition, e.g.:

function DisplayList({id}) {
   const {data: _possiblyIrrelevantList = []} = useGetFoobarQuery(id == null ? skipToken : {id});
   const correctList = id == null ? [] : _possiblyIrrelevantList;
   ...
}

This little hack is necessary to avoid integrity bugs literally everywhere I use RTK query with a skip token. Good library in general, but these subtle details are quite suboptimal & surprising... hopefully improvements can be made in the integrity area.

[HansBrende]

Both of the problems I've mentioned above could be easily worked around if you just exposed one additional field, something like: argsThatQueriedCurrentData.

Then in both cases, I could simply do:

const {data: _data, argsThatQueriedCurrentData} = useGetSomethingOrOtherQuery(args);
const data = shallowEquals(args, argsThatQueriedCurrentData) ? _data : undefined;

This would still be more boilerplate than I would like (as it should really not be opt-in to avoid integrity bugs), but at least this (or an equivalent solution) would be a workable solution and I wouldn't need to migrate to a different library; the situation as it currently stands is a dealbreaker.

BETTER YET would be to simply do this internally yourself:

function useXXXQuery(args) {
    ...
    const result = oldImplementationOf_useXXXQuery(args);
    ...
    if (!shallowEquals(args, argsThatQueriedCurrentData)) {
        result.staleData = result.data;
        result.data = undefined;
    }
    return result;
}

Then, for 99.99999% of use-cases where I care about data integrity, I could simply do:

const {data} = useGetSomethingOrOtherQuery(args);

and not have to worry about bugs. If, for some reason, I really really needed to access the previous, now-incorrect data, I could then just do:

const {data, staleData} = useGetSomethingOrOtherQuery(args);
const dataLackingIntegrity = data ?? staleData;

Alternatively, a solution such as

const {data} = useGetSomethingOrOtherQuery(args, {pleaseDoNotGiveMeBadDataIfArgsHaveChanged: true});

would work quite well and I would be quite thankful for (although it seems like this route would give you less fine-grained control).

@phryneas:
See it from a few other perspective:

• Paginated data. On first load, you don't have data. Show a spinner. Changing to page 2 grays out the data (isFetching), but data from page 1 is still displayed to prevent the UI jumping unnecessarily.

The problem with the above statement is that isFetching === true does not imply that the args have changed. We might still be on page 1, and isFetching is true because of polling, or because of a manual refetch. It would be much better to be able to access the args used to fetch the current data to check if (and how) they are different from the current args. In other words, we need to be able to check whether shallowEquals(args, argsThatFetchedCurrentData) === true.

[phryneas]

The problem, frankly, is that none of this came up in beta and most suggestions right now either add a lot of additional api changes or would be considered breaking (moving data to staleData).

So I appreciate all the comments, but this topic will need a lot of time and thought to find the best approach as to not make things worse. So this will stay open, but probably not resolved in the near future.

And please keep in mind when writing something like

[Not that I should have to do this hacky workaround in the first place! Default library behavior, IMHO, should be the behavior that is least likely to lead to a buggy UI. Bugs should be opt-in, not opt-out. The whole point of using this library was to reduce boilerplate for common use-cases... but if even the most common of use-cases is not supported in the normal idiomatic style without integrity bugs, I'd rather migrate to a more mature library such as React Query which apparently supports this use-case.]

React Query is over two years old and the author of React Query does Open Source as his job while we are doing this in addition to our jobs. It would be pretty hard to live from the 45$ of GitHub sponsors I'm getting out of this, so there is limited time and a lot of stuff to do.
Comments like this are extremely discouraging. If React Query does this better and you need it, please, by all means, use React Query. This is not a competition and I'm happy that you have found the right tool for you, even if it is another tool.
This is also not a carbon copy of another library, but the attempt to deliberately do things differently - and in some places that works amazing, in others it doesn't. We don't know before a few thousand people use it - we've been in beta for half a year without any complaints on this for example.
So now we'll have to iterate - but we'll have to do so in the time we have, with the resources we have, and sometimes with different priorities.

[HansBrende]

@phryneas sorry, didn't mean to send any discouragement your way. Hopefully my comments at least gave some food for thought. But now I feel bad so I will donate lol.

[phryneas]

@HansBrende Please don't just because you feel bad ;)

[HansBrende]

@phryneas too late lol

[phryneas]

@HansBrende Whoa, thanks a lot!

[HansBrende]

@phryneas recognizing that you have constrained development resources/time & want to avoid backwards-incompatible changes, I will try to boil all of my comments down to the shortest backwards-compatible path where I become unblocked. I can potentially implement workarounds in my own codebase to cover the rest, but there are some practical blockers from doing so at the moment. If any ONE of these blockers are alleviated, I'd become unblocked from implementing a workaround (even if it is a painful/hacky workaround):

Blocker #0: no data field exists that is guaranteed to match the query args

This prevents me from taking the easiest path:

const {dataWithIntegrity: data, data: _ignored_} = useGetFoobarQuery(arg);

[Note: this is the default behavior in React Query; accessing previous data is only enabled with the keepPreviousData flag set.]

Blocker #1: I cannot check if the data is previous data from a different query

This prevents me from implementing the following workaround:

    const {data: _data, isPreviousData} = useGetFoobarQuery(arg);
    const data = isPreviousData ? undefined : _data;

[Note: isPreviousData is supported by React Query for queries that used {keepPreviousData: true}; see docs.]

Blocker #2: I cannot access the arg that queried the current data

This prevents me from implementing the slightly more boilerplatey workaround:

    const {data: _data, argThatQueriedData} = useGetFoobarQuery(arg);
    const isPreviousData = !shallowEquals(arg, argThatQueriedData);
    const data = isPreviousData ? undefined : _data;

Blocker #3: I cannot access arg in transformResponse

This prevents me from implementing the vastly more boilerplatey workaround:

    transformResponse(response, meta, argThatQueriedResponse) {
        response.argUsedForQuery = argThatQueriedResponse;
        return response;
    }
    
    ... later ...
    
    const {data: _data} = useGetFoobarQuery(arg);
    const isPreviousData = !shallowEquals(arg, _data?.argUsedForQuery)
    const data = isPreviousData ? undefined : _data;

Surely at least one of these library enhancements wouldn't be too much work to implement? Fingers crossed 🤞

[phryneas]

@HansBrende while 3. is something we should probably do either way (good catch!) I think all these are just really workarounds. I think #1500 might be a good solution, just also exposing the "currentData" in addition to a data attribute, to be used in these use cases.

@agusterodin @pserrer1 Could everyone that has commented in this issue please try that out and leave their feedback? Is this exactly what you need? Just kinda? Does it leave problems or questions open?

You can find easy installation instructions for the CI build on the CodeSandbox build page.

[agusterodin]

Awesome, thanks! Will try out and report back by end of day.

It should coexist fine with selectFromResult, right?

Edit: just left feedback on the PR. Fixes the problem I was having and I like the approach.

[zousandian]

Also: isLoading is only for the first fetch of a hook. If you are changing query args, it will never go back to true again. I'm not entirely sure what behaviour you would want there.

@phryneas hi, you say isLoading will never go back to true. But when I test the pagination example，when I set page, isLoading will become true first, and then become false. Is this expected?

demo link:
https://codesandbox.io/s/admiring-wiles-6rm5t?file=/src/features/posts/PostsManager.tsx

[phryneas]

@zousandian that does indeed look like a bug. I've opened #1519 to track it.

[scwood]

This was closed with multiple PRs but I'm wondering if there's an official solution is to the original problem.

We're using RTK query on a dashboard application that has a bunch of cards and the queries for all the cards use polling.

We want to show spinners on the initial load, but not on subsequent polled requests, so isLoading comes to mind. However, if the user changes the input to the dashboard (and thus the input to the queries) it's no longer desirable to show the old data, so we need to show spinners again.

To solve this right now, for every single query we have an extra piece of state and two useEffects to support that use case:

const [customIsLoading, setCustomIsLoading] = useState(true);

// set customIsLoading flag to true when user changes the input to the dashboard
useEffect(() => setCustomIsLoading(true), [dashboardInput])

// set customIsLoading to false if the query isn't fetching
useEffect(() => {
  if (!queryResult.isFetching) {
    setCustomIsLoading(false);
  }
}, [queryResult.isFetching])

This essentially gives the effect of isLoading but makes it reset based on a piece of state. My question is there a better way to do this? Is there some way to invalidate the cache manually such that isLoading would once again get set to true on the next request? Maybe a different approach altogether?