fix: stop infinite rerenders when prop identity changes (#253) [defer…

…-release]
dhis2 · Nov 6, 2019 · 88f8333 · 88f8333
1 parent 017cf06
commit 88f8333
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 2 deletions.
diff --git a/services/data/src/react/hooks/useQueryExecutor.test.tsx b/services/data/src/react/hooks/useQueryExecutor.test.tsx
@@ -8,7 +8,7 @@ const failingExecute = jest.fn(async () => {
 })
 
 describe('useQueryExecutor', () => {
-    afterAll(() => {
+    afterEach(() => {
         jest.clearAllMocks()
     })
     it('When not immediate, should start with called false and loading false', () => {
@@ -114,6 +114,38 @@ describe('useQueryExecutor', () => {
         })
     })
 
+    it("Shouldn't abort+refetch when inputs change on subsequent renders", async () => {
+        const { result, waitForNextUpdate, rerender } = renderHook(
+            ({ onComplete }) =>
+                useQueryExecutor({
+                    execute,
+                    immediate: true,
+                    singular: true,
+                    variables: {},
+                    onComplete,
+                }),
+            {
+                initialProps: { onComplete: () => {} },
+            }
+        )
+
+        expect(result.current).toMatchObject({
+            called: true,
+            loading: true,
+        })
+
+        rerender({ onComplete: () => {} })
+
+        await waitForNextUpdate()
+        expect(result.current).toMatchObject({
+            called: true,
+            loading: false,
+            data: 42,
+        })
+
+        expect(execute).toHaveBeenCalledTimes(1)
+    })
+
     // it('Should respect abort signal', async () => {
     //     const { result, waitForNextUpdate } = renderHook(() =>
     //         useQueryExecutor({

diff --git a/services/data/src/react/hooks/useQueryExecutor.ts b/services/data/src/react/hooks/useQueryExecutor.ts
@@ -77,12 +77,14 @@ export const useQueryExecutor = <ReturnType>({
         [onComplete, onError, singular, theExecute]
     )
 
+    // Don't include immediate or refetch as deps, otherwise unintentional refetches
+    // may be triggered by changes to input, i.e. recreating the onComplete callback
     useEffect(() => {
         if (immediate) {
             refetch()
         }
         return abort
-    }, [immediate, refetch])
+    }, []) // eslint-disable-line react-hooks/exhaustive-deps
 
     return { refetch, abort, ...state }
 }