From afe8c655f96d6e6f87a13f07b00b04633446521f Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 10 Dec 2021 18:17:06 +0100 Subject: [PATCH 1/2] refactor: make listeners required because the only place where we pass no listener in is in tests --- src/core/subscribable.ts | 8 +++--- src/core/tests/focusManager.test.tsx | 4 +-- src/core/tests/mutationCache.test.tsx | 6 ++--- src/core/tests/onlineManager.test.tsx | 4 +-- src/core/tests/queriesObserver.test.tsx | 2 +- src/core/tests/query.test.tsx | 24 ++++++++--------- src/core/tests/queryClient.test.tsx | 36 ++++++++++++------------- src/core/tests/queryObserver.test.tsx | 14 +++++----- 8 files changed, 48 insertions(+), 50 deletions(-) diff --git a/src/core/subscribable.ts b/src/core/subscribable.ts index 866d8aba7f..2f574ff1b0 100644 --- a/src/core/subscribable.ts +++ b/src/core/subscribable.ts @@ -7,15 +7,13 @@ export class Subscribable { this.listeners = [] } - subscribe(listener?: TListener): () => void { - const callback = listener || (() => undefined) - - this.listeners.push(callback as TListener) + subscribe(listener: TListener): () => void { + this.listeners.push(listener as TListener) this.onSubscribe() return () => { - this.listeners = this.listeners.filter(x => x !== callback) + this.listeners = this.listeners.filter(x => x !== listener) this.onUnsubscribe() } } diff --git a/src/core/tests/focusManager.test.tsx b/src/core/tests/focusManager.test.tsx index ca071ec16d..368ac62d91 100644 --- a/src/core/tests/focusManager.test.tsx +++ b/src/core/tests/focusManager.test.tsx @@ -69,7 +69,7 @@ describe('focusManager', () => { const setEventListenerSpy = jest.spyOn(focusManager, 'setEventListener') - const unsubscribe = focusManager.subscribe() + const unsubscribe = focusManager.subscribe(() => undefined) expect(setEventListenerSpy).toHaveBeenCalledTimes(0) unsubscribe() @@ -88,7 +88,7 @@ describe('focusManager', () => { ) // Should set the default event listener with window event listeners - const unsubscribe = focusManager.subscribe() + const unsubscribe = focusManager.subscribe(() => undefined) expect(addEventListenerSpy).toHaveBeenCalledTimes(2) // Should replace the window default event listener by a new one diff --git a/src/core/tests/mutationCache.test.tsx b/src/core/tests/mutationCache.test.tsx index ff3ca5cea5..5bafcc2e35 100644 --- a/src/core/tests/mutationCache.test.tsx +++ b/src/core/tests/mutationCache.test.tsx @@ -164,7 +164,7 @@ describe('mutationCache', () => { cacheTime: 10, mutationFn: () => Promise.resolve(), }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) expect(queryClient.getMutationCache().getAll()).toHaveLength(0) observer.mutate(1) @@ -231,7 +231,7 @@ describe('mutationCache', () => { }, onSuccess, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) observer.mutate(1) unsubscribe() expect(queryClient.getMutationCache().getAll()).toHaveLength(1) @@ -257,7 +257,7 @@ describe('mutationCache', () => { }, onSuccess, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) observer.mutate(1) unsubscribe() await waitFor(() => { diff --git a/src/core/tests/onlineManager.test.tsx b/src/core/tests/onlineManager.test.tsx index 2e7d35c3f4..d46f3f2389 100644 --- a/src/core/tests/onlineManager.test.tsx +++ b/src/core/tests/onlineManager.test.tsx @@ -64,7 +64,7 @@ describe('onlineManager', () => { const setEventListenerSpy = jest.spyOn(onlineManager, 'setEventListener') - const unsubscribe = onlineManager.subscribe() + const unsubscribe = onlineManager.subscribe(() => undefined) expect(setEventListenerSpy).toHaveBeenCalledTimes(0) unsubscribe() @@ -83,7 +83,7 @@ describe('onlineManager', () => { ) // Should set the default event listener with window event listeners - const unsubscribe = onlineManager.subscribe() + const unsubscribe = onlineManager.subscribe(() => undefined) expect(addEventListenerSpy).toHaveBeenCalledTimes(2) // Should replace the window default event listener by a new one diff --git a/src/core/tests/queriesObserver.test.tsx b/src/core/tests/queriesObserver.test.tsx index c4e5739735..7bf5eef376 100644 --- a/src/core/tests/queriesObserver.test.tsx +++ b/src/core/tests/queriesObserver.test.tsx @@ -248,7 +248,7 @@ describe('queriesObserver', () => { { queryKey: key1, queryFn: queryFn1 }, { queryKey: key2, queryFn: queryFn2 }, ]) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(1) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(1) diff --git a/src/core/tests/query.test.tsx b/src/core/tests/query.test.tsx index 118ac6662b..243225ae18 100644 --- a/src/core/tests/query.test.tsx +++ b/src/core/tests/query.test.tsx @@ -224,7 +224,7 @@ describe('query', () => { queryKey: key, enabled: false, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) unsubscribe() await sleep(100) @@ -253,7 +253,7 @@ describe('query', () => { queryKey: key, enabled: false, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) unsubscribe() await sleep(100) @@ -484,10 +484,10 @@ describe('query', () => { cacheTime: 0, staleTime: Infinity, }) - const unsubscribe1 = observer.subscribe() + const unsubscribe1 = observer.subscribe(() => undefined) unsubscribe1() await waitFor(() => expect(queryCache.find(key)).toBeUndefined()) - const unsubscribe2 = observer.subscribe() + const unsubscribe2 = observer.subscribe(() => undefined) unsubscribe2() await waitFor(() => expect(queryCache.find(key)).toBeUndefined()) @@ -502,7 +502,7 @@ describe('query', () => { cacheTime: 0, }) expect(queryCache.find(key)).toBeDefined() - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) expect(queryCache.find(key)).toBeDefined() unsubscribe() await waitFor(() => expect(queryCache.find(key)).toBeUndefined()) @@ -518,7 +518,7 @@ describe('query', () => { }, cacheTime: 10, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(20) expect(queryCache.find(key)).toBeDefined() observer.refetch() @@ -539,7 +539,7 @@ describe('query', () => { cacheTime: 0, }) expect(queryCache.find(key)).toBeDefined() - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(100) expect(queryCache.find(key)).toBeDefined() unsubscribe() @@ -560,9 +560,9 @@ describe('query', () => { expect(query?.getObserversCount()).toEqual(0) - const unsubscribe1 = observer.subscribe() - const unsubscribe2 = observer2.subscribe() - const unsubscribe3 = observer3.subscribe() + const unsubscribe1 = observer.subscribe(() => undefined) + const unsubscribe2 = observer2.subscribe(() => undefined) + const unsubscribe3 = observer3.subscribe(() => undefined) expect(query?.getObserversCount()).toEqual(3) unsubscribe3() @@ -658,7 +658,7 @@ describe('query', () => { }) const refetchSpy = jest.spyOn(observer, 'refetch') - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryCache.onOnline() // Should refetch the observer @@ -821,7 +821,7 @@ describe('query', () => { retry: false, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(10) expect(consoleMock).toHaveBeenCalledWith('Missing queryFn') diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx index 53de7d72ae..b72f64ded9 100644 --- a/src/core/tests/queryClient.test.tsx +++ b/src/core/tests/queryClient.test.tsx @@ -722,8 +722,8 @@ describe('queryClient', () => { queryKey: key1, enabled: false, }) - observer1.subscribe() - observer2.subscribe() + observer1.subscribe(() => undefined) + observer2.subscribe(() => undefined) await queryClient.refetchQueries() observer1.destroy() observer2.destroy() @@ -743,7 +743,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await queryClient.refetchQueries({ type: 'active', stale: false }) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(2) @@ -761,7 +761,7 @@ describe('queryClient', () => { queryKey: key1, queryFn: queryFn1, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryClient.invalidateQueries(key1) await queryClient.refetchQueries({ stale: true }) unsubscribe() @@ -782,7 +782,7 @@ describe('queryClient', () => { queryKey: key1, queryFn: queryFn1, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await queryClient.refetchQueries( { type: 'active', stale: true }, { cancelRefetch: false } @@ -804,7 +804,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await queryClient.refetchQueries() unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(2) @@ -823,7 +823,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await queryClient.refetchQueries({ type: 'all' }) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(2) @@ -842,7 +842,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await queryClient.refetchQueries({ type: 'active' }) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(2) @@ -861,7 +861,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await queryClient.refetchQueries({ type: 'inactive' }) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(1) @@ -906,7 +906,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryClient.invalidateQueries(key1) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(2) @@ -925,7 +925,7 @@ describe('queryClient', () => { enabled: false, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryClient.invalidateQueries(key1) unsubscribe() expect(queryFn1).toHaveBeenCalledTimes(1) @@ -944,7 +944,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryClient.invalidateQueries(key1, { refetchType: 'none', }) @@ -966,7 +966,7 @@ describe('queryClient', () => { staleTime: Infinity, enabled: false, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryClient.invalidateQueries(key1, { refetchType: 'inactive', }) @@ -987,7 +987,7 @@ describe('queryClient', () => { queryFn: queryFn1, staleTime: Infinity, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) queryClient.invalidateQueries({ refetchType: 'all', }) @@ -1005,7 +1005,7 @@ describe('queryClient', () => { enabled: false, initialData: 1, }) - observer.subscribe() + observer.subscribe(() => undefined) queryClient.fetchQuery(key, ({ signal }) => { const promise = new Promise(resolve => { @@ -1036,7 +1036,7 @@ describe('queryClient', () => { enabled: false, initialData: 1, }) - observer.subscribe() + observer.subscribe(() => undefined) queryClient.fetchQuery(key, ({ signal }) => { const promise = new Promise(resolve => { @@ -1125,8 +1125,8 @@ describe('queryClient', () => { queryFn: queryFn2, enabled: false, }) - observer1.subscribe() - observer2.subscribe() + observer1.subscribe(() => undefined) + observer2.subscribe(() => undefined) await queryClient.resetQueries() observer2.destroy() observer1.destroy() diff --git a/src/core/tests/queryObserver.test.tsx b/src/core/tests/queryObserver.test.tsx index 31fb1ea27b..9daf7d8207 100644 --- a/src/core/tests/queryObserver.test.tsx +++ b/src/core/tests/queryObserver.test.tsx @@ -27,7 +27,7 @@ describe('queryObserver', () => { const key = queryKey() const queryFn = jest.fn() const observer = new QueryObserver(queryClient, { queryKey: key, queryFn }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(1) unsubscribe() expect(queryFn).toHaveBeenCalledTimes(1) @@ -315,7 +315,7 @@ describe('queryObserver', () => { queryFn, enabled: false, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(1) unsubscribe() expect(queryFn).toHaveBeenCalledTimes(0) @@ -441,7 +441,7 @@ describe('queryObserver', () => { retry: 10, retryDelay: 50, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(70) unsubscribe() await sleep(200) @@ -459,7 +459,7 @@ describe('queryObserver', () => { cacheTime: 0, refetchInterval: 1, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) // @ts-expect-error expect(observer.refetchIntervalId).not.toBeUndefined() unsubscribe() @@ -511,7 +511,7 @@ describe('queryObserver', () => { retryDelay: 20, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) // Simulate a race condition when an unsubscribe and a retry occur. await sleep(20) @@ -588,7 +588,7 @@ describe('queryObserver', () => { refetchInterval: 10, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(30) expect(queryFn).toHaveBeenCalledTimes(1) @@ -610,7 +610,7 @@ describe('queryObserver', () => { select: () => data, }) - const unsubscribe = observer.subscribe() + const unsubscribe = observer.subscribe(() => undefined) await sleep(10) expect(observer.getCurrentResult().data).toBe(data) From 7343844bce98ca33f3cd7c1ec060c727e3ca01ab Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 10 Dec 2021 21:35:59 +0100 Subject: [PATCH 2/2] refactor: make notifyOnChangeProps a Set --- src/core/queryObserver.ts | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index 6ed455d60f..c1688865a3 100644 --- a/src/core/queryObserver.ts +++ b/src/core/queryObserver.ts @@ -72,7 +72,7 @@ export class QueryObserver< private staleTimeoutId?: number private refetchIntervalId?: number private currentRefetchInterval?: number | false - private trackedProps!: Array + private trackedProps!: Set constructor( client: QueryClient, @@ -88,7 +88,7 @@ export class QueryObserver< this.client = client this.options = options - this.trackedProps = [] + this.trackedProps = new Set() this.previousSelectError = null this.bindMethods() this.setOptions(options) @@ -235,25 +235,19 @@ export class QueryObserver< ): QueryObserverResult { const trackedResult = {} as QueryObserverResult - const trackProp = (key: keyof QueryObserverResult) => { - if (!this.trackedProps.includes(key)) { - this.trackedProps.push(key) - } - } - Object.keys(result).forEach(key => { Object.defineProperty(trackedResult, key, { configurable: false, enumerable: true, get: () => { - trackProp(key as keyof QueryObserverResult) + this.trackedProps.add(key as keyof QueryObserverResult) return result[key as keyof QueryObserverResult] }, }) }) if (defaultedOptions.useErrorBoundary) { - trackProp('error') + this.trackedProps.add('error') } return trackedResult @@ -605,18 +599,17 @@ export class QueryObserver< if ( notifyOnChangeProps === 'all' || - (!notifyOnChangeProps && !this.trackedProps.length) + (!notifyOnChangeProps && !this.trackedProps.size) ) { return true } - const includedProps = notifyOnChangeProps ?? this.trackedProps + const includedProps = new Set(notifyOnChangeProps ?? this.trackedProps) return Object.keys(result).some(key => { const typedKey = key as keyof QueryObserverResult const changed = result[typedKey] !== prevResult[typedKey] - const isIncluded = includedProps?.some(x => x === key) - return changed && (!includedProps || isIncluded) + return changed && includedProps.has(typedKey) }) }