Repository: zeit/swr (ts)

Issue:
Performance: Component Re-rendered because of hook changes.
Make sure the state returned from the reducer is not a different object that equals by value but different by reference from the previous state:

![image](https://user-images.githubusercontent.com/17661697/70374958-27b02500-18f9-11ea-9097-52affdcc3d39.png)


Pull request:
Reduce re-renders by auto detecting state dependencies
By moving _data, error, isValidating_ away from the state (`useReducer`) to a ref (`stateRef`), the hook won't trigger another re-render anymore when one of these values updates. 

Instead, we add getters to these values (implemented with `defineProperties`). Once a value is read, it will be marked as a dependency immediately. Thus, we can trigger a re-render if any dependency changes in the future.

With this change, `useSWR` will never cause unnecessary re-renders:

```js
const { data } = useSWR(...)
// will not re-render if `isValidating` or `error` changes
```

No changes needed from the user side (and Suspense mode is not affected).

Fixes #184. Related #144, #114.

Published at ~~`swr@0.1.13-beta.0`~~ `swr@0.1.15-beta.0`.

---

This PR has been running on our production for a couple of weeks, and it works very well  🎉

Patch:
diff --git a/src/types.ts b/src/types.ts
index 9a1487e..c4d42ad 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -1,5 +1,3 @@
-import { Reducer } from 'react'
-
 export type fetcherFn<Data> = (...args: any) => Data | Promise<Data>
 export interface ConfigInterface<
   Data = any,
@@ -110,12 +108,3 @@ export type actionType<Data, Error> = {
   error?: Error
   isValidating?: boolean
 }
-
-export type reducerType<Data, Error> = Reducer<
-  {
-    data: Data
-    error: Error
-    isValidating: boolean
-  },
-  actionType<Data, Error>
->
diff --git a/src/use-swr-pages.tsx b/src/use-swr-pages.tsx
index d2babef..2baa17a 100644
--- a/src/use-swr-pages.tsx
+++ b/src/use-swr-pages.tsx
@@ -150,7 +150,12 @@ export function useSWRPages<OffsetType = any, Data = any, Error = any>(
       ) {
         setPageSWRs(swrs => {
           const _swrs = [...swrs]
-          _swrs[id] = swr
+          _swrs[id] = {
+            data: swr.data,
+            error: swr.error,
+            revalidate: swr.revalidate,
+            isValidating: swr.isValidating
+          }
           return _swrs
         })
         if (typeof swr.data !== 'undefined') {
diff --git a/src/use-swr.ts b/src/use-swr.ts
index 1ab5350..1c5a0cc 100644
--- a/src/use-swr.ts
+++ b/src/use-swr.ts
@@ -4,9 +4,11 @@ import {
   useContext,
   useEffect,
   useLayoutEffect,
-  useReducer,
-  useRef
+  useState,
+  useRef,
+  useMemo
 } from 'react'
+
 import defaultConfig, {
   cacheGet,
   cacheSet,
@@ -28,7 +30,6 @@ import {
   fetcherFn,
   keyInterface,
   mutateInterface,
-  reducerType,
   responseInterface,
   RevalidateOptionInterface,
   triggerInterface,
@@ -130,10 +131,6 @@ const mutate: mutateInterface = async (_key, _data, shouldRevalidate) => {
   }
 }
 
-function mergeState(state, payload) {
-  return { ...state, ...payload }
-}
-
 function useSWR<Data = any, Error = any>(
   key: keyInterface
 ): responseInterface<Data, Error>
@@ -189,17 +186,37 @@ function useSWR<Data = any, Error = any>(
   const initialData = cacheGet(key) || config.initialData
   const initialError = cacheGet(keyErr)
 
-  let [state, dispatch] = useReducer<reducerType<Data, Error>>(mergeState, {
+  // if a state is accessed (data, error or isValidating),
+  // we add the state to dependencies so if the state is
+  // updated in the future, we can trigger a rerender
+  const stateDependencies = useRef({
+    data: false,
+    error: false,
+    isValidating: false
+  })
+  const stateRef = useRef({
     data: initialData,
     error: initialError,
     isValidating: false
   })
 
+  const rerender = useState(null)[1]
+  let dispatch = useCallback(payload => {
+    let shouldUpdateState = false
+    for (let k in payload) {
+      stateRef.current[k] = payload[k]
+      if (stateDependencies.current[k]) {
+        shouldUpdateState = true
+      }
+    }
+    if (shouldUpdateState || config.suspense) {
+      rerender({})
+    }
+  }, [])
+
   // error ref inside revalidate (is last request errored?)
   const unmountedRef = useRef(false)
   const keyRef = useRef(key)
-  const dataRef = useRef(initialData)
-  const errorRef = useRef(initialError)
 
   // start a revalidation
   const revalidate = useCallback(
@@ -288,18 +305,16 @@ function useSWR<Data = any, Error = any>(
           isValidating: false
         }
 
-        if (typeof errorRef.current !== 'undefined') {
+        if (typeof stateRef.current.error !== 'undefined') {
           // we don't have an error
           newState.error = undefined
-          errorRef.current = undefined
         }
-        if (deepEqual(dataRef.current, newData)) {
+        if (deepEqual(stateRef.current.data, newData)) {
           // deep compare to avoid extra re-render
           // do nothing
         } else {
           // data changed
           newState.data = newData
-          dataRef.current = newData
         }
 
         // merge the new state
@@ -318,9 +333,7 @@ function useSWR<Data = any, Error = any>(
 
         // get a new error
         // don't use deep equal for errors
-        if (errorRef.current !== err) {
-          errorRef.current = err
-
+        if (stateRef.current.error !== err) {
           // we keep the stale data
           dispatch({
             isValidating: false,
@@ -365,7 +378,7 @@ function useSWR<Data = any, Error = any>(
     // we need to update the data from the cache
     // and trigger a revalidation
 
-    const currentHookData = dataRef.current
+    const currentHookData = stateRef.current.data
     const latestKeyedData = cacheGet(key) || config.initialData
 
     // update the state if the key changed or cache updated
@@ -374,7 +387,6 @@ function useSWR<Data = any, Error = any>(
       !deepEqual(currentHookData, latestKeyedData)
     ) {
       dispatch({ data: latestKeyedData })
-      dataRef.current = latestKeyedData
       keyRef.current = key
     }
 
@@ -418,23 +430,26 @@ function useSWR<Data = any, Error = any>(
     ) => {
       // update hook state
       const newState: actionType<Data, Error> = {}
+      let needUpdate = false
 
       if (
         typeof updatedData !== 'undefined' &&
-        !deepEqual(dataRef.current, updatedData)
+        !deepEqual(stateRef.current.data, updatedData)
       ) {
         newState.data = updatedData
-        dataRef.current = updatedData
+        needUpdate = true
       }
 
       // always update error
       // because it can be `undefined`
-      if (errorRef.current !== updatedError) {
+      if (stateRef.current.error !== updatedError) {
         newState.error = updatedError
-        errorRef.current = updatedError
+        needUpdate = true
       }
 
-      dispatch(newState)
+      if (needUpdate) {
+        dispatch(newState)
+      }
 
       keyRef.current = key
       if (shouldRevalidate) {
@@ -497,7 +512,7 @@ function useSWR<Data = any, Error = any>(
     let timer = null
     const tick = async () => {
       if (
-        !errorRef.current &&
+        !stateRef.current.error &&
         (config.refreshWhenHidden || isDocumentVisible()) &&
         (!config.refreshWhenOffline && isOnline())
       ) {
@@ -569,19 +584,40 @@ function useSWR<Data = any, Error = any>(
       error: latestError,
       data: latestData,
       revalidate,
-      isValidating: state.isValidating
+      isValidating: stateRef.current.isValidating
     }
   }
 
-  return {
-    // `key` might be changed in the upcoming hook re-render,
-    // but the previous state will stay
-    // so we need to match the latest key and data (fallback to `initialData`)
-    error: keyRef.current === key ? state.error : initialError,
-    data: keyRef.current === key ? state.data : initialData,
-    revalidate, // handler
-    isValidating: state.isValidating
-  }
+  // define returned state
+  // can be memorized since the state is a ref
+  return useMemo(() => {
+    const state = { revalidate } as responseInterface<Data, Error>
+    Object.defineProperties(state, {
+      error: {
+        // `key` might be changed in the upcoming hook re-render,
+        // but the previous state will stay
+        // so we need to match the latest key and data (fallback to `initialData`)
+        get: function() {
+          stateDependencies.current.error = true
+          return keyRef.current === key ? stateRef.current.error : initialError
+        }
+      },
+      data: {
+        get: function() {
+          stateDependencies.current.data = true
+          return keyRef.current === key ? stateRef.current.data : initialData
+        }
+      },
+      isValidating: {
+        get: function() {
+          stateDependencies.current.isValidating = true
+          return stateRef.current.isValidating
+        }
+      }
+    })
+
+    return state
+  }, [revalidate])
 }
 
 const SWRConfig = SWRConfigContext.Provider


Test patch:
diff --git a/test/use-swr.test.tsx b/test/use-swr.test.tsx
index 383f384..5f3e901 100644
--- a/test/use-swr.test.tsx
+++ b/test/use-swr.test.tsx
@@ -282,6 +282,82 @@ describe('useSWR', () => {
   })
 })
 
+describe('useSWR - loading', () => {
+  afterEach(cleanup)
+
+  const loadData = () => new Promise(res => setTimeout(() => res('data'), 100))
+
+  it('should return loading state', async () => {
+    let renderCount = 0
+    function Page() {
+      const { data, isValidating } = useSWR('is-validating-1', loadData)
+      renderCount++
+      return (
+        <div>
+          hello, {data}, {isValidating ? 'loading' : 'ready'}
+        </div>
+      )
+    }
+
+    const { container } = render(<Page />)
+    expect(container.textContent).toMatchInlineSnapshot(`"hello, , loading"`)
+    await waitForDomChange({ container })
+    expect(container.textContent).toMatchInlineSnapshot(`"hello, data, ready"`)
+
+    //    data       isValidating
+    // -> undefined, false
+    // -> undefined, true
+    // -> data,      false
+    expect(renderCount).toEqual(3)
+  })
+
+  it('should avoid extra rerenders', async () => {
+    let renderCount = 0
+    function Page() {
+      // we never access `isValidating`, so it will not trigger rerendering
+      const { data } = useSWR('is-validating-2', loadData)
+      renderCount++
+      return <div>hello, {data}</div>
+    }
+
+    const { container } = render(<Page />)
+    await waitForDomChange({ container })
+    expect(container.textContent).toMatchInlineSnapshot(`"hello, data"`)
+
+    //    data
+    // -> undefined
+    // -> data
+    expect(renderCount).toEqual(2)
+  })
+
+  it('should avoid extra rerenders while fetching', async () => {
+    let renderCount = 0,
+      dataLoaded = false
+    const loadDataWithLog = () =>
+      new Promise(res =>
+        setTimeout(() => {
+          dataLoaded = true
+          res('data')
+        }, 100)
+      )
+
+    function Page() {
+      // we never access anything
+      useSWR('is-validating-3', loadDataWithLog)
+      renderCount++
+      return <div>hello</div>
+    }
+
+    const { container } = render(<Page />)
+    expect(container.textContent).toMatchInlineSnapshot(`"hello"`)
+
+    await act(() => new Promise(res => setTimeout(res, 110))) // wait
+    // it doesn't re-render, but fetch was triggered
+    expect(renderCount).toEqual(1)
+    expect(dataLoaded).toEqual(true)
+  })
+})
+
 describe('useSWR - refresh', () => {
   afterEach(cleanup)