[ISSUE-213] refactor: load token pair onto selector context before advancing from pay #461
Conversation
Instead of having initial data and then updating it, load the data when initializing the context if preferences are found.
This will ensure we start with the intended token selector context
Bring back spinning peanutman!
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request introduces significant updates 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: 1
🧹 Outside diff range and nitpick comments (7)
src/components/Request/Pay/Pay.tsx (1)
Line range hint
177-185: Good fix: Correct loading state rendering.This change addresses the issue mentioned in the PR objectives where an incorrect state check caused the peanutman element to disappear. The loading spinner is now correctly displayed when the
linkStateisLOADING.For improved clarity, consider adding a comment explaining the purpose of this condition:
// Display loading spinner while fetching request link details {linkState === _consts.IRequestLinkState.LOADING && ( // ... existing code ... )}This will help future developers understand the intent behind this rendering logic.
src/context/tokenSelector.context.tsx (2)
41-41: Simplify conditional check using optional chainingIn line 41, you can simplify the conditional by using optional chaining, which improves readability.
Apply this diff:
-if (prefs && prefs.tokenAddress && prefs.chainId && prefs.decimals) { +if (prefs?.tokenAddress && prefs.chainId && prefs.decimals) {🧰 Tools
🪛 Biome
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
40-41: Avoid redundant calls togetPeanutPreferences()You are calling
utils.getPeanutPreferences()twice and storing the result inprefsandpreferences. Consider reusing theprefsvariable to avoid redundant calls and improve efficiency.Apply this diff:
// At the beginning of the component const prefs = utils.getPeanutPreferences() // Remove the second call -const preferences = utils.getPeanutPreferences() // In the resetTokenContextProvider function const resetTokenContextProvider = () => { - if (preferences && preferences.tokenAddress == selectedTokenAddress && preferences.chainId == selectedChainID) + if (prefs && prefs.tokenAddress == selectedTokenAddress && prefs.chainId == selectedChainID) return - setSelectedChainID(preferences?.tokenAddress ? preferences.chainId : '10') + setSelectedChainID(prefs?.tokenAddress ? prefs.chainId : '10') setSelectedTokenAddress( - preferences?.tokenAddress ? preferences.tokenAddress : '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85' + prefs?.tokenAddress ? prefs.tokenAddress : '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85' ) setSelectedTokenPrice(undefined) setSelectedTokenDecimals(undefined) }Also applies to: 54-54
🧰 Tools
🪛 Biome
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/Request/Pay/Views/Initial.view.tsx (4)
Line range hint
119-161: Potential undefined value forselectedTokenDataleading to runtime errorsIn the
useEffecthook starting at line 119,selectedTokenDatais used with a non-null assertion (selectedTokenData!). However, there is a scenario whereselectedTokenDatacould beundefined, leading to a runtime error.To ensure safety, add a check to confirm that
selectedTokenDatais defined before callingestimateTxFee():+if (!selectedTokenData) { + return; +} estimateTxFee();Additionally, update the dependency array to include
selectedTokenDatato reflect its usage accurately.
Line range hint
164-172: Unnecessary setting of loading state may affect user experienceIn the
useEffecthook at line 164,setLoadingState('Loading')is called unconditionally wheneverselectedChainIDorselectedTokenAddresschanges. This may lead to unintended loading states and affect the user experience.Consider updating the loading state conditionally:
useEffect(() => { - setLoadingState('Loading') + if (isFetchingTokenData) { + setLoadingState('Loading') + } clearError() const isXChain = selectedChainID !== requestLinkData.chainId || !utils.areTokenAddressesEqual(selectedTokenAddress, requestLinkData.tokenAddress) setIsXChain(isXChain) }, [selectedChainID, selectedTokenAddress, isFetchingTokenData])
Line range hint
174-195: OptimizinguseEffectdependencies and logicThe
useEffecthook starting at line 174 could be optimized by refining the dependencies and avoiding redundant computations.
- Include
fetchTokenSymbolin the dependency array to prevent potential stale closures.- Memoize computations or extract them into separate functions if they are expensive.
- Ensure that
requestLinkDataandtokenPriceDatachanges are accurately reflected.useEffect(() => { const chainDetails = consts.peanutTokenDetails.find((chain) => chain.chainId === requestLinkData.chainId) const logoURI = chainDetails?.tokens.find((token) => utils.areTokenAddressesEqual(token.address, requestLinkData.tokenAddress) )?.logoURI ?? tokenPriceData?.logoURI setTokenRequestedLogoURI(logoURI) let tokenSymbol = requestLinkData.tokenSymbol ?? chainDetails?.tokens.find((token) => utils.areTokenAddressesEqual(token.address, requestLinkData.tokenAddress) )?.symbol?.toUpperCase() ?? tokenPriceData?.symbol if (tokenSymbol) { setTokenRequestedSymbol(tokenSymbol) } else { fetchTokenSymbol(requestLinkData.chainId, requestLinkData.tokenAddress) } -}, [requestLinkData, tokenPriceData]) +}, [requestLinkData, tokenPriceData, fetchTokenSymbol])
197-200: Potential redundant state updates in loading state transitionThe
useEffecthook at line 197 transitions the view to the loading state wheneverisLoadingis true. Ensure that this does not cause unnecessary re-renders or conflicts with other state transitions.Consider checking if the current
viewStateis alreadyLOADINGbefore updating:useEffect(() => { if (isLoading && viewState !== ViewState.LOADING) { setViewState(ViewState.LOADING) } }, [isLoading, viewState])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/components/Request/Pay/Pay.tsx (3 hunks)
- src/components/Request/Pay/Views/Initial.view.tsx (5 hunks)
- src/context/tokenSelector.context.tsx (1 hunks)
🧰 Additional context used
📓 Learnings (2)
src/components/Request/Pay/Views/Initial.view.tsx (10)
Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:76-78 Timestamp: 2024-10-08T20:13:44.480Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:76-78 Timestamp: 2024-10-07T15:28:25.280Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:76-78 Timestamp: 2024-10-08T20:13:42.967Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:76-78 Timestamp: 2024-10-08T20:13:44.480Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:76-78 Timestamp: 2024-10-07T15:25:45.170Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:76-78 Timestamp: 2024-10-08T20:13:42.967Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-08T20:13:45.742Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-08T20:13:42.967Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-07T16:21:26.030Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-08T20:13:44.480Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.src/context/tokenSelector.context.tsx (4)
Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#413 File: src/context/tokenSelector.context.tsx:118-123 Timestamp: 2024-10-08T20:13:42.967Z 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.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#413 File: src/context/tokenSelector.context.tsx:118-123 Timestamp: 2024-10-04T13:40:16.067Z 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.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#404 File: src/context/tokenSelector.context.tsx:121-121 Timestamp: 2024-10-03T09:57:43.885Z Learning: In `TokenContextProvider` within `tokenSelector.context.tsx`, when token data is loaded from preferences, it's acceptable to set `isTokenPriceFetchingComplete` to `true` because the token data is already available.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#404 File: src/context/tokenSelector.context.tsx:121-121 Timestamp: 2024-10-08T20:13:42.967Z Learning: In `TokenContextProvider` within `tokenSelector.context.tsx`, when token data is loaded from preferences, it's acceptable to set `isTokenPriceFetchingComplete` to `true` because the token data is already available.
🪛 Biome
src/context/tokenSelector.context.tsx
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 Gitleaks
src/context/tokenSelector.context.tsx
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (5)
src/components/Request/Pay/Pay.tsx (3)
26-28: LGTM: Context consumption added for token selector.This change aligns well with the PR objective of refactoring the token selector context. It allows the component to update the selected chain and token address, which is crucial for initializing the correct data before rendering.
99-101: Excellent: Initializing token pair before rendering.This change directly addresses the main objective of the PR. By setting the selected chain ID and token address based on the
requestLinkDetails, we ensure that the correct data is loaded before rendering. This prevents the issue of fetching the xchain route with outdated data, which was causing users to get stuck on an error screen.
Line range hint
1-200: Overall: Changes effectively address PR objectives.The modifications in this file successfully implement the main objectives of the PR:
- The token pair is now initialized before rendering, preventing issues with outdated data.
- The token selector context is utilized to manage selected chain and token information.
- The loading state rendering has been corrected.
These changes should resolve the issue of users getting stuck on an error screen during the request pay process.
To ensure the changes work as expected across different scenarios:
This script will help verify that the changes are consistent across the codebase and identify any potential areas that might need additional updates.
src/components/Request/Pay/Views/Initial.view.tsx (2)
46-46:⚠️ Potential issueMissing non-null assertion may cause a runtime error
In the
createXChainUnsignedTxfunction, thelinkDetailsparameter is assignedrequestLink, which may beundefinedif not properly validated. Ensure thatrequestLinkis defined before calling this function to prevent potential runtime errors.Consider adding a check before the function call:
+if (!requestLink) { + throw new Error('requestLink is undefined'); +} const xchainUnsignedTxs = await peanut.prepareXchainRequestFulfillmentTransaction({ fromToken: tokenData.address, fromChainId: tokenData.chainId, senderAddress, squidRouterUrl: 'https://apiplus.squidrouter.com/v2/route', provider: await peanut.getDefaultProvider(tokenData.chainId), tokenType: utils.isAddressZero(tokenData.address) ? interfaces.EPeanutLinkType.native : interfaces.EPeanutLinkType.erc20, fromTokenDecimals: tokenData.decimals as number, linkDetails: requestLink, });⛔ Skipped due to learnings
Learnt from: Hugo0 PR: peanutprotocol/peanut-ui#413 File: src/components/Request/Pay/Views/Initial.view.tsx:71-72 Timestamp: 2024-10-08T20:13:42.967Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, it's acceptable to use the `!` operator in TypeScript to assert that `selectedTokenData` is not `null` or `undefined`, and potential runtime errors from accessing its properties without checks can be disregarded.Learnt from: Hugo0 PR: peanutprotocol/peanut-ui#413 File: src/components/Request/Pay/Views/Initial.view.tsx:71-72 Timestamp: 2024-10-04T13:10:49.199Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, it's acceptable to use the `!` operator in TypeScript to assert that `selectedTokenData` is not `null` or `undefined`, and potential runtime errors from accessing its properties without checks can be disregarded.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-08T20:13:42.967Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-07T16:21:26.030Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-08T20:13:45.742Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#422 File: src/components/Request/Pay/Views/Initial.view.tsx:67-74 Timestamp: 2024-10-08T20:13:44.480Z Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, the functions `getDefaultProvider` and `getTokenContractDetails` used in `fetchTokenSymbol` do not throw errors but can return `undefined`. Additional error handling is unnecessary since this is already handled in the code.
211-214: Ensuring consistent error state transitionsThe
useEffecthook at line 211 updatesviewStatetoERRORwhenevererrorState.showErroris true. Confirm that this logic aligns with the intended error handling flow and does not interfere with other state transitions.Run the following script to check for additional state transitions to
ViewState.ERROR:Ensure that there are no conflicting updates to
viewStatethat might cause unexpected behavior.✅ Verification successful
Verification Complete: No Conflicting
viewState.ERRORTransitions FoundThe
useEffectlogic inInitial.view.tsxcorrectly setsviewStatetoERROR, and no other instances ofsetViewState(ViewState.ERROR)were found that could interfere with state transitions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all instances where viewState is set to ViewState.ERROR rg --type js "setViewState\s*\(\s*ViewState\.ERROR\s*\)"Length of output: 58
Script:
#!/bin/bash # Description: Find all direct assignments where viewState is set to ViewState.ERROR rg --type js "viewState\s*=\s*ViewState\.ERROR"Length of output: 49
Script:
#!/bin/bash # Description: Find all instances where viewState is set to ERROR across all relevant file types rg "setViewState\s*\(\s*ViewState\.ERROR\s*\)" --glob "*.tsx" --glob "*.js" --glob "*.ts"Length of output: 183
On request pay we were trying to fetch xchain route with old data and stayed on error screen, now we ensure that we start with the intended data and only calculate route if needed.
Main change
Refactor
Small fix