feat: Maintain referential stability when deps are used (#1278)

reactive · Sep 20, 2021 · 09481c0 · 09481c0
1 parent d9ee02e
commit 09481c0
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 22 deletions.
diff --git a/packages/hooks/src/__tests__/useLoading.ts b/packages/hooks/src/__tests__/useLoading.ts
@@ -139,4 +139,36 @@ describe('useLoading()', () => {
     // maintain referential equality
     expect(result.current[0]).toBe(wrappedFunc);
   });
+
+  it('should maintain referential equality if function does', async () => {
+    function fun(value: string) {
+      return new Promise<string>((resolve, reject) =>
+        setTimeout(() => resolve(value), 1000),
+      );
+    }
+    const { result, rerender } = renderHook(() => {
+      return useLoading(fun);
+    });
+    const [cb] = result.current;
+    rerender();
+    expect(result.current[0]).toBe(cb);
+  });
+
+  it('should maintain referential equality based on deps', async () => {
+    const { result, rerender } = renderHook(
+      ({ value }: { value: string }) => {
+        return useLoading(() => {
+          return new Promise<string>((resolve, reject) =>
+            setTimeout(() => resolve(value), 1000),
+          );
+        }, [value]);
+      },
+      { initialProps: { value: 'a' } },
+    );
+    const [cb] = result.current;
+    rerender({ value: 'a' });
+    expect(result.current[0]).toBe(cb);
+    rerender({ value: 'b' });
+    expect(result.current[0]).not.toBe(cb);
+  });
 });
diff --git a/packages/hooks/src/useLoading.ts b/packages/hooks/src/useLoading.ts
@@ -18,36 +18,35 @@ import { useEffect, useState, useRef, useCallback } from 'react';
  }
  ```
  */
-export default function useLoading<
-  F extends (...args: any) => Promise<any>,
-  E extends Error,
->(func: F, deps: readonly any[] = []): [F, boolean, E | undefined] {
+export default function useLoading<F extends (...args: any) => Promise<any>>(
+  func: F,
+  deps?: readonly any[],
+): [F, boolean, Error | undefined] {
   const [loading, setLoading] = useState(false);
-  const [error, setError] = useState<undefined | E>(undefined);
+  const [error, setError] = useState<undefined | Error>(undefined);
   const isMountedRef = useRef(true);
   useEffect(
     () => () => {
       isMountedRef.current = false;
     },
     [],
   );
-  const wrappedFunc = useCallback(
-    async (...args: any) => {
-      setLoading(true);
-      let ret;
-      try {
-        ret = await func(...args);
-      } catch (e: any) {
-        setError(e);
-        throw e;
-      } finally {
-        if (isMountedRef.current) {
-          setLoading(false);
-        }
+  const depsList = deps || [func];
+  const wrappedFunc = useCallback(async (...args: any) => {
+    setLoading(true);
+    let ret;
+    try {
+      ret = await func(...args);
+    } catch (e: any) {
+      setError(e);
+      throw e;
+    } finally {
+      if (isMountedRef.current) {
+        setLoading(false);
       }
-      return ret;
-    },
-    [func, ...deps],
-  );
+    }
+    return ret;
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, depsList);
   return [wrappedFunc as any, loading, error];
 }