From 61f5507cd39ad68cea7812bcbc2c6246ec508052 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Tue, 12 Mar 2024 10:26:17 +0100 Subject: [PATCH] fix(core): resumed mutations should not be interrupted by refetches (#7085) * fix(core): resumed mutations should not be interrupted by refetches it's important that we don't run a refetch while we're resuming paused mutations, because that could lead to UI flicker. With this fix, we're awaiting until the resume has finished before triggering a refetchOnReconnect or refetchOnWindowFocus * test: function is async now --- packages/query-core/src/queryClient.ts | 8 +-- .../query-core/src/tests/queryClient.test.tsx | 69 ++++++++++++++++--- packages/query-core/src/tests/utils.ts | 6 +- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/packages/query-core/src/queryClient.ts b/packages/query-core/src/queryClient.ts index f7d02aadcd..da1ce0d561 100644 --- a/packages/query-core/src/queryClient.ts +++ b/packages/query-core/src/queryClient.ts @@ -75,15 +75,15 @@ export class QueryClient { this.#mountCount++ if (this.#mountCount !== 1) return - this.#unsubscribeFocus = focusManager.subscribe((focused) => { + this.#unsubscribeFocus = focusManager.subscribe(async (focused) => { if (focused) { - this.resumePausedMutations() + await this.resumePausedMutations() this.#queryCache.onFocus() } }) - this.#unsubscribeOnline = onlineManager.subscribe((online) => { + this.#unsubscribeOnline = onlineManager.subscribe(async (online) => { if (online) { - this.resumePausedMutations() + await this.resumePausedMutations() this.#queryCache.onOnline() } }) diff --git a/packages/query-core/src/tests/queryClient.test.tsx b/packages/query-core/src/tests/queryClient.test.tsx index 45387a295c..649509772d 100644 --- a/packages/query-core/src/tests/queryClient.test.tsx +++ b/packages/query-core/src/tests/queryClient.test.tsx @@ -1389,6 +1389,10 @@ describe('queryClient', () => { }) describe('focusManager and onlineManager', () => { + afterEach(() => { + onlineManager.setOnline(true) + focusManager.setFocused(undefined) + }) test('should notify queryCache and mutationCache if focused', async () => { const testClient = createQueryClient() testClient.mount() @@ -1411,7 +1415,7 @@ describe('queryClient', () => { expect(mutationCacheResumePausedMutationsSpy).not.toHaveBeenCalled() focusManager.setFocused(true) - expect(queryCacheOnFocusSpy).toHaveBeenCalledTimes(1) + await waitFor(() => expect(queryCacheOnFocusSpy).toHaveBeenCalledTimes(1)) expect(mutationCacheResumePausedMutationsSpy).toHaveBeenCalledTimes(1) expect(queryCacheOnOnlineSpy).not.toHaveBeenCalled() @@ -1419,7 +1423,6 @@ describe('queryClient', () => { queryCacheOnFocusSpy.mockRestore() mutationCacheResumePausedMutationsSpy.mockRestore() queryCacheOnOnlineSpy.mockRestore() - focusManager.setFocused(undefined) }) test('should notify queryCache and mutationCache if online', async () => { @@ -1444,7 +1447,10 @@ describe('queryClient', () => { expect(mutationCacheResumePausedMutationsSpy).not.toHaveBeenCalled() onlineManager.setOnline(true) - expect(queryCacheOnOnlineSpy).toHaveBeenCalledTimes(1) + await waitFor(() => + expect(queryCacheOnOnlineSpy).toHaveBeenCalledTimes(1), + ) + expect(mutationCacheResumePausedMutationsSpy).toHaveBeenCalledTimes(1) expect(queryCacheOnFocusSpy).not.toHaveBeenCalled() @@ -1452,7 +1458,6 @@ describe('queryClient', () => { queryCacheOnFocusSpy.mockRestore() queryCacheOnOnlineSpy.mockRestore() mutationCacheResumePausedMutationsSpy.mockRestore() - onlineManager.setOnline(true) }) test('should resume paused mutations when coming online', async () => { @@ -1481,8 +1486,6 @@ describe('queryClient', () => { expect(observer1.getCurrentResult().status).toBe('success') expect(observer1.getCurrentResult().status).toBe('success') }) - - onlineManager.setOnline(true) }) test('should resume paused mutations one after the other when invoked manually at the same time', async () => { @@ -1600,6 +1603,54 @@ describe('queryClient', () => { newQueryClient.unmount() }) + test('should notify queryCache after resumePausedMutations has finished when coming online', async () => { + const key = queryKey() + + let count = 0 + const results: Array = [] + + const queryObserver = new QueryObserver(queryClient, { + queryKey: key, + queryFn: async () => { + count++ + results.push('data' + count) + await sleep(10) + return 'data' + count + }, + }) + + const unsubscribe = queryObserver.subscribe(() => undefined) + + await waitFor(() => { + expect(queryClient.getQueryData(key)).toBe('data1') + }) + + onlineManager.setOnline(false) + + const observer = new MutationObserver(queryClient, { + mutationFn: async () => { + results.push('mutation') + await sleep(50) + return 1 + }, + }) + + void observer.mutate() + + expect(observer.getCurrentResult().isPaused).toBeTruthy() + + onlineManager.setOnline(true) + + await waitFor(() => { + expect(queryClient.getQueryData(key)).toBe('data2') + }) + + // refetch from coming online should happen after mutations have finished + expect(results).toStrictEqual(['data1', 'mutation', 'data2']) + + unsubscribe() + }) + test('should notify queryCache and mutationCache after multiple mounts and single unmount', async () => { const testClient = createQueryClient() testClient.mount() @@ -1621,11 +1672,13 @@ describe('queryClient', () => { onlineManager.setOnline(false) onlineManager.setOnline(true) - expect(queryCacheOnOnlineSpy).toHaveBeenCalledTimes(1) + await waitFor(() => + expect(queryCacheOnOnlineSpy).toHaveBeenCalledTimes(1), + ) expect(mutationCacheResumePausedMutationsSpy).toHaveBeenCalledTimes(1) focusManager.setFocused(true) - expect(queryCacheOnFocusSpy).toHaveBeenCalledTimes(1) + await waitFor(() => expect(queryCacheOnFocusSpy).toHaveBeenCalledTimes(1)) expect(mutationCacheResumePausedMutationsSpy).toHaveBeenCalledTimes(2) queryCacheOnFocusSpy.mockRestore() diff --git a/packages/query-core/src/tests/utils.ts b/packages/query-core/src/tests/utils.ts index 132a1bbba6..84245d772b 100644 --- a/packages/query-core/src/tests/utils.ts +++ b/packages/query-core/src/tests/utils.ts @@ -1,7 +1,7 @@ import { vi } from 'vitest' import { QueryClient, onlineManager } from '..' import * as utils from '../utils' -import type { SpyInstance } from 'vitest' +import type { MockInstance } from 'vitest' import type { MutationOptions, QueryClientConfig } from '..' export function createQueryClient(config?: QueryClientConfig): QueryClient { @@ -10,13 +10,13 @@ export function createQueryClient(config?: QueryClientConfig): QueryClient { export function mockVisibilityState( value: DocumentVisibilityState, -): SpyInstance<[], DocumentVisibilityState> { +): MockInstance<[], DocumentVisibilityState> { return vi.spyOn(document, 'visibilityState', 'get').mockReturnValue(value) } export function mockOnlineManagerIsOnline( value: boolean, -): SpyInstance<[], boolean> { +): MockInstance<[], boolean> { return vi.spyOn(onlineManager, 'isOnline').mockReturnValue(value) }