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: Error retry should be handled by global revalidator instead of local revalidation function #2415

Merged
merged 8 commits into from
Feb 14, 2023
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
1 change: 1 addition & 0 deletions _internal/constants.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const FOCUS_EVENT = 0
export const RECONNECT_EVENT = 1
export const MUTATE_EVENT = 2
export const ERROR_REVALIDATE_EVENT = 3
6 changes: 4 additions & 2 deletions _internal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,14 +415,16 @@ export type RevalidateEvent =
| typeof revalidateEvents.FOCUS_EVENT
| typeof revalidateEvents.RECONNECT_EVENT
| typeof revalidateEvents.MUTATE_EVENT

| typeof revalidateEvents.ERROR_REVALIDATE_EVENT
type RevalidateCallbackReturnType = {
[revalidateEvents.FOCUS_EVENT]: void
[revalidateEvents.RECONNECT_EVENT]: void
[revalidateEvents.MUTATE_EVENT]: Promise<boolean>
[revalidateEvents.ERROR_REVALIDATE_EVENT]: void
}
export type RevalidateCallback = <K extends RevalidateEvent>(
type: K
type: K,
opts?: any
) => RevalidateCallbackReturnType[K]

export interface Cache<Data = any> {
Expand Down
20 changes: 18 additions & 2 deletions core/use-swr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,15 @@ export const useSWRHandler = <Data = any, Error = any>(
err,
key,
currentConfig,
revalidate,
_opts => {
const revalidators = EVENT_REVALIDATORS[key]
if (revalidators && revalidators[0]) {
revalidators[0](
revalidateEvents.ERROR_REVALIDATE_EVENT,
_opts
)
}
},
{
retryCount: (opts.retryCount || 0) + 1,
dedupe: true
Expand Down Expand Up @@ -511,7 +519,13 @@ export const useSWRHandler = <Data = any, Error = any>(
// Expose revalidators to global event listeners. So we can trigger
// revalidation from the outside.
let nextFocusRevalidatedAt = 0
const onRevalidate = (type: RevalidateEvent) => {
const onRevalidate = (
type: RevalidateEvent,
opts: {
retryCount?: number
dedupe?: boolean
} = {}
) => {
if (type == revalidateEvents.FOCUS_EVENT) {
const now = Date.now()
if (
Expand All @@ -528,6 +542,8 @@ export const useSWRHandler = <Data = any, Error = any>(
}
} else if (type == revalidateEvents.MUTATE_EVENT) {
return revalidate()
} else if (type == revalidateEvents.ERROR_REVALIDATE_EVENT) {
return revalidate(opts)
}
return
}
Expand Down
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ module.exports = {
},
coveragePathIgnorePatterns: ['/node_modules/', '/dist/', '/test/'],
coverageReporters: ['text', 'html'],
reporters: ['default', 'github-actions']
reporters: [['github-actions', { silent: false }], 'summary']
}
12 changes: 6 additions & 6 deletions test/use-swr-config-callbacks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ describe('useSWR - config callbacks', () => {
it('should trigger the onError event with the latest version of the onError callback', async () => {
let state = null
let count = 0

const key = createKey()
function Page(props: { text: string }) {
const { data, mutate, error } = useSWR(
'config callbacks - onError',
key,
() => createResponse(new Error(`Error: ${count++}`)),
{ onError: () => (state = props.text) }
)
Expand Down Expand Up @@ -87,10 +87,10 @@ describe('useSWR - config callbacks', () => {
it('should trigger the onErrorRetry event with the latest version of the onErrorRetry callback', async () => {
let state = null
let count = 0

const key = createKey()
function Page(props: { text: string }) {
const { data, error } = useSWR(
'config callbacks - onErrorRetry',
key,
() => createResponse(new Error(`Error: ${count++}`)),
{
onErrorRetry: (_, __, ___, revalidate, revalidateOpts) => {
Expand Down Expand Up @@ -134,10 +134,10 @@ describe('useSWR - config callbacks', () => {
const LOADING_TIMEOUT = 100
let state = null
let count = 0

const key = createKey()
function Page(props: { text: string }) {
const { data } = useSWR(
'config callbacks - onLoadingSlow',
key,
() => createResponse(count++, { delay: LOADING_TIMEOUT * 2 }),
{
onLoadingSlow: () => {
Expand Down
66 changes: 56 additions & 10 deletions test/use-swr-error.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -495,13 +495,11 @@ describe('useSWR - error', () => {
expect(errorEvents.length).toBe(1)
})

it('should not should not trigger revalidation when a key is already active and has error - isLoading', async () => {
it('should not trigger revalidation when a key is already active and has error - isLoading', async () => {
const key = createKey()
const useData = () => {
return useSWR<any, any>(
key,
() => createResponse(new Error('error!'), { delay: 200 }),
undefined
return useSWR<any, any>(key, () =>
createResponse(new Error('error!'), { delay: 200 })
)
}
const Page = () => {
Expand All @@ -523,13 +521,11 @@ describe('useSWR - error', () => {
await screen.findByText('error!,false')
})

it('should not should not trigger revalidation when a key is already active and has error - isValidating', async () => {
it('should not trigger revalidation when a key is already active and has error - isValidating', async () => {
const key = createKey()
const useData = () => {
return useSWR<any, any>(
key,
() => createResponse(new Error('error!'), { delay: 200 }),
undefined
return useSWR<any, any>(key, () =>
createResponse(new Error('error!'), { delay: 200 })
)
}
const Page = () => {
Expand All @@ -550,4 +546,54 @@ describe('useSWR - error', () => {
screen.getByText('validating')
await screen.findByText('error!,false')
})

it('should trigger revalidation when first hook is unmount', async () => {
const key = createKey()
const fn = jest.fn()
const useData = () => {
return useSWR<any, any>(
key,
() => createResponse(new Error('error!'), { delay: 200 }),
{
onErrorRetry: (_, __, ___, revalidate, opts) => {
setTimeout(() => {
fn()
revalidate(opts)
}, 100)
},
dedupingInterval: 1
}
)
}
const First = () => {
useData()
const [state, setState] = useState(true)
return (
<>
<button onClick={() => setState(!state)}>toggle</button>
{state ? <Second></Second> : null}
</>
)
}
const Second = () => {
useData()
return null
}
const App = () => {
return (
<div>
<First></First>
<Second />
</div>
)
}
renderWithConfig(<App />)
await act(() => sleep(320))
expect(fn).toBeCalledTimes(1)
fireEvent.click(screen.getByText('toggle'))
await act(() => sleep(320))
expect(fn).toBeCalledTimes(2)
await act(() => sleep(320))
expect(fn).toBeCalledTimes(3)
})
})