fix: Fix dependency tracking and useSES bug (#2576)

* fix #2446 * test: update `compare` test snapshot * test: adjust compare order, makes the snapshot easier to understand * add test case * refactor: update `memorizedSanpshot` without changing its reference (#2577) * refactor: always use cacheRef * refactor: update memorizedSnapshot without change its reference * refactor: isEqual * revert example change * add comment --------- Co-authored-by: Yixuan Xu <[email protected]>
vercel · Apr 23, 2023 · 0859b54 · 0859b54
1 parent f8603ab
commit 0859b54
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 30 deletions.
diff --git a/core/use-swr.ts b/core/use-swr.ts
@@ -102,32 +102,29 @@ export const useSWRHandler = <Data = any, Error = any>(
     >(cache, key)
 
   const stateDependencies = useRef<StateDependencies>({}).current
-
   const fallback = isUndefined(fallbackData)
     ? config.fallback[key]
     : fallbackData
 
   const isEqual = (prev: State<Data, any>, current: State<Data, any>) => {
-    let equal = true
     for (const _ in stateDependencies) {
       const t = _ as keyof StateDependencies
       if (t === 'data') {
-        if (!compare(current[t], prev[t])) {
-          if (isUndefined(prev[t])) {
-            if (!compare(current[t], returnedData)) {
-              equal = false
-            }
-          } else {
-            equal = false
+        if (!compare(prev[t], current[t])) {
+          if (!isUndefined(prev[t])) {
+            return false
+          }
+          if (!compare(returnedData, current[t])) {
+            return false
           }
         }
       } else {
         if (current[t] !== prev[t]) {
-          equal = false
+          return false
         }
       }
     }
-    return equal
+    return true
   }
 
   const getSnapshot = useMemo(() => {
@@ -174,9 +171,28 @@ export const useSWRHandler = <Data = any, Error = any>(
     return [
       () => {
         const newSnapshot = getSelectedCache(getCache())
-        return isEqual(newSnapshot, memorizedSnapshot)
-          ? memorizedSnapshot
-          : (memorizedSnapshot = newSnapshot)
+        const compareResult = isEqual(newSnapshot, memorizedSnapshot)
+
+        if (compareResult) {
+          // Mentally, we should always return the `memorizedSnapshot` here
+          // as there's no change between the new and old snapshots.
+          // However, since the `isEqual` function only compares selected fields,
+          // the values of the unselected fields might be changed. That's
+          // simply because we didn't track them.
+          // To support the case in https://github.com/vercel/swr/pull/2576,
+          // we need to update these fields in the `memorizedSnapshot` too
+          // with direct mutations to ensure the snapshot is always up-to-date
+          // even for the unselected fields, but only trigger re-renders when
+          // the selected fields are changed.
+          memorizedSnapshot.data = newSnapshot.data
+          memorizedSnapshot.isLoading = newSnapshot.isLoading
+          memorizedSnapshot.isValidating = newSnapshot.isValidating
+          memorizedSnapshot.error = newSnapshot.error
+          return memorizedSnapshot
+        } else {
+          memorizedSnapshot = newSnapshot
+          return newSnapshot
+        }
       },
       () => serverSnapshot
     ]

diff --git a/package.json b/package.json
@@ -106,7 +106,7 @@
     "@type-challenges/utils": "0.1.1",
     "@types/jest": "^29.4.0",
     "@types/node": "^18.11.18",
-    "@types/react": "^18.0.27",
+    "@types/react": "^18.0.38",
     "@types/use-sync-external-store": "^0.0.3",
     "@typescript-eslint/eslint-plugin": "5.49.0",
     "@typescript-eslint/parser": "5.49.0",

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/test/use-swr-integration.test.tsx b/test/use-swr-integration.test.tsx
@@ -592,4 +592,36 @@ describe('useSWR', () => {
       ]
     `)
   })
+
+  // Test for https://github.com/vercel/swr/issues/2446
+  it('should return latest data synchronously after accessing getter', async () => {
+    const fetcher = async () => {
+      await sleep(10)
+      return 'value'
+    }
+    const key = createKey()
+
+    const logs = []
+
+    function Page() {
+      const swr = useSWR(key, fetcher)
+      const [show, setShow] = useState(false)
+      useEffect(() => {
+        setTimeout(() => {
+          setShow(true)
+        }, 100)
+      }, [])
+      if (!show) return null
+      logs.push(swr.data)
+      return <p>data:{swr.data}</p>
+    }
+
+    renderWithConfig(<Page />)
+    await screen.findByText('data:value')
+    expect(logs).toMatchInlineSnapshot(`
+      [
+        "value",
+      ]
+    `)
+  })
 })
diff --git a/test/use-swr-refresh.test.tsx b/test/use-swr-refresh.test.tsx
@@ -308,25 +308,25 @@ describe('useSWR - refresh', () => {
           },
         ],
         [
+          undefined,
           {
             "timestamp": 1,
             "version": "1.0",
           },
-          undefined,
         ],
         [
+          undefined,
           {
             "timestamp": 1,
             "version": "1.0",
           },
-          undefined,
         ],
         [
-          undefined,
           {
             "timestamp": 1,
             "version": "1.0",
           },
+          undefined,
         ],
         [
           {
@@ -377,25 +377,25 @@ describe('useSWR - refresh', () => {
           },
         ],
         [
+          undefined,
           {
             "timestamp": 1,
             "version": "1.0",
           },
-          undefined,
         ],
         [
+          undefined,
           {
             "timestamp": 1,
             "version": "1.0",
           },
-          undefined,
         ],
         [
-          undefined,
           {
             "timestamp": 1,
             "version": "1.0",
           },
+          undefined,
         ],
         [
           {
@@ -449,21 +449,21 @@ describe('useSWR - refresh', () => {
         ],
         [
           {
-            "timestamp": 2,
+            "timestamp": 1,
             "version": "1.0",
           },
           {
-            "timestamp": 1,
+            "timestamp": 2,
             "version": "1.0",
           },
         ],
         [
           {
-            "timestamp": 1,
+            "timestamp": 2,
             "version": "1.0",
           },
           {
-            "timestamp": 2,
+            "timestamp": 1,
             "version": "1.0",
           },
         ],