fix useWalletClient and useConnectorClient performance issues (#3740

) * fix(useWalletClient): optimize `useEffect` * fix(useConnectorClient): optimize `useEffect` * run changeset * fix(useWalletClient.test.ts): test the `useWalletClient` hook * add tests * cleanup * fix(tsconfig): remove trailing comma * fix ts issues * refactor: tweaks * Update .changeset/olive-ravens-eat.md * refactor: do not remove on first render --------- Co-authored-by: Tom Meagher <tom@meagher.co>
wevm · Mar 27, 2024 · 3373c64 · 3373c64
1 parent 311f007
commit 3373c64
Show file tree

Hide file tree

Showing 8 changed files with 214 additions and 54 deletions.
diff --git a/.changeset/olive-ravens-eat.md b/.changeset/olive-ravens-eat.md
@@ -0,0 +1,5 @@
+---
+"wagmi": patch
+---
+
+Removed unnecessary re-renders from  `useConnectorClient` and `useWalletClient`.
diff --git a/...eact/src/hooks/useConnectorClient.test.ts → ...act/src/hooks/useConnectorClient.test.tsx b/...eact/src/hooks/useConnectorClient.test.ts → ...act/src/hooks/useConnectorClient.test.tsx
@@ -1,8 +1,10 @@
 import { connect, disconnect } from '@wagmi/core'
 import { config, wait } from '@wagmi/test'
-import { renderHook, waitFor } from '@wagmi/test/react'
+import { render, renderHook, waitFor } from '@wagmi/test/react'
+import * as React from 'react'
 import { expect, test } from 'vitest'
 
+import { useAccount } from './useAccount.js'
 import { useConnect } from './useConnect.js'
 import { useConnectorClient } from './useConnectorClient.js'
 import { useDisconnect } from './useDisconnect.js'
