Skip to content

Commit

Permalink
fix(query-core): correct placeholderData prevData value with select fn (
Browse files Browse the repository at this point in the history
#5227)

* fix(query-core): correct placeholderData prevData

* fix(query-core): preserves correct prevQueryResult

This commit preserves the correct previous result between rerenders.

* test(react-query): Test with placeholder & select

* fix(query-core): Add lastDefinedQueryData property

* fix(query-core): Remove console.log

* fix(query-core): Add react-query test
  • Loading branch information
ardeora authored Apr 19, 2023
1 parent 67a563c commit 75a2b4d
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 7 deletions.
19 changes: 12 additions & 7 deletions packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ export class QueryObserver<
TQueryData,
TQueryKey
>
#previousQueryResult?: QueryObserverResult<TData, TError>
#selectError: TError | null
#selectFn?: (data: TQueryData) => TData
#selectResult?: TData
// This property keeps track of the last defined query data.
// It will be used to pass the previous data to the placeholder function between renders.
#lastDefinedQueryData?: TQueryData
#staleTimeoutId?: ReturnType<typeof setTimeout>
#refetchIntervalId?: ReturnType<typeof setInterval>
#currentRefetchInterval?: number | false
Expand Down Expand Up @@ -414,9 +416,6 @@ export class QueryObserver<
const queryInitialState = queryChange
? query.state
: this.#currentQueryInitialState
const prevQueryResult = queryChange
? this.#currentResult
: this.#previousQueryResult

const { state } = query
let { error, errorUpdatedAt, fetchStatus, status } = state
Expand Down Expand Up @@ -490,7 +489,7 @@ export class QueryObserver<
typeof options.placeholderData === 'function'
? (
options.placeholderData as unknown as PlaceholderDataFunction<TQueryData>
)(prevQueryResult?.data as TQueryData | undefined)
)(this.#lastDefinedQueryData)
: options.placeholderData
if (options.select && typeof placeholderData !== 'undefined') {
try {
Expand All @@ -504,7 +503,11 @@ export class QueryObserver<

if (typeof placeholderData !== 'undefined') {
status = 'success'
data = replaceData(prevResult?.data, placeholderData, options) as TData
data = replaceData(
prevResult?.data,
placeholderData as unknown,
options,
) as TData
isPlaceholderData = true
}
}
Expand Down Expand Up @@ -568,6 +571,9 @@ export class QueryObserver<
return
}

if (this.#currentResultState.data !== undefined) {
this.#lastDefinedQueryData = this.#currentResultState.data
}
this.#currentResult = nextResult

// Determine which callbacks to trigger
Expand Down Expand Up @@ -619,7 +625,6 @@ export class QueryObserver<
| undefined
this.#currentQuery = query
this.#currentQueryInitialState = query.state
this.#previousQueryResult = this.#currentResult

if (this.hasListeners()) {
prevQuery?.removeObserver(this)
Expand Down
55 changes: 55 additions & 0 deletions packages/query-core/src/tests/queryObserver.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,61 @@ describe('queryObserver', () => {
expect(observer.getCurrentResult().isPlaceholderData).toBe(false)
})

test('should pass the correct previous data to placeholderData function params when select function is used in conjunction', async () => {
const results: QueryObserverResult[] = []

const key1 = queryKey()
const key2 = queryKey()

const data1 = { value: 'data1' }
const data2 = { value: 'data2' }

const observer = new QueryObserver(queryClient, {
queryKey: key1,
queryFn: () => data1,
placeholderData: (prev) => prev,
select: (data) => data.value,
})

const unsubscribe = observer.subscribe((result) => {
results.push(result)
})

await sleep(1)

observer.setOptions({
queryKey: key2,
queryFn: () => data2,
placeholderData: (prev) => prev,
select: (data) => data.value,
})

await sleep(1)
unsubscribe()

expect(results.length).toBe(4)
expect(results[0]).toMatchObject({
data: undefined,
status: 'pending',
fetchStatus: 'fetching',
}) // Initial fetch
expect(results[1]).toMatchObject({
data: 'data1',
status: 'success',
fetchStatus: 'idle',
}) // Successful fetch
expect(results[2]).toMatchObject({
data: 'data1',
status: 'success',
fetchStatus: 'fetching',
}) // Fetch for new key, but using previous data as placeholder
expect(results[3]).toMatchObject({
data: 'data2',
status: 'success',
fetchStatus: 'idle',
}) // Successful fetch for new key
})

test('setOptions should notify cache listeners', async () => {
const key = queryKey()

Expand Down
153 changes: 153 additions & 0 deletions packages/react-query/src/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,159 @@ describe('useQuery', () => {
})
})

it('should keep the previous data when placeholderData is set and select fn transform is used', async () => {
const key = queryKey()
const states: UseQueryResult<number>[] = []

function Page() {
const [count, setCount] = React.useState(0)

const state = useQuery({
queryKey: [key, count],
queryFn: async () => {
await sleep(10)
return {
count,
}
},
select(data) {
return data.count
},
placeholderData: keepPreviousData,
})

states.push(state)

return (
<div>
<div>data: {state.data}</div>
<button onClick={() => setCount(1)}>setCount</button>
</div>
)
}

const rendered = renderWithClient(queryClient, <Page />)

await waitFor(() => rendered.getByText('data: 0'))

fireEvent.click(rendered.getByRole('button', { name: 'setCount' }))

await waitFor(() => rendered.getByText('data: 1'))

// Initial
expect(states[0]).toMatchObject({
data: undefined,
isFetching: true,
isSuccess: false,
isPlaceholderData: false,
})
// Fetched
expect(states[1]).toMatchObject({
data: 0,
isFetching: false,
isSuccess: true,
isPlaceholderData: false,
})
// Set state
expect(states[2]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPlaceholderData: true,
})
// New data
expect(states[3]).toMatchObject({
data: 1,
isFetching: false,
isSuccess: true,
isPlaceholderData: false,
})
})

it('should show placeholderData between multiple pending queries when select fn transform is used', async () => {
const key = queryKey()
const states: UseQueryResult<number>[] = []

function Page() {
const [count, setCount] = React.useState(0)

const state = useQuery({
queryKey: [key, count],
queryFn: async () => {
await sleep(10)
return {
count,
}
},
select(data) {
return data.count
},
placeholderData: keepPreviousData,
})

states.push(state)

return (
<div>
<div>data: {state.data}</div>
<button onClick={() => setCount((prev) => prev + 1)}>setCount</button>
</div>
)
}

const rendered = renderWithClient(queryClient, <Page />)

await waitFor(() => rendered.getByText('data: 0'))

fireEvent.click(rendered.getByRole('button', { name: 'setCount' }))
fireEvent.click(rendered.getByRole('button', { name: 'setCount' }))
fireEvent.click(rendered.getByRole('button', { name: 'setCount' }))

await waitFor(() => rendered.getByText('data: 3'))
// Initial
expect(states[0]).toMatchObject({
data: undefined,
isFetching: true,
isSuccess: false,
isPlaceholderData: false,
})
// Fetched
expect(states[1]).toMatchObject({
data: 0,
isFetching: false,
isSuccess: true,
isPlaceholderData: false,
})
// Set state -> count = 1
expect(states[2]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPlaceholderData: true,
})
// Set state -> count = 2
expect(states[3]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPlaceholderData: true,
})
// Set state -> count = 3
expect(states[4]).toMatchObject({
data: 0,
isFetching: true,
isSuccess: true,
isPlaceholderData: true,
})
// New data
expect(states[5]).toMatchObject({
data: 3,
isFetching: false,
isSuccess: true,
isPlaceholderData: false,
})
})

it('should transition to error state when placeholderData is set', async () => {
const key = queryKey()
const states: UseQueryResult<number>[] = []
Expand Down

0 comments on commit 75a2b4d

Please sign in to comment.