♻️ refactor lifi usage #681
Conversation
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces static URI-based asset logos with a symbol-driven AssetLogo (which fetches token logos), adds ChainLogo, removes useAccountAssets in favor of usePortfolio, adds LiFi query options/token balance queries, and exposes persistOptions from the query client. Many components updated to the new APIs and props. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the application's approach to handling cryptocurrency assets and blockchain networks. By introducing dedicated Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring of how asset and chain data, particularly logos, are handled throughout the application. By replacing the hardcoded assetLogos map and component-specific logic with new generic AssetLogo and ChainLogo components that fetch data from LI.FI, you've greatly improved maintainability and scalability. The introduction of the usePortfolio hook to replace useAccountAssets and the centralization of LI.FI queries in queryClient.ts are excellent changes that streamline data fetching and caching. Overall, this is a high-quality refactoring. I've included one suggestion to address a React anti-pattern that was present in the code.
src/components/pay-mode/Pay.tsx
Outdated
| if (!selectedAsset.address && assets[0]) { | ||
| const { type } = assets[0]; | ||
| setSelectedAsset({ | ||
| address: type === "external" ? parse(Address, accountAssets[0].address) : parse(Address, accountAssets[0].market), | ||
| address: type === "external" ? parse(Address, assets[0].address) : parse(Address, assets[0].market), | ||
| external: type === "external", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Calling a state setter (setSelectedAsset) directly within the component's render body is a React anti-pattern that can lead to unexpected re-renders and bugs. This logic should be moved into a useEffect hook to ensure it only runs after the component has rendered and when its dependencies change.
useEffect(() => {
if (!selectedAsset.address && assets[0]) {
const { type } = assets[0];
setSelectedAsset({
address: type === "external" ? parse(Address, assets[0].address) : parse(Address, assets[0].market),
external: type === "external",
});
}
}, [assets, selectedAsset.address]);
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## optimize #681 +/- ##
============================================
- Coverage 61.68% 61.37% -0.32%
============================================
Files 169 170 +1
Lines 5304 5364 +60
Branches 1498 1517 +19
============================================
+ Hits 3272 3292 +20
- Misses 1859 1900 +41
+ Partials 173 172 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/swaps/SelectorModal.tsx (1)
33-38: LiftusePortfolioout ofTokenListItemand normalize address matching.
TokenListItemnow depends on app state (usePortfolio), which violates the “dumb component” boundary and can duplicate portfolio computation per row. You already haveassetsinTokenSelectModal, so pass the matching asset down. While refactoring, normalize address casing to avoid mismatches.As per coding guidelines, keep `src/components/**` UI components stateless and fed via props.♻️ Proposed refactor
import usePortfolio from "../../utils/usePortfolio"; import AssetLogo from "../shared/AssetLogo"; @@ import type { Token } from "@lifi/sdk"; +type PortfolioAssetItem = ReturnType<typeof usePortfolio>["assets"][number]; + +function findMatchingAsset(assets: PortfolioAssetItem[], token: Token) { + const tokenAddress = token.address.toLowerCase(); + return assets.find((asset) => { + const assetAddress = asset.type === "protocol" ? asset.asset : asset.address; + return assetAddress?.toLowerCase() === tokenAddress; + }); +} + function TokenListItem({ token, isSelected, onPress, language, + matchingAsset, }: { isSelected: boolean; language: string; onPress: () => void; token: Token; + matchingAsset?: PortfolioAssetItem; }) { - const { assets } = usePortfolio(); - const matchingAsset = assets.find( - (asset) => - (asset.type === "protocol" && asset.asset === token.address) || - (asset.type === "external" && asset.address === token.address), - ); return ( @@ const filteredTokens = useMemo(() => { @@ - const matchesAsset = (token: Token) => - assets.find( - (asset) => - (asset.type === "protocol" && asset.asset === token.address) || - (asset.type === "external" && asset.address === token.address), - ); + function matchesAsset(token: Token) { + return findMatchingAsset(assets, token); + } @@ - renderItem={({ item }) => ( - <TokenListItem - token={item} - isSelected={selectedToken?.address === item.address} - onPress={() => { - onSelect(item); - setSearchQuery(""); - }} - language={language} - /> - )} + renderItem={({ item }) => { + const matchingAsset = findMatchingAsset(assets, item); + return ( + <TokenListItem + token={item} + isSelected={selectedToken?.address === item.address} + onPress={() => { + onSelect(item); + setSearchQuery(""); + }} + language={language} + matchingAsset={matchingAsset} + /> + ); + }}Also applies to: 108-134
🤖 Fix all issues with AI agents
In `@src/components/pay-mode/Pay.tsx`:
- Around line 489-495: The render currently calls setSelectedAsset directly
(selectedAsset, setSelectedAsset, assets, parse, Address), move this
initialization out of render: either initialize selectedAsset lazily in useState
with an initializer that inspects assets[0] and returns the parsed
address/external flag, or wrap the current logic in a useEffect that runs when
assets or selectedAsset.address change (guard: if !selectedAsset.address &&
assets[0]) and calls setSelectedAsset with the same parsed Address and external
boolean; ensure you import/useEffect and keep the parse(Address, ...) logic
unchanged.
In `@src/components/shared/PaymentScheduleSheet.tsx`:
- Line 70: The AssetLogo is being fed a defensive empty-string fallback
(AssetLogo symbol={symbol ?? ""}) even though the parent conditional guarantees
market and thus symbol exist; remove the unnecessary ?? "" fallback and pass
symbol directly (AssetLogo symbol={symbol}) so TypeScript can narrow symbol to
string within the conditional in PaymentScheduleSheet, or alternatively
explicitly assert/convert only if needed (e.g., ensure symbol is typed as string
in the block) to avoid rendering an empty placeholder.
In `@src/utils/lifi.ts`:
- Around line 118-124: The catch block that swallows failures from
getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B") should
log/report the error before returning the fallback tokens; change the anonymous
catch to catch (err) and call the project’s telemetry/logger (e.g., logger.error
or captureException) with context including chain.id and the EXA address, then
return tokens; apply the same change to the other getToken usage around the EXA
fetch at the 187-192 location.
In `@src/utils/queryClient.ts`:
- Around line 42-45: Replace the hardcoded EXA token address string passed to
getToken with a named constant (e.g., EXA_TOKEN_ADDRESS) defined near the top of
the module; update the Promise.all call to use EXA_TOKEN_ADDRESS instead of the
literal string and keep the same .catch fallback, ensuring references to
getToken(chain.id, EXA_TOKEN_ADDRESS) and the exa variable remain unchanged so
the intent is self-documenting.
In `@src/utils/usePortfolio.ts`:
- Around line 117-128: The comparator in the assets sorting (inside
usePortfolio's assets useMemo) can be unstable when both items resolve to "USDC"
because it returns -1 for aSymbol === "USDC" before checking bSymbol; update the
comparator to handle the tie explicitly by adding a tiebreaker when both aSymbol
and bSymbol are "USDC" (for example compare by usdValue, then by a.type vs
b.type or by symbol) so the sort is deterministic and stable.
- Line 130: The current isPending only returns isExternalPending from the token
balances query; update the hook to also include the pending state from
useReadPreviewerExactly (e.g., previewerPending or similar) and return a
combined flag (isPending = isExternalPending || previewerPending) so consumers
know if any portfolio data source is still loading; compute the combined boolean
before the final return and replace the existing isPending with this combined
value, referencing useReadPreviewerExactly and isExternalPending to locate the
sources to merge.
- Around line 90-100: The usdValue calculation in the useMemo for protocolAssets
currently divides BigInt values before Number conversion, losing fractional
precision; update the expression in the protocolAssets mapping (usdValue) to
convert the BigInt product from withdrawLimit(markets, market.market) *
market.usdPrice to a Number first and then divide by (10 ** market.decimals) and
by 1e18 to preserve decimals—adjust the arithmetic ordering in the usdValue
assignment inside the useMemo so it follows the pattern used elsewhere (e.g.,
AssetSelector) by performing Number(...) on the BigInt product before the
floating-point divisions.
| const tokens = response.tokens[chain.id] ?? []; | ||
| try { | ||
| const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B"); | ||
| return [exa, ...tokens]; | ||
| } catch { | ||
| return tokens; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid silent catch blocks for EXA token fetch failures.
Consider reporting the error before returning the fallback list so failures are visible in telemetry.
♻️ Suggested tweak
- } catch {
- return tokens;
- }
+ } catch (error) {
+ reportError(error);
+ return tokens;
+ }
...
- } catch {
- return allowTokens;
- }
+ } catch (error) {
+ reportError(error);
+ return allowTokens;
+ }Also applies to: 187-192
🤖 Prompt for AI Agents
In `@src/utils/lifi.ts` around lines 118 - 124, The catch block that swallows
failures from getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B")
should log/report the error before returning the fallback tokens; change the
anonymous catch to catch (err) and call the project’s telemetry/logger (e.g.,
logger.error or captureException) with context including chain.id and the EXA
address, then return tokens; apply the same change to the other getToken usage
around the EXA fetch at the 187-192 location.
src/utils/queryClient.ts
Outdated
| const [{ tokens }, exa] = await Promise.all([ | ||
| getTokens({ chainTypes: [ChainType.EVM] }), | ||
| getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch(() => undefined), | ||
| ]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract the hardcoded EXA token address to a named constant.
The EXA token address is a magic string. Per coding guidelines, avoid cryptic values and prefer self-documenting code.
♻️ Suggested refactor
+const EXA_TOKEN_ADDRESS = "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B";
+
export const lifiTokensOptions = queryOptions({
queryKey: ["lifi", "tokens"],
staleTime: Infinity,
gcTime: Infinity,
enabled: !chain.testnet && chain.id !== anvil.id,
queryFn: async () => {
try {
const [{ tokens }, exa] = await Promise.all([
getTokens({ chainTypes: [ChainType.EVM] }),
- getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch(() => undefined),
+ getToken(chain.id, EXA_TOKEN_ADDRESS).catch(() => undefined),
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [{ tokens }, exa] = await Promise.all([ | |
| getTokens({ chainTypes: [ChainType.EVM] }), | |
| getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch(() => undefined), | |
| ]); | |
| const EXA_TOKEN_ADDRESS = "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B"; | |
| export const lifiTokensOptions = queryOptions({ | |
| queryKey: ["lifi", "tokens"], | |
| staleTime: Infinity, | |
| gcTime: Infinity, | |
| enabled: !chain.testnet && chain.id !== anvil.id, | |
| queryFn: async () => { | |
| try { | |
| const [{ tokens }, exa] = await Promise.all([ | |
| getTokens({ chainTypes: [ChainType.EVM] }), | |
| getToken(chain.id, EXA_TOKEN_ADDRESS).catch(() => undefined), | |
| ]); |
🤖 Prompt for AI Agents
In `@src/utils/queryClient.ts` around lines 42 - 45, Replace the hardcoded EXA
token address string passed to getToken with a named constant (e.g.,
EXA_TOKEN_ADDRESS) defined near the top of the module; update the Promise.all
call to use EXA_TOKEN_ADDRESS instead of the literal string and keep the same
.catch fallback, ensuring references to getToken(chain.id, EXA_TOKEN_ADDRESS)
and the exa variable remain unchanged so the intent is self-documenting.
| const protocolAssets = useMemo<ProtocolAsset[]>(() => { | ||
| if (!markets) return []; | ||
| return markets | ||
| .filter(({ floatingDepositAssets }) => floatingDepositAssets > 0n) | ||
| .map((market) => ({ | ||
| ...market, | ||
| usdValue: | ||
| Number((withdrawLimit(markets, market.market) * market.usdPrice) / BigInt(10 ** market.decimals)) / 1e18, | ||
| type: "protocol" as const, | ||
| })); | ||
| }, [markets]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Market type definition and usdPrice field
rg -nA10 'interface Market|type Market' --type=ts --type=tsx | head -50Repository: exactly/exa
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Check withdrawLimit function definition
rg -nB2 -A8 'function withdrawLimit|const withdrawLimit' --type=ts --type=tsx | head -50Repository: exactly/exa
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Find other USD value calculations to see if there's a pattern
rg -nC3 'usdPrice.*decimals|usdValue' --type=ts --type=tsx | head -100Repository: exactly/exa
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Find Market type definition - search all files
rg -nA10 'interface Market|type Market' | head -60Repository: exactly/exa
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Find withdrawLimit function
rg -nB2 -A8 'function withdrawLimit|const withdrawLimit' | head -60Repository: exactly/exa
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Find Market.ts or similar type definition file
fd -e ts -e tsx | grep -i market | head -20Repository: exactly/exa
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Check the exact file and surrounding context in usePortfolio.ts
cat -n src/utils/usePortfolio.ts | sed -n '85,105p'Repository: exactly/exa
Length of output: 850
🏁 Script executed:
#!/bin/bash
# Search for Market type/interface definition - try broader search
rg 'Market' src/ -A 5 | grep -A 20 'interface Market\|type Market' | head -40Repository: exactly/exa
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Look at imports at top of usePortfolio.ts to find Market source
head -30 src/utils/usePortfolio.tsRepository: exactly/exa
Length of output: 905
🏁 Script executed:
#!/bin/bash
# Find withdrawLimit definition
rg 'withdrawLimit' src/ -B 3 -A 10 | head -80Repository: exactly/exa
Length of output: 5117
🏁 Script executed:
#!/bin/bash
# Search for type definitions in likely files
fd Market -e ts | head -20Repository: exactly/exa
Length of output: 37
Fix BigInt division order to preserve decimal precision in USD value calculation.
The usdValue calculation divides BigInt values before converting to Number, which truncates and loses fractional parts. Change line 97 from:
Number((withdrawLimit(markets, market.market) * market.usdPrice) / BigInt(10 ** market.decimals)) / 1e18
to:
Number(withdrawLimit(markets, market.market) * market.usdPrice) / (10 ** market.decimals) / 1e18
This matches the established pattern in the codebase (see AssetSelector.tsx) and preserves decimal precision for accurate USD calculations.
🤖 Prompt for AI Agents
In `@src/utils/usePortfolio.ts` around lines 90 - 100, The usdValue calculation in
the useMemo for protocolAssets currently divides BigInt values before Number
conversion, losing fractional precision; update the expression in the
protocolAssets mapping (usdValue) to convert the BigInt product from
withdrawLimit(markets, market.market) * market.usdPrice to a Number first and
then divide by (10 ** market.decimals) and by 1e18 to preserve decimals—adjust
the arithmetic ordering in the usdValue assignment inside the useMemo so it
follows the pattern used elsewhere (e.g., AssetSelector) by performing
Number(...) on the BigInt product before the floating-point divisions.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/swaps/SelectorModal.tsx (1)
10-48: RemoveusePortfoliohook fromTokenListItemand passmatchingAssetas a prop.TokenListItem is a dumb UI component and should not call hooks. Every list item currently subscribes to the portfolio independently, creating redundant subscriptions. The parent
TokenSelectModalalready has assets and computes the matching asset—pass it down instead.Refactor approach
Move the asset lookup logic to the parent component. Define a helper function to find the matching asset, then pass the result as a prop to
TokenListItem:+type PortfolioAsset = ReturnType<typeof usePortfolio>["assets"][number]; function TokenListItem({ token, isSelected, onPress, language, + matchingAsset, }: { isSelected: boolean; language: string; onPress: () => void; token: Token; + matchingAsset?: PortfolioAsset; }) { - const { assets } = usePortfolio(); - const matchingAsset = assets.find( - (asset) => - (asset.type === "protocol" && asset.asset === token.address) || - (asset.type === "external" && asset.address === token.address), - ); return (In the parent
TokenSelectModal, define the lookup function once and use it when rendering:const { assets } = usePortfolio(); +const findMatchingAsset = (token: Token) => + assets.find( + (asset) => + (asset.type === "protocol" && asset.asset === token.address) || + (asset.type === "external" && asset.address === token.address), + ); // In FlatList renderItem: <TokenListItem token={item} isSelected={selectedToken?.address === item.address} + matchingAsset={findMatchingAsset(item)} onPress={() => { onSelect(item); setSearchQuery(""); }} language={language} />
🤖 Fix all issues with AI agents
In `@src/components/loans/LoanSummary.tsx`:
- Line 76: The AssetLogo is being passed an empty string when symbol is
undefined which yields a blank placeholder; update LoanSummary to avoid this by
either conditionally rendering AssetLogo only when symbol is defined (check
symbol before rendering the <AssetLogo ... /> element) or provide a meaningful
fallback prop (e.g., pass "??" or another placeholder instead of ""), ensuring
you update the AssetLogo invocation and any surrounding JSX in the LoanSummary
component so the UI shows a fallback or skeleton while market data loads.
In `@src/components/shared/AssetSelector.tsx`:
- Around line 35-39: The empty-state ("No available assets") can flash because
assets depends on markets but the code only checks usePortfolio's isPending;
update the conditional around assets length to also verify that markets have
finished loading (e.g., ensure markets is defined/non-null or markets.length is
determined from useReadPreviewerExactly) before showing the empty state.
Concretely, in AssetSelector adjust the if (assets.length === 0) branch to also
gate on markets (from useReadPreviewerExactly) or a markets-loading flag so you
only render the empty-state when markets are resolved in addition to isPending
being false.
In `@src/components/shared/ChainLogo.tsx`:
- Around line 22-33: The layout container in ChainLogo.tsx currently uses View;
replace it with the appropriate Tamagui stack (XStack or YStack) to follow
project layout conventions: import XStack/YStack, swap the <View ...> wrapper
around the <Text> with XStack or YStack, keep the visual props (width, height,
borderRadius, backgroundColor) and convert layout props to the stack equivalents
(center alignment/justify), and ensure the inner Text usage
({name.slice(0,2).toUpperCase()}) remains unchanged.
In `@src/components/shared/WeightedRate.tsx`:
- Around line 48-50: Replace the hardcoded spacing in the depositMarkets.map
render for XStack: change the marginRight prop on XStack (inside
depositMarkets.map) from the numeric conditional marginRight={index <
array.length - 1 ? -6 : 0} to use design tokens, e.g. marginRight={index <
array.length - 1 ? "$-2_5" : "$0"}, so XStack/AssetLogo follow the tokenized
spacing standard.
In `@src/utils/usePortfolio.ts`:
- Around line 15-38: Replace inconsistent plain string address typings with the
Hex type from `@exactly/common/validation`: import Hex and update
ProtocolAsset.market (currently `0x${string}`) as well as ProtocolAsset.asset
and ExternalAsset.address (currently string) to use Hex so all asset address
fields share the same strongly-typed address type; keep other fields and union
type PortfolioAsset unchanged.
- Around line 117-128: The sorting logic in the assets useMemo repeats the
"symbol.slice(3)" normalization; extract a helper (e.g., normalizeSymbol(symbol:
string, isProtocol: boolean)) and use it inside the assets comparator to produce
a normalized symbol for both protocol and external assets before the USDC check;
update references in useMemo (assets), anywhere else duplicating the slice
(e.g., Amount.tsx and other occurrences) to call normalizeSymbol to keep
behavior consistent and reduce duplication.
♻️ Duplicate comments (1)
src/components/pay-mode/Pay.tsx (1)
489-493: MovesetSelectedAssetout of render to avoid re-render loops.Calling a state setter during render can cause unstable re-renders and unexpected behavior. Move this to a
useEffectdriven byassetsandselectedAsset.address.🐛 Proposed fix
- if (!selectedAsset.address && assets[0]) { - const { type } = assets[0]; - setSelectedAsset({ - address: type === "external" ? parse(Address, assets[0].address) : parse(Address, assets[0].market), - external: type === "external", - }); - } + useEffect(() => { + if (!selectedAsset.address && assets[0]) { + const { type } = assets[0]; + setSelectedAsset({ + address: type === "external" ? parse(Address, assets[0].address) : parse(Address, assets[0].market), + external: type === "external", + }); + } + }, [assets, selectedAsset.address]);
| {depositMarkets.map(({ market, symbol }, index, array) => ( | ||
| <XStack key={market} marginRight={index < array.length - 1 ? -6 : 0} zIndex={array.length - index}> | ||
| <AssetLogo symbol={symbol} width={16} height={16} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file in question
fd "WeightedRate.tsx" --type fRepository: exactly/exa
Length of output: 93
🏁 Script executed:
# Check the tamagui.config.ts for available spacing tokens
fd "tamagui.config.ts" --type fRepository: exactly/exa
Length of output: 72
🏁 Script executed:
# Read the WeightedRate.tsx file to see the exact code at lines 48-50
cat -n src/components/shared/WeightedRate.tsx | sed -n '40,55p'Repository: exactly/exa
Length of output: 700
🏁 Script executed:
# Read the tamagui.config.ts to check available spacing tokens
cat -n tamagui.config.tsRepository: exactly/exa
Length of output: 29509
Use a design token instead of hardcoded spacing value.
marginRight={index < array.length - 1 ? -6 : 0} uses a hardcoded numeric value. Replace with a token reference like marginRight={index < array.length - 1 ? "$-2_5" : "$0"} to comply with the design token requirement.
🤖 Prompt for AI Agents
In `@src/components/shared/WeightedRate.tsx` around lines 48 - 50, Replace the
hardcoded spacing in the depositMarkets.map render for XStack: change the
marginRight prop on XStack (inside depositMarkets.map) from the numeric
conditional marginRight={index < array.length - 1 ? -6 : 0} to use design
tokens, e.g. marginRight={index < array.length - 1 ? "$-2_5" : "$0"}, so
XStack/AssetLogo follow the tokenized spacing standard.
| export type ProtocolAsset = { | ||
| asset: string; | ||
| assetName: string; | ||
| decimals: number; | ||
| floatingDepositAssets: bigint; | ||
| market: `0x${string}`; | ||
| symbol: string; | ||
| type: "protocol"; | ||
| usdPrice: bigint; | ||
| usdValue: number; | ||
| }; | ||
|
|
||
| export type ExternalAsset = { | ||
| address: string; | ||
| amount?: bigint; | ||
| decimals: number; | ||
| name: string; | ||
| priceUSD: string; | ||
| symbol: string; | ||
| type: "external"; | ||
| usdValue: number; | ||
| }; | ||
|
|
||
| export type PortfolioAsset = ExternalAsset | ProtocolAsset; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consistent address typing across asset types.
ProtocolAsset.market uses the template literal type `0x${string}` while ProtocolAsset.asset and ExternalAsset.address use plain string. For consistency and type safety, consider using the Hex type from @exactly/common/validation for all address fields.
🤖 Prompt for AI Agents
In `@src/utils/usePortfolio.ts` around lines 15 - 38, Replace inconsistent plain
string address typings with the Hex type from `@exactly/common/validation`: import
Hex and update ProtocolAsset.market (currently `0x${string}`) as well as
ProtocolAsset.asset and ExternalAsset.address (currently string) to use Hex so
all asset address fields share the same strongly-typed address type; keep other
fields and union type PortfolioAsset unchanged.
| const assets = useMemo<PortfolioAsset[]>(() => { | ||
| const combined = [...protocolAssets, ...externalAssets]; | ||
| return combined.sort((a, b) => { | ||
| if (options?.sortBy === "usdcFirst") { | ||
| const aSymbol = a.type === "protocol" ? a.symbol.slice(3) : a.symbol; | ||
| const bSymbol = b.type === "protocol" ? b.symbol.slice(3) : b.symbol; | ||
| if (aSymbol === "USDC") return -1; | ||
| if (bSymbol === "USDC") return 1; | ||
| } | ||
| return b.usdValue - a.usdValue; | ||
| }); | ||
| }, [protocolAssets, externalAssets, options?.sortBy]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting symbol normalization to reduce code duplication.
The symbol.slice(3) pattern for stripping the "exa" prefix appears multiple times in the codebase (lines 63, 121-122, and in Amount.tsx line 143). Consider extracting this to a helper function for consistency and maintainability.
♻️ Optional: Extract symbol normalization helper
// In a shared utils file
export function normalizeSymbol(symbol: string, isProtocol: boolean): string {
const normalized = isProtocol ? symbol.slice(3) : symbol;
return normalized === "WETH" ? "ETH" : normalized;
}Then use in sorting:
- const aSymbol = a.type === "protocol" ? a.symbol.slice(3) : a.symbol;
- const bSymbol = b.type === "protocol" ? b.symbol.slice(3) : b.symbol;
+ const aSymbol = normalizeSymbol(a.symbol, a.type === "protocol");
+ const bSymbol = normalizeSymbol(b.symbol, b.type === "protocol");🤖 Prompt for AI Agents
In `@src/utils/usePortfolio.ts` around lines 117 - 128, The sorting logic in the
assets useMemo repeats the "symbol.slice(3)" normalization; extract a helper
(e.g., normalizeSymbol(symbol: string, isProtocol: boolean)) and use it inside
the assets comparator to produce a normalized symbol for both protocol and
external assets before the USDC check; update references in useMemo (assets),
anywhere else duplicating the slice (e.g., Amount.tsx and other occurrences) to
call normalizeSymbol to keep behavior consistent and reduce duplication.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
src/utils/usePortfolio.ts
Outdated
| }); | ||
| }, [protocolAssets, externalAssets, options?.sortBy]); | ||
|
|
||
| return { portfolio, averageRate, assets, protocolAssets, externalAssets, isPending: isExternalPending || isMarketsPending }; |
There was a problem hiding this comment.
🟡 isPending always true on testnet/anvil causing infinite skeleton display
When running on testnet or anvil, the isPending value from usePortfolio is always true, causing UI components to show skeleton loaders indefinitely when the user has no protocol deposits.
Click to expand
Root Cause
The tokenBalancesOptions query has enabled: !!account && !chain.testnet && chain.id !== anvil.id (src/utils/queryClient.ts:62). On testnet/anvil, this is false, so the query never runs.
In TanStack Query v5, when a query is disabled and has no cached data, isPending is true (status is 'pending').
Impact
In usePortfolio.ts:130:
return { ..., isPending: isExternalPending || isMarketsPending };isExternalPending is always true on testnet/anvil, making isPending always true.
In AssetSelector.tsx:38-45:
if (assets.length === 0) {
if (isPending || !markets) {
return <AssetSkeleton />; // Shows skeleton forever on testnet
}
return <Text>No available assets.</Text>;
}When a user has no protocol deposits on testnet/anvil, they see an infinite loading skeleton instead of "No available assets."
Recommendation: Consider using isFetching instead of isPending for the external assets query, or handle the testnet/anvil case explicitly:
const { data: tokenBalances, isFetching: isExternalFetching } = useQuery(tokenBalancesOptions(resolvedAccount));
// ...
return { ..., isPending: isExternalFetching || isMarketsPending };
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/shared/Success.tsx (1)
104-118: Consider tightening the outer condition to requirecurrencyto be defined.The guard on line 117 correctly prevents
AssetLogofrom receiving an undefined symbol. However, the outer condition on line 104 (currency !== "USDC") still evaluates totruewhencurrencyisundefined, causing the block to render with "with [amount]" but empty currency text and no logo.If rendering this block only makes sense when a non-USDC currency is actually provided, consider adjusting the outer condition:
Proposed fix
- {currency !== "USDC" && ( + {currency && currency !== "USDC" && ( <XStack gap="$s2" alignItems="center"> ... - {currency && <AssetLogo height={22} width={22} symbol={currency} />} + <AssetLogo height={22} width={22} symbol={currency} /> </XStack> )}This makes the inner guard redundant (since
currencyis guaranteed to be defined) and prevents incomplete UI when no currency is provided.src/components/home/AssetList.tsx (1)
139-146: GuardpriceUSDbefore parseUnits to prevent runtime errors.
parseUnitsexpects a string; ifpriceUSDis missing or undefined from the LiFi SDK data, this will throw. A defensive guard is warranted here, as similar code insrc/components/send-funds/Amount.tsxalready uses this pattern.🛠️ Proposed fix
- usdPrice: parseUnits(priceUSD, 18), + usdPrice: parseUnits(priceUSD ?? "0", 18),
🤖 Fix all issues with AI agents
In `@src/components/add-funds/SupportedAssetsSheet.tsx`:
- Line 19: The filter callback in supportedAssets uses an abbreviated parameter
name `s`; rename it to a descriptive name like `symbol` for readability and
consistency (update the arrow function inside Object.keys(assetLogos).filter to
use `symbol` and adjust its references so the check excludes "USDC.e" and "DAI"
correctly). Ensure the variable `supportedAssets` and any downstream uses
continue to work with the renamed parameter.
In `@src/utils/queryClient.ts`:
- Around line 45-47: The catch callback for the getToken call uses an
implicitly-typed error parameter causing the lint rule; change the catch handler
to accept error: unknown, then narrow it before passing to reportError (e.g.,
coerce to Error safely or extract message) and return undefined as before.
Update the catch on the getToken(...) call that assigns to const exa and ensure
reportError is invoked with a proper Error or string derived from the unknown
(or wrap unknown in new Error(String(error))) so the code satisfies
`@typescript-eslint/use-unknown-in-catch-callback-variable`.
♻️ Duplicate comments (5)
src/utils/lifi.ts (2)
118-124: Do not swallow token fetch errors.The EXA token fetch failure is silently ignored, which hides telemetry. Please report the error before falling back.
♻️ Suggested fix
- } catch { - return tokens; - } + } catch (error) { + reportError(error); + return tokens; + }
187-192: Do not swallow allowlist token fetch errors.Same concern as above; log the failure before returning the fallback list.
♻️ Suggested fix
- } catch { - return allowTokens; - } + } catch (error) { + reportError(error); + return allowTokens; + }src/utils/usePortfolio.ts (1)
90-98: Avoid BigInt division before Number conversion in usdValue.Dividing BigInt before converting truncates decimals and under-reports value. Convert to Number first, then divide by decimals.
How does JavaScript BigInt division handle fractional results, and why does it truncate decimals?🐛 Proposed fix
- usdValue: - Number((withdrawLimit(markets, market.market) * market.usdPrice) / BigInt(10 ** market.decimals)) / 1e18, + usdValue: + Number(withdrawLimit(markets, market.market) * market.usdPrice) / (10 ** market.decimals) / 1e18,Based on learnings, this preserves decimal precision for display.
src/components/shared/WeightedRate.tsx (1)
48-51: Replace hardcoded overlap spacing with a design token.
marginRight={index < array.length - 1 ? -6 : 0}should use a spacing token to comply with design-token-only styling.♻️ Proposed fix
- <XStack key={market} marginRight={index < array.length - 1 ? -6 : 0} zIndex={array.length - index}> + <XStack + key={market} + marginRight={index < array.length - 1 ? "$-2_5" : "$0"} + zIndex={array.length - index} + >As per coding guidelines, use design tokens for spacing.
src/components/loans/LoanSummary.tsx (1)
76-76: Avoid rendering AssetLogo with an empty symbol.Passing
""yields a blank fallback. Render only whensymbolexists (or provide a real fallback).♻️ Suggested fix
- <AssetLogo height={16} symbol={symbol ?? ""} width={16} /> + {symbol ? <AssetLogo height={16} symbol={symbol} width={16} /> : null}
src/utils/queryClient.ts
Outdated
| const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch((error) => { | ||
| reportError(error); | ||
| return undefined; |
There was a problem hiding this comment.
Fix lint error: use unknown in catch callback.
CI fails with @typescript-eslint/use-unknown-in-catch-callback-variable on this callback.
🐛 Proposed fix
- const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch((error) => {
+ const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch((error: unknown) => {
reportError(error);
return undefined;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch((error) => { | |
| reportError(error); | |
| return undefined; | |
| const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch((error: unknown) => { | |
| reportError(error); | |
| return undefined; | |
| }); |
🧰 Tools
🪛 GitHub Actions: test
[error] 45-45: eslint: Prefer the safe 'unknown' in a catch callback variable. @typescript-eslint/use-unknown-in-catch-callback-variable. Step: 'nx run @exactly/mobile:"test:eslint"', command: 'eslint ./src --max-warnings=0'
🤖 Prompt for AI Agents
In `@src/utils/queryClient.ts` around lines 45 - 47, The catch callback for the
getToken call uses an implicitly-typed error parameter causing the lint rule;
change the catch handler to accept error: unknown, then narrow it before passing
to reportError (e.g., coerce to Error safely or extract message) and return
undefined as before. Update the catch on the getToken(...) call that assigns to
const exa and ensure reportError is invoked with a proper Error or string
derived from the unknown (or wrap unknown in new Error(String(error))) so the
code satisfies `@typescript-eslint/use-unknown-in-catch-callback-variable`.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/components/add-funds/AddCrypto.tsx`:
- Around line 145-148: The XStack mapping over supportedAssets uses a hardcoded
negative marginRight (-12) which violates the token-only spacing rule; update
the XStack rendered in the supportedAssets.map (the XStack element that wraps
AssetLogo) to use a Tamagui space token instead of the literal -12 (e.g.,
replace the conditional marginRight value with the appropriate negative space
token from your theme/spacing tokens), ensuring the token is referenced directly
(not a computed pixel literal) and preserving the conditional logic for the last
item.
In `@src/components/add-funds/Bridge.tsx`:
- Around line 700-711: The View wrapping ChainLogo currently uses a hardcoded
borderColor="white"; replace that with the design token
borderColor="$borderNeutralSoft" to match the existing border usage pattern in
this component (see other Asset/Chain logo wrappers using $borderNeutralSoft) so
the View containing ChainLogo and the AssetLogo remain consistent with theme
tokens and not use $background which is empty.
In `@src/components/loans/AmountSelector.tsx`:
- Around line 84-90: selectedMarket can be undefined causing AssetLogo to
receive an empty string and render an empty badge; update AmountSelector to
guard or provide a clear placeholder: compute a symbol before rendering (use
selectedMarket?.symbol.slice(3), map "WETH" -> "ETH") and if selectedMarket is
undefined render a small skeleton/placeholder component or pass a meaningful
default symbol (e.g., "UNKNOWN" or "—") to the AssetLogo prop so AssetLogo never
receives an empty string; adjust the render around AssetLogo in AmountSelector
accordingly.
In `@src/components/shared/AssetLogo.tsx`:
- Around line 26-45: AssetLogo uses a computed numeric fontSize (fontSize={width
* 0.4}) which violates design-token rules; replace that numeric size with a
Tamagui token by mapping the incoming width to an appropriate token (e.g.,
small/medium/large tokens) and pass that token string into the Text component's
fontSize prop instead; update the Text in AssetLogo to use the chosen token (and
add a small helper or inline conditional mapping based on the width prop) so the
component uses token-based sizing while leaving getTokenLogoURI, useQuery, and
StyledImage unchanged.
In `@src/components/shared/CopyAddressSheet.tsx`:
- Line 18: The component CopyAddressSheet currently derives supportedAssets from
Object.keys(assetLogos) which creates an indirect dependency on assetLogos;
extract a dedicated constant array (e.g., SUPPORTED_ASSETS) listing the allowed
symbols (excluding "USDC.e" and "DAI") and replace the derived supportedAssets
with this constant; update any references to supportedAssets in CopyAddressSheet
to use SUPPORTED_ASSETS and remove the unused dependency on assetLogos in this
file (or keep assetLogos import only if still needed elsewhere).
♻️ Duplicate comments (7)
src/utils/lifi.ts (2)
119-124: Avoid silent catch blocks for EXA token fetch failures.The catch block swallows failures from
getToken()without reporting. Consider logging the error before returning the fallback to maintain visibility in telemetry.♻️ Suggested fix
try { const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B"); return [exa, ...tokens]; - } catch { + } catch (error) { + reportError(error); return tokens; }
187-192: Same silent catch pattern here.Apply consistent error reporting for the EXA token fetch failure.
♻️ Suggested fix
try { const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B"); return [exa, ...allowTokens]; - } catch { + } catch (error) { + reportError(error); return allowTokens; }src/utils/queryClient.ts (1)
35-54: Extract the hardcoded EXA token address to a named constant.The EXA token address appears multiple times in the codebase. Per coding guidelines, avoid magic strings and prefer self-documenting code.
♻️ Suggested refactor
+const EXA_TOKEN_ADDRESS = "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B"; + export const lifiTokensOptions = queryOptions({ // ... queryFn: async () => { try { // ... if (chain.id !== optimism.id) return allTokens; - const exa = await getToken(chain.id, "0x1e925De1c68ef83bD98eE3E130eF14a50309C01B").catch((error: unknown) => { + const exa = await getToken(chain.id, EXA_TOKEN_ADDRESS).catch((error: unknown) => { reportError(error); }); return exa ? [exa, ...allTokens] : allTokens; } catch (error) { // ... } }, });This constant could also be reused in
src/utils/lifi.tswhere the same address appears ingetAllTokensandgetAllowTokens.src/utils/usePortfolio.ts (2)
94-98: Fix usdValue precision loss from BigInt division.Line 97 divides in BigInt space before conversion, truncating decimals. Convert to Number first, then divide to preserve precision.
🐛 Proposed fix
- usdValue: - Number((withdrawLimit(markets, market.market) * market.usdPrice) / BigInt(10 ** market.decimals)) / 1e18, + usdValue: + Number(withdrawLimit(markets, market.market) * market.usdPrice) / (10 ** market.decimals) / 1e18,Based on learnings, prefer
Number(bigintAmount) / 10 ** decimalsto avoid truncation.
52-52:isPendingcan stay true when tokenBalances query is disabled.On testnet/anvil the query is disabled, but TanStack Query reports
isPending: true, so Line 136 can keep the UI in a perpetual loading state. Consider deriving pending fromisFetchingonly when the query is enabled.🧩 Suggested adjustment
- const { data: tokenBalances, isPending: isExternalPending } = useQuery(tokenBalancesOptions(resolvedAccount)); + const tokenBalancesQuery = tokenBalancesOptions(resolvedAccount); + const { data: tokenBalances, isFetching: isExternalFetching } = useQuery(tokenBalancesQuery); + const isExternalPending = tokenBalancesQuery.enabled ? isExternalFetching : false;Also applies to: 136-136
src/components/shared/WeightedRate.tsx (1)
48-52: LGTM on symbol-based AssetLogo migration.The mapping correctly passes
symboltoAssetLogofor each deposit market.Note: The hardcoded
marginRight={-6}spacing value should use a design token instead (e.g.,"$-2_5"), but this was already flagged in a previous review.src/components/loans/LoanSummary.tsx (1)
76-76: Consider handling undefined symbol more gracefully.When
symbolisundefined, passing an empty string results in a blank placeholder. SinceisMarketFetchingis now tracked, this is less likely to occur, but conditional rendering would be safer.
| {supportedAssets.map((symbol, index) => ( | ||
| <XStack key={symbol} marginRight={index < supportedAssets.length - 1 ? -12 : 0} zIndex={index}> | ||
| <AssetLogo symbol={symbol} width={32} height={32} /> | ||
| </XStack> |
There was a problem hiding this comment.
Replace hardcoded negative spacing with a token-derived value.
marginRight={... ? -12 : 0} violates the token-only spacing rule. Use a Tamagui space token value instead.
🛠️ Suggested fix using Tamagui tokens
-import { ScrollView, XStack, YStack } from "tamagui";
+import { ScrollView, XStack, YStack, useToken } from "tamagui";
@@
export default function AddCrypto() {
const router = useRouter();
const fontScale = PixelRatio.getFontScale();
+ const overlapSpacing = -useToken("space", "s3");
const { address } = useAccount();
@@
- {supportedAssets.map((symbol, index) => (
- <XStack key={symbol} marginRight={index < supportedAssets.length - 1 ? -12 : 0} zIndex={index}>
+ {supportedAssets.map((symbol, index) => (
+ <XStack key={symbol} marginRight={index < supportedAssets.length - 1 ? overlapSpacing : 0} zIndex={index}>
<AssetLogo symbol={symbol} width={32} height={32} />
</XStack>
))}As per coding guidelines, use design tokens for spacing values.
🤖 Prompt for AI Agents
In `@src/components/add-funds/AddCrypto.tsx` around lines 145 - 148, The XStack
mapping over supportedAssets uses a hardcoded negative marginRight (-12) which
violates the token-only spacing rule; update the XStack rendered in the
supportedAssets.map (the XStack element that wraps AssetLogo) to use a Tamagui
space token instead of the literal -12 (e.g., replace the conditional
marginRight value with the appropriate negative space token from your
theme/spacing tokens), ensuring the token is referenced directly (not a computed
pixel literal) and preserving the conditional logic for the last item.
| <AssetLogo symbol={destinationToken.symbol} width={40} height={40} /> | ||
| <View | ||
| position="absolute" | ||
| bottom={0} | ||
| right={0} | ||
| width={20} | ||
| height={20} | ||
| borderWidth={1} | ||
| borderColor="white" | ||
| borderRadius={10} | ||
| borderRadius="$r_0" | ||
| overflow="hidden" | ||
| > | ||
| <OptimismImage width="100%" height="100%" /> | ||
| <ChainLogo size={20} /> | ||
| </View> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the tamagui config file
find . -name "tamagui.config.*" -type f 2>/dev/null | head -20Repository: exactly/exa
Length of output: 74
🏁 Script executed:
#!/bin/bash
# Get basic info about the Bridge.tsx file to understand context
wc -l src/components/add-funds/Bridge.tsx
head -30 src/components/add-funds/Bridge.tsxRepository: exactly/exa
Length of output: 1174
🏁 Script executed:
#!/bin/bash
# Examine the tamagui config to see color tokens
cat tamagui.config.ts | head -200Repository: exactly/exa
Length of output: 6719
🏁 Script executed:
#!/bin/bash
# Look at the specific lines in Bridge.tsx to understand context
sed -n '690,720p' src/components/add-funds/Bridge.tsxRepository: exactly/exa
Length of output: 1731
🏁 Script executed:
#!/bin/bash
# Check if there are more color tokens defined, particularly looking for background or neutral tokens
grep -n "background\|neutral\|border" tamagui.config.ts | head -50Repository: exactly/exa
Length of output: 2863
🏁 Script executed:
#!/bin/bash
# Get the complete color tokens section to see if there's a white or neutral color
sed -n '/color: {/,/},/p' tamagui.config.ts | tail -100Repository: exactly/exa
Length of output: 3341
🏁 Script executed:
#!/bin/bash
# Also check if there's any design token documentation or if $background is actually defined
grep -n "\$background\|background:" tamagui.config.tsRepository: exactly/exa
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Check the exact hex value for grayscaleLight1 to confirm it's white-ish
grep -n "grayscaleLight1:" tamagui.config.tsRepository: exactly/exa
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Let's see the full tamagui config around line 262-268 to understand the theme tokens better
sed -n '260,275p' tamagui.config.tsRepository: exactly/exa
Length of output: 927
🏁 Script executed:
#!/bin/bash
# Verify all available color tokens in the light theme
sed -n '260,275p' tamagui.config.tsRepository: exactly/exa
Length of output: 927
🏁 Script executed:
#!/bin/bash
# Check for any other white-like or neutral border tokens
grep -n "uiNeutral\|borderNeutral" tamagui.config.ts | head -20Repository: exactly/exa
Length of output: 1276
🏁 Script executed:
#!/bin/bash
# Search for other uses of borderColor in Bridge.tsx to see pattern
grep -n "borderColor" src/components/add-funds/Bridge.tsxRepository: exactly/exa
Length of output: 374
🏁 Script executed:
#!/bin/bash
# Search for other badge/overlay patterns with borders in the codebase
rg "borderColor.*white|borderColor.*\$ui|borderColor.*\$background" --type=tsx -C2 | head -50Repository: exactly/exa
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Get more context about the light/dark theme tokens and what's available
sed -n '260,380p' tamagui.config.ts | grep -E "border|inverse"Repository: exactly/exa
Length of output: 853
Hardcoded color violates design token guidelines; proposed fix is incorrect.
Line 706 uses borderColor="white" instead of a design token. However, the proposed fix using $background is incorrect—that token is defined as an empty string in the config. Looking at similar border usage patterns in this file (lines 614, 628, 672, 918), the appropriate token is $borderNeutralSoft, which maps to the light grayscale border color.
Proposed fix
<View
position="absolute"
bottom={0}
right={0}
borderWidth={1}
- borderColor="white"
+ borderColor="$borderNeutralSoft"
borderRadius="$r_0"
overflow="hidden"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <AssetLogo symbol={destinationToken.symbol} width={40} height={40} /> | |
| <View | |
| position="absolute" | |
| bottom={0} | |
| right={0} | |
| width={20} | |
| height={20} | |
| borderWidth={1} | |
| borderColor="white" | |
| borderRadius={10} | |
| borderRadius="$r_0" | |
| overflow="hidden" | |
| > | |
| <OptimismImage width="100%" height="100%" /> | |
| <ChainLogo size={20} /> | |
| </View> | |
| <AssetLogo symbol={destinationToken.symbol} width={40} height={40} /> | |
| <View | |
| position="absolute" | |
| bottom={0} | |
| right={0} | |
| borderWidth={1} | |
| borderColor="$borderNeutralSoft" | |
| borderRadius="$r_0" | |
| overflow="hidden" | |
| > | |
| <ChainLogo size={20} /> | |
| </View> |
🤖 Prompt for AI Agents
In `@src/components/add-funds/Bridge.tsx` around lines 700 - 711, The View
wrapping ChainLogo currently uses a hardcoded borderColor="white"; replace that
with the design token borderColor="$borderNeutralSoft" to match the existing
border usage pattern in this component (see other Asset/Chain logo wrappers
using $borderNeutralSoft) so the View containing ChainLogo and the AssetLogo
remain consistent with theme tokens and not use $background which is empty.
| <AssetLogo | ||
| source={{ | ||
| uri: assetLogos[ | ||
| selectedMarket?.symbol.slice(3) === "WETH" | ||
| ? "ETH" | ||
| : (selectedMarket?.symbol.slice(3) as keyof typeof assetLogos) | ||
| ], | ||
| }} | ||
| symbol={ | ||
| selectedMarket?.symbol.slice(3) === "WETH" ? "ETH" : (selectedMarket?.symbol.slice(3) ?? "") | ||
| } | ||
| width={32} | ||
| height={32} | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider handling the undefined market edge case for AssetLogo.
When selectedMarket is undefined, this passes an empty string to symbol, causing AssetLogo to render a fallback badge with empty initials. While this is a brief state during initial load, consider either showing a skeleton or a placeholder symbol.
♻️ Optional improvement
<AssetLogo
- symbol={
- selectedMarket?.symbol.slice(3) === "WETH" ? "ETH" : (selectedMarket?.symbol.slice(3) ?? "")
- }
+ symbol={
+ selectedMarket
+ ? selectedMarket.symbol.slice(3) === "WETH"
+ ? "ETH"
+ : selectedMarket.symbol.slice(3)
+ : "?"
+ }
width={32}
height={32}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <AssetLogo | |
| source={{ | |
| uri: assetLogos[ | |
| selectedMarket?.symbol.slice(3) === "WETH" | |
| ? "ETH" | |
| : (selectedMarket?.symbol.slice(3) as keyof typeof assetLogos) | |
| ], | |
| }} | |
| symbol={ | |
| selectedMarket?.symbol.slice(3) === "WETH" ? "ETH" : (selectedMarket?.symbol.slice(3) ?? "") | |
| } | |
| width={32} | |
| height={32} | |
| /> | |
| <AssetLogo | |
| symbol={ | |
| selectedMarket | |
| ? selectedMarket.symbol.slice(3) === "WETH" | |
| ? "ETH" | |
| : selectedMarket.symbol.slice(3) | |
| : "?" | |
| } | |
| width={32} | |
| height={32} | |
| /> |
🤖 Prompt for AI Agents
In `@src/components/loans/AmountSelector.tsx` around lines 84 - 90, selectedMarket
can be undefined causing AssetLogo to receive an empty string and render an
empty badge; update AmountSelector to guard or provide a clear placeholder:
compute a symbol before rendering (use selectedMarket?.symbol.slice(3), map
"WETH" -> "ETH") and if selectedMarket is undefined render a small
skeleton/placeholder component or pass a meaningful default symbol (e.g.,
"UNKNOWN" or "—") to the AssetLogo prop so AssetLogo never receives an empty
string; adjust the render around AssetLogo in AmountSelector accordingly.
| export default function AssetLogo({ height, symbol, width }: { height: number; symbol: string; width: number }) { | ||
| const { data: tokens = [] } = useQuery(lifiTokensOptions); | ||
| const uri = getTokenLogoURI(tokens, symbol); | ||
| if (!uri) { | ||
| return ( | ||
| <View | ||
| width={width} | ||
| height={height} | ||
| borderRadius="$r_0" | ||
| backgroundColor="$backgroundStrong" | ||
| alignItems="center" | ||
| justifyContent="center" | ||
| > | ||
| <Text fontSize={width * 0.4} fontWeight="bold" color="$uiNeutralSecondary"> | ||
| {symbol.slice(0, 2).toUpperCase()} | ||
| </Text> | ||
| </View> | ||
| ); | ||
| } | ||
| return <StyledImage source={{ uri }} width={width} height={height} />; |
There was a problem hiding this comment.
Use Tamagui font-size tokens instead of a computed numeric size.
fontSize={width * 0.4} introduces a hardcoded numeric style. Please switch to a token-based size (optionally mapped from width) to comply with the design-token rule.
🛠️ Suggested fix using token-based sizing
export default function AssetLogo({ height, symbol, width }: { height: number; symbol: string; width: number }) {
+ const fontSize = width >= 40 ? "$7" : width >= 32 ? "$6" : width >= 24 ? "$5" : "$4";
const { data: tokens = [] } = useQuery(lifiTokensOptions);
const uri = getTokenLogoURI(tokens, symbol);
if (!uri) {
return (
<View
width={width}
height={height}
borderRadius="$r_0"
backgroundColor="$backgroundStrong"
alignItems="center"
justifyContent="center"
>
- <Text fontSize={width * 0.4} fontWeight="bold" color="$uiNeutralSecondary">
+ <Text fontSize={fontSize} fontWeight="bold" color="$uiNeutralSecondary">
{symbol.slice(0, 2).toUpperCase()}
</Text>
</View>
);
}
return <StyledImage source={{ uri }} width={width} height={height} />;
}As per coding guidelines, keep font sizing on design tokens.
🤖 Prompt for AI Agents
In `@src/components/shared/AssetLogo.tsx` around lines 26 - 45, AssetLogo uses a
computed numeric fontSize (fontSize={width * 0.4}) which violates design-token
rules; replace that numeric size with a Tamagui token by mapping the incoming
width to an appropriate token (e.g., small/medium/large tokens) and pass that
token string into the Text component's fontSize prop instead; update the Text in
AssetLogo to use the chosen token (and add a small helper or inline conditional
mapping based on the width prop) so the component uses token-based sizing while
leaving getTokenLogoURI, useQuery, and StyledImage unchanged.
| const supportedAssets = Object.entries(assetLogos) | ||
| .filter(([symbol]) => symbol !== "USDC.e" && symbol !== "DAI") | ||
| .map(([symbol, image]) => ({ symbol, image })); | ||
| const supportedAssets = Object.keys(assetLogos).filter((s) => s !== "USDC.e" && s !== "DAI"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting supported assets to a constant array.
The assetLogos import is now only used to derive symbol keys, not for the actual URIs. This creates an indirect dependency where changing assetLogos affects supported assets even though the logo URIs are no longer consumed here.
Proposed refactor
-import assetLogos from "../../utils/assetLogos";
+// TODO consider moving supported assets to a shared constant
+const supportedAssets = ["USDC", "WETH", "wstETH", "OP", "WBTC", "cbBTC", "AERO"];
-
-const supportedAssets = Object.keys(assetLogos).filter((s) => s !== "USDC.e" && s !== "DAI");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const supportedAssets = Object.keys(assetLogos).filter((s) => s !== "USDC.e" && s !== "DAI"); | |
| // TODO consider moving supported assets to a shared constant | |
| const supportedAssets = ["USDC", "WETH", "wstETH", "OP", "WBTC", "cbBTC", "AERO"]; |
🤖 Prompt for AI Agents
In `@src/components/shared/CopyAddressSheet.tsx` at line 18, The component
CopyAddressSheet currently derives supportedAssets from Object.keys(assetLogos)
which creates an indirect dependency on assetLogos; extract a dedicated constant
array (e.g., SUPPORTED_ASSETS) listing the allowed symbols (excluding "USDC.e"
and "DAI") and replace the derived supportedAssets with this constant; update
any references to supportedAssets in CopyAddressSheet to use SUPPORTED_ASSETS
and remove the unused dependency on assetLogos in this file (or keep assetLogos
import only if still needed elsewhere).
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.