Skip to content

Commit

Permalink
fix(queryObserver): cache select errors (TanStack#3554)
Browse files Browse the repository at this point in the history
there is no need to re-run the select function if it is referentially stable, even when it throws an error. if no dependency has changed, we can re-use the cached error, as the select function should be idempotent and thus return the same error anyway.

further, if we have cached data from a previously successful select, that should be returned as `data` alongside the error.
  • Loading branch information
TkDodo authored and MMMikeM committed May 23, 2022
1 parent 157e994 commit d882161
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 16 deletions.
15 changes: 6 additions & 9 deletions src/core/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,21 +462,18 @@ 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) {
if (process.env.NODE_ENV !== 'production') {
this.client.getLogger().error(selectError)
Expand Down Expand Up @@ -521,7 +518,7 @@ export class QueryObserver<
placeholderData
)
}
this.previousSelectError = null
this.selectError = null
} catch (selectError) {
if (process.env.NODE_ENV !== 'production') {
this.client.getLogger().error(selectError)
Expand Down
61 changes: 54 additions & 7 deletions src/core/tests/queryObserver.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -249,22 +249,21 @@ describe('queryObserver', () => {
test('should always run the selector again if selector throws an error', async () => {
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)
})
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,
Expand All @@ -285,10 +284,58 @@ 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,
})
})

Expand Down

0 comments on commit d882161

Please sign in to comment.