Skip to content

Commit

Permalink
fix(queryObserver): memoized select functions should not return outda…
Browse files Browse the repository at this point in the history
…ted values (#3140)

not all runs to createResult are saved on `this.currentResult` (optimistically created updates do not - we just create the result again later). With the fix from #3098, we made sure that memoized select does not run again in those cases by storing every run. However, that now means that we cannot rely on `prevResult.data` because it might still contain outdated data.

The solution is to store a pair of previous selectFn + data and return the exact data that was computed from the previous select fn if we get the exact same function passed
  • Loading branch information
TkDodo authored Dec 28, 2021
1 parent b977cf4 commit d7faf86
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 4 deletions.
14 changes: 10 additions & 4 deletions src/core/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ export class QueryObserver<
>
private previousQueryResult?: QueryObserverResult<TData, TError>
private previousSelectError: Error | null
private previousSelectFn?: (data: TQueryData) => TData
private previousSelect?: {
fn: (data: TQueryData) => TData
result: TData
}
private staleTimeoutId?: number
private refetchIntervalId?: number
private currentRefetchInterval?: number | false
Expand Down Expand Up @@ -497,14 +500,17 @@ export class QueryObserver<
if (
prevResult &&
state.data === prevResultState?.data &&
options.select === this.previousSelectFn &&
options.select === this.previousSelect?.fn &&
!this.previousSelectError
) {
data = prevResult.data
data = this.previousSelect.result
} else {
try {
this.previousSelectFn = options.select
data = options.select(state.data)
this.previousSelect = {
fn: options.select,
result: data,
}
if (options.structuralSharing !== false) {
data = replaceEqualDeep(prevResult?.data, data)
}
Expand Down
50 changes: 50 additions & 0 deletions src/react/tests/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3998,6 +3998,56 @@ describe('useQuery', () => {
expect(selectRun).toBe(3)
})

it('select should always return the correct state', async () => {
const key1 = queryKey()

function Page() {
const [count, inc] = React.useReducer(prev => prev + 1, 2)
const [forceValue, forceUpdate] = React.useReducer(prev => prev + 1, 1)

const state = useQuery(
key1,
async () => {
await sleep(10)
return 0
},
{
select: React.useCallback(
(data: number) => {
return `selected ${data + count}`
},
[count]
),
placeholderData: 99,
}
)

return (
<div>
<h2>Data: {state.data}</h2>
<h2>forceValue: {forceValue}</h2>
<button onClick={inc}>inc: {count}</button>
<button onClick={forceUpdate}>forceUpdate</button>
</div>
)
}

const rendered = renderWithClient(queryClient, <Page />)
await waitFor(() => rendered.getByText('Data: selected 101')) // 99 + 2

await waitFor(() => rendered.getByText('Data: selected 2')) // 0 + 2

rendered.getByRole('button', { name: /inc/i }).click()

await waitFor(() => rendered.getByText('Data: selected 3')) // 0 + 3

rendered.getByRole('button', { name: /forceUpdate/i }).click()

await waitFor(() => rendered.getByText('forceValue: 2'))
// data should still be 3 after an independent re-render
await waitFor(() => rendered.getByText('Data: selected 3'))
})

it('should cancel the query function when there are no more subscriptions', async () => {
const key = queryKey()
let cancelFn: jest.Mock = jest.fn()
Expand Down

1 comment on commit d7faf86

@vercel
Copy link

@vercel vercel bot commented on d7faf86 Dec 28, 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.