Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useQuery keepPreviousData is not keeping previous data on error #3046

Closed
HARIKSREEE opened this issue Dec 3, 2021 · 14 comments
Closed

useQuery keepPreviousData is not keeping previous data on error #3046

HARIKSREEE opened this issue Dec 3, 2021 · 14 comments

Comments

@HARIKSREEE
Copy link

useQuery used with keepPreviousData as true is not keeping the previous query data when an error occurs in a paginated list fetch.

Steps:

  1. Created a custom hook for fetching my paginated list (keepPreviousData is true)
  2. First fetch is successful. (Fetched with limit = 10, offset = 0)
  3. Second fetch failed due to a random API issue (Fetched with limit = 10, offset = 10)
  4. data property from the query becomes undefined

Expected behavior
When the second fetch failed, I am expecting the data from the first fetch to be kept for showing in the UI.

Intention:
Keep the data from last successful fetch and show a error toast for the failure

Screenshots
image

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chromium Edge
  • Version: 3.34.0 (React Query Version)

Additional context
I referred to the stackoverflow thread and seen a comment that the data will not become undefined .
Stackoverflow link

@TkDodo
Copy link
Collaborator

TkDodo commented Dec 3, 2021

When the second fetch failed, I am expecting the data from the first fetch to be kept for showing in the UI.

nope, this is not how keepPreviousData works. When the query key changes, it will show you the data of the previous query key while you are fetching for the next query key to emulate a "transition".

but once the new query key has finished loading, you will just get what that query has. if it's a success state with data, you'll get that, and if it's an error state without data, you'll get that as well. I don't see a good use-case for showing fields from the previous query (like data) mixed with other fields from the current query (like error) after it has settled.

@HARIKSREEE
Copy link
Author

HARIKSREEE commented Dec 3, 2021

Thank you for the quick response.
My use case is like below.

  1. I have a table to show list of items
  2. Below the table I have a pagination component
  3. For example, when I click page 2 , I will be getting items from page 2 and I can show that in the table.
  4. Suppose, when I try to go to page 3, something happened at backend and that fetch failed.
  5. In that case , instead of showing an empty table, I need to keep the last successful data and indicate the user that some error
    happened using a toast or any other kind of notification.

I understand that from your above comment, the data will not be available when an error occurred since the query changed.

I am starting to think React Query is not suitable for my use case.

By the way, I tried a way to achieve the behavior by setting the latest successful data as initialData through the initialData callback function and at the same time, filter out any error query cache.
Could you please give an advice whether this is a considerable solution?
I am trying to understand if there is any straight forward way using react query.

@TkDodo
Copy link
Collaborator

TkDodo commented Dec 3, 2021

you can of course set initialData from the previous query as data for your "next" query if you want that. That data will then be put into the cache for that page as well, and with the default staleTime of 0, you will get a background refetch fetching the real data.

however, if it errors, you will be:

  • on page 3, and page 3 will be in your local state
  • so your page navigation component will likely show that you are on page 3
  • but the data will be from page 2 🤷

if I would want that behaviour, I would:

  • store the previous page and go back to that one in case of error.
  • show data from a previous page with queryClient.getQueryData in case I have an error
  • maybe write the previous data to a ref?
const { isPreviousData, data } = useQuery(....)
const previousData = useRef()

useEffect(() => {
  if (isPreviousData) {
    previousData.current = data
  }
}, [isPreviousData, data])

