Skip to content

Commit 38178ba

Browse files
TkDodoTkDodo
authoredApr 12, 2025··
fix(query-core): make sure we don't invoke select too often when using placeholderData (#9007)
* fix(query-core): make sure we don't invoke select too often when using placeholderData * ref: run through placeholderData first, then through this helps us memoizing select calls, and we can get rid of some duplication around how select is called again after placeholderData * chore: skip-nx-cache attempt * test: increase timeout * chore: re-enable caching
·
v5.74.5v5.73.3
1 parent d5ba5d1 commit 38178ba

File tree

4 files changed

+195
-169
lines changed

4 files changed

+195
-169
lines changed
 

Diff for: ‎packages/eslint-plugin-query/src/__tests__/no-void-query-fn.test.ts

+90-88
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import path from 'node:path'
22
import { RuleTester } from '@typescript-eslint/rule-tester'
3-
import { afterAll, describe, it } from 'vitest'
3+
import { afterAll, describe, it, test } from 'vitest'
44
import { rule } from '../rules/no-void-query-fn/no-void-query-fn.rule'
55
import { normalizeIndent } from './test-utils'
66

@@ -18,11 +18,12 @@ const ruleTester = new RuleTester({
1818
},
1919
})
2020

21-
ruleTester.run('no-void-query-fn', rule, {
22-
valid: [
23-
{
24-
name: 'queryFn returns a value',
25-
code: normalizeIndent`
21+
test('should run rule tests', () => {
22+
ruleTester.run('no-void-query-fn', rule, {
23+
valid: [
24+
{
25+
name: 'queryFn returns a value',
26+
code: normalizeIndent`
2627
import { useQuery } from '@tanstack/react-query'
2728
2829
function Component() {
@@ -33,10 +34,10 @@ ruleTester.run('no-void-query-fn', rule, {
3334
return null
3435
}
3536
`,
36-
},
37-
{
38-
name: 'queryFn returns a Promise',
39-
code: normalizeIndent`
37+
},
38+
{
39+
name: 'queryFn returns a Promise',
40+
code: normalizeIndent`
4041
import { useQuery } from '@tanstack/react-query'
4142
4243
function Component() {
@@ -47,10 +48,10 @@ ruleTester.run('no-void-query-fn', rule, {
4748
return null
4849
}
4950
`,
50-
},
51-
{
52-
name: 'queryFn returns Promise.resolve',
53-
code: normalizeIndent`
51+
},
52+
{
53+
name: 'queryFn returns Promise.resolve',
54+
code: normalizeIndent`
5455
import { useQuery } from '@tanstack/react-query'
5556
5657
function Component() {
@@ -61,10 +62,10 @@ ruleTester.run('no-void-query-fn', rule, {
6162
return null
6263
}
6364
`,
64-
},
65-
{
66-
name: 'queryFn with explicit Promise type',
67-
code: normalizeIndent`
65+
},
66+
{
67+
name: 'queryFn with explicit Promise type',
68+
code: normalizeIndent`
6869
import { useQuery } from '@tanstack/react-query'
6970
7071
interface Data {
@@ -81,10 +82,10 @@ ruleTester.run('no-void-query-fn', rule, {
8182
return null
8283
}
8384
`,
84-
},
85-
{
86-
name: 'queryFn with generic Promise type',
87-
code: normalizeIndent`
85+
},
86+
{
87+
name: 'queryFn with generic Promise type',
88+
code: normalizeIndent`
8889
import { useQuery } from '@tanstack/react-query'
8990
9091
interface Response<T> {
@@ -101,10 +102,10 @@ ruleTester.run('no-void-query-fn', rule, {
101102
return null
102103
}
103104
`,
104-
},
105-
{
106-
name: 'queryFn with external async function',
107-
code: normalizeIndent`
105+
},
106+
{
107+
name: 'queryFn with external async function',
108+
code: normalizeIndent`
108109
import { useQuery } from '@tanstack/react-query'
109110
110111
async function fetchData(): Promise<{ data: string }> {
@@ -119,10 +120,10 @@ ruleTester.run('no-void-query-fn', rule, {
119120
return null
120121
}
121122
`,
122-
},
123-
{
124-
name: 'queryFn returns null',
125-
code: normalizeIndent`
123+
},
124+
{
125+
name: 'queryFn returns null',
126+
code: normalizeIndent`
126127
import { useQuery } from '@tanstack/react-query'
127128
128129
function Component() {
@@ -133,10 +134,10 @@ ruleTester.run('no-void-query-fn', rule, {
133134
return null
134135
}
135136
`,
136-
},
137-
{
138-
name: 'queryFn returns 0',
139-
code: normalizeIndent`
137+
},
138+
{
139+
name: 'queryFn returns 0',
140+
code: normalizeIndent`
140141
import { useQuery } from '@tanstack/react-query'
141142
142143
function Component() {
@@ -147,10 +148,10 @@ ruleTester.run('no-void-query-fn', rule, {
147148
return null
148149
}
149150
`,
150-
},
151-
{
152-
name: 'queryFn returns false',
153-
code: normalizeIndent`
151+
},
152+
{
153+
name: 'queryFn returns false',
154+
code: normalizeIndent`
154155
import { useQuery } from '@tanstack/react-query'
155156
156157
function Component() {
@@ -161,12 +162,12 @@ ruleTester.run('no-void-query-fn', rule, {
161162
return null
162163
}
163164
`,
164-
},
165-
],
166-
invalid: [
167-
{
168-
name: 'queryFn returns void',
169-
code: normalizeIndent`
165+
},
166+
],
167+
invalid: [
168+
{
169+
name: 'queryFn returns void',
170+
code: normalizeIndent`
170171
import { useQuery } from '@tanstack/react-query'
171172
172173
function Component() {
@@ -179,11 +180,11 @@ ruleTester.run('no-void-query-fn', rule, {
179180
return null
180181
}
181182
`,
182-
errors: [{ messageId: 'noVoidReturn' }],
183-
},
184-
{
185-
name: 'queryFn returns undefined',
186-
code: normalizeIndent`
183+
errors: [{ messageId: 'noVoidReturn' }],
184+
},
185+
{
186+
name: 'queryFn returns undefined',
187+
code: normalizeIndent`
187188
import { useQuery } from '@tanstack/react-query'
188189
189190
function Component() {
@@ -194,11 +195,11 @@ ruleTester.run('no-void-query-fn', rule, {
194195
return null
195196
}
196197
`,
197-
errors: [{ messageId: 'noVoidReturn' }],
198-
},
199-
{
200-
name: 'async queryFn returns void',
201-
code: normalizeIndent`
198+
errors: [{ messageId: 'noVoidReturn' }],
199+
},
200+
{
201+
name: 'async queryFn returns void',
202+
code: normalizeIndent`
202203
import { useQuery } from '@tanstack/react-query'
203204
204205
function Component() {
@@ -211,11 +212,11 @@ ruleTester.run('no-void-query-fn', rule, {
211212
return null
212213
}
213214
`,
214-
errors: [{ messageId: 'noVoidReturn' }],
215-
},
216-
{
217-
name: 'queryFn with explicit void Promise',
218-
code: normalizeIndent`
215+
errors: [{ messageId: 'noVoidReturn' }],
216+
},
217+
{
218+
name: 'queryFn with explicit void Promise',
219+
code: normalizeIndent`
219220
import { useQuery } from '@tanstack/react-query'
220221
221222
function Component() {
@@ -228,11 +229,11 @@ ruleTester.run('no-void-query-fn', rule, {
228229
return null
229230
}
230231
`,
231-
errors: [{ messageId: 'noVoidReturn' }],
232-
},
233-
{
234-
name: 'queryFn with Promise.resolve(undefined)',
235-
code: normalizeIndent`
232+
errors: [{ messageId: 'noVoidReturn' }],
233+
},
234+
{
235+
name: 'queryFn with Promise.resolve(undefined)',
236+
code: normalizeIndent`
236237
import { useQuery } from '@tanstack/react-query'
237238
238239
function Component() {
@@ -243,11 +244,11 @@ ruleTester.run('no-void-query-fn', rule, {
243244
return null
244245
}
245246
`,
246-
errors: [{ messageId: 'noVoidReturn' }],
247-
},
248-
{
249-
name: 'queryFn with external void async function',
250-
code: normalizeIndent`
247+
errors: [{ messageId: 'noVoidReturn' }],
248+
},
249+
{
250+
name: 'queryFn with external void async function',
251+
code: normalizeIndent`
251252
import { useQuery } from '@tanstack/react-query'
252253
253254
async function voidOperation(): Promise<void> {
@@ -262,11 +263,11 @@ ruleTester.run('no-void-query-fn', rule, {
262263
return null
263264
}
264265
`,
265-
errors: [{ messageId: 'noVoidReturn' }],
266-
},
267-
{
268-
name: 'queryFn with conditional return (one branch missing)',
269-
code: normalizeIndent`
266+
errors: [{ messageId: 'noVoidReturn' }],
267+
},
268+
{
269+
name: 'queryFn with conditional return (one branch missing)',
270+
code: normalizeIndent`
270271
import { useQuery } from '@tanstack/react-query'
271272
272273
function Component() {
@@ -282,11 +283,11 @@ ruleTester.run('no-void-query-fn', rule, {
282283
return null
283284
}
284285
`,
285-
errors: [{ messageId: 'noVoidReturn' }],
286-
},
287-
{
288-
name: 'queryFn with ternary operator returning undefined',
289-
code: normalizeIndent`
286+
errors: [{ messageId: 'noVoidReturn' }],
287+
},
288+
{
289+
name: 'queryFn with ternary operator returning undefined',
290+
code: normalizeIndent`
290291
import { useQuery } from '@tanstack/react-query'
291292
292293
function Component() {
@@ -297,11 +298,11 @@ ruleTester.run('no-void-query-fn', rule, {
297298
return null
298299
}
299300
`,
300-
errors: [{ messageId: 'noVoidReturn' }],
301-
},
302-
{
303-
name: 'async queryFn with try/catch missing return in catch',
304-
code: normalizeIndent`
301+
errors: [{ messageId: 'noVoidReturn' }],
302+
},
303+
{
304+
name: 'async queryFn with try/catch missing return in catch',
305+
code: normalizeIndent`
305306
import { useQuery } from '@tanstack/react-query'
306307
307308
function Component() {
@@ -319,7 +320,8 @@ ruleTester.run('no-void-query-fn', rule, {
319320
return null
320321
}
321322
`,
322-
errors: [{ messageId: 'noVoidReturn' }],
323-
},
324-
],
325-
})
323+
errors: [{ messageId: 'noVoidReturn' }],
324+
},
325+
],
326+
})
327+
}, 10_000)

Diff for: ‎packages/query-core/src/__tests__/queryObserver.test.tsx

+74-38
Original file line numberDiff line numberDiff line change
@@ -925,42 +925,6 @@ describe('queryObserver', () => {
925925
expect(observer.getCurrentResult().data).toBe(selectedData2)
926926
})
927927

928-
test('should not use an undefined value returned by select as placeholderData', () => {
929-
const key = queryKey()
930-
931-
const data = { value: 'data' }
932-
const selectedData = { value: 'data' }
933-
const placeholderData1 = { value: 'data' }
934-
const placeholderData2 = { value: 'data' }
935-
936-
const observer = new QueryObserver(queryClient, {
937-
queryKey: key,
938-
queryFn: () => data,
939-
select: () => data,
940-
})
941-
942-
observer.setOptions({
943-
queryKey: key,
944-
queryFn: () => data,
945-
select: () => {
946-
return selectedData
947-
},
948-
placeholderData: placeholderData1,
949-
})
950-
951-
expect(observer.getCurrentResult().isPlaceholderData).toBe(true)
952-
953-
observer.setOptions({
954-
queryKey: key,
955-
queryFn: () => data,
956-
// @ts-expect-error
957-
select: () => undefined,
958-
placeholderData: placeholderData2,
959-
})
960-
961-
expect(observer.getCurrentResult().isPlaceholderData).toBe(false)
962-
})
963-
964928
test('should pass the correct previous queryKey (from prevQuery) to placeholderData function params with select', async () => {
965929
const results: Array<QueryObserverResult> = []
966930
const keys: Array<ReadonlyArray<unknown> | null> = []
@@ -1036,11 +1000,16 @@ describe('queryObserver', () => {
10361000
const data1 = { value: 'data1' }
10371001
const data2 = { value: 'data2' }
10381002

1003+
let selectCount = 0
1004+
10391005
const observer = new QueryObserver(queryClient, {
10401006
queryKey: key1,
10411007
queryFn: () => data1,
10421008
placeholderData: (prev) => prev,
1043-
select: (data) => data.value,
1009+
select: (data) => {
1010+
selectCount++
1011+
return data.value
1012+
},
10441013
})
10451014

10461015
const unsubscribe = observer.subscribe((result) => {
@@ -1053,7 +1022,70 @@ describe('queryObserver', () => {
10531022
queryKey: key2,
10541023
queryFn: () => data2,
10551024
placeholderData: (prev) => prev,
1056-
select: (data) => data.value,
1025+
select: (data) => {
1026+
selectCount++
1027+
return data.value
1028+
},
1029+
})
1030+
1031+
await sleep(1)
1032+
unsubscribe()
1033+
1034+
expect(results.length).toBe(4)
1035+
expect(results[0]).toMatchObject({
1036+
data: undefined,
1037+
status: 'pending',
1038+
fetchStatus: 'fetching',
1039+
}) // Initial fetch
1040+
expect(results[1]).toMatchObject({
1041+
data: 'data1',
1042+
status: 'success',
1043+
fetchStatus: 'idle',
1044+
}) // Successful fetch
1045+
expect(results[2]).toMatchObject({
1046+
data: 'data1',
1047+
status: 'success',
1048+
fetchStatus: 'fetching',
1049+
}) // Fetch for new key, but using previous data as placeholder
1050+
expect(results[3]).toMatchObject({
1051+
data: 'data2',
1052+
status: 'success',
1053+
fetchStatus: 'idle',
1054+
}) // Successful fetch for new key
1055+
1056+
// it's 3 because select is an inline function
1057+
expect(selectCount).toBe(3)
1058+
})
1059+
1060+
test('should use cached selectResult when switching between queries and placeholderData returns previousData', async () => {
1061+
const results: Array<QueryObserverResult> = []
1062+
1063+
const key1 = queryKey()
1064+
const key2 = queryKey()
1065+
1066+
const data1 = { value: 'data1' }
1067+
const data2 = { value: 'data2' }
1068+
1069+
const stableSelect = vi.fn((data: { value: string }) => data.value)
1070+
1071+
const observer = new QueryObserver(queryClient, {
1072+
queryKey: key1,
1073+
queryFn: () => data1,
1074+
placeholderData: (prev) => prev,
1075+
select: stableSelect,
1076+
})
1077+
1078+
const unsubscribe = observer.subscribe((result) => {
1079+
results.push(result)
1080+
})
1081+
1082+
await sleep(1)
1083+
1084+
observer.setOptions({
1085+
queryKey: key2,
1086+
queryFn: () => data2,
1087+
placeholderData: (prev) => prev,
1088+
select: stableSelect,
10571089
})
10581090

10591091
await sleep(1)
@@ -1080,6 +1112,10 @@ describe('queryObserver', () => {
10801112
status: 'success',
10811113
fetchStatus: 'idle',
10821114
}) // Successful fetch for new key
1115+
1116+
expect(stableSelect).toHaveBeenCalledTimes(2)
1117+
expect(stableSelect.mock.calls[0]![0]).toEqual(data1)
1118+
expect(stableSelect.mock.calls[1]![0]).toEqual(data2)
10831119
})
10841120

10851121
test('setOptions should notify cache listeners', () => {

Diff for: ‎packages/query-core/src/queryObserver.ts

+31-34
Original file line numberDiff line numberDiff line change
@@ -474,33 +474,11 @@ export class QueryObserver<
474474

475475
let { error, errorUpdatedAt, status } = newState
476476

477-
// Select data if needed
478-
if (options.select && newState.data !== undefined) {
479-
// Memoize select result
480-
if (
481-
prevResult &&
482-
newState.data === prevResultState?.data &&
483-
options.select === this.#selectFn
484-
) {
485-
data = this.#selectResult
486-
} else {
487-
try {
488-
this.#selectFn = options.select
489-
data = options.select(newState.data)
490-
data = replaceData(prevResult?.data, data, options)
491-
this.#selectResult = data
492-
this.#selectError = null
493-
} catch (selectError) {
494-
this.#selectError = selectError as TError
495-
}
496-
}
497-
}
498-
// Use query data
499-
else {
500-
data = newState.data as unknown as TData
501-
}
477+
// Per default, use query data
478+
data = newState.data as unknown as TData
479+
let skipSelect = false
502480

503-
// Show placeholder data if needed
481+
// use placeholderData if needed
504482
if (
505483
options.placeholderData !== undefined &&
506484
data === undefined &&
@@ -514,7 +492,11 @@ export class QueryObserver<
514492
options.placeholderData === prevResultOptions?.placeholderData
515493
) {
516494
placeholderData = prevResult.data
495+
// we have to skip select when reading this memoization
496+
// because prevResult.data is already "selected"
497+
skipSelect = true
517498
} else {
499+
// compute placeholderData
518500
placeholderData =
519501
typeof options.placeholderData === 'function'
520502
? (
@@ -524,14 +506,6 @@ export class QueryObserver<
524506
this.#lastQueryWithDefinedData as any,
525507
)
526508
: options.placeholderData
527-
if (options.select && placeholderData !== undefined) {
528-
try {
529-
placeholderData = options.select(placeholderData)
530-
this.#selectError = null
531-
} catch (selectError) {
532-
this.#selectError = selectError as TError
533-
}
534-
}
535509
}
536510

537511
if (placeholderData !== undefined) {
@@ -545,6 +519,29 @@ export class QueryObserver<
545519
}
546520
}
547521

522+
// Select data if needed
523+
// this also runs placeholderData through the select function
524+
if (options.select && data !== undefined && !skipSelect) {
525+
// Memoize select result
526+
if (
527+
prevResult &&
528+
data === prevResultState?.data &&
529+
options.select === this.#selectFn
530+
) {
531+
data = this.#selectResult
532+
} else {
533+
try {
534+
this.#selectFn = options.select
535+
data = options.select(data as any)
536+
data = replaceData(prevResult?.data, data, options)
537+
this.#selectResult = data
538+
this.#selectError = null
539+
} catch (selectError) {
540+
this.#selectError = selectError as TError
541+
}
542+
}
543+
}
544+
548545
if (this.#selectError) {
549546
error = this.#selectError as any
550547
data = this.#selectResult

Diff for: ‎packages/vue-query/package.json

-9
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,6 @@
5959
"src",
6060
"!src/__tests__"
6161
],
62-
"nx": {
63-
"targets": {
64-
"test:lib": {
65-
"dependsOn": [
66-
"test:types"
67-
]
68-
}
69-
}
70-
},
7162
"dependencies": {
7263
"@tanstack/match-sorter-utils": "^8.19.4",
7364
"@tanstack/query-core": "workspace:*",

0 commit comments

Comments
 (0)
Please sign in to comment.