Skip error retrying when document is not active and improve tests (vercel#1742)

* use the babel coverage reporter

* improve coverage

Author: shuding

jest.config.js

@@ -19,6 +19,5 @@ module.exports = {
     ]
   },
   coveragePathIgnorePatterns: ['/node_modules/', '/dist/', '/test/'],
-  coverageProvider: 'v8',
-  coverageReporters: ['text']
+  coverageReporters: ['text', 'html']
 }

src/index.ts

@@ -8,7 +8,7 @@ export { useSWRConfig } from './utils/use-swr-config'
 export { mutate } from './utils/config'
 
 // Types
-export {
+export type {
   SWRConfiguration,
   Revalidator,
   RevalidatorOptions,

src/use-swr.ts

@@ -76,6 +76,7 @@ export const useSWRHandler = <Data = any, Error = any>(
   const fetcherRef = useRef(fetcher)
   const configRef = useRef(config)
   const getConfig = () => configRef.current
+  const isActive = () => getConfig().isVisible() && getConfig().isOnline()
 
   // Get the current state that SWR should return.
   const cached = cache.get(key)
@@ -305,10 +306,14 @@ export const useSWRHandler = <Data = any, Error = any>(
             getConfig().onError(err, key, config)
             if (config.shouldRetryOnError) {
               // When retrying, dedupe is always enabled
-              getConfig().onErrorRetry(err, key, config, revalidate, {
-                retryCount: (opts.retryCount || 0) + 1,
-                dedupe: true
-              })
+              if (isActive()) {
+                // If it's active, stop. It will auto revalidate when refocusing
+                // or reconnecting.
+                getConfig().onErrorRetry(err, key, config, revalidate, {
+                  retryCount: (opts.retryCount || 0) + 1,
+                  dedupe: true
+                })
+              }
             }
           }
         }
@@ -366,8 +371,6 @@ export const useSWRHandler = <Data = any, Error = any>(
     const keyChanged = initialMountedRef.current
     const softRevalidate = revalidate.bind(UNDEFINED, WITH_DEDUPE)
 
-    const isActive = () => getConfig().isVisible() && getConfig().isOnline()
-
     // Expose state updater to global event listeners. So we can update hook's
     // internal state from the outside.
     const onStateUpdate: StateUpdateCallback<Data, Error> = (

src/utils/config.ts

@@ -20,11 +20,6 @@ const onErrorRetry = (
   revalidate: Revalidator,
   opts: Required<RevalidatorOptions>
 ): void => {
-  if (!preset.isVisible()) {
-    // If it's hidden, stop. It will auto revalidate when refocusing.
-    return
-  }
-
   const maxRetryCount = config.errorRetryCount
   const currentRetryCount = opts.retryCount
 

src/utils/web-preset.ts

@@ -30,10 +30,7 @@ const offDocumentEvent = hasDoc
 
 const isVisible = () => {
   const visibilityState = hasDoc && document.visibilityState
-  if (!isUndefined(visibilityState)) {
-    return visibilityState !== 'hidden'
-  }
-  return true
+  return isUndefined(visibilityState) || visibilityState !== 'hidden'
 }
 
 const initFocus = (callback: () => void) => {

test/unit/serialize.test.ts

@@ -0,0 +1,13 @@
+import { unstable_serialize } from 'swr'
+import { stableHash } from '../../src/utils/hash'
+
+describe('SWR - unstable_serialize', () => {
+  it('should serialize arguments correctly', async () => {
+    expect(unstable_serialize([])).toBe('')
+    expect(unstable_serialize(null)).toBe('')
+    expect(unstable_serialize('key')).toBe('key')
+    expect(unstable_serialize([1, { foo: 2, bar: 1 }, ['a', 'b', 'c']])).toBe(
+      stableHash([1, { foo: 2, bar: 1 }, ['a', 'b', 'c']])
+    )
+  })
+})

test/use-swr-error.test.tsx

@@ -1,7 +1,13 @@
 import { act, fireEvent, screen } from '@testing-library/react'
 import React, { useEffect, useState } from 'react'
 import useSWR from 'swr'
-import { sleep, createResponse, createKey, renderWithConfig } from './utils'
+import {
+  sleep,
+  createResponse,
+  createKey,
+  renderWithConfig,
+  mockVisibilityHidden
+} from './utils'
 
 describe('useSWR - error', () => {
   it('should handle errors', async () => {
@@ -72,6 +78,70 @@ describe('useSWR - error', () => {
     screen.getByText('error: 2')
   })
 
+  it('should stop retrying when document is not visible', async () => {
+    const key = createKey()
+    let count = 0
+    function Page() {
+      const { data, error } = useSWR(
+        key,
+        () => createResponse(new Error('error: ' + count++), { delay: 100 }),
+        {
+          onErrorRetry: (_, __, ___, revalidate, revalidateOpts) => {
+            revalidate(revalidateOpts)
+          },
+          dedupingInterval: 0
+        }
+      )
+      if (error) return <div>{error.message}</div>
+      return <div>hello, {data}</div>
+    }
+    renderWithConfig(<Page />)
+    screen.getByText('hello,')
+
+    // mount
+    await screen.findByText('error: 0')
+
+    // errored, retrying
+    await act(() => sleep(50))
+    const resetVisibility = mockVisibilityHidden()
+
+    await act(() => sleep(100))
+    screen.getByText('error: 1')
+
+    await act(() => sleep(100)) // stopped due to invisible
+    screen.getByText('error: 1')
+
+    resetVisibility()
+  })
+
+  it('should not retry when shouldRetryOnError is disabled', async () => {
+    const key = createKey()
+    let count = 0
+    function Page() {
+      const { data, error } = useSWR(
+        key,
+        () => createResponse(new Error('error: ' + count++), { delay: 100 }),
+        {
+          onErrorRetry: (_, __, ___, revalidate, revalidateOpts) => {
+            revalidate(revalidateOpts)
+          },
+          dedupingInterval: 0,
+          shouldRetryOnError: false
+        }
+      )
+      if (error) return <div>{error.message}</div>
+      return <div>hello, {data}</div>
+    }
+    renderWithConfig(<Page />)
+    screen.getByText('hello,')
+
+    // mount
+    await screen.findByText('error: 0')
+
+    await act(() => sleep(150))
+    screen.getByText('error: 0')
+  })
+
   it('should trigger the onLoadingSlow and onSuccess event', async () => {
     const key = createKey()
     let loadingSlow = null,

test/use-swr-reconnect.test.tsx

@@ -4,7 +4,8 @@ import useSWR from 'swr'
 import {
   nextTick as waitForNextTick,
   renderWithConfig,
-  createKey
+  createKey,
+  mockVisibilityHidden
 } from './utils'
 
 describe('useSWR - reconnect', () => {
@@ -90,4 +91,37 @@ describe('useSWR - reconnect', () => {
     // should not be revalidated
     screen.getByText('data: 0')
   })
+
+  it("shouldn't revalidate on reconnect if invisible", async () => {
+    let value = 0
+
+    const key = createKey()
+    function Page() {
+      const { data } = useSWR(key, () => value++, {
+        dedupingInterval: 0,
+        isOnline: () => false
+      })
+      return <div>data: {data}</div>
+    }
+
+    renderWithConfig(<Page />)
+    // hydration
+    screen.getByText('data:')
+
+    // mount
+    await screen.findByText('data: 0')
+
+    await waitForNextTick()
+
+    const resetVisibility = mockVisibilityHidden()
+
+    // trigger reconnect
+    fireEvent(window, createEvent('offline', window))
+    fireEvent(window, createEvent('online', window))
+
+    // should not be revalidated
+    screen.getByText('data: 0')
+
+    resetVisibility()
+  })
 })

test/utils.tsx

@@ -53,3 +53,9 @@ export const renderWithGlobalCache = (
 ): ReturnType<typeof _renderWithConfig> => {
   return _renderWithConfig(element, { ...config })
 }
+
+export const mockVisibilityHidden = () => {
+  const mockVisibilityState = jest.spyOn(document, 'visibilityState', 'get')
+  mockVisibilityState.mockImplementation(() => 'hidden')
+  return () => mockVisibilityState.mockRestore()
+}