Conversation
WalkthroughThis update introduces wallet portfolio integration into the token swap UI. It adds a new API service and types for fetching and modeling wallet portfolio data, and enhances token selection components to display tokens and balances from the user's wallet. The Redux state is extended to store wallet portfolio data, and new actions are added for managing it. Token search and selection logic is refactored to conditionally use wallet portfolio tokens or search results based on UI state. Components now support and display token balances and prices. Several component prop types are updated to accommodate these changes, and related tests are refactored accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DropdownTokensList
participant ReduxStore
participant pillarXApiWalletPortfolio
participant TokenSearchInput
User->>DropdownTokensList: Opens token dropdown
DropdownTokensList->>pillarXApiWalletPortfolio: Fetch wallet portfolio (if wallet connected)
pillarXApiWalletPortfolio-->>DropdownTokensList: Returns portfolio data
DropdownTokensList->>ReduxStore: Dispatch setWalletPortfolio(portfolio)
User->>TokenSearchInput: Types in search box
TokenSearchInput->>ReduxStore: Checks isReceiveOpen / isSwapOpen, walletPortfolio
alt Receive UI open
TokenSearchInput->>pillarXApiWalletPortfolio: API search by receive chain
pillarXApiWalletPortfolio-->>TokenSearchInput: Search results
TokenSearchInput->>ReduxStore: Dispatch search results
else Swap UI open and walletPortfolio available
TokenSearchInput->>TokenSearchInput: Local fuzzy search via Fuse.js
TokenSearchInput->>ReduxStore: Dispatch search results
end
DropdownTokensList->>User: Renders tokens from wallet or search results
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
Deploying x with
|
| Latest commit: |
b119a50
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bc36693f.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3162-the-exchange-c.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
src/apps/the-exchange/components/SelectToken/SelectToken.tsx (1)
46-55: Effective conditional rendering of token balance and USD value.The implementation correctly handles the display of token balances and their USD value when available. Consider adding handling for very small balances or potential edge cases.
{tokenBalance && ( <Body className="font-normal"> - {limitDigits(tokenBalance)}{' '} + {tokenBalance > 0 ? limitDigits(tokenBalance) : '0.00'}{' '} {tokenPrice && ( <span className="text-black_grey/[.4]"> - ${(tokenPrice * tokenBalance).toFixed(4)} + ${isFinite(tokenPrice * tokenBalance) ? (tokenPrice * tokenBalance).toFixed(4) : '0.00'} </span> )} </Body> )}src/services/pillarXApiWalletPortfolio.ts (1)
57-61: Consider adding a comment about the chain IDs array.The conditional chain IDs selection isn't immediately obvious without context.
Consider adding a brief comment about why different chain IDs are used for testnet vs mainnet:
+ // For testnet, only use Sepolia testnet (11155111) + // For mainnet, use all supported production chains const chainIds = isTestnet ? [11155111] : CompatibleChains.map((chain) => chain.chainId);src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (1)
2-2: Addfuse.jsto project dependencies
Fuseis imported but I don’t see it declared inpackage.json. A missing dependency will break CI builds.# package.json "dependencies": { + "fuse.js": "^7.0.0", ... }src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx (1)
212-218: Avoid double invocation oftokenBalanceLimit
tokenBalanceLimitis executed twice per render – once to test truthiness and once to render the message. Compute once for minor perf/readability gain:- {tokenBalanceLimit(Number(inputValue), tokenBalance) && ( - <BodySmall ...> - {tokenBalanceLimit(Number(inputValue), tokenBalance)} - </BodySmall> - )} + {(() => { + const limitMsg = tokenBalanceLimit(Number(inputValue), tokenBalance); + return ( + limitMsg && ( + <BodySmall id="token-balance-limit-exchange" data-testid="error-max-limit"> + {limitMsg} + </BodySmall> + ) + ); + })()}src/types/api.ts (3)
605-611: Inconsistent type naming: use singularContractBalanceMobula
The new type is namedContractsBalanceMobula(plural), but we already have a legacyContractBalance(singular) for non‑Mobula data. For consistency and clearer symmetry, consider renaming this toContractBalanceMobulaand updating all references accordingly.-export type ContractsBalanceMobula = { +export type ContractBalanceMobula = { address: string; balance: number; balanceRaw: string; chainId: string; decimals: number; };
678-687: Consider suffixingPortfolioDatawithMobulafor clarity
We have an earlierWalletPortfolioData; this new type could be confused with it. To maintain clear boundaries between legacy and Mobula models, you might rename this toPortfolioDataMobula.
693-696: Remove commented-out stub code
The leftoverWalletBalancesstub is commented out and unused. Please delete these lines to keep the codebase clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/CardsSwap/test/__snapshots__/CardSwap.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/DropdownTokensList/test/__snapshots__/DropdownTokensList.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/SelectToken/test/__snapshots__/SelectToken.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/TokenListItem/test/__snapshots__/TokenListItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (14)
src/apps/the-exchange/components/CardsSwap/CardsSwap.tsx(1 hunks)src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx(1 hunks)src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx(7 hunks)src/apps/the-exchange/components/DropdownTokensList/TokenRow.tsx(1 hunks)src/apps/the-exchange/components/EnterAmount/EnterAmount.tsx(5 hunks)src/apps/the-exchange/components/SelectToken/SelectToken.tsx(4 hunks)src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx(3 hunks)src/apps/the-exchange/components/TokenListItem/TokenListItem.tsx(2 hunks)src/apps/the-exchange/components/TokenListItem/test/TokenListItem.test.tsx(2 hunks)src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx(5 hunks)src/apps/the-exchange/reducer/theExchangeSlice.ts(6 hunks)src/services/pillarXApiWalletPortfolio.ts(1 hunks)src/services/tokensData.ts(1 hunks)src/types/api.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (1)
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (1)
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
🧬 Code Graph Analysis (6)
src/apps/the-exchange/components/SelectToken/SelectToken.tsx (1)
src/apps/token-atlas/utils/converters.ts (1)
limitDigits(8-23)
src/services/pillarXApiWalletPortfolio.ts (4)
src/types/api.ts (2)
PortfolioData(678-687)WalletPortfolioMobulaResponse(689-691)src/services/tokensData.ts (2)
Token(11-21)chainIdToChainNameTokensData(213-234)src/utils/blockchain.ts (2)
isTestnet(29-35)CompatibleChains(260-289)src/store.ts (1)
addMiddleware(48-58)
src/apps/the-exchange/components/TokenListItem/test/TokenListItem.test.tsx (1)
src/apps/token-atlas/utils/converters.ts (1)
limitDigits(8-23)
src/apps/the-exchange/components/TokenListItem/TokenListItem.tsx (3)
src/services/tokensData.ts (3)
Token(11-21)chainNameToChainIdTokensData(237-258)chainNameDataCompatibility(46-66)src/utils/blockchain.ts (1)
getLogoForChainId(158-192)src/apps/token-atlas/utils/converters.ts (1)
limitDigits(8-23)
src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (6)
src/apps/the-exchange/hooks/useReducerHooks.tsx (1)
useAppSelector(6-6)src/apps/the-exchange/utils/types.tsx (1)
ChainType(28-31)src/types/api.ts (1)
PortfolioData(678-687)src/apps/the-exchange/reducer/theExchangeSlice.ts (6)
setWalletPortfolio(107-112)setSwapChain(65-67)setIsReceiveOpen(62-64)setReceiveChain(68-70)setSearchTokenResult(86-88)setSearchToken(98-100)src/services/pillarXApiWalletPortfolio.ts (1)
convertPortfolioAPIResponseToToken(13-33)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(237-258)
src/apps/the-exchange/reducer/theExchangeSlice.ts (1)
src/types/api.ts (1)
PortfolioData(678-687)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (46)
src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx (1)
205-205: Good test setup updateAdding the feature flag
REACT_APP_SWAP_BUTTON_SWITCHto the test environment properly aligns with the component changes in CardsSwap.tsx, ensuring that tests will continue to pass with the new conditional rendering logic.src/apps/the-exchange/components/CardsSwap/CardsSwap.tsx (1)
113-115:Details
✅ Verification successful
Feature flag implementation LGTM
The addition of the
REACT_APP_SWAP_BUTTON_SWITCHenvironment variable check provides more control over the UI by making the switch button's visibility configurable without code changes.To ensure consistent feature flag naming across the codebase:
🏁 Script executed:
#!/bin/bash # Check for other feature flags to verify naming convention consistency rg "process\.env\.REACT_APP_.*_SWITCH" --type ts --type tsxLength of output: 89
🏁 Script executed:
#!/bin/bash # Retry searching across .ts and .tsx files for feature-flag naming consistency rg -E 'process\.env\.REACT_APP_.*_SWITCH' --glob '*.{ts,tsx}'Length of output: 460
🏁 Script executed:
#!/bin/bash # Properly search for any feature flags ending with _SWITCH in .ts and .tsx files rg 'process\.env\.REACT_APP_.*_SWITCH' --glob '*.ts' --glob '*.tsx'Length of output: 309
Feature flag naming consistency confirmed
A search for all
REACT_APP_*_SWITCHflags returned only these two occurrences:
- src/apps/the-exchange/components/CardsSwap/CardsSwap.tsx
- src/apps/the-exchange/components/CardsSwap/test/CardSwap.test.tsx
No further instances or naming mismatches found—no changes required.
src/services/tokensData.ts (1)
19-20: Enhancement of the Token data model to support wallet portfolio integration.The addition of optional
balanceandpricefields to theTokentype enables components to display token balances and value calculations. This is a clean, non-breaking change that follows the Open-Closed Principle by extending the existing model without modifying current functionality.src/apps/the-exchange/components/DropdownTokensList/TokenRow.tsx (1)
26-26: Good simplification by unifying token data props.Passing the entire token object instead of individual properties reduces prop drilling and simplifies the component interface. This change supports the new balance and price display features while making the code more maintainable.
src/apps/the-exchange/components/SwapReceiveCard/SwapReceiveCard.tsx (3)
108-108: Improved container flexibility with min-height and full-height.Changing from a fixed height to
min-h-[200px] h-fullallows the container to adapt to content size while maintaining a minimum size, which provides better responsive behavior.
125-130: Token balance and price data correctly passed to SelectToken component.The conditional passing of balance and price data only when in SWAP position aligns with the application's intended behavior for displaying user balances.
139-141: Token balance appropriately passed for validation in EnterAmount component.Passing the balance to EnterAmount enables amount validation against available balance, enhancing the user experience by preventing over-spending.
src/apps/the-exchange/components/SelectToken/SelectToken.tsx (3)
6-6: Appropriate utility import for balance formatting.The
limitDigitsutility will ensure consistent numeric formatting across the application.
19-20: Well-designed props extension for token balance and price.Adding these optional props maintains backward compatibility while enabling new functionality.
37-37: Added margin improves vertical spacing.The added bottom margin provides better visual separation between the token information and other elements.
src/apps/the-exchange/components/TokenListItem/test/TokenListItem.test.tsx (7)
4-6: Good addition of the limitDigits utility.The inclusion of the
limitDigitsutility from token-atlas is appropriate for testing the token balance formatting.
11-21: Well-structured test fixture data.The
tokenExampleobject provides comprehensive test data including balance and price properties, which aligns with the updated Token interface. This is a good practice for creating reusable test fixtures.
27-27: Updated component prop structure.The test now correctly passes a single
tokenobject instead of separate primitive props, matching the component's new API.
35-44: Thorough verification of rendered token data.The test now properly verifies that token details, including the formatted balance and computed total value, are correctly rendered in the component.
54-58: Proper type handling for edge case testing.Using TypeScript comments to silence the type error is appropriate here since you're intentionally testing the component's behavior with incomplete data (undefined logo).
69-70: Updated element selection strategy.Tests now locate elements by the token's name from the token object, which is more resilient than using fixed strings.
80-81: Consistent element selection approach.The hover test is also updated to use the token name from the object, maintaining consistency with the click test.
src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx (11)
15-20: Updated reducer actions to support wallet portfolio.Added
setSearchTokenandsetWalletPortfolioimports from the reducer to manage the new wallet portfolio state.
23-27: New wallet portfolio API integration.Added imports for the wallet portfolio API service and conversion utility, which will enable token balance display.
39-41: Added type imports for portfolio data.The
PortfolioDataandChainTypeimports support the new portfolio functionality.
73-75: Added Redux state selectors for new features.Properly added selectors for
isReceiveOpen,swapChain, andwalletPortfoliostate, which are needed for the enhanced portfolio and chain-based filtering functionality.Also applies to: 92-97
102-126: Wallet portfolio data fetching and state management.The component uses the RTK Query hook to fetch wallet portfolio data and manages related loading states. The useEffect correctly updates Redux state based on the API response.
I notice you've excluded some dependencies from the dependency array in the useEffect. Based on the retrieved learning from PR #275, this seems intentional as you're focusing only on portfolio data changes.
197-219: Smart token list rendering with portfolio integration.The component now intelligently chooses between wallet portfolio tokens and search results based on UI state. The filtering by chain ID is a nice feature that improves UX when users select specific chains.
233-240: Improved dropdown close behavior.When closing the dropdown, the component now properly resets the chain filter to "all" and clears the search token. This ensures a clean state when reopening the dropdown.
263-264: Updated search placeholder.The updated placeholder "Search name / contract" is more informative about what users can search for.
271-282: Enhanced error and empty state handling.The component now handles both token search errors and wallet portfolio errors, and displays an appropriate message when no tokens are found in the wallet.
286-290: Improved loading state.The loading indicator now considers both token search loading and wallet portfolio loading/fetching states, providing a better user experience.
291-312: Updated token list rendering with proper filtering.The List component now uses the updated token list data structure and properly filters out tokens that are already selected in either the swap or receive positions.
src/services/pillarXApiWalletPortfolio.ts (4)
13-33: Well-implemented portfolio data conversion.The
convertPortfolioAPIResponseToTokenfunction effectively converts the portfolio API response into the expectedTokenstructure. It handles null checks and properly maps chain IDs to chain names using the existing utility.
35-45: Robust API query configuration with retry logic.The use of retry logic with fetchBaseQuery is a good practice for handling network failures. The dynamic base URL based on testnet/mainnet environment is also a good approach.
47-75: Well-structured RTK Query API definition.The API is properly configured to fetch wallet portfolio data with the correct request structure. The dynamic chainIds handling based on environment is a good approach for supporting different networks.
77-79: Proper Redux store integration.The service is correctly integrated with the Redux store using
addMiddlewareand exports the generated hook for component usage.src/apps/the-exchange/components/TokenListItem/TokenListItem.tsx (7)
2-8: Updated imports to support enhanced token display.Added imports for Token type, chain utilities, and the limitDigits formatter to support the new balance and price display functionality.
15-19: Simplified prop interface.The component now accepts a single
tokenobject instead of multiple primitive props, which simplifies the API and improves type safety.
21-21: Updated function signature.The function signature now uses the new prop structure.
30-38: Enhanced token logo display.The TokenLogo component now receives both the token logo and a chain-specific logo based on the token's blockchain, improving visual identification.
40-45: Updated token detail rendering.Token name and symbol are now accessed from the token object properties.
48-62: Added conditional rendering for token balance information.The component now conditionally displays either the token balance and USD value (when available) or the chain compatibility text. The use of limitDigits for formatting the balance and proper calculation of the USD value improve the user experience.
55-55: Verify valid price before calculating value.Good use of the optional chaining to only attempt calculation if price exists.
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (1)
179-186: Missing dependencies inuseEffect
isLoading,isFetching, anderrorare referenced inside the effect but not listed in the dependency array. The loading/error flags may become stale.- }, [ + }, [ searchData, debouncedSearchText, isSwapOpen, swapChain.chainId, receiveChain.chainId, isReceiveOpen, walletPortfolio, + isLoading, + isFetching, + error, ]);src/apps/the-exchange/reducer/theExchangeSlice.ts (1)
23-24: State shape change looks correct
searchTokenis now optional – good alignment with the UI where an empty search isundefinedrather than''. No issues spotted here.Also applies to: 49-50
src/types/api.ts (5)
613-618: NewCrossChainBalanceMobulatype looks good
Fields align with the Mobula API response and mirror the legacyCrossChainBalance. No changes needed here.
624-632: Verifydecimalsshould bestring[]
TheAssetMobulatype declaresdecimals: string[], but decimals are typically numeric. If the API really returns string‑encoded decimals, this is fine. Otherwise, consider usingnumber[]to reflect numeric precision and simplify downstream calculations.
634-650: NewAssetDataMobulatype aligned with API spec
This captures balances, price data, PnL metrics, and wallets as expected. References toContractsBalanceMobulaandCrossChainBalancesnow map correctly (once the mapping type is fixed).
652-657: Clarify PnL history value types
PnLHistoryusesstring[][], but it’s unclear what each inner string represents (timestamp, value, etc.). Please confirm the API returns nested string arrays and, if possible, introduce a tuple type (e.g.,[timestamp: number, value: number][]) for better type safety.
689-692: Wrapper response type is correctly defined
WalletPortfolioMobulaResponsematches the API shape (result.data). No changes needed.
| export type TotalPnLHistory = { | ||
| '24h': { | ||
| realized: number; | ||
| unrealized: number; | ||
| }; | ||
| '7d': { | ||
| realized: number; | ||
| unrealized: number; | ||
| }; | ||
| '30d': { | ||
| realized: number; | ||
| unrealized: number; | ||
| }; | ||
| '1y': { | ||
| realized: number; | ||
| unrealized: number; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Naming collision: unify TotalPnLHistory vs. existing TotalPnlHistory
There’s already a TotalPnlHistory defined earlier. The new TotalPnLHistory only differs by capitalization of “L”. This risks confusion and import errors. Consider deprecating one or renaming the new type to TotalPnlHistoryMobula (or vice versa) for clear separation.
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3162/The-Exchange-changes)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (3)
105-111:⚠️ Potential issueAdd null-safety check for API response data
The code is missing a null check for
searchData?.result?.databefore passing it toconvertAPIResponseToTokens(). Without this check, the code could throw an error if the API returns a malformed payload or a 204 response.Apply this fix:
if (isReceiveOpen) { if (!searchData) return; + if (!searchData?.result?.data) { + dispatch(setSearchTokenResult([])); + return; + } + const result = convertAPIResponseToTokens( - searchData?.result?.data as TokenAssetResponse[], + searchData.result.data as TokenAssetResponse[], debouncedSearchText );
136-176:⚠️ Potential issueFix fuzzy search logic overwriting exact matches
The exact-match branch (for contract addresses) is immediately overwritten by the fuzzy search branch. After dispatching the results for contract address search, the code creates another Fuse instance and overwrites the Redux state, causing users to never see the exact-match results.
Apply this fix:
if (debouncedSearchText.length > 40) { const fuse = new Fuse(tokensWithBalances, { keys: ['name', 'symbol', 'contract'], // Fields to search in threshold: 0.2, // Allow some fuzziness for queries that are not contract like minMatchCharLength: 3, useExtendedSearch: true, // Enables exact match using '=' }); // Check if query length is above 40 characters have an exact match (likely a contract address) const searchQuery = debouncedSearchText.length > 40 ? `="${debouncedSearchText}"` : debouncedSearchText; const results = fuse.search(searchQuery).map((r) => r.item); dispatch(setSearchTokenResult(results)); + return; // Early return to prevent overwriting exact matches } const fuse = new Fuse(tokensWithBalances, {
130-158: 🛠️ Refactor suggestionOptimize Fuse.js instance creation with useMemo
Fuse instances are recreated on every render cycle, which is inefficient for large token lists. Use
useMemoto cache these instances and their input data.Move the Fuse setup outside the effect and use
useMemo:+ // Memoize tokens with balances to avoid unnecessary conversions + const tokensWithBalances = useMemo(() => { + if (!walletPortfolio) return []; + return convertPortfolioAPIResponseToToken(walletPortfolio); + }, [walletPortfolio]); + + // Memoize Fuse instance for contract address searches + const contractFuse = useMemo(() => { + return new Fuse(tokensWithBalances, { + keys: ['name', 'symbol', 'contract'], + threshold: 0.2, + minMatchCharLength: 3, + useExtendedSearch: true, + }); + }, [tokensWithBalances]); + + // Memoize Fuse instance for normal searches + const normalFuse = useMemo(() => { + return new Fuse(tokensWithBalances, { + keys: ['name', 'symbol', 'contract'], + threshold: 0.3, + }); + }, [tokensWithBalances]); useEffect(() => { // ...existing code... if (isSwapOpen && walletPortfolio) { - // This sets the token results list that will be displayed in the UI - const tokensWithBalances = - convertPortfolioAPIResponseToToken(walletPortfolio); - // Since the list of available tokens with balance is loaded, we just need // here a simpler search with Fuse.js with auto select when the contract // address is entered if (debouncedSearchText.length > 40) { - const fuse = new Fuse(tokensWithBalances, { - keys: ['name', 'symbol', 'contract'], // Fields to search in - threshold: 0.2, // Allow some fuzziness for queries that are not contract like - minMatchCharLength: 3, - useExtendedSearch: true, // Enables exact match using '=' - }); - // Check if query length is above 40 characters have an exact match (likely a contract address) const searchQuery = debouncedSearchText.length > 40 ? `="${debouncedSearchText}"` : debouncedSearchText; - const results = fuse.search(searchQuery).map((r) => r.item); + const results = contractFuse.search(searchQuery).map((r) => r.item); dispatch(setSearchTokenResult(results)); + return; } - const fuse = new Fuse(tokensWithBalances, { - keys: ['name', 'symbol', 'contract'], // Fields to search in - threshold: 0.3, // Allow some fuzziness for queries that are not contract like - }); - const results = fuse
🧹 Nitpick comments (2)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (2)
159-175: Extract common chain filtering logicThe chain filtering logic is duplicated between the receive and swap cases. Consider extracting this into a helper function.
+ // Helper function to filter tokens by chain ID + const filterTokensByChain = (tokens, chainId) => { + if (chainId === 0) return tokens; + return tokens.filter( + (token) => chainNameToChainIdTokensData(token.blockchain) === chainId + ); + }; // In the receive case: dispatch( setSearchTokenResult( - receiveChain.chainId === 0 - ? result - : result - .filter( - (tokens) => - chainNameToChainIdTokensData(tokens.blockchain) === - receiveChain.chainId - ) - .map((tokens) => tokens) + filterTokensByChain(result, receiveChain.chainId) ) ); // In the swap case: dispatch( setSearchTokenResult( - swapChain.chainId === 0 - ? results - : results - .filter( - (tokens) => - chainNameToChainIdTokensData(tokens.blockchain) === - swapChain.chainId - ) - .map((tokens) => tokens) + filterTokensByChain(results, swapChain.chainId) ) );
146-148: Simplify contract address search query logicThe
searchQueryassignment has a redundant condition since you already checkeddebouncedSearchText.length > 40in the outer if statement.// Check if query length is above 40 characters have an exact match (likely a contract address) - const searchQuery = - debouncedSearchText.length > 40 - ? `="${debouncedSearchText}"` - : debouncedSearchText; + const searchQuery = `="${debouncedSearchText}"`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx(6 hunks)src/types/api.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/api.ts
🧰 Additional context used
🧠 Learnings (1)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (1)
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: Cloudflare Pages
- GitHub Check: build
🔇 Additional comments (2)
src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsx (2)
178-186:Details
✅ Verification successful
Pay attention to useEffect dependencies
The dependencies list now includes
walletPortfoliowhich could be a complex object that causes the effect to run frequently. Consider using a more stable dependency if possible.Check if
walletPortfoliochanges frequently:
🏁 Script executed:
#!/bin/bash # Look for redux actions that modify walletPortfolio grep -r "walletPortfolio" --include="*.ts" --include="*.tsx" src/apps/the-exchange/reducerLength of output: 362
🏁 Script executed:
#!/bin/bash # Search for the definition of PortfolioData to understand its shape rg "type PortfolioData" -A10 src/apps/the-exchange rg "interface PortfolioData" -A10 src/apps/the-exchangeLength of output: 110
🏁 Script executed:
#!/bin/bash # Search for PortfolioData type definition across the repository rg "PortfolioData" -A5 .Length of output: 12690
🏁 Script executed:
#!/bin/bash # Show the useEffect callback in TokenSearchInput to see how walletPortfolio is used rg -n "useEffect" -A5 -B5 src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsxLength of output: 1580
🏁 Script executed:
#!/bin/bash # Print lines 60-200 to inspect useEffect with walletPortfolio dependency sed -n '60,200p' src/apps/the-exchange/components/TokenSearchInput/TokenSearchInput.tsxLength of output: 4458
Effect dependencies are appropriate
After reviewing the reducer and selector,walletPortfoliois only updated via explicitsetWalletPortfoliodispatches (on initial fetch or error), so its reference doesn’t change on every render. Including it ensures the effect runs once when real portfolio data arrives—there’s no unnecessary re‑execution.
93-96: 🛠️ Refactor suggestionAlways skip query when search text is empty
The API query skips only when
debouncedSearchTextis empty, but doesn't consider which mode is active. This could still trigger large, empty searches.} = useGetSearchTokensQuery( { searchInput: debouncedSearchText, filterBlockchains: chainIdToChainNameTokensData(receiveChain.chainId), }, - { skip: !debouncedSearchText } + { skip: !debouncedSearchText || isSwapOpen } );This will skip the query when either:
- The search text is empty (existing behavior)
- The swap UI is open (since you're using local wallet portfolio data in this case)
Likely an incorrect or invalid review comment.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Chores