Skip to content

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

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

Merged
merged 2 commits into from
Dec 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,5 @@ module.exports = {
]
},
coveragePathIgnorePatterns: ['/node_modules/', '/dist/', '/test/'],
coverageProvider: 'v8',
coverageReporters: ['text']
coverageReporters: ['text', 'html']
}
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export { useSWRConfig } from './utils/use-swr-config'
export { mutate } from './utils/config'

// Types
export {
export type {
SWRConfiguration,
Revalidator,
RevalidatorOptions,
Expand Down
15 changes: 9 additions & 6 deletions src/use-swr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,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)
Expand Down Expand Up @@ -306,10 +307,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
})
}
}
}
}
Expand Down Expand Up @@ -367,8 +372,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> = (
Expand Down
5 changes: 0 additions & 5 deletions src/utils/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 1 addition & 4 deletions src/utils/web-preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
13 changes: 13 additions & 0 deletions test/unit/serialize.test.ts
Original file line number Diff line number Diff line change
@@ -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']])
)
})
})
72 changes: 71 additions & 1 deletion test/use-swr-error.test.tsx
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand Down Expand Up @@ -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,
Expand Down
36 changes: 35 additions & 1 deletion test/use-swr-reconnect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import useSWR from 'swr'
import {
nextTick as waitForNextTick,
renderWithConfig,
createKey
createKey,
mockVisibilityHidden
} from './utils'

describe('useSWR - reconnect', () => {
Expand Down Expand Up @@ -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()
})
})
6 changes: 6 additions & 0 deletions test/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}