fix: Refactor core revalidate function and fix isValidating state bug

src/use-swr.ts

@@ -149,11 +149,16 @@ export const useSWRHandler = <Data = any, Error = any>(
         }
       }
 
+      // The new state object when request finishes.
+      const newState: State<Data, Error> = { isValidating: false }
+      const finishRequestAndUpdateState = () => {
+        cache.set(keyValidating, false)
+        setState(newState)
+      }
+
       // Start fetching. Change the `isValidating` state, update the cache.
       cache.set(keyValidating, true)
-      setState({
-        isValidating: true
-      })
+      setState({ isValidating: true })
 
       try {
         if (shouldStartNewRequest) {
@@ -196,7 +201,7 @@ export const useSWRHandler = <Data = any, Error = any>(
           }
         }
 
-        // if there're other ongoing request(s), started after the current one,
+        // If there're other ongoing request(s), started after the current one,
         // we need to ignore the current one to avoid possible race conditions:
         //   req1------------------>res1        (current one)
         //        req2---------------->res2
@@ -206,14 +211,11 @@ export const useSWRHandler = <Data = any, Error = any>(
           return false
         }
 
+        // Clear error.
         cache.set(keyErr, UNDEFINED)
-        cache.set(keyValidating, false)
+        newState.error = UNDEFINED
 
-        const newState: State<Data, Error> = {
-          isValidating: false
-        }
-
-        // if there're other mutations(s), overlapped with the current revalidation:
+        // If there're other mutations(s), overlapped with the current revalidation:
         // case 1:
         //   req------------------>res
         //       mutate------>end
@@ -234,72 +236,57 @@ export const useSWRHandler = <Data = any, Error = any>(
             // case 3
             MUTATION_END_TS[key] === 0)
         ) {
-          setState(newState)
+          finishRequestAndUpdateState()
           return false
         }
 
-        if (!isUndefined(stateRef.current.error)) {
-          newState.error = UNDEFINED
-        }
-
         // Deep compare with latest state to avoid extra re-renders.
         // For local state, compare and assign.
         if (!compare(stateRef.current.data, newData)) {
           newState.data = newData
         }
+
         // For global state, it's possible that the key has changed.
         // https://github.com/vercel/swr/pull/1058
         if (!compare(cache.get(key), newData)) {
           cache.set(key, newData)
         }
-
-        // merge the new state
-        setState(newState)
-
-        if (shouldStartNewRequest) {
-          // also update other hooks
-          broadcastState(cache, key, newData, newState.error, false)
-        }
       } catch (err) {
-        // Reset the state immediately.
         // @ts-ignore
         cleanupState(startAt)
-        cache.set(keyValidating, false)
-        if (getConfig().isPaused()) {
-          setState({
-            isValidating: false
-          })
-          return false
-        }
 
-        // Get a new error, don't use deep comparison for errors.
-        cache.set(keyErr, err)
-        if (stateRef.current.error !== err) {
-          // Keep the stale data but update error.
-          setState({
-            isValidating: false,
-            error: err as Error
-          })
-          if (shouldStartNewRequest) {
-            // Broadcast to update the states of other hooks.
-            broadcastState(cache, key, UNDEFINED, err, false)
-          }
-        }
+        // Not paused, we continue handling the error. Otherwise discard it.
+        if (!getConfig().isPaused()) {
+          // Get a new error, don't use deep comparison for errors.
+          cache.set(keyErr, err)
+          newState.error = err as Error
 
-        // Error event and retry logic.
-        if (isCallbackSafe()) {
-          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
-            })
+          // Error event and retry logic.
+          if (isCallbackSafe()) {
+            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
+              })
+            }
           }
         }
       }
 
+      // Mark loading as stopped.
       loading = false
+
+      // Update the current hook's state.
+      finishRequestAndUpdateState()
+
+      // Here is the source of the request, need to tell all other hooks to
+      // update their states.
+      if (shouldStartNewRequest) {
+        broadcastState(cache, key, newState.data, newState.error, false)
+      }
+
       return true
     },
     // `setState` is immutable, and `eventsCallback`, `fnArgs`, `keyErr`,

test/use-swr-loading.test.tsx

@@ -1,7 +1,13 @@
-import { act, screen } from '@testing-library/react'
+import { act, screen, fireEvent } from '@testing-library/react'
 import React from 'react'
 import useSWR from 'swr'
-import { createResponse, createKey, sleep, renderWithConfig } from './utils'
+import {
+  createResponse,
+  createKey,
+  sleep,
+  renderWithConfig,
+  nextTick
+} from './utils'
 
 describe('useSWR - loading', () => {
   it('should return loading state', async () => {
@@ -89,4 +95,101 @@ describe('useSWR - loading', () => {
     renderWithConfig(<Page />)
     screen.getByText('data,error,isValidating,mutate')
   })
+
+  it('should sync loading states', async () => {
+    const key = createKey()
+    const fetcher = jest.fn()
+
+    function Foo() {
+      const { isValidating } = useSWR(key, async () => {
+        fetcher()
+        return 'foo'
+      })
+      return isValidating ? <>loading</> : <>stopped</>
+    }
+
+    function Page() {
+      return (
+        <>
+          <Foo />,<Foo />
+        </>
+      )
+    }
+
+    renderWithConfig(<Page />)
+    screen.getByText('loading,loading')
+    await nextTick()
+    screen.getByText('stopped,stopped')
+    expect(fetcher).toBeCalledTimes(1)
+  })
+
+  it('should sync all loading states if errored', async () => {
+    const key = createKey()
+
+    function Foo() {
+      const { isValidating } = useSWR(key, async () => {
+        throw new Error(key)
+      })
+
+      return isValidating ? <>loading</> : <>stopped</>
+    }
+
+    function Page() {
+      return (
+        <>
+          <Foo />,<Foo />
+        </>
+      )
+    }
+
+    renderWithConfig(<Page />)
+    screen.getByText('loading,loading')
+    await nextTick()
+    screen.getByText('stopped,stopped')
+  })
+
+  it('should sync all loading states if errored but paused', async () => {
+    const key = createKey()
+    let paused = false
+
+    function Foo() {
+      const { isValidating } = useSWR(key, {
+        isPaused: () => paused,
+        fetcher: async () => {
+          await sleep(50)
+          throw new Error(key)
+        },
+        dedupingInterval: 0
+      })
+
+      return isValidating ? <>loading</> : <>stopped</>
+    }
+
+    function Page() {
+      const [mountSecondRequest, setMountSecondRequest] = React.useState(false)
+      return (
+        <>
+          <Foo />,{mountSecondRequest ? <Foo /> : null}
+          <br />
+          <button onClick={() => setMountSecondRequest(true)}>start</button>
+        </>
+      )
+    }
+
+    renderWithConfig(<Page />)
+    screen.getByText('loading,')
+    await act(() => sleep(70))
+    screen.getByText('stopped,')
+
+    fireEvent.click(screen.getByText('start'))
+    await act(() => sleep(20))
+    screen.getByText('loading,loading')
+
+    // Pause before it resolves
+    paused = true
+    await act(() => sleep(50))
+
+    // They should both stop
+    screen.getByText('stopped,stopped')
+  })
 })