-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Batch support for Basenames and ENS names #2102
Merged
+590
−11
Merged
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
2da4b4f
feat: batch ens basenames
cpcramer 423b136
Exporting types
cpcramer 062392a
Temp testing code
cpcramer d83e25d
Batching example
cpcramer a6e7cdd
Remove playground testing code.
cpcramer 16e39ed
testing
cpcramer d382047
cleaning
cpcramer ef96965
Linting
cpcramer 08dc556
Remove avatars stuff
cpcramer 8f3f1dd
Changeset
cpcramer 6544327
Update type
cpcramer 33d3271
nits
cpcramer 3913b44
refactor
cpcramer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@coinbase/onchainkit": patch | ||
--- | ||
|
||
- **feat**: Batch support for Basenames and ENS names. By @cpcramer #2102 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
import { publicClient } from '@/core/network/client'; | ||
import { renderHook, waitFor } from '@testing-library/react'; | ||
import type { Address } from 'viem'; | ||
import { base, mainnet, optimism } from 'viem/chains'; | ||
import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
import { getNames } from '../utils/getNames'; | ||
import { getNewReactQueryTestProvider } from './getNewReactQueryTestProvider'; | ||
import { useNames } from './useNames'; | ||
|
||
vi.mock('@/core/network/client'); | ||
vi.mock('@/core/network/getChainPublicClient', () => ({ | ||
...vi.importActual('@/core/network/getChainPublicClient'), | ||
getChainPublicClient: vi.fn(() => publicClient), | ||
})); | ||
|
||
vi.mock('../utils/getNames'); | ||
|
||
describe('useNames', () => { | ||
const testAddresses = [ | ||
'0x1234567890123456789012345678901234567890', | ||
'0x2345678901234567890123456789012345678901', | ||
'0x3456789012345678901234567890123456789012', | ||
] as Address[]; | ||
|
||
beforeEach(() => { | ||
vi.clearAllMocks(); | ||
vi.resetAllMocks(); | ||
}); | ||
|
||
it('returns the correct ENS names and loading state', async () => { | ||
const testEnsNames = ['user1.eth', 'user2.eth', 'user3.eth']; | ||
|
||
vi.mocked(getNames).mockResolvedValue(testEnsNames); | ||
|
||
const { result } = renderHook( | ||
() => useNames({ addresses: testAddresses }), | ||
{ | ||
wrapper: getNewReactQueryTestProvider(), | ||
}, | ||
); | ||
|
||
expect(result.current.isPending).toBe(true); | ||
expect(result.current.data).toBe(undefined); | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toEqual(testEnsNames); | ||
expect(result.current.isPending).toBe(false); | ||
}); | ||
|
||
expect(getNames).toHaveBeenCalledWith({ | ||
addresses: testAddresses, | ||
chain: mainnet, | ||
}); | ||
}); | ||
|
||
it('returns the correct names for custom chain', async () => { | ||
const testBaseNames = ['user1.base', 'user2.base', 'user3.base']; | ||
|
||
vi.mocked(getNames).mockResolvedValue(testBaseNames); | ||
|
||
const { result } = renderHook( | ||
() => | ||
useNames({ | ||
addresses: testAddresses, | ||
chain: base as unknown as typeof mainnet, | ||
}), | ||
{ | ||
wrapper: getNewReactQueryTestProvider(), | ||
}, | ||
); | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toEqual(testBaseNames); | ||
expect(result.current.isPending).toBe(false); | ||
}); | ||
|
||
expect(getNames).toHaveBeenCalledWith({ | ||
addresses: testAddresses, | ||
chain: base, | ||
}); | ||
}); | ||
|
||
it('returns error for unsupported chain', async () => { | ||
const errorMessage = | ||
'ChainId not supported, name resolution is only supported on Ethereum and Base.'; | ||
vi.mocked(getNames).mockRejectedValue(errorMessage); | ||
|
||
const { result } = renderHook( | ||
() => | ||
useNames({ | ||
addresses: testAddresses, | ||
chain: optimism as unknown as typeof mainnet, | ||
}), | ||
{ | ||
wrapper: getNewReactQueryTestProvider(), | ||
}, | ||
); | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toBe(undefined); | ||
expect(result.current.isPending).toBe(false); | ||
expect(result.current.isError).toBe(true); | ||
expect(result.current.error).toBe(errorMessage); | ||
}); | ||
}); | ||
|
||
it('is disabled when addresses array is empty', async () => { | ||
vi.mocked(getNames).mockImplementation(() => { | ||
throw new Error('This should not be called'); | ||
}); | ||
|
||
const { result } = renderHook(() => useNames({ addresses: [] }), { | ||
wrapper: getNewReactQueryTestProvider(), | ||
}); | ||
|
||
await new Promise((resolve) => setTimeout(resolve, 100)); | ||
|
||
expect(getNames).not.toHaveBeenCalled(); | ||
expect(result.current.isError).toBe(false); | ||
}); | ||
|
||
it('uses the default query options when no queryOptions are provided', async () => { | ||
const testEnsNames = ['user1.eth', 'user2.eth', 'user3.eth']; | ||
|
||
vi.mocked(getNames).mockResolvedValue(testEnsNames); | ||
|
||
renderHook(() => useNames({ addresses: testAddresses }), { | ||
wrapper: getNewReactQueryTestProvider(), | ||
}); | ||
|
||
await waitFor(() => { | ||
expect(getNames).toHaveBeenCalled(); | ||
}); | ||
}); | ||
|
||
it('merges custom queryOptions with default options', async () => { | ||
const testEnsNames = ['user1.eth', 'user2.eth', 'user3.eth']; | ||
const customCacheTime = 120000; | ||
|
||
vi.mocked(getNames).mockResolvedValue(testEnsNames); | ||
|
||
const { result } = renderHook( | ||
() => | ||
useNames({ addresses: testAddresses }, { cacheTime: customCacheTime }), | ||
{ | ||
wrapper: getNewReactQueryTestProvider(), | ||
}, | ||
); | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toEqual(testEnsNames); | ||
}); | ||
|
||
expect(getNames).toHaveBeenCalled(); | ||
}); | ||
|
||
it('creates a stable query key based on addresses and chain', async () => { | ||
const testEnsNames = ['user1.eth', 'user2.eth', 'user3.eth']; | ||
vi.mocked(getNames).mockResolvedValue(testEnsNames); | ||
|
||
const { result, rerender } = renderHook( | ||
(props: { addresses: Address[] }) => | ||
useNames({ addresses: props.addresses }), | ||
{ | ||
wrapper: getNewReactQueryTestProvider(), | ||
initialProps: { addresses: testAddresses }, | ||
}, | ||
); | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toEqual(testEnsNames); | ||
}); | ||
|
||
vi.clearAllMocks(); | ||
|
||
rerender({ addresses: [...testAddresses] }); | ||
|
||
expect(getNames).not.toHaveBeenCalled(); | ||
}); | ||
|
||
it('handles partial failures in name resolution', async () => { | ||
const partialResults = [null, null, null]; | ||
vi.mocked(getNames).mockResolvedValue(partialResults); | ||
|
||
const { result } = renderHook( | ||
() => useNames({ addresses: testAddresses }), | ||
{ | ||
wrapper: getNewReactQueryTestProvider(), | ||
}, | ||
); | ||
|
||
await waitFor(() => { | ||
expect(result.current.data).toEqual(partialResults); | ||
expect(result.current.isPending).toBe(false); | ||
expect(result.current.isError).toBe(false); | ||
}); | ||
|
||
expect(getNames).toHaveBeenCalledWith({ | ||
addresses: testAddresses, | ||
chain: mainnet, | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { getNames } from '@/identity/utils/getNames'; | ||
import { DEFAULT_QUERY_OPTIONS } from '@/internal/constants'; | ||
import { useQuery } from '@tanstack/react-query'; | ||
import type { Address } from 'viem'; | ||
import { mainnet } from 'viem/chains'; | ||
import type { GetNameReturnType, UseQueryOptions } from '../types'; | ||
|
||
export type UseNamesOptions = { | ||
addresses: Address[]; | ||
chain?: typeof mainnet; | ||
}; | ||
|
||
/** | ||
* A React hook that leverages the `@tanstack/react-query` for fetching and optionally caching | ||
* multiple Basenames or ENS names in a single batch request. | ||
*/ | ||
export const useNames = ( | ||
{ addresses, chain = mainnet }: UseNamesOptions, | ||
queryOptions?: UseQueryOptions, | ||
) => { | ||
const { enabled, cacheTime, staleTime, refetchOnWindowFocus } = { | ||
...DEFAULT_QUERY_OPTIONS, | ||
...queryOptions, | ||
}; | ||
|
||
const addressesKey = addresses.join(','); | ||
const queryKey = ['useNames', addressesKey, chain.id]; | ||
|
||
return useQuery<GetNameReturnType[]>({ | ||
queryKey, | ||
queryFn: () => getNames({ addresses, chain }), | ||
gcTime: cacheTime, | ||
staleTime, | ||
enabled: enabled && addresses.length > 0, | ||
refetchOnWindowFocus, | ||
}); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import type { HTMLAttributes, ImgHTMLAttributes, ReactNode } from 'react'; | ||
import type { Address, Chain } from 'viem'; | ||
import type { mainnet } from 'viem/_types/chains/definitions/mainnet'; | ||
|
||
/** | ||
* Note: exported as public Type | ||
|
@@ -160,7 +161,7 @@ export type GetAttestationsOptions = { | |
* Note: exported as public Type | ||
*/ | ||
export type GetAvatar = { | ||
/** The ENS name to fetch the avatar for. */ | ||
/** The ENS or Basename to fetch the avatar for. */ | ||
ensName: string; | ||
/** Optional chain for domain resolution */ | ||
chain?: Chain; | ||
|
@@ -186,6 +187,16 @@ export type GetName = { | |
*/ | ||
export type GetNameReturnType = string | Basename | null; | ||
|
||
/** | ||
* Note: exported as public Type | ||
*/ | ||
export type GetNames = { | ||
/** Array of Ethereum addresses to resolve names for */ | ||
addresses: Address[]; | ||
/** Optional chain for domain resolution */ | ||
chain?: typeof mainnet; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why wouldn't this be Chain? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be Chain, good catch |
||
}; | ||
|
||
/** | ||
* Note: exported as public Type | ||
*/ | ||
|
@@ -272,6 +283,26 @@ export type UseAvatarOptions = { | |
chain?: Chain; | ||
}; | ||
|
||
/** | ||
* Note: exported as public Type | ||
*/ | ||
export type UseNameOptions = { | ||
/** The address for which the ENS or Basename is to be fetched. */ | ||
address: Address; | ||
/** Optional chain for domain resolution */ | ||
chain?: Chain; | ||
}; | ||
|
||
/** | ||
* Note: exported as public Type | ||
*/ | ||
export type UseNamesOptions = { | ||
/** Array of addresses to resolve ENS or Basenames for */ | ||
addresses: Address[]; | ||
/** Optional chain for domain resolution */ | ||
chain?: typeof mainnet; | ||
}; | ||
|
||
/** | ||
* Note: exported as public Type | ||
*/ | ||
|
@@ -283,13 +314,3 @@ export type UseQueryOptions = { | |
/** Stale time in milliseconds */ | ||
staleTime?: number; | ||
}; | ||
|
||
/** | ||
* Note: exported as public Type | ||
*/ | ||
export type UseNameOptions = { | ||
/** The Ethereum address for which the ENS name is to be fetched. */ | ||
address: Address; | ||
/** Optional chain for domain resolution */ | ||
chain?: Chain; | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, allow dev to pass through any query options that Tanstack supports - will make the hook a lot more useful to support specific use cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fast follow along with Avatars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you expose gcTime instead of cacheTime so that when you switch to Tanstack query options it won't be a breaking change and we won't have to support both?