Skip to content

Commit d5b78fe

Browse files
jvuotijvuoti
committed
Refactor to findMatchingObservers
This removes extra calls to setOptions from getOptimisticResult, and gets rid of extra renders in tests. setOptions is now explicitly only called in updateObservers.
1 parent b349049 commit d5b78fe

File tree

2 files changed

+80
-114
lines changed

2 files changed

+80
-114
lines changed

‎src/core/queriesObserver.ts‎

Lines changed: 65 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -64,69 +64,64 @@ export class QueriesObserver extends Subscribable<QueriesObserverListener> {
6464
}
6565

6666
getOptimisticResult(queries: QueryObserverOptions[]): QueryObserverResult[] {
67-
return this.getUpdatedObservers(queries).newResult
67+
return this.findMatchingObservers(queries).map(match =>
68+
match.observer.getCurrentResult()
69+
)
6870
}
6971

70-
private getUpdatedObservers(
71-
queries: QueryObserverOptions[],
72-
notifyOptions?: NotifyOptions
73-
): QueryObserverUpdate {
72+
private findMatchingObservers(
73+
queries: QueryObserverOptions[]
74+
): QueryObserverMatch[] {
7475
const prevObservers = this.observers
75-
const prevObserversMap = this.observersMap
76-
const newResult: QueryObserverResult[] = []
77-
const newObservers: QueryObserver[] = []
78-
const newObserversMap: Record<string, QueryObserver> = {}
79-
8076
const defaultedQueryOptions = queries.map(options =>
8177
this.client.defaultQueryObserverOptions(options)
8278
)
83-
const matchingObservers = defaultedQueryOptions
84-
.map(options => {
85-
const match = prevObserversMap[options.queryHash!]
79+
80+
const matchingObservers: QueryObserverMatch[] = defaultedQueryOptions
81+
.map(defaultedOptions => {
82+
const match = prevObservers.find(
83+
observer => observer.options.queryHash === defaultedOptions.queryHash!
84+
)
8685
if (match != null) {
87-
match.setOptions(options, notifyOptions)
88-
return match
86+
return { defaultedQueryOptions: defaultedOptions, observer: match }
8987
}
9088
return null
9189
})
9290
.filter(notNullOrUndefined)
9391

9492
const matchedQueryHashes = matchingObservers.map(
95-
observer => observer?.options.queryHash
93+
match => match.defaultedQueryOptions.queryHash
9694
)
9795
const unmatchedQueries = defaultedQueryOptions.filter(
98-
options => !matchedQueryHashes.includes(options.queryHash)
96+
defaultedOptions =>
97+
!matchedQueryHashes.includes(defaultedOptions.queryHash)
9998
)
10099

101100
const unmatchedObservers = prevObservers.filter(
102-
prevObserver => !matchingObservers.includes(prevObserver)
101+
prevObserver =>
102+
!matchingObservers.some(match => match.observer === prevObserver)
103103
)
104104

105-
const newlyMatchedOrCreatedObservers = unmatchedQueries.map(options => {
106-
if (options.keepPreviousData && unmatchedObservers.length > 0) {
107-
// use the first observer but no longer matched query to keep query data for any new queries
108-
const firstObserver = unmatchedObservers.splice(0, 1)[0]
109-
if (firstObserver !== undefined) {
110-
firstObserver.setOptions(options, notifyOptions)
111-
return firstObserver
105+
const newOrReusedObservers: QueryObserverMatch[] = unmatchedQueries.map(
106+
(options, index) => {
107+
if (options.keepPreviousData && unmatchedObservers.length > 0) {
108+
// return previous data from one of the observers that no longer match
109+
const previouslyUsedObserver = unmatchedObservers[index]
110+
if (previouslyUsedObserver !== undefined) {
111+
return {
112+
defaultedQueryOptions: options,
113+
observer: previouslyUsedObserver,
114+
}
115+
}
116+
}
117+
return {
118+
defaultedQueryOptions: options,
119+
observer: this.getObserver(options),
112120
}
113121
}
114-
return this.getObserver(options)
115-
})
116-
117-
matchingObservers
118-
.concat(newlyMatchedOrCreatedObservers)
119-
.forEach(observer => {
120-
newObservers.push(observer)
121-
newResult.push(observer.getCurrentResult())
122-
newObserversMap[observer.options.queryHash!] = observer
123-
})
122+
)
124123

125-
return {
126-
newResult: newResult,
127-
newObservers: newObservers,
128-
newObserversMap: newObserversMap,
129-
}
124+
return matchingObservers.concat(newOrReusedObservers)
130125
}
131126

132127
private getObserver(options: QueryObserverOptions): QueryObserver {
@@ -138,42 +133,46 @@ export class QueriesObserver extends Subscribable<QueriesObserverListener> {
138133
private updateObservers(notifyOptions?: NotifyOptions): void {
139134
notifyManager.batch(() => {
140135
const prevObservers = this.observers
141-
const updatedObservers = this.getUpdatedObservers(
142-
this.queries,
143-
notifyOptions
136+
137+
const newObserverMatches = this.findMatchingObservers(this.queries)
138+
139+
// set options for the new observers to notify of changes
140+
newObserverMatches.forEach(match =>
141+
match.observer.setOptions(match.defaultedQueryOptions, notifyOptions)
144142
)
145143

146-
const hasIndexChange = updatedObservers.newObservers.some(
144+
const newObservers = newObserverMatches.map(match => match.observer)
145+
const newObserversMap = Object.fromEntries(
146+
newObservers.map(observer => [observer.options.queryHash, observer])
147+
)
148+
const newResult = newObservers.map(observer =>
149+
observer.getCurrentResult()
150+
)
151+
152+
const hasIndexChange = newObservers.some(
147153
(observer, index) => observer !== prevObservers[index]
148154
)
149-
if (
150-
prevObservers.length === updatedObservers.newObservers.length &&
151-
!hasIndexChange
152-
) {
155+
if (prevObservers.length === newObservers.length && !hasIndexChange) {
153156
return
154157
}
155158

156-
this.observers = updatedObservers.newObservers
157-
this.observersMap = updatedObservers.newObserversMap
158-
this.result = updatedObservers.newResult
159+
this.observers = newObservers
160+
this.observersMap = newObserversMap
161+
this.result = newResult
159162

160163
if (!this.hasListeners()) {
161164
return
162165
}
163166

164-
difference(prevObservers, updatedObservers.newObservers).forEach(
165-
observer => {
166-
observer.destroy()
167-
}
168-
)
167+
difference(prevObservers, newObservers).forEach(observer => {
168+
observer.destroy()
169+
})
169170

170-
difference(updatedObservers.newObservers, prevObservers).forEach(
171-
observer => {
172-
observer.subscribe(result => {
173-
this.onUpdate(observer, result)
174-
})
175-
}
176-
)
171+
difference(newObservers, prevObservers).forEach(observer => {
172+
observer.subscribe(result => {
173+
this.onUpdate(observer, result)
174+
})
175+
})
177176

178177
this.notify()
179178
})
@@ -196,8 +195,7 @@ export class QueriesObserver extends Subscribable<QueriesObserverListener> {
196195
}
197196
}
198197

199-
type QueryObserverUpdate = {
200-
newResult: QueryObserverResult[]
201-
newObservers: QueryObserver[]
202-
newObserversMap: Record<string, QueryObserver>
198+
type QueryObserverMatch = {
199+
defaultedQueryOptions: QueryObserverOptions
200+
observer: QueryObserver
203201
}

‎src/react/tests/useQueries.test.tsx‎

Lines changed: 15 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe('useQueries', () => {
9898

9999
renderWithClient(queryClient, <Page />)
100100

101-
await waitFor(() => expect(states.length).toBe(8))
101+
await waitFor(() => expect(states.length).toBe(7))
102102

103103
expect(states[0]).toMatchObject([
104104
{
@@ -128,22 +128,18 @@ describe('useQueries', () => {
128128
{ status: 'success', data: 5, isPreviousData: false, isFetching: false },
129129
])
130130
expect(states[3]).toMatchObject([
131-
{ status: 'success', data: 2, isPreviousData: true, isFetching: true },
132-
{ status: 'success', data: 5, isPreviousData: true, isFetching: true },
131+
{ status: 'success', data: 2, isPreviousData: false, isFetching: false },
132+
{ status: 'success', data: 5, isPreviousData: false, isFetching: false },
133133
])
134134
expect(states[4]).toMatchObject([
135135
{ status: 'success', data: 2, isPreviousData: true, isFetching: true },
136136
{ status: 'success', data: 5, isPreviousData: true, isFetching: true },
137137
])
138138
expect(states[5]).toMatchObject([
139-
{ status: 'success', data: 2, isPreviousData: true, isFetching: true },
140-
{ status: 'success', data: 5, isPreviousData: true, isFetching: true },
141-
])
142-
expect(states[6]).toMatchObject([
143139
{ status: 'success', data: 4, isPreviousData: false, isFetching: false },
144140
{ status: 'success', data: 5, isPreviousData: true, isFetching: true },
145141
])
146-
expect(states[7]).toMatchObject([
142+
expect(states[6]).toMatchObject([
147143
{ status: 'success', data: 4, isPreviousData: false, isFetching: false },
148144
{ status: 'success', data: 10, isPreviousData: false, isFetching: false },
149145
])
@@ -179,7 +175,7 @@ describe('useQueries', () => {
179175

180176
renderWithClient(queryClient, <Page />)
181177

182-
await waitFor(() => expect(states.length).toBe(10))
178+
await waitFor(() => expect(states.length).toBe(8))
183179

184180
expect(states[0]).toMatchObject([
185181
{
@@ -210,8 +206,8 @@ describe('useQueries', () => {
210206
])
211207

212208
expect(states[3]).toMatchObject([
213-
{ status: 'success', data: 4, isPreviousData: true, isFetching: true },
214-
{ status: 'success', data: 8, isPreviousData: true, isFetching: true },
209+
{ status: 'success', data: 4, isPreviousData: false, isFetching: false },
210+
{ status: 'success', data: 8, isPreviousData: false, isFetching: false },
215211
{
216212
status: 'loading',
217213
data: undefined,
@@ -230,26 +226,6 @@ describe('useQueries', () => {
230226
},
231227
])
232228
expect(states[5]).toMatchObject([
233-
{ status: 'success', data: 4, isPreviousData: true, isFetching: true },
234-
{ status: 'success', data: 8, isPreviousData: true, isFetching: true },
235-
{
236-
status: 'loading',
237-
data: undefined,
238-
isPreviousData: false,
239-
isFetching: true,
240-
},
241-
])
242-
expect(states[6]).toMatchObject([
243-
{ status: 'success', data: 4, isPreviousData: true, isFetching: true },
244-
{ status: 'success', data: 8, isPreviousData: true, isFetching: true },
245-
{
246-
status: 'loading',
247-
data: undefined,
248-
isPreviousData: false,
249-
isFetching: true,
250-
},
251-
])
252-
expect(states[7]).toMatchObject([
253229
{ status: 'success', data: 6, isPreviousData: false, isFetching: false },
254230
{ status: 'success', data: 8, isPreviousData: true, isFetching: true },
255231
{
@@ -259,7 +235,7 @@ describe('useQueries', () => {
259235
isFetching: true,
260236
},
261237
])
262-
expect(states[8]).toMatchObject([
238+
expect(states[6]).toMatchObject([
263239
{ status: 'success', data: 6, isPreviousData: false, isFetching: false },
264240
{ status: 'success', data: 12, isPreviousData: false, isFetching: false },
265241
{
@@ -269,7 +245,7 @@ describe('useQueries', () => {
269245
isFetching: true,
270246
},
271247
])
272-
expect(states[9]).toMatchObject([
248+
expect(states[7]).toMatchObject([
273249
{ status: 'success', data: 6, isPreviousData: false, isFetching: false },
274250
{ status: 'success', data: 12, isPreviousData: false, isFetching: false },
275251
{ status: 'success', data: 18, isPreviousData: false, isFetching: false },
@@ -317,7 +293,7 @@ describe('useQueries', () => {
317293

318294
renderWithClient(queryClient, <Page />)
319295

320-
await waitFor(() => expect(states.length).toBe(12))
296+
await waitFor(() => expect(states.length).toBe(10))
321297

322298
expect(states[0]).toMatchObject([
323299
{
@@ -348,7 +324,7 @@ describe('useQueries', () => {
348324
])
349325
expect(states[3]).toMatchObject([
350326
{ status: 'success', data: 5, isPreviousData: false, isFetching: false },
351-
{ status: 'success', data: 10, isPreviousData: true, isFetching: true },
327+
{ status: 'success', data: 10, isPreviousData: false, isFetching: false },
352328
])
353329
expect(states[4]).toMatchObject([
354330
{ status: 'success', data: 5, isPreviousData: false, isFetching: false },
@@ -359,26 +335,18 @@ describe('useQueries', () => {
359335
{ status: 'success', data: 15, isPreviousData: false, isFetching: false },
360336
])
361337
expect(states[6]).toMatchObject([
362-
{ status: 'success', data: 10, isPreviousData: false, isFetching: true },
363-
{ status: 'success', data: 15, isPreviousData: false, isFetching: true },
338+
{ status: 'success', data: 15, isPreviousData: false, isFetching: false },
339+
{ status: 'success', data: 5, isPreviousData: false, isFetching: false },
364340
])
365341
expect(states[7]).toMatchObject([
366342
{ status: 'success', data: 10, isPreviousData: false, isFetching: true },
367-
{ status: 'success', data: 15, isPreviousData: false, isFetching: true },
343+
{ status: 'success', data: 15, isPreviousData: false, isFetching: false },
368344
])
369345
expect(states[8]).toMatchObject([
370346
{ status: 'success', data: 10, isPreviousData: false, isFetching: true },
371-
{ status: 'success', data: 15, isPreviousData: false, isFetching: true },
347+
{ status: 'success', data: 15, isPreviousData: false, isFetching: false },
372348
])
373349
expect(states[9]).toMatchObject([
374-
{ status: 'success', data: 10, isPreviousData: false, isFetching: true },
375-
{ status: 'success', data: 15, isPreviousData: false, isFetching: true },
376-
])
377-
expect(states[10]).toMatchObject([
378-
{ status: 'success', data: 10, isPreviousData: false, isFetching: false },
379-
{ status: 'success', data: 15, isPreviousData: false, isFetching: true },
380-
])
381-
expect(states[11]).toMatchObject([
382350
{ status: 'success', data: 10, isPreviousData: false, isFetching: false },
383351
{ status: 'success', data: 15, isPreviousData: false, isFetching: false },
384352
])

0 commit comments

Comments
 (0)