@@ -122,19 +124,15 @@ test('behavior: connect and disconnect', async () => {
 })
 
 test('behavior: switch chains', async () => {
+  await connect(config, { connector })
+
   const { result } = renderHook(() => ({
-    useConnect: useConnect(),
     useConnectorClient: useConnectorClient(),
-    useDisconnect: useDisconnect(),
     useSwitchChain: useSwitchChain(),
   }))
 
   expect(result.current.useConnectorClient.data).not.toBeDefined()
 
-  result.current.useConnect.connect({
-    connector: result.current.useConnect.connectors[0]!,
-  })
-
   await waitFor(() =>
     expect(result.current.useConnectorClient.data).toBeDefined(),
   )
@@ -152,11 +150,7 @@ test('behavior: switch chains', async () => {
   )
   expect(result.current.useConnectorClient.data?.chain.id).toEqual(1)
 
-  result.current.useDisconnect.disconnect()
-
-  await waitFor(() =>
-    expect(result.current.useConnectorClient.data).not.toBeDefined(),
-  )
+  await disconnect(config, { connector })
 })
 
 test('behavior: disabled when properties missing', async () => {
@@ -165,3 +159,72 @@ test('behavior: disabled when properties missing', async () => {
   await wait(100)
   await waitFor(() => expect(result.current.isPending).toBeTruthy())
 })
+
+test('behavior: re-render does not invalidate query', async () => {
+  const { getByTestId } = render(<Parent />)
+
+  getByTestId('connect').click()
+  await waitFor(() => {
+    expect(getByTestId('address').innerText).toContain(
+      '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266',
+    )
+    expect(getByTestId('client').innerText).toBeTruthy()
+
+    expect(getByTestId('child-client').innerText).toBeTruthy()
+    expect(getByTestId('render-count').innerText).toEqual('1')
+  })
+
+  const initialClient = getByTestId('child-client').innerText
+
+  getByTestId('rerender').click()
+  await waitFor(() => {
+    expect(getByTestId('render-count').innerText).toEqual('2')
+  })
+  await wait(200)
+
+  expect(getByTestId('child-client').innerText).toEqual(initialClient)
+})
+
+function Parent() {
+  const [renderCount, setRenderCount] = React.useState(1)
+
+  const { connectors, connect } = useConnect()
+  const { address } = useAccount()
+  const { data } = useConnectorClient()
+
+  return (
+    <>
+      <div data-testid="address">{address}</div>
+      <div data-testid="client">{data?.uid}</div>
+      <Child key={renderCount} renderCount={renderCount} />
+
+      <button
+        type="button"
+        data-testid="connect"
+        onClick={() => connect({ connector: connectors[0]! })}
+      >
+        Connect
+      </button>
+      <button
+        type="button"
+        data-testid="rerender"
+        onClick={() => setRenderCount((prev) => prev + 1)}
+      >
+        Re-render
+      </button>
+    </>
+  )
+}
+
+function Child(props: {
+  renderCount: number
+}) {
+  const { renderCount } = props
+  const { data } = useConnectorClient()
+  return (
+    <div>
+      <span data-testid="child-client">{data?.uid}</span>
+      <span data-testid="render-count">{renderCount}</span>
+    </div>
+  )
+}
diff --git a/packages/react/src/hooks/useConnectorClient.ts b/packages/react/src/hooks/useConnectorClient.ts
@@ -14,7 +14,7 @@ import {
   type GetConnectorClientQueryKey,
   getConnectorClientQueryOptions,
 } from '@wagmi/core/query'
-import { useEffect } from 'react'
+import { useEffect, useRef } from 'react'
 
 import type { ConfigParameter } from '../types/properties.js'
 import {
@@ -63,9 +63,9 @@ export function useConnectorClient<
 >(
   parameters: UseConnectorClientParameters<config, chainId, selectData> = {},
 ): UseConnectorClientReturnType<config, chainId, selectData> {
-  const { query = {} } = parameters
+  const { query = {}, ...rest } = parameters
 
-  const config = useConfig(parameters)
+  const config = useConfig(rest)
   const queryClient = useQueryClient()
   const { address, connector, status } = useAccount()
   const chainId = useChainId()
@@ -80,11 +80,19 @@ export function useConnectorClient<
   })
   const enabled = Boolean(status !== 'disconnected' && (query.enabled ?? true))
 
+  const addressRef = useRef(address)
   // biome-ignore lint/nursery/useExhaustiveDependencies: `queryKey` not required
   useEffect(() => {
-    // invalidate when address changes
-    if (address) queryClient.invalidateQueries({ queryKey })
-    else queryClient.removeQueries({ queryKey }) // remove when account is disconnected
+    const previousAddress = addressRef.current
+    if (!address && previousAddress) {
+      // remove when account is disconnected
+      queryClient.removeQueries({ queryKey })
+      addressRef.current = undefined
+    } else if (address !== previousAddress) {
+      // invalidate when address changes
+      queryClient.invalidateQueries({ queryKey })
+      addressRef.current = address
+    }
   }, [address, queryClient])
 
   return useQuery({

diff --git a/...s/react/src/hooks/useWalletClient.test.ts → .../react/src/hooks/useWalletClient.test.tsx b/...s/react/src/hooks/useWalletClient.test.ts → .../react/src/hooks/useWalletClient.test.tsx
@@ -1,20 +1,22 @@
 import { connect, disconnect } from '@wagmi/core'
-import { config } from '@wagmi/test'
-import { renderHook, waitFor } from '@wagmi/test/react'
+import { config, wait } from '@wagmi/test'
+import { render, renderHook, waitFor } from '@wagmi/test/react'
+import * as React from 'react'
 import { expect, test } from 'vitest'
 
+import { useAccount } from './useAccount.js'
 import { useConnect } from './useConnect.js'
-import { useConnectorClient } from './useConnectorClient.js'
 import { useDisconnect } from './useDisconnect.js'
 import { useSwitchChain } from './useSwitchChain.js'
+import { useWalletClient } from './useWalletClient.js'
 
 // Almost identical implementation to `useConnectorClient` (except for return type)
 // Should update both in tandem
 
 const connector = config.connectors[0]!
 
 test('default', async () => {
-  const { result } = renderHook(() => useConnectorClient())
+  const { result } = renderHook(() => useWalletClient())
 
   await waitFor(() => expect(result.current.isPending).toBeTruthy())
 
@@ -43,7 +45,7 @@ test('default', async () => {
       "isStale": true,
       "isSuccess": false,
       "queryKey": [
-        "connectorClient",
+        "walletClient",
         {
           "chainId": 1,
           "connectorUid": undefined,
@@ -58,7 +60,7 @@ test('default', async () => {
 test('behavior: connected on mount', async () => {
   await connect(config, { connector })
 
-  const { result } = renderHook(() => useConnectorClient())
+  const { result } = renderHook(() => useWalletClient())
 
   await waitFor(() => expect(result.current.isSuccess).toBeTruthy())
 
@@ -103,61 +105,118 @@ test('behavior: connected on mount', async () => {
 test('behavior: connect and disconnect', async () => {
   const { result } = renderHook(() => ({
     useConnect: useConnect(),
-    useConnectorClient: useConnectorClient(),
+    useWalletClient: useWalletClient(),
     useDisconnect: useDisconnect(),
   }))
 
-  expect(result.current.useConnectorClient.data).not.toBeDefined()
+  expect(result.current.useWalletClient.data).not.toBeDefined()
 
   result.current.useConnect.connect({
     connector: result.current.useConnect.connectors[0]!,
   })
 
-  await waitFor(() =>
-    expect(result.current.useConnectorClient.data).toBeDefined(),
-  )
+  await waitFor(() => expect(result.current.useWalletClient.data).toBeDefined())
 
   result.current.useDisconnect.disconnect()
 
   await waitFor(() =>
-    expect(result.current.useConnectorClient.data).not.toBeDefined(),
+    expect(result.current.useWalletClient.data).not.toBeDefined(),
   )
 })
 
 test('behavior: switch chains', async () => {
+  await connect(config, { connector })
+
   const { result } = renderHook(() => ({
-    useConnect: useConnect(),
-    useConnectorClient: useConnectorClient(),
-    useDisconnect: useDisconnect(),
+    useWalletClient: useWalletClient(),
     useSwitchChain: useSwitchChain(),
   }))
 
-  expect(result.current.useConnectorClient.data).not.toBeDefined()
+  expect(result.current.useWalletClient.data).not.toBeDefined()
 
-  result.current.useConnect.connect({
-    connector: result.current.useConnect.connectors[0]!,
-  })
-
-  await waitFor(() =>
-    expect(result.current.useConnectorClient.data).toBeDefined(),
-  )
+  await waitFor(() => expect(result.current.useWalletClient.data).toBeDefined())
 
   result.current.useSwitchChain.switchChain({ chainId: 456 })
   await waitFor(() => {
     expect(result.current.useSwitchChain.isSuccess).toBeTruthy()
     result.current.useSwitchChain.reset()
   })
-  expect(result.current.useConnectorClient.data?.chain.id).toEqual(456)
+  expect(result.current.useWalletClient.data?.chain.id).toEqual(456)
 
   result.current.useSwitchChain.switchChain({ chainId: 1 })
   await waitFor(() =>
     expect(result.current.useSwitchChain.isSuccess).toBeTruthy(),
   )
-  expect(result.current.useConnectorClient.data?.chain.id).toEqual(1)
+  expect(result.current.useWalletClient.data?.chain.id).toEqual(1)
 
-  result.current.useDisconnect.disconnect()
+  await disconnect(config, { connector })
+})
 
-  await waitFor(() =>
-    expect(result.current.useConnectorClient.data).not.toBeDefined(),
-  )
+test('behavior: re-render does not invalidate query', async () => {
+  const { getByTestId } = render(<Parent />)
+
+  getByTestId('connect').click()
+  await waitFor(() => {
+    expect(getByTestId('address').innerText).toContain(
+      '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266',
+    )
+    expect(getByTestId('client').innerText).toBeTruthy()
+
+    expect(getByTestId('child-client').innerText).toBeTruthy()
+    expect(getByTestId('render-count').innerText).toEqual('1')
+  })
+
+  const initialClient = getByTestId('child-client').innerText
+
+  getByTestId('rerender').click()
+  await waitFor(() => {
+    expect(getByTestId('render-count').innerText).toEqual('2')
+  })
+  await wait(200)
+
+  expect(getByTestId('child-client').innerText).toEqual(initialClient)
 })
+
+function Parent() {
+  const [renderCount, setRenderCount] = React.useState(1)
+
+  const { connectors, connect } = useConnect()
+  const { address } = useAccount()
+  const { data } = useWalletClient()
+
+  return (
+    <>
+      <div data-testid="address">{address}</div>
+      <div data-testid="client">{data?.uid}</div>
+      <Child key={renderCount} renderCount={renderCount} />
+
+      <button
+        type="button"
+        data-testid="connect"
+        onClick={() => connect({ connector: connectors[0]! })}
+      >
+        Connect
+      </button>
+      <button
+        type="button"
+        data-testid="rerender"
+        onClick={() => setRenderCount((prev) => prev + 1)}
+      >
+        Re-render
+      </button>
+    </>
+  )
+}
+
+function Child(props: {
+  renderCount: number
+}) {
+  const { renderCount } = props
+  const { data } = useWalletClient()
+  return (
+    <div>
+      <span data-testid="child-client">{data?.uid}</span>
+      <span data-testid="render-count">{renderCount}</span>
+    </div>
+  )
+}
diff --git a/packages/react/src/hooks/useWalletClient.ts b/packages/react/src/hooks/useWalletClient.ts
@@ -17,7 +17,7 @@ import {
   type GetWalletClientQueryKey,
   getWalletClientQueryOptions,
 } from '@wagmi/core/query'
-import { useEffect } from 'react'
+import { useEffect, useRef } from 'react'
 
 import type { ConfigParameter } from '../types/properties.js'
 import {
@@ -83,11 +83,19 @@ export function useWalletClient<
   )
   const enabled = Boolean(status !== 'disconnected' && (query.enabled ?? true))
 
+  const addressRef = useRef(address)
   // biome-ignore lint/nursery/useExhaustiveDependencies: `queryKey` not required
   useEffect(() => {
-    // invalidate when address changes
-    if (address) queryClient.invalidateQueries({ queryKey })
-    else queryClient.removeQueries({ queryKey }) // remove when account is disconnected
+    const previousAddress = addressRef.current
+    if (!address && previousAddress) {
+      // remove when account is disconnected
+      queryClient.removeQueries({ queryKey })
+      addressRef.current = undefined
+    } else if (address !== previousAddress) {
+      // invalidate when address changes
+      queryClient.invalidateQueries({ queryKey })
+      addressRef.current = address
+    }
   }, [address, queryClient])
 
   return useQuery({

diff --git a/packages/react/tsconfig.json b/packages/react/tsconfig.json
@@ -1,5 +1,8 @@
 {
   "extends": "./tsconfig.build.json",
-  "include": ["src/**/*.ts", "test/**/*.ts"],
+  "compilerOptions": {
+    "jsx": "preserve"
+  },
+  "include": ["src/**/*.ts", "src/**/*.tsx", "test/**/*.ts", "test/**/*.tsx"],
   "exclude": []
 }