From 779e4851b684a0c2308e9dce9819c40686c6617f Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Tue, 4 Apr 2023 22:23:19 -0400 Subject: [PATCH 1/6] fix(query-core): correct placeholderData prevData --- packages/query-core/src/queryObserver.ts | 7 +-- .../src/tests/queryObserver.test.tsx | 55 +++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 6d8685e705..c7b6dd8c7c 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -64,7 +64,6 @@ export class QueryObserver< TQueryData, TQueryKey > - #previousQueryResult?: QueryObserverResult #selectError: TError | null #selectFn?: (data: TQueryData) => TData #selectResult?: TData @@ -414,9 +413,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 @@ -490,7 +486,7 @@ export class QueryObserver< typeof options.placeholderData === 'function' ? ( options.placeholderData as unknown as PlaceholderDataFunction - )(prevQueryResult?.data as TQueryData | undefined) + )(prevResultState?.data) : options.placeholderData if (options.select && typeof placeholderData !== 'undefined') { try { @@ -619,7 +615,6 @@ export class QueryObserver< | undefined this.#currentQuery = query this.#currentQueryInitialState = query.state - this.#previousQueryResult = this.#currentResult if (this.hasListeners()) { prevQuery?.removeObserver(this) diff --git a/packages/query-core/src/tests/queryObserver.test.tsx b/packages/query-core/src/tests/queryObserver.test.tsx index 47d1567402..1311966132 100644 --- a/packages/query-core/src/tests/queryObserver.test.tsx +++ b/packages/query-core/src/tests/queryObserver.test.tsx @@ -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() From 984f1e1ae79d419c2a2264fe8cec25dffe9e8412 Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Sun, 16 Apr 2023 00:07:28 -0400 Subject: [PATCH 2/6] fix(query-core): preserves correct prevQueryResult This commit preserves the correct previous result between rerenders. --- packages/query-core/src/queryObserver.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index c7b6dd8c7c..f2d91e1528 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -64,6 +64,7 @@ export class QueryObserver< TQueryData, TQueryKey > + #previousQueryResult?: QueryObserverResult #selectError: TError | null #selectFn?: (data: TQueryData) => TData #selectResult?: TData @@ -413,6 +414,9 @@ export class QueryObserver< const queryInitialState = queryChange ? query.state : this.#currentQueryInitialState + const prevQueryResult = queryChange + ? prevResultState + : this.#previousQueryResult const { state } = query let { error, errorUpdatedAt, fetchStatus, status } = state @@ -486,7 +490,7 @@ export class QueryObserver< typeof options.placeholderData === 'function' ? ( options.placeholderData as unknown as PlaceholderDataFunction - )(prevResultState?.data) + )(prevQueryResult?.data as TQueryData | undefined) : options.placeholderData if (options.select && typeof placeholderData !== 'undefined') { try { @@ -615,6 +619,8 @@ export class QueryObserver< | undefined this.#currentQuery = query this.#currentQueryInitialState = query.state + this.#previousQueryResult = this + .#currentResultState as unknown as QueryObserverResult if (this.hasListeners()) { prevQuery?.removeObserver(this) From 87ee0ec0e253963927241ac81fa01e96ac5e0d13 Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Sun, 16 Apr 2023 00:42:46 -0400 Subject: [PATCH 3/6] test(react-query): Test with placeholder & select --- .../src/__tests__/useQuery.test.tsx | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 58bb17457e..4a4282bb63 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -1728,6 +1728,75 @@ 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[] = [] + + 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 ( +
+
data: {state.data}
+ +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + 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 transition to error state when placeholderData is set', async () => { const key = queryKey() const states: UseQueryResult[] = [] From 40cf3bcd6c242bc2cc209f5757f84aba1fdbd6c0 Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Tue, 18 Apr 2023 16:44:58 -0400 Subject: [PATCH 4/6] fix(query-core): Add lastDefinedQueryData property --- packages/query-core/src/queryObserver.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index f2d91e1528..1760ed8b57 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -64,10 +64,12 @@ export class QueryObserver< TQueryData, TQueryKey > - #previousQueryResult?: QueryObserverResult #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 #refetchIntervalId?: ReturnType #currentRefetchInterval?: number | false @@ -414,9 +416,6 @@ export class QueryObserver< const queryInitialState = queryChange ? query.state : this.#currentQueryInitialState - const prevQueryResult = queryChange - ? prevResultState - : this.#previousQueryResult const { state } = query let { error, errorUpdatedAt, fetchStatus, status } = state @@ -484,13 +483,14 @@ export class QueryObserver< prevResult?.isPlaceholderData && options.placeholderData === prevResultOptions?.placeholderData ) { + console.log('memoized placeholder data') placeholderData = prevResult.data } else { placeholderData = typeof options.placeholderData === 'function' ? ( options.placeholderData as unknown as PlaceholderDataFunction - )(prevQueryResult?.data as TQueryData | undefined) + )(this.#lastDefinedQueryData) : options.placeholderData if (options.select && typeof placeholderData !== 'undefined') { try { @@ -504,7 +504,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 } } @@ -568,6 +572,9 @@ export class QueryObserver< return } + if (this.#currentResultState.data !== undefined) { + this.#lastDefinedQueryData = this.#currentResultState.data + } this.#currentResult = nextResult // Determine which callbacks to trigger @@ -619,8 +626,6 @@ export class QueryObserver< | undefined this.#currentQuery = query this.#currentQueryInitialState = query.state - this.#previousQueryResult = this - .#currentResultState as unknown as QueryObserverResult if (this.hasListeners()) { prevQuery?.removeObserver(this) From a5ed15f16f188be6b4947b025243273e0a72afb2 Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Tue, 18 Apr 2023 16:47:08 -0400 Subject: [PATCH 5/6] fix(query-core): Remove console.log --- packages/query-core/src/queryObserver.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 1760ed8b57..33f837eef6 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -483,7 +483,6 @@ export class QueryObserver< prevResult?.isPlaceholderData && options.placeholderData === prevResultOptions?.placeholderData ) { - console.log('memoized placeholder data') placeholderData = prevResult.data } else { placeholderData = From 24ed7f9e2576969187cc18bbaff6717f5bb5d812 Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Tue, 18 Apr 2023 17:10:09 -0400 Subject: [PATCH 6/6] fix(query-core): Add react-query test --- .../src/__tests__/useQuery.test.tsx | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 4a4282bb63..eb0294438d 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -1797,6 +1797,90 @@ describe('useQuery', () => { }) }) + it('should show placeholderData between multiple pending queries when select fn transform is used', async () => { + const key = queryKey() + const states: UseQueryResult[] = [] + + 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 ( +
+
data: {state.data}
+ +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + 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[] = []