fix: use justaname batch api call for resolving to ens#786
fix: use justaname batch api call for resolving to ens#786Hugo0 merged 2 commits intopeanut-wallet-devfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request integrates ENS name retrieval into the application by shifting the fetching logic from the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant HP as HomePage Component
participant B as usePrimaryNameBatch Hook
participant WC as WalletCard Component
U->>HP: Load Home Page
HP->>HP: Map wallet addresses (useMemo)
HP->>B: Fetch ENS names for addresses
B-->>HP: Return allPrimaryNames
HP->>WC: Pass primaryName prop to WalletCard
WC-->>U: Render wallet with ENS names
sequenceDiagram
participant U as User
participant WP as WalletDetailsPage Component
participant H as usePrimaryName Hook
participant WC as WalletCard Component
U->>WP: Open Wallet Details
WP->>H: Retrieve primaryName for address (onChain priority)
H-->>WP: Return primaryName
WP->>WC: Pass primaryName prop to WalletCard
WC-->>U: Render wallet with ENS name
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Home/WalletCard.tsx (1)
337-337: Consider adding more robust nullish checking.While the function signature has been correctly updated to require
primaryName, the implementation could be more robust when handling potential edge cases.function getWalletDisplayInfo(wallet: IWallet, username: string, primaryName: string, isRewardsWallet: boolean) { // ... return { - displayName: primaryName || shortenAddressLong(wallet.address), - copyText: primaryName || wallet.address, + displayName: primaryName && primaryName.trim() ? primaryName : shortenAddressLong(wallet.address), + copyText: primaryName && primaryName.trim() ? primaryName : wallet.address, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/(mobile-ui)/home/page.tsx(3 hunks)src/app/(mobile-ui)/wallet/page.tsx(3 hunks)src/components/Home/WalletCard.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (8)
src/app/(mobile-ui)/wallet/page.tsx (3)
20-20: Good addition of the ENS resolution library.Adding the
usePrimaryNamehook from justaname.id is appropriate for enabling ENS name resolution functionality.
42-45: Well implemented ENS resolution.The implementation of the
usePrimaryNamehook with 'onChain' priority is a good approach for fetching ENS names. The hook is correctly set up to use the wallet address when available.
149-149: Proper fallback for primaryName.Good implementation of passing the primaryName to the WalletCard component with a fallback to the wallet's address if primaryName is null or undefined.
src/app/(mobile-ui)/home/page.tsx (3)
21-21: Good choice of batch API.Using the batch version of the justaname API is appropriate for this context where multiple wallet addresses need to be resolved.
51-55: Efficient implementation of batch ENS resolution.The implementation is well-structured:
- Using
useMemofor the wallet addresses array is efficient- The batch hook is properly configured with the
enabledflag- This approach is more efficient than making individual calls for each wallet
235-235: Correct usage of batch resolution results.The primary name is correctly retrieved from the
allPrimaryNamesobject using the wallet address as a key, with a proper fallback to the wallet address if no ENS name is found.src/components/Home/WalletCard.tsx (2)
35-35: Interface update correctly reflects new requirements.Adding the
primaryNameproperty to theWalletCardWalletinterface ensures type safety and makes the dependency on this property explicit.
77-77: Component structure improved.The component now receives the
primaryNameas a prop rather than fetching it internally, which is a good separation of concerns.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/components/Global/ImageGeneration/LinkPreview.tsx (2)
20-37:⚠️ Potential issueCritical issue: React hook used inside a regular function
The
usePrimaryNamehook is being called inside theformatDisplayAddressfunction, which violates React's Rules of Hooks. Hooks can only be called from React function components or custom hooks (names starting with "use").This will cause unpredictable behavior in your application. Here's how to fix it:
- function formatDisplayAddress(address: string): string { - const { primaryName } = usePrimaryName({ - address, - priority: 'onChain', - }) - - if (primaryName) { - return primaryName - } - +// Separate hook function that can be used in components +export function useFormattedAddress(address: string): string { + const { primaryName } = usePrimaryName({ + address, + priority: 'onChain', + }) + + return formatDisplayAddress(address, primaryName) +} + +// Pure function that doesn't use hooks +function formatDisplayAddress(address: string, primaryName?: string | null): string { + if (primaryName) { + return primaryName + } + if (address.startsWith('0x')) { if (isAddress(address)) { return printableAddress(address) } return address } return address }Then, modify the component to use the new hook:
<label style={{ fontSize: '16px', fontWeight: 'bold', color: 'black' }}> - {formatDisplayAddress(address)}{' '} + {useFormattedAddress(address)}{' '} {showEmptyAmountMessage ? PREVIEW_TYPES[previewType].emptyAmountMessage : PREVIEW_TYPES[previewType].message} </label>
39-124: 🛠️ Refactor suggestionUpdate the LinkPreviewImg component to use the new hook pattern
The
LinkPreviewImgcomponent also needs to be updated to properly use the ENS resolution via a hook at the component level rather than within theformatDisplayAddressfunction.Here's how to refactor the component:
export function LinkPreviewImg({ amount, tokenSymbol, address, previewType, }: { amount: string tokenSymbol?: string address: string previewType: PreviewType }) { + // Use the hook at component level + const { primaryName } = usePrimaryName({ + address, + priority: 'onChain', + }) + + const formattedAddress = formatDisplayAddress(address, primaryName) const previewBg = `${ process.env.NEXT_PUBLIC_VERCEL_URL ? `https://${process.env.NEXT_PUBLIC_VERCEL_URL}` : process.env.NEXT_PUBLIC_BASE_URL }/social-preview-bg.png` const isEmptyAmount = !amount || parseFloat(amount) === 0 const showEmptyAmountMessage = previewType === PreviewType.REQUEST && isEmptyAmount // Rest of the component... // In the JSX where address is displayed: <label style={{ fontSize: '16px', fontWeight: 'bold', color: 'black' }}> - {formatDisplayAddress(address)}{' '} + {formattedAddress}{' '} {showEmptyAmountMessage ? PREVIEW_TYPES[previewType].emptyAmountMessage : PREVIEW_TYPES[previewType].message} </label>This approach ensures the hook is called at the component level, following React's rules of hooks, while still providing the ENS name resolution functionality.
🧹 Nitpick comments (2)
src/components/Global/ImageGeneration/LinkPreview.tsx (2)
21-24: Consider adding error handling for the ENS resolutionThe current implementation doesn't handle potential errors that might occur during ENS name resolution.
// In the refactored component export function LinkPreviewImg({...}) { + const [error, setError] = useState<Error | null>(null); const { primaryName, error: ensError } = usePrimaryName({ address, priority: 'onChain', + onError: (err) => { + console.error('Error resolving ENS name:', err); + setError(err); + } }) + // Handle error state if needed + useEffect(() => { + if (ensError) { + console.warn('Failed to resolve ENS name:', ensError); + } + }, [ensError]); const formattedAddress = formatDisplayAddress(address, primaryName)This ensures any issues with the ENS resolution are properly logged and don't silently fail.
2-2: Consider using the batch hook for better performanceAccording to the PR objectives, this PR is about implementing
justaname's batch API call. While you're importing the single-address hook here, consider if this component would benefit from the batch implementation as mentioned in the AI summary.If multiple instances of
LinkPreviewImgare rendered simultaneously with different addresses, using the batch hookusePrimaryNameBatchmight be more efficient than making separate API calls for each address.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Global/ImageGeneration/LinkPreview.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/Global/ImageGeneration/LinkPreview.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#460
File: src/components/Global/ImageGeneration/LinkPreview.tsx:125-128
Timestamp: 2025-04-02T19:11:13.628Z
Learning: 'tokenPrice' is not used currently and will be removed in future PRs.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
usePrimaryNameBatchhook to resolve to ens namesSummary by CodeRabbit
New Features
Refactor