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
vercel · Mar 2, 2021 · 8c0461c · 8c0461c
1 parent 679b95d
commit 8c0461c
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 62 deletions.
diff --git 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
@@ -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,26 +21,33 @@ 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)
     window.addEventListener('focus', () => cb(), false)
   }
 }
 
-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
@@ -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,28 +77,28 @@ 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')
 
     await waitForNextTick()
     // 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
@@ -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 <div>data: {data}</div>
+    }
+    const { container } = render(<Page />)
+
+    // 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 <div>data: {data}</div>
+    }
+    const { container } = render(<Page />)
+
+    // 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"`)
+  })
+})