It is not technically impossible to return previous data in case of error from react-query perspective, I just think it would be more confusing than helpful (but maybe I'm wrong), as the main use-case was to avoid this:

The UI jumps in and out of the success and loading states because each new page is treated like a brand new query.

@TkDodo
Copy link
Collaborator

TkDodo commented Dec 3, 2021

fyi, I just found the bugfix where this behaviour was introduced:

I think the desired effect is that if an error is encountered in the new/latest query, the status should not be success with the previous data, but error with the new query.

@HARIKSREEE
Copy link
Author

you can of course set initialData from the previous query as data for your "next" query if you want that. That data will then be put into the cache for that page as well, and with the default staleTime of 0, you will get a background refetch fetching the real data.

however, if it errors, you will be:

  • on page 3, and page 3 will be in your local state
  • so your page navigation component will likely show that you are on page 3
  • but the data will be from page 2 🤷

if I would want that behaviour, I would:

  • store the previous page and go back to that one in case of error.
  • show data from a previous page with queryClient.getQueryData in case I have an error
  • maybe write the previous data to a ref?
const { isPreviousData, data } = useQuery(....)
const previousData = useRef()

useEffect(() => {
  if (isPreviousData) {
    previousData.current = data
  }
}, [isPreviousData, data])

It is not technically impossible to return previous data in case of error from react-query perspective, I just think it would be more confusing than helpful (but maybe I'm wrong), as the main use-case was to avoid this:

The UI jumps in and out of the success and loading states because each new page is treated like a brand new query.

Thanks a lot
May be we should change the property name to something like, preventUiJump from KeepPreviousData

@TkDodo TkDodo closed this as completed Dec 3, 2021
@kostia1st
Copy link

May be we should change the property name to something like, preventUiJump from KeepPreviousData

However funny that sounds, I support that idea in general. Because right now, keepPreviousData does NOT keep the previous data when it's supposed to, and the data prop returned from useQuery is described as The last successfully resolved data for the query which in conjunction with keepPreviousData should be returning the previous successful result.

Currently, keepPreviousData is more of keepPreviousDataWhileTransitioningToAnotherKeyButNoMore

@kostia1st
Copy link

kostia1st commented Sep 8, 2022

I ended up implementing a wrapper hook which does what is expected. Posting here in case someone could also benefit from the code.

export const useAppQuery = <
  TQueryFnData,
  TError,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
>(
  ...args: Parameters<typeof useQuery<TQueryFnData, TError, TData, TQueryKey>>
) => {
  const previousDataRef = useRef<TData>();
  const result = useQuery(...args);
  if (!result.isError) {
    previousDataRef.current = result.data;
  } else {
    result.data = previousDataRef.current;
  }
  return result;
};

@siteassist-benjamin
Copy link

@TkDodo I have come across this thread and it is exactly what I am looking for! 🙏

Our use case for react-query is to also cache data for offline use/on poor connection. When data currently fails to load (even if its cached in a persistor) onError as clarified above, the cached data gets replaced with only the error.

This is not ideal because it is overriding data that we are persisting on purpose and react-query is acting as our store. We want to keep old data in cache until more up to date data is retrieved.

I would think other people may come across this use case too since this library seems to be trying to remove a lot of redux/store use cases

Do you see any other way that this could be solved?

Thanks in advance!

@TkDodo
Copy link
Collaborator

TkDodo commented Jul 18, 2023

When data currently fails to load (even if its cached in a persistor) onError as clarified above, the cached data gets replaced with only the error.

please show a codesandbox reproduction. Generally, we don't remove data if it's there. When you get an error for a refetch while you have data, you will be in error state, but data should still be defined.

@siteassist-benjamin
Copy link

siteassist-benjamin commented Jul 18, 2023

When data currently fails to load (even if its cached in a persistor) onError as clarified above, the cached data gets replaced with only the error.

please show a codesandbox reproduction. Generally, we don't remove data if it's there. When you get an error for a refetch while you have data, you will be in error state, but data should still be defined.

Thanks so much for your reply!

The data is defined onError but when the hook is destroyed, the next time the hook is used, the data is not there anymore.

I will try to somehow create a codesandbox reproduction however its hard because especially now that you mention it, the behaviour probably has to do with persistence or hook re-instantiation.

Do you have any ideas what it could be?

@TkDodo
Copy link
Collaborator

TkDodo commented Jul 18, 2023

probably has to do with persistence or hook re-instantiation.

queries in error state are not persisted because Errors cannot be serialized. Even if you have data, the error would "disappear".

see: https://tanstack.com/query/v4/docs/react/reference/hydration#limitations

@HARIKSREEE
Copy link
Author

@siteassist-benjamin
I had to use redux toolkit rtk query to work with this situation.
That worked pretty well.
Please note, there are some features not available in rtk query compared to react query. So please check the comparison table before taking a decision.

@siteassist-benjamin
Copy link

probably has to do with persistence or hook re-instantiation.

queries in error state are not persisted because Errors cannot be serialized. Even if you have data, the error would "disappear".

see: https://tanstack.com/query/v4/docs/react/reference/hydration#limitations

Thank you so much @TkDodo doing shouldDehydrateQuery: () => true fixed my issue, thats exactly what I was looking for. We didn't need to serialise the error, we just needed access to the data

Really appreciate the help ❤️

@TkDodo
Copy link
Collaborator

TkDodo commented Jul 18, 2023

fixed my issue, thats exactly what I was looking for. We didn't need to serialise the error, we just needed access to the data

just keep in mind that that means that you can be in isError state, but potentially have no error, even though the types say so. So if you do:

if (isError) {
  return <div>{error.message}</div>
}

you will get a runtime error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants