refactor: use getter functions to avoid unnecessary re-renders

[koba04]

This PR fixes #780. I've left a comment on the intention.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 352e03f:

Sandbox	Source
SWR-Basic	Configuration
SWR-States	Configuration
SWR-Infinite	Configuration

[huozhi]

Object.assign seems to have the same effect with object spread?

var a = { get x() { console.log('getx'); return 'x' } }
Object.assign({}, a) // "getx"

I think you can just assign to swr itself? the 1st arg won't be traversed.

Suggested change

	return Object.assign({}, swr, {
	return Object.assign(swr, {

[koba04]

Thank you for your feedback!
You are right, to avoid triggering the getter function, I have to mutate a directly.

var a = {
  get x() { console.log('getx'); return 'x' }
}

Object.assign(a, { foo: 'foo'}) // dosn't trigger the getter

But, that would break the current behavior, so I'll investigate a way to avoid triggering re-renders.

[koba04]

A way I came up with is to define getter functions to evaluate properties lazily.

diff --git a/src/use-swr-infinite.ts b/src/use-swr-infinite.ts
index 3b26b67..949cdc8 100644
--- a/src/use-swr-infinite.ts
+++ b/src/use-swr-infinite.ts
@@ -216,12 +216,16 @@ function useSWRInfinite<Data = any, Error = any>(
     [mutate, pageCountCacheKey]
   )
 
-  // Use Object.assign to avoid unnecessary re-renders caused by triggering all the getters of the returned swr object
-  return Object.assign({}, swr, {
+  // Use getter functions to avoid unnecessary re-renders caused by triggering all the getters of the returned swr object
+  return {
+    get error() { return swr.error },
+    get data() { return swr.data },
+    get revalidate() { return swr.revalidate },
+    get isValidating() { return swr.isValidating },
     mutate,
     size,
     setSize
-  }) as SWRInfiniteResponseInterface<Data, Error>
+  } as SWRInfiniteResponseInterface<Data, Error>
 }
 
 export {

[shuding]

Can you explain a bit why Object.assign(swr, { ... }) won't work? It does trigger the getter when accessing a.x:

[koba04]

@shuding
The reason is that this mutates swr directly, and it breaks where the swr.mutate variable is used.

swr/src/use-swr-infinite.ts

Line 198 in 3121592

return swr.mutate(data, shouldRevalidate)

It leads to failing many tests. I can fix it like the following, but the useSWRInfinite › should keep mutate referential equal test is still failing. Should I investigate the way to pass the test rather than using getter functions?

diff --git a/src/use-swr-infinite.ts b/src/use-swr-infinite.ts
index a0eda9c..ab0541e 100644
--- a/src/use-swr-infinite.ts
+++ b/src/use-swr-infinite.ts
@@ -184,6 +184,8 @@ function useSWRInfinite<Data = any, Error = any>(
     dataRef.current = swr.data
   }, [swr.data])

+  const _mutate = swr.mutate
+
   const mutate = useCallback(
     (data, shouldRevalidate = true) => {
       if (shouldRevalidate && typeof data !== 'undefined') {
@@ -195,9 +197,9 @@ function useSWRInfinite<Data = any, Error = any>(
         cache.set(contextCacheKey, { force: true })
       }

-      return swr.mutate(data, shouldRevalidate)
+      return _mutate(data, shouldRevalidate)
     },
-    [swr.mutate, contextCacheKey]
+    [_mutate, contextCacheKey]
   )

[koba04]

The failed test is here.

swr/test/use-swr-infinite.test.tsx

Line 441 in 3121592

expect(setSize).toEqual(setters[0])

[shuding]

That's a good point! I think we need to find an elegant way to fix it then... let me see

[koba04]

@shuding I agree with you, that's not an ideal solution. I'll try to find it. Thank you for your feedback!

[shuding]

Let's use the same logic like use-swr then:

const swrInfinite = { size, setSize, mutate }
Object.defineProperties(swrInfinite, {
  error: {
    get: () => swr.error,
    enumerable: true
  },
  ...
})

The reason of not using the latest get syntax is that we have to specify enumerable: true.

[koba04]

I've updated specifing enumerable: true!

I also came up with the way to use useMemo like 116fced, but React says "You may rely on useMemo as a performance optimization, not as a semantic guarantee. ", so I've reverted the change.

[koba04]

I've updated the implementation from Object.assign to getter functions to address the issue mentioned at #890 (comment).

[shuding]

Thanks for the help!

[koba04]

Thank you!