refactor: usedeepcomparecallback with useDeepCompareMemoize

packages/graphql-hooks/package.json

@@ -38,7 +38,6 @@
   "dependencies": {
     "@aws-crypto/sha256-browser": "^4.0.0",
     "buffer": "^6.0.3",
-    "dequal": "^2.0.0",
     "events": "^3.3.0",
     "extract-files": "^11.0.0",
     "use-deep-compare-effect": "^1.8.1"

packages/graphql-hooks/src/useClientRequest.ts

@@ -1,13 +1,14 @@
-import { dequal } from 'dequal'
-import React, { DependencyList } from 'react'
+import React from 'react'
 import ClientContext from './ClientContext'
 import { Events } from './events'
+import { useDeepCompareCallback } from './useDeepCompareCallback'
 import {
   UseClientRequestOptions,
   FetchData,
   UseClientRequestResult,
   ResetFunction,
-  CacheKeyObject
+  CacheKeyObject,
+  GraphQLResponseError
 } from './types/common-types'
 
 const actionTypes = {
@@ -73,16 +74,6 @@ function reducer(state, action) {
   }
 }
 
-function useDeepCompareCallback(callback, deps: DependencyList) {
-  const ref = React.useRef<DependencyList>()
-
-  if (!dequal(deps, ref.current)) {
-    ref.current = deps
-  }
-
-  return React.useCallback(callback, ref.current as any)
-}
-
 /*
   options include:
 
@@ -94,7 +85,7 @@ function useDeepCompareCallback(callback, deps: DependencyList) {
 function useClientRequest<
   ResponseData = any,
   Variables = object,
-  TGraphQLError = object
+  TGraphQLError extends GraphQLResponseError = GraphQLResponseError
 >(
   query: string,
   initialOpts: UseClientRequestOptions<ResponseData, Variables> = {}
@@ -171,106 +162,118 @@ function useClientRequest<
   }, [])
 
   // arguments to fetchData override the useClientRequest arguments
-  const fetchData = useDeepCompareCallback(
-    newOpts => {
-      const revisedOpts = {
-        ...initialOpts,
-        ...newOpts
-      }
+  const fetchData: FetchData<ResponseData, Variables, TGraphQLError> =
+    useDeepCompareCallback(
+      newOpts => {
+        const revisedOpts = {
+          ...initialOpts,
+          ...newOpts
+        }
 
-      const revisedOperation = {
-        ...operation,
-        variables: revisedOpts.variables,
-        operationName: revisedOpts.operationName
-      }
+        const revisedOperation = {
+          ...operation,
+          variables: revisedOpts.variables,
+          operationName: revisedOpts.operationName
+        }
 
-      if (!isMounted.current) {
-        return Promise.resolve({
-          error: {
-            fetchError: new Error(
-              'fetchData should not be called after hook unmounted'
-            )
-          },
-          loading: false,
-          cacheHit: false
-        })
-      }
+        if (!isMounted.current) {
+          return Promise.resolve({
+            error: {
+              fetchError: new Error(
+                'fetchData should not be called after hook unmounted'
+              )
+            },
+            loading: false,
+            cacheHit: false
+          })
+        }
 
-      const revisedCacheKey = client.getCacheKey(revisedOperation, revisedOpts)
+        const revisedCacheKey = client.getCacheKey(
+          revisedOperation,
+          revisedOpts
+        )
 
-      // NOTE: There is a possibility of a race condition whereby
-      // the second query could finish before the first one, dispatching an old result
-      // see https://github.com/nearform/graphql-hooks/issues/150
-      activeCacheKey.current = revisedCacheKey
+        // NOTE: There is a possibility of a race condition whereby
+        // the second query could finish before the first one, dispatching an old result
+        // see https://github.com/nearform/graphql-hooks/issues/150
+        activeCacheKey.current = revisedCacheKey
 
-      const cacheHit = revisedOpts.skipCache
-        ? null
-        : client.getCache(revisedCacheKey)
+        const cacheHit = revisedOpts.skipCache
+          ? null
+          : (client.getCache(revisedCacheKey) as UseClientRequestResult<
+              ResponseData,
+              TGraphQLError
+            >)
 
-      if (cacheHit) {
-        dispatch({
-          type: actionTypes.CACHE_HIT,
-          result: cacheHit,
-          resetState: stringifiedCacheKey !== JSON.stringify(state.cacheKey)
-        })
+        if (cacheHit) {
+          dispatch({
+            type: actionTypes.CACHE_HIT,
+            result: cacheHit,
+            resetState: stringifiedCacheKey !== JSON.stringify(state.cacheKey)
+          })
 
-        return Promise.resolve(cacheHit)
-      }
+          return Promise.resolve(cacheHit)
+        }
 
-      dispatch({ type: actionTypes.LOADING, initialState })
+        dispatch({ type: actionTypes.LOADING, initialState })
 
-      return client.request(revisedOperation, revisedOpts).then(result => {
-        if (
-          revisedOpts.updateData &&
-          typeof revisedOpts.updateData !== 'function'
-        ) {
-          throw new Error('options.updateData must be a function')
-        }
+        return client
+          .request<ResponseData, TGraphQLError>(revisedOperation, revisedOpts)
+          .then(result => {
+            if (
+              revisedOpts.updateData &&
+              typeof revisedOpts.updateData !== 'function'
+            ) {
+              throw new Error('options.updateData must be a function')
+            }
 
-        const actionResult: any = { ...result }
-        if (revisedOpts.useCache) {
-          actionResult.useCache = true
-          actionResult.cacheKey = revisedCacheKey
-
-          if (client.ssrMode) {
-            const cacheValue = {
-              error: actionResult.error,
-              data: revisedOpts.updateData
-                ? revisedOpts.updateData(state.data, actionResult.data)
-                : actionResult.data
+            const actionResult: any = { ...result }
+            if (revisedOpts.useCache) {
+              actionResult.useCache = true
+              actionResult.cacheKey = revisedCacheKey
+
+              if (client.ssrMode) {
+                const cacheValue = {
+                  error: actionResult.error,
+                  data: revisedOpts.updateData
+                    ? revisedOpts.updateData(state.data, actionResult.data)
+                    : actionResult.data
+                }
+                client.saveCache(revisedCacheKey, cacheValue)
+              }
             }
-            client.saveCache(revisedCacheKey, cacheValue)
-          }
-        }
 
-        if (isMounted.current && revisedCacheKey === activeCacheKey.current) {
-          dispatch({
-            type: actionTypes.REQUEST_RESULT,
-            updateData: revisedOpts.updateData,
-            result: actionResult
-          })
-        }
+            if (
+              isMounted.current &&
+              revisedCacheKey === activeCacheKey.current
+            ) {
+              dispatch({
+                type: actionTypes.REQUEST_RESULT,
+                updateData: revisedOpts.updateData,
+                result: actionResult
+              })
+            }
 
-        if (initialOpts.isMutation) {
-          client.mutationsEmitter.emit(query, {
-            ...revisedOperation,
-            mutation: query,
-            result: actionResult
-          })
-        }
+            if (initialOpts.isMutation) {
+              client.mutationsEmitter.emit(query, {
+                ...revisedOperation,
+                mutation: query,
+                result: actionResult
+              })
+            }
 
-        if (!result?.error && revisedOpts.onSuccess) {
-          if (typeof revisedOpts.onSuccess !== 'function') {
-            throw new Error('options.onSuccess must be a function')
-          }
-          revisedOpts.onSuccess(result, revisedOperation.variables)
-        }
+            if (!result?.error && revisedOpts.onSuccess) {
+              if (typeof revisedOpts.onSuccess !== 'function') {
+                throw new Error('options.onSuccess must be a function')
+              }
+              revisedOpts.onSuccess(result, revisedOperation.variables)
+            }
 
-        return result
-      })
-    },
-    [client, initialOpts, operation]
-  )
+            return result as UseClientRequestResult<ResponseData, TGraphQLError>
+          })
+      },
+      [client, initialOpts, operation]
+    )
 
   // We perform caching after reducer update
   // to include the outcome of updateData.

packages/graphql-hooks/src/useDeepCompareCallback.ts

@@ -0,0 +1,13 @@
+import { useDeepCompareMemoize } from 'use-deep-compare-effect'
+import React from 'react'
+
+type UseCallbackParameters = Parameters<typeof React.useCallback>
+type UseCallbackCallback = UseCallbackParameters[0]
+type DependencyList = UseCallbackParameters[1]
+
+export function useDeepCompareCallback<T extends UseCallbackCallback>(
+  callback: T,
+  deps: DependencyList
+) {
+  return React.useCallback(callback, useDeepCompareMemoize(deps))
+}

packages/graphql-hooks/src/useQuery.ts

@@ -3,7 +3,11 @@ import ClientContext from './ClientContext'
 import createRefetchMutationsMap from './createRefetchMutationsMap'
 import useClientRequest from './useClientRequest'
 
-import { UseQueryOptions, UseQueryResult } from './types/common-types'
+import {
+  GraphQLResponseError,
+  UseQueryOptions,
+  UseQueryResult
+} from './types/common-types'
 
 const defaultOpts = {
   useCache: true,
@@ -14,7 +18,7 @@ const defaultOpts = {
 function useQuery<
   ResponseData = any,
   Variables = object,
-  TGraphQLError = object
+  TGraphQLError extends GraphQLResponseError = GraphQLResponseError
 >(
   query: string,
   opts: UseQueryOptions<ResponseData, Variables> = {}

packages/graphql-hooks/test-jsdom/unit/useDeepCompareCallback.test.ts

@@ -0,0 +1,56 @@
+import { renderHook } from '@testing-library/react'
+import { useDeepCompareCallback } from '../../src/useDeepCompareCallback'
+
+function renderHookWithDeps<T, U>(callback: (props: T) => U, props: T) {
+  return renderHook(callback, {
+    initialProps: props
+  })
+}
+
+describe('useDeepCompareCallback', () => {
+  it('should get the same instance if the deps is an empty array and rerender an empty array', () => {
+    const { result, rerender } = renderHookWithDeps(
+      ({ deps }) =>
+        useDeepCompareCallback(() => {
+          return deps
+        }, deps),
+      {
+        deps: []
+      }
+    )
+    const firstResult = result.current
+    rerender({ deps: [] })
+    expect(result.current).toEqual(firstResult)
+  })
+
+  it('should get the same instance if the deps is defined and rerender with a clone of the deps', () => {
+    const deps = [{ a: 1 }, { b: 1 }]
+    const { result, rerender } = renderHookWithDeps(
+      ({ deps }) =>
+        useDeepCompareCallback(() => {
+          return deps
+        }, deps),
+      {
+        deps
+      }
+    )
+    const firstResult = result.current
+    rerender({ deps: structuredClone(deps) })
+    expect(result.current).toEqual(firstResult)
+  })
+
+  it('should get a new instance if the deps is changed', () => {
+    const { result, rerender } = renderHookWithDeps(
+      ({ deps }) =>
+        useDeepCompareCallback(() => {
+          return deps
+        }, deps),
+      {
+        deps: [{ a: 1 }]
+      }
+    )
+    const firstResult = result.current
+    rerender({ deps: [{ a: 2 }] })
+    expect(result.current).not.toEqual(firstResult)
+  })
+})