diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index c3fa0f6839..7dff4fb575 100644 --- a/src/core/queryObserver.ts +++ b/src/core/queryObserver.ts @@ -68,11 +68,9 @@ export class QueryObserver< TQueryKey > private previousQueryResult?: QueryObserverResult - private previousSelectError: Error | null - private previousSelect?: { - fn: (data: TQueryData) => TData - result: TData - } + private selectError: Error | null + private selectFn?: (data: TQueryData) => TData + private selectResult?: TData private staleTimeoutId?: number private refetchIntervalId?: number private currentRefetchInterval?: number | false @@ -93,7 +91,7 @@ export class QueryObserver< this.client = client this.options = options this.trackedProps = [] - this.previousSelectError = null + this.selectError = null this.bindMethods() this.setOptions(options) } @@ -508,27 +506,21 @@ export class QueryObserver< if ( prevResult && state.data === prevResultState?.data && - options.select === this.previousSelect?.fn && - !this.previousSelectError + options.select === this.selectFn ) { - data = this.previousSelect.result + data = this.selectResult } else { try { + this.selectFn = options.select data = options.select(state.data) if (options.structuralSharing !== false) { data = replaceEqualDeep(prevResult?.data, data) } - this.previousSelect = { - fn: options.select, - result: data, - } - this.previousSelectError = null + this.selectResult = data + this.selectError = null } catch (selectError) { getLogger().error(selectError) - error = selectError - this.previousSelectError = selectError - errorUpdatedAt = Date.now() - status = 'error' + this.selectError = selectError } } } @@ -565,13 +557,10 @@ export class QueryObserver< placeholderData ) } - this.previousSelectError = null + this.selectError = null } catch (selectError) { getLogger().error(selectError) - error = selectError - this.previousSelectError = selectError - errorUpdatedAt = Date.now() - status = 'error' + this.selectError = selectError } } } @@ -583,6 +572,13 @@ export class QueryObserver< } } + if (this.selectError) { + error = this.selectError as any + data = this.selectResult + errorUpdatedAt = Date.now() + status = 'error' + } + const result: QueryObserverBaseResult = { status, isLoading: status === 'loading', diff --git a/src/core/tests/queryObserver.test.tsx b/src/core/tests/queryObserver.test.tsx index 1cba155cbd..9ad4123a54 100644 --- a/src/core/tests/queryObserver.test.tsx +++ b/src/core/tests/queryObserver.test.tsx @@ -245,18 +245,17 @@ describe('queryObserver', () => { expect(observerResult2.data).toMatchObject({ myCount: 1 }) }) - test('should always run the selector again if selector throws an error', async () => { + test('should always run the selector again if selector throws an error and selector is not referentially stable', async () => { const consoleMock = mockConsoleError() const key = queryKey() const results: QueryObserverResult[] = [] - const select = () => { - throw new Error('selector error') - } const queryFn = () => ({ count: 1 }) const observer = new QueryObserver(queryClient, { queryKey: key, queryFn, - select, + select: () => { + throw new Error('selector error') + }, }) const unsubscribe = observer.subscribe(result => { results.push(result) @@ -264,7 +263,7 @@ describe('queryObserver', () => { await sleep(1) await observer.refetch() unsubscribe() - expect(results.length).toBe(5) + expect(results.length).toBe(4) expect(results[0]).toMatchObject({ status: 'loading', isFetching: true, @@ -285,11 +284,60 @@ describe('queryObserver', () => { isFetching: false, data: undefined, }) - expect(results[4]).toMatchObject({ + consoleMock.mockRestore() + }) + + test('should return stale data if selector throws an error', async () => { + const consoleMock = mockConsoleError() + const key = queryKey() + const results: QueryObserverResult[] = [] + let shouldError = false + const error = new Error('select error') + const observer = new QueryObserver(queryClient, { + queryKey: key, + queryFn: () => (shouldError ? 2 : 1), + select: num => { + if (shouldError) { + throw error + } + shouldError = true + return String(num) + }, + }) + + const unsubscribe = observer.subscribe(result => { + results.push(result) + }) + await sleep(10) + await observer.refetch() + unsubscribe() + + expect(results.length).toBe(4) + expect(results[0]).toMatchObject({ + status: 'loading', + isFetching: true, + data: undefined, + error: null, + }) + expect(results[1]).toMatchObject({ + status: 'success', + isFetching: false, + data: '1', + error: null, + }) + expect(results[2]).toMatchObject({ + status: 'success', + isFetching: true, + data: '1', + error: null, + }) + expect(results[3]).toMatchObject({ status: 'error', isFetching: false, - data: undefined, + data: '1', + error, }) + consoleMock.mockRestore() }) diff --git a/src/react/tests/useQuery.test.tsx b/src/react/tests/useQuery.test.tsx index 8ffc82a6bb..f9eb777662 100644 --- a/src/react/tests/useQuery.test.tsx +++ b/src/react/tests/useQuery.test.tsx @@ -820,6 +820,47 @@ describe('useQuery', () => { consoleMock.mockRestore() }) + it('should not re-run a stable select when it re-renders if selector throws an error', async () => { + const consoleMock = mockConsoleError() + const key = queryKey() + const error = new Error('Select Error') + let runs = 0 + + function Page() { + const [, rerender] = React.useReducer(() => ({}), {}) + const state = useQuery( + key, + () => (runs === 0 ? 'test' : 'test2'), + { + select: React.useCallback(() => { + runs++ + throw error + }, []), + } + ) + return ( +
+
error: {state.error?.message}
+ + +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + await waitFor(() => rendered.getByText('error: Select Error')) + expect(runs).toEqual(1) + fireEvent.click(rendered.getByRole('button', { name: 'rerender' })) + await sleep(10) + expect(runs).toEqual(1) + fireEvent.click(rendered.getByRole('button', { name: 'refetch' })) + await sleep(10) + expect(runs).toEqual(2) + + consoleMock.mockRestore() + }) + it('should re-render when dataUpdatedAt changes but data remains the same', async () => { const key = queryKey() const states: UseQueryResult[] = [] @@ -4134,7 +4175,7 @@ describe('useQuery', () => { await waitFor(() => rendered.getByText('Data: selected 3')) }) - it('select should structually share data', async () => { + it('select should structurally share data', async () => { const key1 = queryKey() const states: Array> = []