fix: Handle unmounting whilst request is in flight (#96)

Prevents `fetchData` within `useClientRequest` from updating state when a request returns, if the component has unmounted.

closes #54

- [x] I have checked the [contributing document](../blob/master/CONTRIBUTING.md)
- [x] I have added or updated any relevant documentation
- [x] I have added or updated any relevant tests

Author: Joezo

packages/graphql-hooks/src/useClientRequest.js

@@ -52,6 +52,7 @@ function reducer(state, action) {
 */
 function useClientRequest(query, initialOpts = {}) {
   const client = React.useContext(ClientContext)
+  const isMounted = React.useRef(true)
   const operation = {
     query,
     variables: initialOpts.variables,
@@ -73,12 +74,22 @@ function useClientRequest(query, initialOpts = {}) {
   // if so the state would be invalid, this effect ensures we reset it back
   const stringifiedCacheKey = JSON.stringify(cacheKey)
   React.useEffect(() => {
-    if (initialOpts.updateData) return // if using updateData we can assume that the consumer cares about the previous data
-    dispatch({ type: actionTypes.RESET_STATE, initialState })
+    if (!initialOpts.updateData) {
+      // if using updateData we can assume that the consumer cares about the previous data
+      dispatch({ type: actionTypes.RESET_STATE, initialState })
+    }
   }, [stringifiedCacheKey]) // eslint-disable-line react-hooks/exhaustive-deps
 
+  React.useEffect(() => {
+    isMounted.current = true
+    return () => {
+      isMounted.current = false
+    }
+  }, [])
+
   // arguments to fetchData override the useClientRequest arguments
   function fetchData(newOpts) {
+    if (!isMounted.current) return Promise.resolve()
     const revisedOpts = {
       ...initialOpts,
       ...newOpts
@@ -118,10 +129,12 @@ function useClientRequest(query, initialOpts = {}) {
         client.cache.set(revisedCacheKey, result)
       }
 
-      dispatch({
-        type: actionTypes.REQUEST_RESULT,
-        result
-      })
+      if (isMounted.current) {
+        dispatch({
+          type: actionTypes.REQUEST_RESULT,
+          result
+        })
+      }
 
       return result
     })

packages/graphql-hooks/test/unit/useClientRequest.test.js

@@ -205,6 +205,50 @@ describe('useClientRequest', () => {
       expect(state).toEqual({ cacheHit: false, loading: false, data: 'data' })
     })
 
+    it('calls request with options & doesnt update state if component unmounts whilst request is in flight', async () => {
+      let fetchData, state
+      const { unmount } = renderHook(
+        () =>
+          ([fetchData, state] = useClientRequest(TEST_QUERY, {
+            variables: { limit: 2 },
+            operationName: 'test'
+          })),
+        {
+          wrapper: Wrapper
+        }
+      )
+
+      const fetchDataPromise = fetchData()
+      unmount()
+      await fetchDataPromise
+
+      expect(mockClient.request).toHaveBeenCalledWith(
+        { operationName: 'test', variables: { limit: 2 }, query: TEST_QUERY },
+        { operationName: 'test', variables: { limit: 2 } }
+      )
+      expect(state).toEqual({ cacheHit: false, loading: true })
+    })
+
+    it('returns undefined instantly if not mounted', async () => {
+      let fetchData, state
+      const { unmount } = renderHook(
+        () =>
+          ([fetchData, state] = useClientRequest(TEST_QUERY, {
+            variables: { limit: 2 },
+            operationName: 'test'
+          })),
+        {
+          wrapper: Wrapper
+        }
+      )
+
+      unmount()
+      const result = await fetchData()
+      expect(result).toBe(undefined)
+      expect(mockClient.request).not.toHaveBeenCalled()
+      expect(state).toEqual({ cacheHit: false, loading: true })
+    })
+
     it('calls request with revised options', async () => {
       let fetchData
       renderHook(