Skip to content

Commit

Permalink
fix(useQuery): fix tracked queries and error boundaries (#2993)
Browse files Browse the repository at this point in the history
if you useErrorBoundary, you're likely not observing the error, because you're not using the returned error field - the ErrorBoundary handles the error for you. So when the actual error then happens, we don't inform the observer about it, because we're only informing them if a prop has changed that they are interested in, resulting in the error boundary never being shown.

The solution would be to always track the error property if you've set useErrorBoundary
  • Loading branch information
TkDodo committed Nov 20, 2021
1 parent c2727c1 commit 58dc1ce
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 7 deletions.
26 changes: 20 additions & 6 deletions src/core/queryObserver.ts
Expand Up @@ -232,24 +232,38 @@ export class QueryObserver<
}

trackResult(
result: QueryObserverResult<TData, TError>
result: QueryObserverResult<TData, TError>,
defaultedOptions: QueryObserverOptions<
TQueryFnData,
TError,
TData,
TQueryData,
TQueryKey
>
): QueryObserverResult<TData, TError> {
const trackedResult = {} as QueryObserverResult<TData, TError>

const trackProp = (key: keyof QueryObserverResult) => {
if (!this.trackedProps.includes(key)) {
this.trackedProps.push(key)
}
}

Object.keys(result).forEach(key => {
Object.defineProperty(trackedResult, key, {
configurable: false,
enumerable: true,
get: () => {
const typedKey = key as keyof QueryObserverResult
if (!this.trackedProps.includes(typedKey)) {
this.trackedProps.push(typedKey)
}
return result[typedKey]
trackProp(key as keyof QueryObserverResult)
return result[key as keyof QueryObserverResult]
},
})
})

if (defaultedOptions.useErrorBoundary || defaultedOptions.suspense) {
trackProp('error')
}

return trackedResult
}

Expand Down
60 changes: 60 additions & 0 deletions src/react/tests/QueryResetErrorBoundary.test.tsx
Expand Up @@ -592,4 +592,64 @@ describe('QueryErrorResetBoundary', () => {

expect(rendered.queryByText('page')).not.toBeNull()
})

it('should show error boundary when using tracked queries even though we do not track the error field', async () => {
const key = queryKey()

let succeed = false
const consoleMock = mockConsoleError()

function Page() {
const { data } = useQuery(
key,
async () => {
await sleep(10)
if (!succeed) {
throw new Error('Error')
} else {
return 'data'
}
},
{
retry: false,
useErrorBoundary: true,
notifyOnChangeProps: 'tracked',
}
)
return <div>{data}</div>
}

const rendered = renderWithClient(
queryClient,
<QueryErrorResetBoundary>
{({ reset }) => (
<ErrorBoundary
onReset={reset}
fallbackRender={({ resetErrorBoundary }) => (
<div>
<div>error boundary</div>
<button
onClick={() => {
resetErrorBoundary()
}}
>
retry
</button>
</div>
)}
>
<Page />
</ErrorBoundary>
)}
</QueryErrorResetBoundary>
)

await waitFor(() => rendered.getByText('error boundary'))
await waitFor(() => rendered.getByText('retry'))
succeed = true
fireEvent.click(rendered.getByText('retry'))
await waitFor(() => rendered.getByText('data'))

consoleMock.mockRestore()
})
})
2 changes: 1 addition & 1 deletion src/react/useBaseQuery.ts
Expand Up @@ -144,7 +144,7 @@ export function useBaseQuery<

// Handle result property usage tracking
if (defaultedOptions.notifyOnChangeProps === 'tracked') {
result = observer.trackResult(result)
result = observer.trackResult(result, defaultedOptions)
}

return result
Expand Down

1 comment on commit 58dc1ce

@vercel
Copy link

@vercel vercel bot commented on 58dc1ce Nov 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.