Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,26 @@ export type PreferredCurrencyDisplayProps = {
sourceAmount: Decimal | null | undefined
sourceAssetId: string
forceFallback?: boolean
usdPrice?: Decimal
} & Omit<CurrencyDisplayProps, 'currency' | 'value'>

export const PreferredCurrencyDisplay = (
props: PreferredCurrencyDisplayProps,
) => {
const { sourceAmount, sourceAssetId, forceFallback, ...displayProps } =
props
const {
sourceAmount,
sourceAssetId,
forceFallback,
usdPrice,
...displayProps
} = props
const { displayCurrency, convertedValue, isPending } =
usePreferredCurrencyDisplay(sourceAmount, sourceAssetId, forceFallback)
usePreferredCurrencyDisplay(
sourceAmount,
sourceAssetId,
forceFallback,
usdPrice,
)

return (
<CurrencyDisplay
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,35 @@ describe('usePreferredCurrencyDisplay', () => {
expect(result.current.isPending).toBe(true)
expect(result.current.convertedValue).toBeNull()
})

it('uses pre-fetched usdPrice and skips asset price query', () => {
mockUseCurrency.mockReturnValue({
preferredCurrency: 'EUR',
fallbackCurrency: 'USD',
usdToPreferred: (v: Decimal) => v.mul(Decimal('0.85')),
})

// Return empty map — should not matter since preFetchedUsdPrice is provided
mockUseAssetPricesQuery.mockReturnValue({
data: new Map(),
isPending: false,
})

const { result } = renderHook(() =>
usePreferredCurrencyDisplay(
Decimal(10),
'123',
false,
Decimal('1.50'),
),
)

expect(result.current.displayCurrency).toBe('EUR')
// 10 * 1.50 = 15 USD, usdToPreferred(15) = 15 * 0.85 = 12.75
expect(result.current.convertedValue).toEqual(Decimal(12.75))
expect(result.current.isPending).toBe(false)

// Verify it was called with empty array (no per-item fetch)
expect(mockUseAssetPricesQuery).toHaveBeenCalledWith([])
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const usePreferredCurrencyDisplay = (
sourceAmount: Decimal | null | undefined,
sourceAssetId: string,
forceFallback?: boolean,
preFetchedUsdPrice?: Decimal,
): UsePreferredCurrencyDisplayResult => {
const { preferredCurrency, fallbackCurrency, usdToPreferred } =
useCurrency()
Expand All @@ -45,8 +46,11 @@ export const usePreferredCurrencyDisplay = (
return preferredCurrency
}, [needsFallback, fallbackCurrency, preferredCurrency])

// Asset prices in USD
const priceIDs = useMemo(() => [sourceAssetId], [sourceAssetId])
// Skip per-item price fetch when a pre-fetched price is provided (bulk query optimization)
const priceIDs = useMemo(
() => (preFetchedUsdPrice !== undefined ? [] : [sourceAssetId]),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may consider adding no-negated-condition rule. What do you think?

Suggested change
() => (preFetchedUsdPrice !== undefined ? [] : [sourceAssetId]),
() => (!!preFetchedUsdPrice ? [] : [sourceAssetId]),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can definitely discuss it, but I'm personally not a fan. There are some cases which are much easier to assess if we use negated conditions, so I don't see much benefit on not allowing that options to be used. But would be happy to do so in a follow up PR refactoring everywhere in case we agree on that.

[preFetchedUsdPrice, sourceAssetId],
)
const { data: usdPrices, isPending: usdPricesPending } =
useAssetPricesQuery(priceIDs)

Expand All @@ -55,19 +59,29 @@ export const usePreferredCurrencyDisplay = (
usePreferredCurrencyPriceQuery(fallbackCurrency, needsFallback)

const isPending = useMemo(() => {
if (preFetchedUsdPrice !== undefined) {
return needsFallback ? fallbackRatePending : false
}
if (needsFallback) {
return usdPricesPending || fallbackRatePending
}
return usdPricesPending
}, [needsFallback, fallbackRatePending, usdPricesPending])
}, [
preFetchedUsdPrice,
needsFallback,
fallbackRatePending,
usdPricesPending,
])

const convertedValue = useMemo(() => {
if (sourceAmount == null) return null
if (!sourceAmount) return null
if (isPending) return null

const assetPrice = usdPrices?.get(sourceAssetId)
const usdPrice = assetPrice?.usdPrice ?? Decimal(0)
const usdValue = sourceAmount.mul(usdPrice)
const resolvedUsdPrice =
preFetchedUsdPrice ??
usdPrices?.get(sourceAssetId)?.usdPrice ??
Decimal(0)
const usdValue = sourceAmount.mul(resolvedUsdPrice)

if (!needsFallback) {
return usdToPreferred(usdValue)
Expand All @@ -79,6 +93,7 @@ export const usePreferredCurrencyDisplay = (
sourceAmount,
isPending,
needsFallback,
preFetchedUsdPrice,
usdPrices,
sourceAssetId,
usdToPreferred,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ export const AccountAssetList = ({
<SwipeableAssetItem
item={item}
isSwipeEnabled={isSwipeable}
usdPrice={
renderItemProps.assetPrices.get(item.assetId)?.usdPrice
}
onPress={renderItemProps.goToAssetScreen}
onOptOut={renderItemProps.handleOptOut}
/>
Expand All @@ -117,6 +120,8 @@ export const AccountAssetList = ({
renderItem={renderItem}
scrollEnabled={scrollEnabled}
keyExtractor={item => item.assetId}
estimatedItemSize={72}
recycleItems
automaticallyAdjustKeyboardInsets
keyboardDismissMode='interactive'
contentContainerStyle={styles.rootContainer}
Expand Down Expand Up @@ -179,7 +184,7 @@ export const AccountAssetList = ({
<LoadingView
variant='skeleton'
size='sm'
count={3}
count={8}
/>
</PWView>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,21 @@ import {
} from '@components/core'
import { AccountAssetItemView } from '@modules/assets/components/AssetItem/AccountAssetItemView'
import { AssetWithAccountBalance } from '@perawallet/wallet-core-accounts'
import { Decimal } from 'decimal.js'
import { useStyles } from './styles'

export type SwipeableAssetItemProps = {
item: AssetWithAccountBalance
isSwipeEnabled: boolean
usdPrice?: Decimal
onPress: (item: AssetWithAccountBalance) => void
onOptOut: (item: AssetWithAccountBalance) => void
}

const SwipeableAssetItemInner = ({
item,
isSwipeEnabled,
usdPrice,
onPress,
onOptOut,
}: SwipeableAssetItemProps) => {
Expand Down Expand Up @@ -69,7 +72,10 @@ const SwipeableAssetItemInner = ({
style={styles.itemContainer}
onPress={handlePress}
>
<AccountAssetItemView accountBalance={item} />
<AccountAssetItemView
accountBalance={item}
usdPrice={usdPrice}
/>
</PWTouchableOpacity>
)
}
Expand All @@ -86,7 +92,10 @@ const SwipeableAssetItemInner = ({
style={styles.itemContainer}
onPress={handlePress}
>
<AccountAssetItemView accountBalance={item} />
<AccountAssetItemView
accountBalance={item}
usdPrice={usdPrice}
/>
</PWTouchableOpacity>
</PWView>
</PWSwipeable>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import {
WalletAccount,
AssetWithAccountBalance,
} from '@perawallet/wallet-core-accounts'
import { useAssetsQuery } from '@perawallet/wallet-core-assets'
import {
useAssetsQuery,
useAssetPricesQuery,
type AssetPrices,
} from '@perawallet/wallet-core-assets'
import { useModalState, ModalState } from '@hooks/useModalState'
import { useDebouncedValue } from '@hooks/useDebouncedValue'
import { useSortedAssetBalances } from './useSortedAssetBalances'
Expand Down Expand Up @@ -48,6 +52,7 @@ type UseAccountAssetListResult = {
getEmptyBody: () => string
renderItemProps: {
isWatch: boolean
assetPrices: AssetPrices
goToAssetScreen: (asset: AssetWithAccountBalance) => void
handleOptOut: (item: AssetWithAccountBalance) => void
}
Expand Down Expand Up @@ -80,6 +85,7 @@ export const useAccountAssetList = ({
[balanceData],
)
const { data: assets } = useAssetsQuery(assetIDs)
const { data: assetPrices } = useAssetPricesQuery(assetIDs)
const navigation = useNavigation<NativeStackNavigationProp<ParamListBase>>()

const { sortedBalances, hideZeroBalance } = useSortedAssetBalances(
Expand Down Expand Up @@ -181,10 +187,11 @@ export const useAccountAssetList = ({
const renderItemProps = useMemo(
() => ({
isWatch,
assetPrices,
goToAssetScreen,
handleOptOut,
}),
[isWatch, goToAssetScreen, handleOptOut],
[isWatch, assetPrices, goToAssetScreen, handleOptOut],
)

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
limitations under the License
*/

import { Decimal } from 'decimal.js'
import { AssetIcon } from '../AssetIcon'
import { CurrencyDisplay } from '@components/CurrencyDisplay'
import { PreferredCurrencyDisplay } from '@components/PreferredCurrencyDisplay'
import {
PWIcon,
PWIconSize,
PWSkeleton,
PWText,
PWView,
PWViewProps,
Expand All @@ -27,21 +29,31 @@ import { useMemo } from 'react'

export type AccountAssetItemViewProps = {
accountBalance: AssetWithAccountBalance
usdPrice?: Decimal
iconSize?: PWIconSize
} & PWViewProps

export const AccountAssetItemView = ({
accountBalance,
usdPrice,
iconSize,
...rest
}: AccountAssetItemViewProps) => {
const styles = useStyles()

const { data: assets } = useAssetsQuery([accountBalance.assetId])
// Use pre-fetched asset data when available to avoid N+1 queries.
// Falls back to individual fetch for callers that don't populate accountBalance.asset.
const assetIds = useMemo(
() => (accountBalance.asset ? [] : [accountBalance.assetId]),
[accountBalance.asset, accountBalance.assetId],
)
const { data: fetchedAssets } = useAssetsQuery(assetIds)

const asset = useMemo(() => {
return assets?.get(accountBalance.assetId)
}, [assets, accountBalance.assetId])
return (
accountBalance.asset ?? fetchedAssets?.get(accountBalance.assetId)
)
}, [accountBalance.asset, fetchedAssets, accountBalance.assetId])

const isAlgo = useMemo(
() => asset?.assetId === ALGO_ASSET_ID,
Expand Down Expand Up @@ -77,7 +89,18 @@ export const AccountAssetItemView = ({
}, [asset, accountBalance.assetId])

if (!asset?.unitName) {
return <></>
return (
<PWView style={[styles.container, rest.style]}>
<PWSkeleton
height={40}
width={40}
circle
/>
<PWView style={styles.dataContainer}>
<PWSkeleton height={16} />
</PWView>
</PWView>
)
}

return (
Expand Down Expand Up @@ -121,6 +144,7 @@ export const AccountAssetItemView = ({
<PreferredCurrencyDisplay
sourceAmount={accountBalance.amount}
sourceAssetId={accountBalance.assetId}
usdPrice={usdPrice}
precision={2}
minPrecision={2}
showSymbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,15 @@ vi.mock('@components/PreferredCurrencyDisplay', () => ({
}))

describe('AccountAssetItemView', () => {
it('renders asset info for Algo', () => {
it('renders asset info for Algo using pre-fetched asset data', () => {
const accountBalance = {
assetId: 0,
asset: {
assetId: 0,
unitName: 'ALGO',
name: 'Algorand',
decimals: 6,
},
amount: '1000000',
} as unknown as AssetWithAccountBalance

Expand All @@ -65,7 +71,25 @@ describe('AccountAssetItemView', () => {
expect(screen.getByText('ALGO')).toBeTruthy()
})

it('renders asset units and IDs for non-algo assets', async () => {
it('renders asset units and IDs for non-algo assets using pre-fetched data', () => {
const accountBalance = {
assetId: 123,
asset: {
assetId: 123,
unitName: 'TEST',
name: 'Test Asset',
decimals: 2,
},
amount: '500',
} as unknown as AssetWithAccountBalance

render(<AccountAssetItemView accountBalance={accountBalance} />)

expect(screen.getByText('Test Asset')).toBeTruthy()
expect(screen.getByText('TEST - 123')).toBeTruthy()
})

it('falls back to useAssetsQuery when asset is not pre-fetched', async () => {
const { useAssetsQuery } =
await import('@perawallet/wallet-core-assets')
vi.mocked(useAssetsQuery).mockReturnValue({
Expand Down Expand Up @@ -93,4 +117,27 @@ describe('AccountAssetItemView', () => {
expect(screen.getByText('Test Asset')).toBeTruthy()
expect(screen.getByText('TEST - 123')).toBeTruthy()
})

it('renders skeleton placeholder when asset data is not available', async () => {
const { useAssetsQuery } =
await import('@perawallet/wallet-core-assets')
vi.mocked(useAssetsQuery).mockReturnValue({
data: new Map(),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any)

const accountBalance = {
assetId: 999,
amount: '0',
} as unknown as AssetWithAccountBalance

const { container } = render(
<AccountAssetItemView accountBalance={accountBalance} />,
)

// Should not render asset name since data isn't available
expect(screen.queryByText('Algo')).toBeNull()
// Should render something (the skeleton placeholder)
expect(container).toBeTruthy()
})
})
Loading
Loading