From 8c0461c30aa069da27e35ddd42de67cd36922eea Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 2 Mar 2021 18:21:23 +0800 Subject: [PATCH] Fix: do not depend on navigator.onLine; code optimizations (#1004) * fix: do not depend on navigator.onLine * chore: use anonymous functions * micro optimization * micro optimization * add test --- src/config.ts | 48 ++++++++++++------------- src/libs/web-preset.ts | 48 ++++++++++++++----------- test/use-swr-focus.test.tsx | 37 ++++++++++--------- test/use-swr-offline.test.tsx | 67 +++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 62 deletions(-) create mode 100644 test/use-swr-offline.test.tsx diff --git a/src/config.ts b/src/config.ts index b2d26df09..3d7c8c74c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -47,34 +47,32 @@ const slowConnection = ['slow-2g', '2g'].indexOf(navigator['connection'].effectiveType) !== -1 // config -const defaultConfig: ConfigInterface = { - // events - onLoadingSlow: () => {}, - onSuccess: () => {}, - onError: () => {}, - onErrorRetry, +const defaultConfig: ConfigInterface = Object.assign( + { + // events + onLoadingSlow: () => {}, + onSuccess: () => {}, + onError: () => {}, + onErrorRetry, - errorRetryInterval: (slowConnection ? 10 : 5) * 1000, - focusThrottleInterval: 5 * 1000, - dedupingInterval: 2 * 1000, - loadingTimeout: (slowConnection ? 5 : 3) * 1000, + errorRetryInterval: (slowConnection ? 10 : 5) * 1000, + focusThrottleInterval: 5 * 1000, + dedupingInterval: 2 * 1000, + loadingTimeout: (slowConnection ? 5 : 3) * 1000, - refreshInterval: 0, - revalidateOnFocus: true, - revalidateOnReconnect: true, - refreshWhenHidden: false, - refreshWhenOffline: false, - shouldRetryOnError: true, - suspense: false, - compare: dequal, + refreshInterval: 0, + revalidateOnFocus: true, + revalidateOnReconnect: true, + refreshWhenHidden: false, + refreshWhenOffline: false, + shouldRetryOnError: true, + suspense: false, + compare: dequal, - fetcher: webPreset.fetcher, - isOnline: webPreset.isOnline, - isDocumentVisible: webPreset.isDocumentVisible, - isPaused: () => false, - registerOnFocus: webPreset.registerOnFocus, - registerOnReconnect: webPreset.registerOnReconnect -} + isPaused: () => false + }, + webPreset +) export { cache } export default defaultConfig diff --git a/src/libs/web-preset.ts b/src/libs/web-preset.ts index 437f6fa61..c57ed7be2 100644 --- a/src/libs/web-preset.ts +++ b/src/libs/web-preset.ts @@ -1,18 +1,17 @@ -function isOnline(): boolean { - if ( - typeof navigator !== 'undefined' && - typeof navigator.onLine !== 'undefined' - ) { - return navigator.onLine - } - // always assume it's online - return true -} +/** + * Due to bug https://bugs.chromium.org/p/chromium/issues/detail?id=678075, + * it's not reliable to detect if the browser is currently online or offline + * based on `navigator.onLine`. + * As a work around, we always assume it's online on first load, and change + * the status upon `online` or `offline` events. + */ +let online = true +const isOnline = () => online -function isDocumentVisible(): boolean { +const isDocumentVisible = () => { if ( typeof document !== 'undefined' && - typeof document.visibilityState !== 'undefined' + document.visibilityState !== undefined ) { return document.visibilityState !== 'hidden' } @@ -22,12 +21,12 @@ function isDocumentVisible(): boolean { const fetcher = url => fetch(url).then(res => res.json()) -function registerOnFocus(cb: () => void) { +const registerOnFocus = (cb: () => void) => { if ( typeof window !== 'undefined' && - typeof window.addEventListener !== 'undefined' && + window.addEventListener !== undefined && typeof document !== 'undefined' && - typeof document.addEventListener !== 'undefined' + document.addEventListener !== undefined ) { // focus revalidate document.addEventListener('visibilitychange', () => cb(), false) @@ -35,13 +34,20 @@ function registerOnFocus(cb: () => void) { } } -function registerOnReconnect(cb: () => void) { - if ( - typeof window !== 'undefined' && - typeof window.addEventListener !== 'undefined' - ) { +const registerOnReconnect = (cb: () => void) => { + if (typeof window !== 'undefined' && window.addEventListener !== undefined) { // reconnect revalidate - window.addEventListener('online', () => cb(), false) + window.addEventListener( + 'online', + () => { + online = true + cb() + }, + false + ) + + // nothing to revalidate, just update the status + window.addEventListener('offline', () => (online = false), false) } } diff --git a/test/use-swr-focus.test.tsx b/test/use-swr-focus.test.tsx index 8a326c5d7..8790c2304 100644 --- a/test/use-swr-focus.test.tsx +++ b/test/use-swr-focus.test.tsx @@ -3,7 +3,11 @@ import React, { useState } from 'react' import useSWR from '../src' import { sleep } from './utils' -const waitForNextTick = async () => await act(() => sleep(1)) +const waitForNextTick = () => act(() => sleep(1)) +const focusWindow = () => + act(async () => { + fireEvent.focus(window) + }) describe('useSWR - focus', () => { it('should revalidate on focus by default', async () => { @@ -24,7 +28,8 @@ describe('useSWR - focus', () => { await waitForNextTick() // trigger revalidation - fireEvent.focus(window) + await focusWindow() + await screen.findByText('data: 1') }) @@ -47,7 +52,7 @@ describe('useSWR - focus', () => { await waitForNextTick() // trigger revalidation - fireEvent.focus(window) + await focusWindow() // should not be revalidated expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: 0"`) }) @@ -72,20 +77,20 @@ describe('useSWR - focus', () => { await waitForNextTick() // trigger revalidation - fireEvent.focus(window) + await focusWindow() // data should not change expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: 0"`) // change revalidateOnFocus to true fireEvent.click(container.firstElementChild) // trigger revalidation - fireEvent.focus(window) + await focusWindow() // data should update await screen.findByText('data: 1') await waitForNextTick() // trigger revalidation - fireEvent.focus(window) + await focusWindow() // data should update await screen.findByText('data: 2') @@ -93,7 +98,7 @@ describe('useSWR - focus', () => { // change revalidateOnFocus to false fireEvent.click(container.firstElementChild) // trigger revalidation - fireEvent.focus(window) + await focusWindow() // data should not change expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: 2"`) }) @@ -122,17 +127,17 @@ describe('useSWR - focus', () => { await waitForNextTick() // trigger revalidation - fireEvent.focus(window) + await focusWindow() // still in throttling interval await act(() => sleep(20)) // should be throttled - fireEvent.focus(window) + await focusWindow() await screen.findByText('data: 1') // wait for focusThrottleInterval await act(() => sleep(100)) // trigger revalidation again - fireEvent.focus(window) + await focusWindow() await screen.findByText('data: 2') }) @@ -161,11 +166,11 @@ describe('useSWR - focus', () => { await waitForNextTick() // trigger revalidation - fireEvent.focus(window) + await focusWindow() // wait for throttle interval await act(() => sleep(100)) // trigger revalidation - fireEvent.focus(window) + await focusWindow() await screen.findByText('data: 2') await waitForNextTick() @@ -174,21 +179,21 @@ describe('useSWR - focus', () => { // wait for throttle interval await act(() => sleep(100)) // trigger revalidation - fireEvent.focus(window) + await focusWindow() // wait for throttle interval await act(() => sleep(100)) // should be throttled - fireEvent.focus(window) + await focusWindow() await screen.findByText('data: 3') // wait for throttle interval await act(() => sleep(150)) // trigger revalidation - fireEvent.focus(window) + await focusWindow() // wait for throttle intervals await act(() => sleep(150)) // trigger revalidation - fireEvent.focus(window) + await focusWindow() await screen.findByText('data: 5') }) }) diff --git a/test/use-swr-offline.test.tsx b/test/use-swr-offline.test.tsx new file mode 100644 index 000000000..6185e6aa2 --- /dev/null +++ b/test/use-swr-offline.test.tsx @@ -0,0 +1,67 @@ +import { act, fireEvent, render, screen } from '@testing-library/react' +import React from 'react' +import useSWR from '../src' +import { sleep } from './utils' + +const waitForNextTick = () => act(() => sleep(1)) +const focusWindow = () => + act(async () => { + fireEvent.focus(window) + }) +const dispatchWindowEvent = event => + act(async () => { + window.dispatchEvent(new Event(event)) + }) + +describe('useSWR - offline', () => { + it('should not revalidate when offline', async () => { + let value = 0 + + function Page() { + const { data } = useSWR('offline-1', () => value++, { + dedupingInterval: 0 + }) + return
data: {data}
+ } + const { container } = render() + + // hydration + expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: "`) + // mount + await screen.findByText('data: 0') + + // simulate offline + await waitForNextTick() + await dispatchWindowEvent('offline') + + // trigger focus revalidation + await focusWindow() + + // should not be revalidated + expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: 0"`) + }) + + it('should revalidate immediately when becoming online', async () => { + let value = 0 + + function Page() { + const { data } = useSWR('offline-2', () => value++, { + dedupingInterval: 0 + }) + return
data: {data}
+ } + const { container } = render() + + // hydration + expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: "`) + // mount + await screen.findByText('data: 0') + + // simulate online + await waitForNextTick() + await dispatchWindowEvent('online') + + // should be revalidated + expect(container.firstChild.textContent).toMatchInlineSnapshot(`"data: 1"`) + }) +})