Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(onlineManager): always initialize with online: true #5714

Merged
merged 13 commits into from
Jul 18, 2023
8 changes: 8 additions & 0 deletions docs/react/guides/migrating-to-v5.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,14 @@ There are some caveats to this change however, which you must be aware of:

The `visibilitychange` event is used exclusively now. This is possible because we only support browsers that support the `visibilitychange` event. This fixes a bunch of issues [as listed here](https://github.com/TanStack/query/pull/4805).

### Network status no longer relies on the `navigator.onLine` property

`navigator.onLine` doesn't work well in Chromium based browsers. There are [a lot of issues](https://bugs.chromium.org/p/chromium/issues/list?q=navigator.online) around false negatives, which lead to Queries being wrongfully marked as `offline`.

To circumvent this, we now always start with `online: true` and only listen to `online` and `offline` events to update the status.

This should reduce the likelihood of false negatives, however, it might mean false positives for offline apps that load via serviceWorkers, which can work even without an internet connection.

### Removed custom `context` prop in favor of custom `queryClient` instance

In v4, we introduced the possibility to pass a custom `context` to all react-query hooks. This allowed for proper isolation when using MicroFrontends.
Expand Down
15 changes: 14 additions & 1 deletion docs/react/reference/focusManager.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ It can be used to change the default event listeners or to manually change the f
Its available methods are:

- [`setEventListener`](#focusmanagerseteventlistener)
- [`subscribe`](#focusmanagersubscribe)
- [`setFocused`](#focusmanagersetfocused)
- [`isFocused`](#focusmanagerisfocused)

Expand All @@ -33,9 +34,21 @@ focusManager.setEventListener((handleFocus) => {
})
```

## `focusManager.subscribe`

`subscribe` can be used to subscribe to changes in the visibility state. It returns an unsubscribe function:

```tsx
import { focusManager } from '@tanstack/react-query'

const unsubscribe = focusManager.subscribe(isVisible => {
console.log('isVisible', isVisible)
})
```

## `focusManager.setFocused`

`setFocused` can be used to manually set the focus state. Set `undefined` to fallback to the default focus check.
`setFocused` can be used to manually set the focus state. Set `undefined` to fall back to the default focus check.

```tsx
import { focusManager } from '@tanstack/react-query'
Expand Down
30 changes: 23 additions & 7 deletions docs/react/reference/onlineManager.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,20 @@ id: OnlineManager
title: OnlineManager
---

The `OnlineManager` manages the online state within TanStack Query.
The `OnlineManager` manages the online state within TanStack Query. It can be used to change the default event listeners or to manually change the online state.

It can be used to change the default event listeners or to manually change the online state.
> Per default, the `onlineManager` assumes an active network connection, and listens to the `online` and `offline` events on the `window` object to detect changes.

> In previous versions, `navigator.onLine` was used to determine the network status. However, it doesn't work well in Chromium based browsers. There are [a lot of issues](https://bugs.chromium.org/p/chromium/issues/list?q=navigator.online) around false negatives, which lead to Queries being wrongfully marked as `offline`.

> To circumvent this, we now always start with `online: true` and only listen to `online` and `offline` events to update the status.

> This should reduce the likelihood of false negatives, however, it might mean false positives for offline apps that load via serviceWorkers, which can work even without an internet connection.

Its available methods are:

- [`setEventListener`](#onlinemanagerseteventlistener)
- [`subscribe`](#onlinemanagersubscribe)
- [`setOnline`](#onlinemanagersetonline)
- [`isOnline`](#onlinemanagerisonline)

Expand All @@ -28,9 +35,21 @@ onlineManager.setEventListener(setOnline => {
})
```

## `onlineManager.subscribe`

`subscribe` can be used to subscribe to changes in the online state. It returns an unsubscribe function:

```tsx
import { onlineManager } from '@tanstack/react-query'

const unsubscribe = onlineManager.subscribe(isOnline => {
console.log('isOnline', isOnline)
})
```

## `onlineManager.setOnline`

`setOnline` can be used to manually set the online state. Set `undefined` to fallback to the default online check.
`setOnline` can be used to manually set the online state.

```tsx
import { onlineManager } from '@tanstack/react-query'
Expand All @@ -40,14 +59,11 @@ onlineManager.setOnline(true)

// Set to offline
onlineManager.setOnline(false)

// Fallback to the default online check
onlineManager.setOnline(undefined)
```

**Options**

- `online: boolean | undefined`
- `online: boolean`

## `onlineManager.isOnline`

Expand Down
57 changes: 16 additions & 41 deletions packages/query-core/src/onlineManager.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { Subscribable } from './subscribable'
import { isServer } from './utils'

type SetupFn = (
setOnline: (online?: boolean) => void,
) => (() => void) | undefined
type Listener = (online: boolean) => void
type SetupFn = (setOnline: Listener) => (() => void) | undefined

const onlineEvents = ['online', 'offline'] as const

export class OnlineManager extends Subscribable {
#online?: boolean
export class OnlineManager extends Subscribable<Listener> {
#online = true
#cleanup?: () => void

#setup: SetupFn
Expand All @@ -19,17 +16,16 @@ export class OnlineManager extends Subscribable {
// addEventListener does not exist in React Native, but window does
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!isServer && window.addEventListener) {
const listener = () => onOnline()
const onlineListener = () => onOnline(true)
const offlineListener = () => onOnline(false)
// Listen to online
onlineEvents.forEach((event) => {
window.addEventListener(event, listener, false)
})
window.addEventListener('online', onlineListener, false)
window.addEventListener('offline', offlineListener, false)

return () => {
// Be sure to unsubscribe if a new handler is set
onlineEvents.forEach((event) => {
window.removeEventListener(event, listener)
})
window.removeEventListener('online', onlineListener)
window.removeEventListener('offline', offlineListener)
}
}

Expand All @@ -53,43 +49,22 @@ export class OnlineManager extends Subscribable {
setEventListener(setup: SetupFn): void {
this.#setup = setup
this.#cleanup?.()
this.#cleanup = setup((online?: boolean) => {
if (typeof online === 'boolean') {
this.setOnline(online)
} else {
this.onOnline()
}
})
this.#cleanup = setup(this.setOnline.bind(this))
}

setOnline(online?: boolean): void {
setOnline(online: boolean): void {
const changed = this.#online !== online

if (changed) {
this.#online = online
this.onOnline()
this.listeners.forEach((listener) => {
listener(online)
})
}
}

onOnline(): void {
this.listeners.forEach((listener) => {
listener()
})
}

isOnline(): boolean {
if (typeof this.#online === 'boolean') {
return this.#online
}

if (
typeof navigator === 'undefined' ||
typeof navigator.onLine === 'undefined'
) {
return true
}

return navigator.onLine
return this.#online
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/query-core/src/tests/hydration.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { MutationCache } from '../mutationCache'
import {
createQueryClient,
executeMutation,
mockNavigatorOnLine,
mockOnlineManagerIsOnline,
sleep,
} from './utils'

Expand Down Expand Up @@ -347,7 +347,7 @@ describe('dehydration and rehydration', () => {
test('should be able to dehydrate mutations and continue on hydration', async () => {
const consoleMock = vi.spyOn(console, 'error')
consoleMock.mockImplementation(() => undefined)
const onlineMock = mockNavigatorOnLine(false)
const onlineMock = mockOnlineManagerIsOnline(false)

const serverAddTodo = vi
.fn()
Expand Down
16 changes: 6 additions & 10 deletions packages/query-core/src/tests/onlineManager.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('onlineManager', () => {
test('setEventListener should use online boolean arg', async () => {
let count = 0

const setup = (setOnline: (online?: boolean) => void) => {
const setup = (setOnline: (online: boolean) => void) => {
setTimeout(() => {
count++
setOnline(false)
Expand Down Expand Up @@ -154,19 +154,15 @@ describe('onlineManager', () => {

onlineManager.subscribe(listener)

onlineManager.setOnline(true)
onlineManager.setOnline(true)

expect(listener).toHaveBeenCalledTimes(1)

onlineManager.setOnline(false)
onlineManager.setOnline(false)

expect(listener).toHaveBeenCalledTimes(2)
expect(listener).toHaveBeenNthCalledWith(1, false)

onlineManager.setOnline(undefined)
onlineManager.setOnline(undefined)
onlineManager.setOnline(true)
onlineManager.setOnline(true)

expect(listener).toHaveBeenCalledTimes(3)
expect(listener).toHaveBeenCalledTimes(2)
expect(listener).toHaveBeenNthCalledWith(2, true)
})
})
18 changes: 9 additions & 9 deletions packages/query-core/src/tests/queryClient.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { waitFor } from '@testing-library/react'
import '@testing-library/jest-dom'

import { vi } from 'vitest'
import {
MutationObserver,
QueryObserver,
Expand All @@ -11,7 +10,7 @@ import {
import { noop } from '../utils'
import {
createQueryClient,
mockNavigatorOnLine,
mockOnlineManagerIsOnline,
queryKey,
sleep,
} from './utils'
Expand Down Expand Up @@ -1074,7 +1073,7 @@ describe('queryClient', () => {
const key1 = queryKey()
const queryFn1 = vi.fn<unknown[], string>().mockReturnValue('data1')
await queryClient.fetchQuery({ queryKey: key1, queryFn: queryFn1 })
const onlineMock = mockNavigatorOnLine(false)
const onlineMock = mockOnlineManagerIsOnline(false)

await queryClient.refetchQueries({ queryKey: key1 })

Expand All @@ -1088,7 +1087,7 @@ describe('queryClient', () => {
queryClient.setQueryDefaults(key1, { networkMode: 'always' })
const queryFn1 = vi.fn<unknown[], string>().mockReturnValue('data1')
await queryClient.fetchQuery({ queryKey: key1, queryFn: queryFn1 })
const onlineMock = mockNavigatorOnLine(false)
const onlineMock = mockOnlineManagerIsOnline(false)

await queryClient.refetchQueries({ queryKey: key1 })

Expand Down Expand Up @@ -1394,7 +1393,7 @@ describe('queryClient', () => {
queryCacheOnFocusSpy.mockRestore()
queryCacheOnOnlineSpy.mockRestore()
mutationCacheResumePausedMutationsSpy.mockRestore()
onlineManager.setOnline(undefined)
onlineManager.setOnline(true)
})

test('should resume paused mutations when coming online', async () => {
Expand Down Expand Up @@ -1424,7 +1423,7 @@ describe('queryClient', () => {
expect(observer1.getCurrentResult().status).toBe('success')
})

onlineManager.setOnline(undefined)
onlineManager.setOnline(true)
})

test('should resume paused mutations one after the other when invoked manually at the same time', async () => {
Expand Down Expand Up @@ -1459,7 +1458,7 @@ describe('queryClient', () => {
expect(observer2.getCurrentResult().isPaused).toBeTruthy()
})

onlineManager.setOnline(undefined)
onlineManager.setOnline(true)
void queryClient.resumePausedMutations()
await sleep(5)
await queryClient.resumePausedMutations()
Expand Down Expand Up @@ -1491,6 +1490,7 @@ describe('queryClient', () => {
'resumePausedMutations',
)

onlineManager.setOnline(false)
onlineManager.setOnline(true)
expect(queryCacheOnOnlineSpy).toHaveBeenCalledTimes(1)
expect(mutationCacheResumePausedMutationsSpy).toHaveBeenCalledTimes(1)
Expand All @@ -1503,7 +1503,7 @@ describe('queryClient', () => {
queryCacheOnOnlineSpy.mockRestore()
mutationCacheResumePausedMutationsSpy.mockRestore()
focusManager.setFocused(undefined)
onlineManager.setOnline(undefined)
onlineManager.setOnline(true)
})

test('should not notify queryCache and mutationCache after multiple mounts/unmounts', async () => {
Expand Down Expand Up @@ -1538,7 +1538,7 @@ describe('queryClient', () => {
queryCacheOnOnlineSpy.mockRestore()
mutationCacheResumePausedMutationsSpy.mockRestore()
focusManager.setFocused(undefined)
onlineManager.setOnline(undefined)
onlineManager.setOnline(true)
})
})

Expand Down
8 changes: 5 additions & 3 deletions packages/query-core/src/tests/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { act } from '@testing-library/react'
import { vi } from 'vitest'
import { QueryClient } from '..'
import { QueryClient, onlineManager } from '..'
import * as utils from '../utils'
import type { SpyInstance } from 'vitest'
import type { MutationOptions, QueryClientConfig } from '..'
Expand All @@ -15,8 +15,10 @@ export function mockVisibilityState(
return vi.spyOn(document, 'visibilityState', 'get').mockReturnValue(value)
}

export function mockNavigatorOnLine(value: boolean): SpyInstance<[], boolean> {
return vi.spyOn(navigator, 'onLine', 'get').mockReturnValue(value)
export function mockOnlineManagerIsOnline(
value: boolean,
): SpyInstance<[], boolean> {
return vi.spyOn(onlineManager, 'isOnline').mockReturnValue(value)
}

let queryKeyCount = 0
Expand Down
4 changes: 1 addition & 3 deletions packages/query-devtools/src/Devtools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -485,13 +485,11 @@ export const DevtoolsPanel: Component<DevtoolsPanelProps> = (props) => {
<button
onClick={() => {
if (offline()) {
onlineManager().setOnline(undefined)
onlineManager().setOnline(true)
setOffline(false)
window.dispatchEvent(new Event('online'))
} else {
onlineManager().setOnline(false)
setOffline(true)
window.dispatchEvent(new Event('offline'))
}
}}
class={styles.actionsBtn}
Expand Down
Loading