[TASK-6945] refactor(claim): use new token selector #535
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/context/tokenSelector.context.tsx (1)
Line range hint
117-124: Optimize loading state managementThere's a potential race condition with
setIsFetchingTokenData(true)being called twice:
- Before
setRefetchXchainRoute- Inside
fetchAndSetTokenPriceThis could lead to inconsistent loading states if the component unmounts between these calls.
Consider consolidating the loading state management:
if (selectedTokenAddress && selectedChainID) { - setIsFetchingTokenData(true) setSelectedTokenData(undefined) setRefetchXchainRoute(true) setSelectedTokenPrice(undefined) setSelectedTokenDecimals(undefined) setInputDenomination('TOKEN') fetchAndSetTokenPrice(selectedTokenAddress, selectedChainID) return () => { isCurrent = false } }src/components/Claim/Claim.tsx (3)
139-141: Consider cleaning up states before early return.When a link is already claimed, we should reset any previously set states (like selectedChainID and selectedTokenAddress) before returning, to prevent stale state issues.
if (linkDetails.claimed) { setLinkState(_consts.claimLinkStateType.ALREADY_CLAIMED) + setSelectedChainID(undefined) + setSelectedTokenAddress(undefined) return }
147-157: Simplify user-specific logic and improve null safety.The code can be improved by:
- Removing redundant address check in estimatePoints
- Simplifying the USD amount calculation
if (address) { setRecipient({ name: '', address }) const estimatedPoints = await estimatePoints({ - address: address ?? '', + address, chainId: linkDetails.chainId, - amountUSD: Number(linkDetails.tokenAmount) * (tokenPrice?.price ?? 0), + amountUSD: Number(linkDetails.tokenAmount) * tokenPrice, actionType: ActionType.CLAIM, }) setEstimatedPoints(estimatedPoints) }
159-162: Fix indentation in the else block.The indentation in the else block is inconsistent with the surrounding code.
if (address && linkDetails.senderAddress === address) { setLinkState(_consts.claimLinkStateType.CLAIM_SENDER) } else { - setLinkState(_consts.claimLinkStateType.CLAIM) + setLinkState(_consts.claimLinkStateType.CLAIM) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Claim/Claim.tsx(1 hunks)src/components/Claim/Link/Initial.view.tsx(11 hunks)src/components/Global/TokenSelector/TokenSelectorXChain.tsx(0 hunks)src/context/tokenSelector.context.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Global/TokenSelector/TokenSelectorXChain.tsx
🧰 Additional context used
📓 Learnings (1)
src/context/tokenSelector.context.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#413
File: src/context/tokenSelector.context.tsx:118-123
Timestamp: 2024-11-12T09:39:20.720Z
Learning: In the `TokenContextProvider` component within `src/context/tokenSelector.context.tsx`, in the TypeScript React application, when data changes and before calling `fetchAndSetTokenPrice`, it is necessary to reset `selectedTokenData`, `selectedTokenPrice`, `selectedTokenDecimals`, and `inputDenomination` to discard stale data.
🔇 Additional comments (5)
src/context/tokenSelector.context.tsx (3)
Line range hint 12-19: LGTM: Context interface changes are well-structured
The new refetchXchainRoute state and its setter are properly integrated into the context interface with appropriate typing.
Line range hint 45-54: LGTM: State management is consistent
The new refetchXchainRoute state is properly initialized and integrated into the existing state management pattern.
117-117: Verify integration with claim flow
Since this change is part of updating the claim flow, let's verify the integration with the claim components.
✅ Verification successful
Let me check the specific usage context in the Initial.view.tsx to understand how refetchXchainRoute is being used.
Integration with claim flow is properly implemented
The verification shows that refetchXchainRoute is correctly integrated in the claim flow:
- It's properly consumed in
Initial.view.tsxwhere it triggers route fetching when set to true - The context update in
tokenSelector.context.tsxcorrectly triggers a re-fetch of cross-chain routes when token or chain selection changes - The state reset and cleanup is handled appropriately in both components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using the token selector context in the claim flow
echo "Searching for claim components using tokenSelectorContext..."
rg -l "tokenSelectorContext" "src/components/Claim"
# Search for specific usage of refetchXchainRoute
echo "Checking usage of refetchXchainRoute..."
rg "refetchXchainRoute" "src/components/Claim"
Length of output: 725
Script:
#!/bin/bash
# Get the context around refetchXchainRoute usage in Initial.view
echo "Checking context of refetchXchainRoute usage in Initial.view.tsx..."
rg -B 5 -A 5 "refetchXchainRoute" "src/components/Claim/Link/Initial.view.tsx"
# Check the token selector context implementation
echo "Checking token selector context implementation..."
rg -B 5 -A 5 "setRefetchXchainRoute" "src/context/tokenSelector.context.tsx"
Length of output: 3642
src/components/Claim/Claim.tsx (2)
134-135: LGTM: Token selector state initialization is properly placed.
The immediate state updates for chain ID and token address after link validation ensure correct initialization of the token selector.
134-162: Verify integration with the new token selector across the codebase.
The changes successfully integrate the new token selector into the claim flow. Let's verify that all components consuming these states are updated accordingly.
✅ Verification successful
Based on the search results, I can now provide a final response:
The token selector integration is properly implemented across the codebase
The changes in the Claim component correctly integrate with the token selector context. The context is consistently used across all relevant components including:
- Claim flow components (Claim.tsx, Initial.view.tsx)
- Token selector components
- Related features (Create, Request, Offramp)
All components are using the same context (tokenSelectorContext) and accessing the state variables (selectedChainID, selectedTokenAddress) in a consistent manner.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using the token selector context
rg -l "tokenSelectorContext" --type tsx
# Search for any remaining references to the old TokenSelectorXChain component
rg -l "TokenSelectorXChain"
Length of output: 155
Script:
#!/bin/bash
# Search for components using the token selector context with correct file extension
rg -l "tokenSelectorContext" --type ts --type tsx
# Search for token selector related state variables in Claim component
rg "setSelectedChainID|setSelectedTokenAddress" -A 2 -B 2 src/components/Claim/
Length of output: 1945
Script:
#!/bin/bash
# Search for the token selector context definition and usage
rg "tokenSelectorContext" -A 5 -B 5
# Search for any other components that might be using the token selector states
rg "selectedChainID|selectedTokenAddress" --type ts --type tsx
Length of output: 24063
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/Claim/Link/Initial.view.tsx (3)
Line range hint
116-135: Consider enhancing error message handlingWhile the error handling is good, the generic "Error claiming link" message could be more informative. Consider passing through the specific error details from the ErrorHandler.
- throw new Error('Error claiming link') + throw new Error('Failed to claim link: Transaction unsuccessful')
Line range hint
152-171: Simplify route availability check logicThe nested conditions for checking route availability could be simplified for better readability.
- if (!tokenName || !chainName) { - if (!crossChainDetails) { - setErrorState({ - showError: true, - errorMessage: 'offramp unavailable', - }) - return - } + if ((!tokenName || !chainName) && !crossChainDetails) { + setErrorState({ + showError: true, + errorMessage: 'offramp unavailable', + }) + return + }
294-305: Consider memoizing chain comparison valuesThe chain and token address comparisons in the useEffect could benefit from memoization to prevent unnecessary re-renders.
+ const isSameChain = useMemo( + () => + selectedChainID === claimLinkData.chainId && + areTokenAddressesEqual(selectedTokenAddress, claimLinkData.tokenAddress), + [selectedChainID, selectedTokenAddress, claimLinkData.chainId, claimLinkData.tokenAddress] + ) useEffect(() => { - if ( - selectedChainID === claimLinkData.chainId && - areTokenAddressesEqual(selectedTokenAddress, claimLinkData.tokenAddress) - ) { + if (isSameChain) { setIsXChain(false) setSelectedRoute(null) setHasFetchedRoute(false) } else { setIsXChain(true) } - }, [selectedChainID, selectedTokenAddress, claimLinkData.chainId, claimLinkData.tokenAddress]) + }, [isSameChain])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Claim/Link/Initial.view.tsx(12 hunks)
🔇 Additional comments (3)
src/components/Claim/Link/Initial.view.tsx (3)
12-20: LGTM! Improved import organization
The change to import utility functions individually rather than using a catch-all import improves code maintainability and makes dependencies explicit.
73-82: LGTM! Enhanced token selector context
The expanded context usage properly handles cross-chain functionality with clear state management for chain ID, token address, and cross-chain route refetching.
426-434: LGTM! Clean token selector implementation
The new TokenSelector implementation with reset functionality properly aligns with the PR objectives of replacing the old token selector.
Claim flow as still using old token selector. This PR removes that token selector and uses the new one that is already used in others flows (request)