useQuery().promise will always be a rejected promise if the query is in an error state

[KATT]

Describe the bug

If a query has ever errored, getOptimisticResult() will always return an error status (rather than a pending one)

Your minimal, reproducible example

https://codesandbox.io/p/devbox/winter-darkness-k48yny

Steps to reproduce

1. Fail a query
2. Try to use useQuery().promise somewhere else
3. Component always fails, rather than optimistically fetching when it renders

Expected behavior

If the getOptimisticResult() should reflect the actual optimistic result, it should be pending

How often does this bug happen?

None

Screenshots or Videos

No response

Platform

CodeSandbox

Tanstack Query adapter

None

TanStack Query version

latest

TypeScript version

latest

Additional context

Discovered while trying to fix #8499 and tried to fix in the same PR, but will try to have a stab on doing it separately

[TkDodo]

before you go to deep into this @KATT, I’m pretty sure getOptimisticResult() works fine. The problem is that experimental_prefetchInRender treats the query as if it “suspends”, so we set retryOnMount to false to prevent too many refetches when we have an error that will be thrown to an error boundary:

query/packages/react-query/src/errorBoundaryUtils.ts

Lines 29 to 38 in 74f0d6a

	if (
	options.suspense ||
	options.throwOnError ||
	options.experimental_prefetchInRender
	) {
	// Prevent retrying failed query if the error boundary has not been reset yet
	if (!errorResetBoundary.isReset()) {
	options.retryOnMount = false
	}
	}

This could be wrong, but on the other hand, without that, we would never see an error boundary as we would attempt to refetch before it is ever shown.

If we keep this behaviour, I think it’s fine to show the error from the error boundary, and then the boundary needs a “retry” button that also resets our flag as shown here:

https://tanstack.com/query/latest/docs/framework/react/guides/suspense#resetting-error-boundaries

[KATT]

Thank you! I'm glad I posted this issue before going too deep :) I wanted your thoughts before doing too much

[KATT]

Could you have a look at this test in #8508 and reaffirm that you also think that this should pass without futzing around with error boundaries?

I'm just making sure there's an error in the cache:

https://github.com/KATT/react-query/blob/1aefaeda94a45209379e81f71c4e1c98219d6b44/packages/react-query/src/__tests__/useQuery.promise.test.tsx#L148-L156

and trying to fetch it

https://github.com/KATT/react-query/blob/1aefaeda94a45209379e81f71c4e1c98219d6b44/packages/react-query/src/__tests__/useQuery.promise.test.tsx#L160-L166

[KATT]

The way I'm thinking about it, result.promise at …

query/packages/react-query/src/useBaseQuery.ts

Line 85 in 3c5d8e3

const result = observer.getOptimisticResult(defaultedOptions)

… must be a pending promise of the next result rather than the error & that it could either be done by

a) futzing around with getOptimisticResult() or,
b) to do to transform the result + trigger the fetchOptimistic() together if prefetchInRender===true

[TkDodo]

There are two things I’m seeing in your test:

1. prefetchInRender won’t trigger a re-fetch in render when useQuery mounts, because we already have a cached entry (an error). In that case, if you use(promise) in the same component as useQuery, there is nothing that will actually fire the re-fetch. This is the same conceptual problem as the one with enabled.

2. If we break it up into multiple components, I agree it should work.

[KATT]

There are two things I’m seeing in your test:

1. prefetchInRender won’t trigger a re-fetch in render when useQuery mounts, because we already have a cached entry (an error). In that case, if you use(promise) in the same component as useQuery, there is nothing that will actually fire the re-fetch. This is the same conceptual problem as the one with enabled.

Not exactly, that's why I didn't try to fix them together in #8501:

• For the enabled-one, it'll have a pending promise which later resolves (after that bug is fixed)
• In this case, it'll be a rejected promise that immediately triggers a suspense boundary.

Conceptually, they can be solved together which is what a fix to this would likely look like

2. If we break it up into multiple components, I agree it should work.

wdym? Are you saying prefetching in the test isn't good enough?

[TkDodo]

In this case, it'll be a rejected promise that immediately triggers a suspense boundary.

Yeah, I’m just not sure why this is wrong 🤔 . If a query errors we expect it to show the error in an error boundary until the user has acknowledged that error. A prefetch doesn’t really change that?

wdym? Are you saying prefetching in the test isn't good enough?

I mean we need to split up the useQuery and the use call. Your test does this:

function MyComponent() {
  useTrackRenders()
  const query = useQuery({
    queryKey: key,
    queryFn,
    staleTime: Infinity,
  })

  const data = React.use(query.promise)
  return <>{data}</>
}

by the time useQuery mounts, it finds a query in error state in the cache. Even if query.promise would be a pending one, useQuery doesn’t trigger a refetch because prefetchInRender can’t do it as a cache entry exists already, so it would be an observable side effect, and useQuery can only do it in an effect, but we unmount MyComponent.

That’s why I said it’s conceptually similar to the enabled example

[KATT]

Hm. Maybe this isn't a bug and aligns with how standard useQuery() works in the case of previously errored queries.

I just encountered it and expected it to refetch as it mounts even if it has errors in the cache.