Skip to content

Commit

Permalink
fix(core): resumed mutations should not be interrupted by refetches (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
TkDodo committed Mar 12, 2024
1 parent 7e4d772 commit 61f5507
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 15 deletions.
8 changes: 4 additions & 4 deletions packages/query-core/src/queryClient.ts
Expand Up @@ -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()
}
})
Expand Down
69 changes: 61 additions & 8 deletions packages/query-core/src/tests/queryClient.test.tsx
Expand Up @@ -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()
Expand All @@ -1411,15 +1415,14 @@ 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()

queryCacheOnFocusSpy.mockRestore()
mutationCacheResumePausedMutationsSpy.mockRestore()
queryCacheOnOnlineSpy.mockRestore()
focusManager.setFocused(undefined)
})

test('should notify queryCache and mutationCache if online', async () => {
Expand All @@ -1444,15 +1447,17 @@ 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()

queryCacheOnFocusSpy.mockRestore()
queryCacheOnOnlineSpy.mockRestore()
mutationCacheResumePausedMutationsSpy.mockRestore()
onlineManager.setOnline(true)
})

test('should resume paused mutations when coming online', async () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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<string> = []

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()
Expand All @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions 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 {
Expand All @@ -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)
}

Expand Down

0 comments on commit 61f5507

Please sign in to comment.