Skip to content

fix(query-core): correct placeholderData prevData value with select fn #5227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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