From 4557ecd8f3b2db90a73c5c4e89c0002c7fe228ef Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sun, 14 Nov 2021 13:53:37 +0100 Subject: [PATCH 01/10] feat: mutation cachetime stramline queryCache / mutationCache events by combining them into notifiable.ts --- src/core/mutation.ts | 18 ++++++++++++- src/core/mutationCache.ts | 57 ++++++++++++++++++++++++++++----------- src/core/notifiable.ts | 12 +++++++++ src/core/query.ts | 4 +-- src/core/queryCache.ts | 32 ++++++++-------------- src/core/queryObserver.ts | 7 ++--- 6 files changed, 87 insertions(+), 43 deletions(-) create mode 100644 src/core/notifiable.ts diff --git a/src/core/mutation.ts b/src/core/mutation.ts index 54ba5473a9..feda111976 100644 --- a/src/core/mutation.ts +++ b/src/core/mutation.ts @@ -107,11 +107,23 @@ export class Mutation< addObserver(observer: MutationObserver): void { if (this.observers.indexOf(observer) === -1) { this.observers.push(observer) + + this.mutationCache.notify({ + type: 'mutationObserverAdded', + mutation: this, + observer, + }) } } removeObserver(observer: MutationObserver): void { this.observers = this.observers.filter(x => x !== observer) + + this.mutationCache.notify({ + type: 'mutationObserverRemoved', + mutation: this, + observer, + }) } cancel(): Promise { @@ -249,7 +261,11 @@ export class Mutation< this.observers.forEach(observer => { observer.onMutationUpdate(action) }) - this.mutationCache.notify(this) + this.mutationCache.notify({ + mutation: this, + type: 'mutationUpdated', + action, + }) }) } } diff --git a/src/core/mutationCache.ts b/src/core/mutationCache.ts index 02b97b19b4..78ca6835f1 100644 --- a/src/core/mutationCache.ts +++ b/src/core/mutationCache.ts @@ -1,9 +1,10 @@ +import { MutationObserver } from './mutationObserver' import type { MutationOptions } from './types' import type { QueryClient } from './queryClient' import { notifyManager } from './notifyManager' -import { Mutation, MutationState } from './mutation' +import { Action, Mutation, MutationState } from './mutation' import { matchMutation, MutationFilters, noop } from './utils' -import { Subscribable } from './subscribable' +import { Notifiable } from './notifiable' // TYPES @@ -12,21 +13,53 @@ interface MutationCacheConfig { error: unknown, variables: unknown, context: unknown, - mutation: Mutation + mutation: Mutation ) => void onSuccess?: ( data: unknown, variables: unknown, context: unknown, - mutation: Mutation + mutation: Mutation ) => void } -type MutationCacheListener = (mutation?: Mutation) => void +interface NotifyEventMutationAdded { + type: 'mutationAdded' + mutation: Mutation +} +interface NotifyEventMutationRemoved { + type: 'mutationRemoved' + mutation: Mutation +} + +interface NotifyEventMutationObserverAdded { + type: 'mutationObserverAdded' + mutation: Mutation + observer: MutationObserver +} + +interface NotifyEventMutationObserverRemoved { + type: 'mutationObserverRemoved' + mutation: Mutation + observer: MutationObserver +} + +interface NotifyEventMutationUpdated { + type: 'mutationUpdated' + mutation: Mutation + action: Action +} + +type MutationCacheNotifyEvent = + | NotifyEventMutationAdded + | NotifyEventMutationRemoved + | NotifyEventMutationObserverAdded + | NotifyEventMutationObserverRemoved + | NotifyEventMutationUpdated // CLASS -export class MutationCache extends Subscribable { +export class MutationCache extends Notifiable { config: MutationCacheConfig private mutations: Mutation[] @@ -61,13 +94,13 @@ export class MutationCache extends Subscribable { add(mutation: Mutation): void { this.mutations.push(mutation) - this.notify(mutation) + this.notify({ type: 'mutationAdded', mutation }) } remove(mutation: Mutation): void { this.mutations = this.mutations.filter(x => x !== mutation) mutation.cancel() - this.notify(mutation) + this.notify({ type: 'mutationRemoved', mutation }) } clear(): void { @@ -96,14 +129,6 @@ export class MutationCache extends Subscribable { return this.mutations.filter(mutation => matchMutation(filters, mutation)) } - notify(mutation?: Mutation) { - notifyManager.batch(() => { - this.listeners.forEach(listener => { - listener(mutation) - }) - }) - } - onFocus(): void { this.resumePausedMutations() } diff --git a/src/core/notifiable.ts b/src/core/notifiable.ts new file mode 100644 index 0000000000..3d2bcd857f --- /dev/null +++ b/src/core/notifiable.ts @@ -0,0 +1,12 @@ +import { Subscribable } from './subscribable' +import { notifyManager } from '../core/notifyManager' + +export class Notifiable extends Subscribable<(event: TEvent) => void> { + notify(event: TEvent) { + notifyManager.batch(() => { + this.listeners.forEach(listener => { + listener(event) + }) + }) + } +} diff --git a/src/core/query.ts b/src/core/query.ts index a466b137bc..5889dfbf74 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -322,7 +322,7 @@ export class Query< // Stop the query from being garbage collected this.clearGcTimeout() - this.cache.notify({ type: 'observerAdded', query: this, observer }) + this.cache.notify({ type: 'queryObserverAdded', query: this, observer }) } } @@ -348,7 +348,7 @@ export class Query< } } - this.cache.notify({ type: 'observerRemoved', query: this, observer }) + this.cache.notify({ type: 'queryObserverRemoved', query: this, observer }) } } diff --git a/src/core/queryCache.ts b/src/core/queryCache.ts index f57b970ead..f3f93bed27 100644 --- a/src/core/queryCache.ts +++ b/src/core/queryCache.ts @@ -8,7 +8,7 @@ import { Action, Query, QueryState } from './query' import type { QueryKey, QueryOptions } from './types' import { notifyManager } from './notifyManager' import type { QueryClient } from './queryClient' -import { Subscribable } from './subscribable' +import { Notifiable } from './notifiable' import { QueryObserver } from './queryObserver' // TYPES @@ -38,20 +38,20 @@ interface NotifyEventQueryUpdated { action: Action } -interface NotifyEventObserverAdded { - type: 'observerAdded' +interface NotifyEventQueryObserverAdded { + type: 'queryObserverAdded' query: Query observer: QueryObserver } -interface NotifyEventObserverRemoved { - type: 'observerRemoved' +interface NotifyEventQueryObserverRemoved { + type: 'queryObserverRemoved' query: Query observer: QueryObserver } -interface NotifyEventObserverResultsUpdated { - type: 'observerResultsUpdated' +interface NotifyEventQueryObserverResultsUpdated { + type: 'queryObserverResultsUpdated' query: Query } @@ -59,15 +59,13 @@ type QueryCacheNotifyEvent = | NotifyEventQueryAdded | NotifyEventQueryRemoved | NotifyEventQueryUpdated - | NotifyEventObserverAdded - | NotifyEventObserverRemoved - | NotifyEventObserverResultsUpdated - -type QueryCacheListener = (event?: QueryCacheNotifyEvent) => void + | NotifyEventQueryObserverAdded + | NotifyEventQueryObserverRemoved + | NotifyEventQueryObserverResultsUpdated // CLASS -export class QueryCache extends Subscribable { +export class QueryCache extends Notifiable { config: QueryCacheConfig private queries: Query[] @@ -179,14 +177,6 @@ export class QueryCache extends Subscribable { : this.queries } - notify(event: QueryCacheNotifyEvent) { - notifyManager.batch(() => { - this.listeners.forEach(listener => { - listener(event) - }) - }) - } - onFocus(): void { notifyManager.batch(() => { this.queries.forEach(query => { diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index fe9cba01aa..8ff440f555 100644 --- a/src/core/queryObserver.ts +++ b/src/core/queryObserver.ts @@ -711,9 +711,10 @@ export class QueryObserver< // Then the cache listeners if (notifyOptions.cache) { - this.client - .getQueryCache() - .notify({ query: this.currentQuery, type: 'observerResultsUpdated' }) + this.client.getQueryCache().notify({ + query: this.currentQuery, + type: 'queryObserverResultsUpdated', + }) } }) } From 89ee8b9b877869c7df127d37f58a0229c412abbd Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sun, 14 Nov 2021 15:51:14 +0100 Subject: [PATCH 02/10] feat: mutation cachetime removable --- src/core/query.ts | 34 ++++++++-------------------------- src/core/removable.ts | 37 +++++++++++++++++++++++++++++++++++++ src/core/types.ts | 1 + 3 files changed, 46 insertions(+), 26 deletions(-) create mode 100644 src/core/removable.ts diff --git a/src/core/query.ts b/src/core/query.ts index 5889dfbf74..8c0af25a9f 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -2,7 +2,6 @@ import { getAbortController, Updater, functionalUpdate, - isValidTimeout, noop, replaceEqualDeep, timeUntilStale, @@ -24,6 +23,7 @@ import type { QueryObserver } from './queryObserver' import { notifyManager } from './notifyManager' import { getLogger } from './logger' import { Retryer, isCancelledError } from './retryer' +import { Removable } from './removable' // TYPES @@ -146,25 +146,25 @@ export class Query< TError = unknown, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey -> { +> extends Removable { queryKey: TQueryKey queryHash: string options!: QueryOptions initialState: QueryState revertState?: QueryState state: QueryState - cacheTime!: number meta: QueryMeta | undefined private cache: QueryCache private promise?: Promise - private gcTimeout?: number private retryer?: Retryer private observers: QueryObserver[] private defaultOptions?: QueryOptions private abortSignalConsumed: boolean constructor(config: QueryConfig) { + super() + this.abortSignalConsumed = false this.defaultOptions = config.defaultOptions this.setOptions(config.options) @@ -185,11 +185,7 @@ export class Query< this.meta = options?.meta - // Default to 5 minutes if not cache time is set - this.cacheTime = Math.max( - this.cacheTime || 0, - this.options.cacheTime ?? 5 * 60 * 1000 - ) + this.updateCacheTime(this.options.cacheTime) } setDefaultOptions( @@ -198,22 +194,7 @@ export class Query< this.defaultOptions = options } - private scheduleGc(): void { - this.clearGcTimeout() - - if (isValidTimeout(this.cacheTime)) { - this.gcTimeout = setTimeout(() => { - this.optionalRemove() - }, this.cacheTime) - } - } - - private clearGcTimeout() { - clearTimeout(this.gcTimeout) - this.gcTimeout = undefined - } - - private optionalRemove() { + protected optionalRemove() { if (!this.observers.length && !this.state.isFetching) { this.cache.remove(this) } @@ -260,7 +241,8 @@ export class Query< } destroy(): void { - this.clearGcTimeout() + super.destroy() + this.cancel({ silent: true }) } diff --git a/src/core/removable.ts b/src/core/removable.ts new file mode 100644 index 0000000000..7efd5a8f6a --- /dev/null +++ b/src/core/removable.ts @@ -0,0 +1,37 @@ +import { isValidTimeout } from './utils' + +export class Removable { + cacheTime!: number + private gcTimeout?: number + + destroy(): void { + this.clearGcTimeout() + } + + protected scheduleGc(): void { + this.clearGcTimeout() + + if (isValidTimeout(this.cacheTime)) { + this.gcTimeout = setTimeout(() => { + this.optionalRemove() + }, this.cacheTime) + } + } + + protected updateCacheTime(newCacheTime: number | undefined): void { + // Default to 5 minutes if no cache time is set + this.cacheTime = Math.max( + this.cacheTime || 0, + newCacheTime ?? 5 * 60 * 1000 + ) + } + + protected clearGcTimeout() { + clearTimeout(this.gcTimeout) + this.gcTimeout = undefined + } + + protected optionalRemove() { + // Do nothing + } +} diff --git a/src/core/types.ts b/src/core/types.ts index 7d186fa1e4..dc7a90a402 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -534,6 +534,7 @@ export interface MutationOptions< ) => Promise | void retry?: RetryValue retryDelay?: RetryDelayValue + cacheTime?: number _defaulted?: boolean } From fcb3f7bb09ab468a8dc22e5cf87814da4b60d8c3 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sun, 14 Nov 2021 16:06:50 +0100 Subject: [PATCH 03/10] feat: mutation cachetime add gc to mutations --- src/core/mutation.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/core/mutation.ts b/src/core/mutation.ts index feda111976..f7105c1727 100644 --- a/src/core/mutation.ts +++ b/src/core/mutation.ts @@ -3,6 +3,7 @@ import type { MutationCache } from './mutationCache' import type { MutationObserver } from './mutationObserver' import { getLogger } from './logger' import { notifyManager } from './notifyManager' +import { Removable } from './removable' import { Retryer } from './retryer' import { noop } from './utils' @@ -80,7 +81,7 @@ export class Mutation< TError = unknown, TVariables = void, TContext = unknown -> { +> extends Removable { state: MutationState options: MutationOptions mutationId: number @@ -90,6 +91,8 @@ export class Mutation< private retryer?: Retryer constructor(config: MutationConfig) { + super() + this.options = { ...config.defaultOptions, ...config.options, @@ -98,6 +101,8 @@ export class Mutation< this.mutationCache = config.mutationCache this.observers = [] this.state = config.state || getDefaultState() + this.updateCacheTime(this.options.cacheTime) + this.scheduleGc() } setState(state: MutationState): void { @@ -108,6 +113,9 @@ export class Mutation< if (this.observers.indexOf(observer) === -1) { this.observers.push(observer) + // Stop the query from being garbage collected + this.clearGcTimeout() + this.mutationCache.notify({ type: 'mutationObserverAdded', mutation: this, @@ -119,6 +127,12 @@ export class Mutation< removeObserver(observer: MutationObserver): void { this.observers = this.observers.filter(x => x !== observer) + if (this.cacheTime) { + this.scheduleGc() + } else { + this.mutationCache.remove(this) + } + this.mutationCache.notify({ type: 'mutationObserverRemoved', mutation: this, From bed509289a2d1d82714733969f7a1991b3682921 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sun, 14 Nov 2021 16:17:47 +0100 Subject: [PATCH 04/10] feat: mutation cachetime streamline event types between queries and mutations --- src/broadcastQueryClient-experimental/index.ts | 12 ++++++------ src/core/mutation.ts | 6 +++--- src/core/mutationCache.ts | 14 +++++++------- src/core/query.ts | 6 +++--- src/core/queryCache.ts | 16 ++++++++-------- src/core/queryObserver.ts | 2 +- src/core/tests/queryCache.test.tsx | 4 ++-- 7 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/broadcastQueryClient-experimental/index.ts b/src/broadcastQueryClient-experimental/index.ts index 9f6181553a..6a36d1c339 100644 --- a/src/broadcastQueryClient-experimental/index.ts +++ b/src/broadcastQueryClient-experimental/index.ts @@ -33,20 +33,20 @@ export function broadcastQueryClient({ } = queryEvent if ( - queryEvent.type === 'queryUpdated' && + queryEvent.type === 'updated' && queryEvent.action?.type === 'success' ) { channel.postMessage({ - type: 'queryUpdated', + type: 'updated', queryHash, queryKey, state, }) } - if (queryEvent.type === 'queryRemoved') { + if (queryEvent.type === 'removed') { channel.postMessage({ - type: 'queryRemoved', + type: 'removed', queryHash, queryKey, }) @@ -61,7 +61,7 @@ export function broadcastQueryClient({ tx(() => { const { type, queryHash, queryKey, state } = action - if (type === 'queryUpdated') { + if (type === 'updated') { const query = queryCache.get(queryHash) if (query) { @@ -77,7 +77,7 @@ export function broadcastQueryClient({ }, state ) - } else if (type === 'queryRemoved') { + } else if (type === 'removed') { const query = queryCache.get(queryHash) if (query) { diff --git a/src/core/mutation.ts b/src/core/mutation.ts index f7105c1727..5bdbc6ed1e 100644 --- a/src/core/mutation.ts +++ b/src/core/mutation.ts @@ -117,7 +117,7 @@ export class Mutation< this.clearGcTimeout() this.mutationCache.notify({ - type: 'mutationObserverAdded', + type: 'observerAdded', mutation: this, observer, }) @@ -134,7 +134,7 @@ export class Mutation< } this.mutationCache.notify({ - type: 'mutationObserverRemoved', + type: 'observerRemoved', mutation: this, observer, }) @@ -277,7 +277,7 @@ export class Mutation< }) this.mutationCache.notify({ mutation: this, - type: 'mutationUpdated', + type: 'updated', action, }) }) diff --git a/src/core/mutationCache.ts b/src/core/mutationCache.ts index 78ca6835f1..d64dd0ea43 100644 --- a/src/core/mutationCache.ts +++ b/src/core/mutationCache.ts @@ -24,28 +24,28 @@ interface MutationCacheConfig { } interface NotifyEventMutationAdded { - type: 'mutationAdded' + type: 'added' mutation: Mutation } interface NotifyEventMutationRemoved { - type: 'mutationRemoved' + type: 'removed' mutation: Mutation } interface NotifyEventMutationObserverAdded { - type: 'mutationObserverAdded' + type: 'observerAdded' mutation: Mutation observer: MutationObserver } interface NotifyEventMutationObserverRemoved { - type: 'mutationObserverRemoved' + type: 'observerRemoved' mutation: Mutation observer: MutationObserver } interface NotifyEventMutationUpdated { - type: 'mutationUpdated' + type: 'updated' mutation: Mutation action: Action } @@ -94,13 +94,13 @@ export class MutationCache extends Notifiable { add(mutation: Mutation): void { this.mutations.push(mutation) - this.notify({ type: 'mutationAdded', mutation }) + this.notify({ type: 'added', mutation }) } remove(mutation: Mutation): void { this.mutations = this.mutations.filter(x => x !== mutation) mutation.cancel() - this.notify({ type: 'mutationRemoved', mutation }) + this.notify({ type: 'removed', mutation }) } clear(): void { diff --git a/src/core/query.ts b/src/core/query.ts index 8c0af25a9f..643825e381 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -304,7 +304,7 @@ export class Query< // Stop the query from being garbage collected this.clearGcTimeout() - this.cache.notify({ type: 'queryObserverAdded', query: this, observer }) + this.cache.notify({ type: 'observerAdded', query: this, observer }) } } @@ -330,7 +330,7 @@ export class Query< } } - this.cache.notify({ type: 'queryObserverRemoved', query: this, observer }) + this.cache.notify({ type: 'observerRemoved', query: this, observer }) } } @@ -490,7 +490,7 @@ export class Query< observer.onQueryUpdate(action) }) - this.cache.notify({ query: this, type: 'queryUpdated', action }) + this.cache.notify({ query: this, type: 'updated', action }) }) } diff --git a/src/core/queryCache.ts b/src/core/queryCache.ts index f3f93bed27..a5dadc7022 100644 --- a/src/core/queryCache.ts +++ b/src/core/queryCache.ts @@ -23,35 +23,35 @@ interface QueryHashMap { } interface NotifyEventQueryAdded { - type: 'queryAdded' + type: 'added' query: Query } interface NotifyEventQueryRemoved { - type: 'queryRemoved' + type: 'removed' query: Query } interface NotifyEventQueryUpdated { - type: 'queryUpdated' + type: 'updated' query: Query action: Action } interface NotifyEventQueryObserverAdded { - type: 'queryObserverAdded' + type: 'observerAdded' query: Query observer: QueryObserver } interface NotifyEventQueryObserverRemoved { - type: 'queryObserverRemoved' + type: 'observerRemoved' query: Query observer: QueryObserver } interface NotifyEventQueryObserverResultsUpdated { - type: 'queryObserverResultsUpdated' + type: 'observerResultsUpdated' query: Query } @@ -109,7 +109,7 @@ export class QueryCache extends Notifiable { this.queriesMap[query.queryHash] = query this.queries.push(query) this.notify({ - type: 'queryAdded', + type: 'added', query, }) } @@ -127,7 +127,7 @@ export class QueryCache extends Notifiable { delete this.queriesMap[query.queryHash] } - this.notify({ type: 'queryRemoved', query }) + this.notify({ type: 'removed', query }) } } diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index 8ff440f555..fceea94f37 100644 --- a/src/core/queryObserver.ts +++ b/src/core/queryObserver.ts @@ -713,7 +713,7 @@ export class QueryObserver< if (notifyOptions.cache) { this.client.getQueryCache().notify({ query: this.currentQuery, - type: 'queryObserverResultsUpdated', + type: 'observerResultsUpdated', }) } }) diff --git a/src/core/tests/queryCache.test.tsx b/src/core/tests/queryCache.test.tsx index 4a17d2d0a0..0b3e5e0333 100644 --- a/src/core/tests/queryCache.test.tsx +++ b/src/core/tests/queryCache.test.tsx @@ -23,7 +23,7 @@ describe('queryCache', () => { queryClient.setQueryData(key, 'foo') const query = queryCache.find(key) await sleep(1) - expect(subscriber).toHaveBeenCalledWith({ query, type: 'queryAdded' }) + expect(subscriber).toHaveBeenCalledWith({ query, type: 'added' }) unsubscribe() }) @@ -43,7 +43,7 @@ describe('queryCache', () => { queryClient.prefetchQuery(key, () => 'data') const query = queryCache.find(key) await sleep(100) - expect(callback).toHaveBeenCalledWith({ query, type: 'queryAdded' }) + expect(callback).toHaveBeenCalledWith({ query, type: 'added' }) }) test('should notify subscribers when new query with initialData is added', async () => { From 8e8a5d005d810f486a9e71b1b6de715cc1f31a75 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sun, 14 Nov 2021 16:50:00 +0100 Subject: [PATCH 05/10] feat: mutation cachetime tests, and I forgot to implement optionalRemove, so make it abstract --- src/core/mutation.ts | 8 +++++- src/core/removable.ts | 6 ++-- src/core/tests/mutationCache.test.tsx | 41 +++++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/core/mutation.ts b/src/core/mutation.ts index 5bdbc6ed1e..9f25e5582f 100644 --- a/src/core/mutation.ts +++ b/src/core/mutation.ts @@ -113,7 +113,7 @@ export class Mutation< if (this.observers.indexOf(observer) === -1) { this.observers.push(observer) - // Stop the query from being garbage collected + // Stop the mutation from being garbage collected this.clearGcTimeout() this.mutationCache.notify({ @@ -140,6 +140,12 @@ export class Mutation< }) } + protected optionalRemove() { + if (!this.observers.length && this.state.status !== 'loading') { + this.mutationCache.remove(this) + } + } + cancel(): Promise { if (this.retryer) { this.retryer.cancel() diff --git a/src/core/removable.ts b/src/core/removable.ts index 7efd5a8f6a..c33f08202c 100644 --- a/src/core/removable.ts +++ b/src/core/removable.ts @@ -1,6 +1,6 @@ import { isValidTimeout } from './utils' -export class Removable { +export abstract class Removable { cacheTime!: number private gcTimeout?: number @@ -31,7 +31,5 @@ export class Removable { this.gcTimeout = undefined } - protected optionalRemove() { - // Do nothing - } + protected abstract optionalRemove(): void } diff --git a/src/core/tests/mutationCache.test.tsx b/src/core/tests/mutationCache.test.tsx index 425c438907..9fd2aa85c8 100644 --- a/src/core/tests/mutationCache.test.tsx +++ b/src/core/tests/mutationCache.test.tsx @@ -1,5 +1,5 @@ -import { queryKey, mockConsoleError } from '../../react/tests/utils' -import { MutationCache, QueryClient } from '../..' +import { queryKey, mockConsoleError, sleep } from '../../react/tests/utils' +import { MutationCache, MutationObserver, QueryClient } from '../..' describe('mutationCache', () => { describe('MutationCacheConfig.onError', () => { @@ -106,4 +106,41 @@ describe('mutationCache', () => { ).toEqual([mutation2]) }) }) + + describe('garbage collection', () => { + test('should remove unused mutations after cacheTime has elapsed', async () => { + const testCache = new MutationCache() + const testClient = new QueryClient({ mutationCache: testCache }) + await testClient.executeMutation({ + mutationKey: ['a', 1], + variables: 1, + cacheTime: 10, + mutationFn: () => Promise.resolve(), + }) + + expect(testCache.getAll()).toHaveLength(1) + await sleep(10) + expect(testCache.getAll()).toHaveLength(0) + }) + + test('should not remove mutations if there are active observers', async () => { + const queryClient = new QueryClient() + const observer = new MutationObserver(queryClient, { + variables: 1, + cacheTime: 10, + mutationFn: () => Promise.resolve(), + }) + const unsubscribe = observer.subscribe() + + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + observer.mutate(1) + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + await sleep(10) + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + unsubscribe() + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + await sleep(10) + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + }) + }) }) From 2ccdcb2c3bd46394bdfb22e8f01ac3985eafd161 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 17 Nov 2021 17:08:16 +0100 Subject: [PATCH 06/10] feat: mutation cachetime replicate gc behavior from https://github.com/tannerlinsley/react-query/pull/2950 and add more tests --- src/core/mutation.ts | 8 +++-- src/core/tests/mutationCache.test.tsx | 49 +++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/core/mutation.ts b/src/core/mutation.ts index cf586c3e2f..f44f9b50f6 100644 --- a/src/core/mutation.ts +++ b/src/core/mutation.ts @@ -145,8 +145,12 @@ export class Mutation< } protected optionalRemove() { - if (!this.observers.length && this.state.status !== 'loading') { - this.mutationCache.remove(this) + if (!this.observers.length) { + if (this.state.status === 'loading') { + this.scheduleGc() + } else { + this.mutationCache.remove(this) + } } } diff --git a/src/core/tests/mutationCache.test.tsx b/src/core/tests/mutationCache.test.tsx index 9fd2aa85c8..72417a8811 100644 --- a/src/core/tests/mutationCache.test.tsx +++ b/src/core/tests/mutationCache.test.tsx @@ -1,3 +1,4 @@ +import { waitFor } from '@testing-library/react' import { queryKey, mockConsoleError, sleep } from '../../react/tests/utils' import { MutationCache, MutationObserver, QueryClient } from '../..' @@ -111,16 +112,19 @@ describe('mutationCache', () => { test('should remove unused mutations after cacheTime has elapsed', async () => { const testCache = new MutationCache() const testClient = new QueryClient({ mutationCache: testCache }) + const onSuccess = jest.fn() await testClient.executeMutation({ mutationKey: ['a', 1], variables: 1, cacheTime: 10, mutationFn: () => Promise.resolve(), + onSuccess, }) expect(testCache.getAll()).toHaveLength(1) await sleep(10) expect(testCache.getAll()).toHaveLength(0) + expect(onSuccess).toHaveBeenCalledTimes(1) }) test('should not remove mutations if there are active observers', async () => { @@ -142,5 +146,50 @@ describe('mutationCache', () => { await sleep(10) expect(queryClient.getMutationCache().getAll()).toHaveLength(0) }) + + test('should be garbage collected later when unsubscribed and mutation is loading', async () => { + const queryClient = new QueryClient() + const onSuccess = jest.fn() + const observer = new MutationObserver(queryClient, { + variables: 1, + cacheTime: 10, + mutationFn: async () => { + await sleep(20) + return 'data' + }, + onSuccess, + }) + const unsubscribe = observer.subscribe() + observer.mutate(1) + unsubscribe() + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + await sleep(10) + // unsubscribe should not remove even though cacheTime has elapsed b/c mutation is still loading + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + await sleep(10) + // should be removed after an additional cacheTime wait + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + expect(onSuccess).toHaveBeenCalledTimes(1) + }) + + test('should call callbacks even with cacheTime 0 and mutation still loading', async () => { + const queryClient = new QueryClient() + const onSuccess = jest.fn() + const observer = new MutationObserver(queryClient, { + variables: 1, + cacheTime: 0, + mutationFn: async () => { + return 'data' + }, + onSuccess, + }) + const unsubscribe = observer.subscribe() + observer.mutate(1) + unsubscribe() + await waitFor(() => { + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + }) + expect(onSuccess).toHaveBeenCalledTimes(1) + }) }) }) From 8550b2c34411e4adeb0dd38090513dcec876d5f7 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 17 Nov 2021 17:29:09 +0100 Subject: [PATCH 07/10] feat: mutation cachetime get test coverage back to 100% --- src/core/tests/mutationCache.test.tsx | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/core/tests/mutationCache.test.tsx b/src/core/tests/mutationCache.test.tsx index 72417a8811..5ffc47c383 100644 --- a/src/core/tests/mutationCache.test.tsx +++ b/src/core/tests/mutationCache.test.tsx @@ -147,6 +147,44 @@ describe('mutationCache', () => { expect(queryClient.getMutationCache().getAll()).toHaveLength(0) }) + test('should only remove when the last observer unsubscribes', async () => { + const queryClient = new QueryClient() + const observer1 = new MutationObserver(queryClient, { + variables: 1, + cacheTime: 10, + mutationFn: async () => { + await sleep(10) + return 'update1' + }, + }) + + const observer2 = new MutationObserver(queryClient, { + cacheTime: 10, + mutationFn: async () => { + await sleep(10) + return 'update2' + }, + }) + + await observer1.mutate() + + // we currently have no way to add multiple observers to the same mutation + const currentMutation = observer1['currentMutation']! + currentMutation?.addObserver(observer1) + currentMutation?.addObserver(observer2) + + expect(currentMutation['observers'].length).toEqual(2) + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + + currentMutation?.removeObserver(observer1) + currentMutation?.removeObserver(observer2) + expect(currentMutation['observers'].length).toEqual(0) + expect(queryClient.getMutationCache().getAll()).toHaveLength(1) + // wait for cacheTime to gc + await sleep(10) + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + }) + test('should be garbage collected later when unsubscribed and mutation is loading', async () => { const queryClient = new QueryClient() const onSuccess = jest.fn() From d10cad065893df99447f3b1dd7d2382c1ef8f59a Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 17 Nov 2021 17:40:36 +0100 Subject: [PATCH 08/10] feat: mutation cachetime docs --- .../guides/migrating-to-react-query-4.md | 27 +++++++++++++++++++ docs/src/pages/reference/MutationCache.md | 6 ++--- docs/src/pages/reference/useMutation.md | 6 ++++- 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/docs/src/pages/guides/migrating-to-react-query-4.md b/docs/src/pages/guides/migrating-to-react-query-4.md index c3ec75d6cb..b3cff94804 100644 --- a/docs/src/pages/guides/migrating-to-react-query-4.md +++ b/docs/src/pages/guides/migrating-to-react-query-4.md @@ -93,3 +93,30 @@ For the same reason, those have also been combined: ``` This flag defaults to `active` because `refetchActive` defaulted to `true`. This means we also need a way to tell `invalidateQueries` to not refetch at all, which is why a fourth option (`none`) is also allowed here. + +### Streamlined NotifyEvents + +Subscribing manually to the `QueryCache` has always given you a `QueryCacheNotifyEvent`, but this was not true for the `MutationCache`. We have streamlined the behavior and also adapted event names accordingly. + +#### QueryCacheNotifyEvent + +```diff +- type: 'queryAdded' ++ type: 'added' +- type: 'queryRemoved' ++ type: 'removed' +- type: 'queryUpdated' ++ type: 'updated' +``` + +#### MutationCacheNotifyEvent + +The `MutationCacheNotifyEvent` uses the same types as the `QueryCacheNotifyEvent`. + +> Note: This is only relevant if you manually subscribe to the caches via `queryCache.subscribe` or `mutationCache.subscribe` + +## New Features 🚀 + +### Mutation Cache Garbage Collection + +Mutations can now also be garbage collected automatically, just like queries. The default `cacheTime` for mutations is also set to 5 minutes. diff --git a/docs/src/pages/reference/MutationCache.md b/docs/src/pages/reference/MutationCache.md index 8bb76b4745..2b4d966cc3 100644 --- a/docs/src/pages/reference/MutationCache.md +++ b/docs/src/pages/reference/MutationCache.md @@ -60,8 +60,8 @@ const mutations = mutationCache.getAll() The `subscribe` method can be used to subscribe to the mutation cache as a whole and be informed of safe/known updates to the cache like mutation states changing or mutations being updated, added or removed. ```js -const callback = mutation => { - console.log(mutation) +const callback = event => { + console.log(event.type, event.mutation) } const unsubscribe = mutationCache.subscribe(callback) @@ -69,7 +69,7 @@ const unsubscribe = mutationCache.subscribe(callback) **Options** -- `callback: (mutation?: Mutation) => void` +- `callback: (mutation?: MutationCacheNotifyEvent) => void` - This function will be called with the mutation cache any time it is updated. **Returns** diff --git a/docs/src/pages/reference/useMutation.md b/docs/src/pages/reference/useMutation.md index e8b2e9cdcb..6f4571dcbb 100644 --- a/docs/src/pages/reference/useMutation.md +++ b/docs/src/pages/reference/useMutation.md @@ -17,13 +17,14 @@ const { reset, status, } = useMutation(mutationFn, { + cacheTime, mutationKey, onError, onMutate, onSettled, onSuccess, useErrorBoundary, - meta, + meta }) mutate(variables, { @@ -39,6 +40,9 @@ mutate(variables, { - **Required** - A function that performs an asynchronous task and returns a promise. - `variables` is an object that `mutate` will pass to your `mutationFn` +- `cacheTime: number | Infinity` + - The time in milliseconds that unused/inactive cache data remains in memory. When a mutation's cache becomes unused or inactive, that cache data will be garbage collected after this duration. When different cache times are specified, the longest one will be used. + - If set to `Infinity`, will disable garbage collection - `mutationKey: string` - Optional - A mutation key can be set to inherit defaults set with `queryClient.setMutationDefaults` or to identify the mutation in the devtools. From 08c32a739b7ef78d304c268fc7a1856773202902 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 17 Nov 2021 17:47:09 +0100 Subject: [PATCH 09/10] feat: mutation cachetime try to make tests more resilient --- src/core/tests/mutationCache.test.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/core/tests/mutationCache.test.tsx b/src/core/tests/mutationCache.test.tsx index 5ffc47c383..bdeeaa8f5b 100644 --- a/src/core/tests/mutationCache.test.tsx +++ b/src/core/tests/mutationCache.test.tsx @@ -123,7 +123,9 @@ describe('mutationCache', () => { expect(testCache.getAll()).toHaveLength(1) await sleep(10) - expect(testCache.getAll()).toHaveLength(0) + await waitFor(() => { + expect(testCache.getAll()).toHaveLength(0) + }) expect(onSuccess).toHaveBeenCalledTimes(1) }) @@ -144,7 +146,9 @@ describe('mutationCache', () => { unsubscribe() expect(queryClient.getMutationCache().getAll()).toHaveLength(1) await sleep(10) - expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + await waitFor(() => { + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + }) }) test('should only remove when the last observer unsubscribes', async () => { @@ -182,7 +186,9 @@ describe('mutationCache', () => { expect(queryClient.getMutationCache().getAll()).toHaveLength(1) // wait for cacheTime to gc await sleep(10) - expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + await waitFor(() => { + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + }) }) test('should be garbage collected later when unsubscribed and mutation is loading', async () => { @@ -206,7 +212,9 @@ describe('mutationCache', () => { expect(queryClient.getMutationCache().getAll()).toHaveLength(1) await sleep(10) // should be removed after an additional cacheTime wait - expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + await waitFor(() => { + expect(queryClient.getMutationCache().getAll()).toHaveLength(0) + }) expect(onSuccess).toHaveBeenCalledTimes(1) }) From d665de26c80204fed243ab3ab2f89d8aec9cd556 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 17 Nov 2021 21:23:02 +0100 Subject: [PATCH 10/10] feat: mutation cachetime fix imports after merge conflict --- src/core/tests/mutationCache.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/tests/mutationCache.test.tsx b/src/core/tests/mutationCache.test.tsx index 1f4c51a97d..37e3f3017b 100644 --- a/src/core/tests/mutationCache.test.tsx +++ b/src/core/tests/mutationCache.test.tsx @@ -1,5 +1,5 @@ import { waitFor } from '@testing-library/react' -import { queryKey, mockConsoleError } from '../../reactjs/tests/utils' +import { queryKey, mockConsoleError, sleep } from '../../reactjs/tests/utils' import { MutationCache, MutationObserver, QueryClient } from '../..' describe('mutationCache', () => {