useSuspenseQuery - throw error while having stale data

[grzesiek-ds]

Hello TanStack team!

First of all I would like to thank You for Your hard work - TanStack Query is a great tool and I enjoy using it.

Example situation

Consider a case where we have component which is suppose to always display fresh (not-stale) data.

• When data is loading for first time, loader should be displayed.
• When fresh data is available, it should be displayed.
• When we did not manage to fetch fresh data, and data in cache becomes stale or theres no data in cache, error boundary should be displayed.
• When error boundary is resetting, loader should be displayed (since data in cache is stale, or theres no data in cache).

Problem statement

I believe that it could be achieved by using useQuery and custom throwOnError, but while using useSuspenseQuery, it appears that custom useEffect is needed.

With useQuery

// Note - I did not test it!
const QueryConsumer = () => {
  const result = useQuery({
    ...,
    staleTime: 30 * 1000,
    throwOnError: (_, query) => typeof query.state.data === 'undefined' || query.isStaleByTime(30 * 1000),
  })
}

With usesSuspenseQuery

// Also did not test it!
const QueryConsumer = () => {
  const result = useQuery({
    ...,
  })
  // useLayoutEffect instead to throw error before rendering?
  useEffect(() => {
    if (result.isStale && result.error) {
      throw result.error;
    }
  }, [result.isStale, result.error]);
}

Question

Since throwOnError is not available for useSuspenseQuery.
https://tanstack.com/query/latest/docs/framework/react/guides/suspense#throwonerror-default

Would You consider allowing throwOnError for useSuspenseQuery while also preserving current defaultThrowOnError logic?

Current implementation

const defaultThrowOnError = (...) => typeof query.state.data === 'undefined'

const useSuspenseQuery = (...) => {
  return useBaseQuery({
    ...,
    throwOnError: defaultThrowOnError,
  })
}

const useBaseQuery = (...) => {
  ...
  if (getHasError(...)) {
    throw result.error
  }
  ...
}

export const getHasError = (...) => {
  return (
    result.isError &&
    !errorResetBoundary.isReset() &&
    !result.isFetching &&
    query &&
    shouldThrowError(throwOnError, [result.error, query])
  )
}

Proposed change

// useSuspenseQuery can define throwOnError, which is combined with defaultThrownError
const useSuspenseQuery = ({ ..., throwOnError }) => {
  return useBaseQuery({
    ...,
    throwOnError: (error, query) => defaultThrowOnError(error, query) || throwOnError?.(error, query)
  })
}

// Or defaultThrowOnError logic is defined in getHasError - this affects useQuery behaviour
export const getHasError = (...) => {
  return (
    result.isError &&
    !errorResetBoundary.isReset() &&
    !result.isFetching &&
    query &&
    (
      // Logic from defaultThrowOnError is defined in getHasError
      typeof query.state.data === 'undefined' ||
      // Custom, optional throwOnError is allowed
      shouldThrowError(throwOnError, [result.error, query])
    )
  )
}

[TkDodo]

throwOnError is not allowed on useSuspenseQuery because you could set it to () => false, and that would void the guarantee that data is always defined after useSuspenseQuery has been called.

What you can do is just throw during rendering, without needing a useEffect:

if (result.isStale && result.error) {
  throw result.error;
}

that should trigger the error boundary. It's also in the docs: https://tanstack.com/query/latest/docs/framework/react/guides/suspense#throwonerror-default

[grzesiek-ds]

@TkDodo I think that I didnt phrase it correctly - sorry.
My proposition is to merge the current behaviour of defaultThrowOnError with custom value of throwOnError.

Therefore when settings throwOnError: () => false in reality check would be: typeof query.state.data === 'undefined' || false.

As result it would be ensured that data is defined.

[TkDodo]

My proposition is to merge the current behaviour of defaultThrowOnError with custom value of throwOnError.

interesting idea. Wouldn't it be quite confusing to pass throwOnError: () => false but still get errors thrown to boundaries if they are for an initial fetch? It would also be a different behaviour of the same option between suspense and non-suspense hooks. Is the throw-in-an-if-statement approach not good enough ?

[grzesiek-ds]

I agree that defining throwOnError for a suspense query is not great, as the error on the initial fetch cannot be ignored.
Would it be desirable to introduce such functionality as a separate prop, with a more self-explanatory name, just for useSuspenseQuery? For example, throwOnSubsequentError?
I think that good documentation would be enough to explain this — I'm willing to contribute to both the change and documentation.

There's nothing wrong with the custom error handling you mentioned above.
My goal was to propose the "normalization" of features offered by both useQuery and useSuspenseQuery. Of course, I'm motivated by a specific case that I have 🙃.

[TkDodo]

imo normalization of features only works if we have them behave the same, which we just agreed we cannot do.

[grzesiek-ds]

So the answer is "no we don't want this"? 😄

[TkDodo]

right now I'd say that's right :)