-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Graceful Perps icon fallback #24340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -184,5 +184,14 @@ | |
| android:resource="@xml/filepaths" | ||
| /> | ||
| </provider> | ||
| <!-- EAS Update configuration --> | ||
| <meta-data android:name="expo.modules.updates.UPDATES_CONFIGURATION_REQUEST_HEADERS_KEY" android:value="{"expo-channel-name":"rc"}"/> | ||
| <meta-data android:name="expo.modules.updates.EXPO_RUNTIME_VERSION" android:value="7.62.0" /> | ||
| <meta-data android:name="expo.modules.updates.EXPO_UPDATE_URL" android:value="https://u.expo.dev/" /> | ||
| <meta-data android:name="expo.modules.updates.EXPO_UPDATES_CHECK_ON_LAUNCH" android:value="NEVER" /> | ||
| <meta-data android:name="expo.modules.updates.EXPO_UPDATES_LAUNCH_WAIT_MS" android:value="0" /> | ||
| <meta-data android:name="expo.modules.updates.CODE_SIGNING_CERTIFICATE" android:value="-----BEGIN CERTIFICATE-----
MIIC0zCCAbugAwIBAgIJGH+Ulb6OVEeJMA0GCSqGSIb3DQEBCwUAMBMxETAPBgNV
BAMTCE1ldGFNYXNrMB4XDTI1MTIxOTIzMTY1NFoXDTM1MTIxOTIzMTY1NFowEzER
MA8GA1UEAxMITWV0YU1hc2swggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB
AQDWvpseYyys+mFZrxGXMDD4VRGIs0u5mXgjwDf7fEOJWI7uldvVcVDPCO1v+/Ig
0nlz/NgDdVvYNgKFJJq4JMLDJNzxNIcaqSCKlO9IU5qWvnnfEeyWx6Pv0NQssHOD
Sc+WnvfCXub3akM9CE6Noy/KbIHpyUNwypux1eU5KGnZ704kNVsWmU7PeRn5Olnl
6t5Q93sIe2fbESDvHYh0TKC9eQvgkvrCCKlMgyqnZb4fdylXGbWRjivp+AKh/DHz
37bz9KtQO7YF8oZ1QvTiSVOb0hS1rzAE/YMatTbC214FQu5/w3vHCe7ejKcNCjzM
xFm6nSd41ho/SpW4r92dt0xvAgMBAAGjKjAoMA4GA1UdDwEB/wQEAwIHgDAWBgNV
HSUBAf8EDDAKBggrBgEFBQcDAzANBgkqhkiG9w0BAQsFAAOCAQEAn8OKneCOuUcI
wUGRVqWDcej4yYGugWWiIdEgy4as+vXUhMCk47uPwWuKNSILDuPeSxt2lo+AND8U
4N3I87+oYbLktOyph5FtpwUEMC8R/YE8Q5bNmi0LHzzGteenfUhSc+MVhpVVwZ1I
SbHGrj6/oet39FFFqJhWAU+RbMwSnYZKrZTfYELuEppP3WO03P9I8T3XR0d+CmMb
YdeLUvYilZI+3VxKL6tg/UWlZOX8MH6JbtNTm+2YMi1fp/hE8mrd+rR2Re5d87ub
srOY8HZvM0JX4RPddITpfSEwLwrPhdSFRwxyAVmBALYWVHeuiFn2pI3jfNeHF2Lt
SS2hIbf10Q==
-----END CERTIFICATE-----
" /> | ||
| <meta-data android:name="expo.modules.updates.CODE_SIGNING_METADATA" android:value="{"keyid":"rc","alg":"rsa-v1_5-sha256"}" /> | ||
| <meta-data android:name="expo.modules.updates.ENABLED" android:value="true" /> | ||
|
||
| </application> | ||
| </manifest> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| import { Image } from 'expo-image'; | ||
| import React, { memo, useMemo } from 'react'; | ||
| import React, { memo, useCallback, useEffect, useMemo, useState } from 'react'; | ||
| import { ActivityIndicator, View } from 'react-native'; | ||
| import Text, { | ||
| TextVariant, | ||
| } from '../../../../../component-library/components/Texts/Text'; | ||
| import { useTokenLogo } from '../../../../hooks/useTokenLogo'; | ||
| import { | ||
| getAssetIconUrl, | ||
| getAssetIconUrls, | ||
| getPerpsDisplaySymbol, | ||
| } from '../../utils/marketUtils'; | ||
| import { | ||
|
|
@@ -23,12 +23,27 @@ const PerpsTokenLogo: React.FC<PerpsTokenLogoProps> = ({ | |
| testID, | ||
| recyclingKey, | ||
| }) => { | ||
| // SVG URL - expo-image handles SVG rendering properly | ||
| const imageUri = useMemo(() => { | ||
| // Track if we should use fallback URL (after primary fails) | ||
| const [useFallbackUrl, setUseFallbackUrl] = useState(false); | ||
|
|
||
| // Get both primary (MetaMask) and fallback (HyperLiquid) URLs | ||
| const iconUrls = useMemo(() => { | ||
| if (!symbol) return null; | ||
| return getAssetIconUrl(symbol, K_PREFIX_ASSETS); | ||
| return getAssetIconUrls(symbol, K_PREFIX_ASSETS); | ||
| }, [symbol]); | ||
|
|
||
| // Reset fallback state when symbol changes | ||
| useEffect(() => { | ||
| setUseFallbackUrl(false); | ||
| }, [symbol]); | ||
|
|
||
| // Select current image URL based on fallback state | ||
| const imageUri = iconUrls | ||
| ? useFallbackUrl | ||
| ? iconUrls.fallback | ||
| : iconUrls.primary | ||
| : null; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale fallback state causes wrong URL on symbol changeLow Severity When the symbol changes after a primary URL failure, 🔬 Verification TestTest code: // This test demonstrates the bug - checking intermediate state
it('should use primary URL immediately when symbol changes (BUG: uses fallback first)', () => {
// Simulate the render cycle manually
let useFallbackUrl = true; // State from previous symbol's primary failure
const newSymbol = 'ETH';
const iconUrls = {
primary: 'https://metamask/.../ETH.svg',
fallback: 'https://hyperliquid/.../ETH.svg'
};
// First render with new symbol (before useEffect runs)
const imageUri = iconUrls
? useFallbackUrl
? iconUrls.fallback // <-- BUG: This branch is taken
: iconUrls.primary
: null;
console.log('First render imageUri:', imageUri);
console.log('Expected primary URL:', iconUrls.primary);
console.log('Bug present:', imageUri === iconUrls.fallback);
});Command run: Output: Why this proves the bug: The output shows that when |
||
|
|
||
| // Extract display symbol (e.g., "TSLA" from "xyz:TSLA") | ||
| const fallbackText = useMemo(() => { | ||
| const displaySymbol = getPerpsDisplaySymbol(symbol || ''); | ||
|
|
@@ -53,6 +68,22 @@ const PerpsTokenLogo: React.FC<PerpsTokenLogoProps> = ({ | |
| assetsRequiringDarkBg: ASSETS_REQUIRING_DARK_BG, | ||
| }); | ||
|
|
||
| // Handle image error with fallback logic: | ||
| // 1. If primary URL fails, try fallback URL | ||
| // 2. If fallback URL also fails, show text fallback | ||
| const handleImageError = useCallback(() => { | ||
| if (!useFallbackUrl && iconUrls?.fallback) { | ||
| // Primary failed - try fallback URL | ||
| setUseFallbackUrl(true); | ||
| } else { | ||
| // Both URLs failed - show text fallback | ||
| handleError(); | ||
| } | ||
| }, [useFallbackUrl, iconUrls?.fallback, handleError]); | ||
|
|
||
| // Image key includes fallback state for proper re-render when switching URLs | ||
| const imageKey = `${recyclingKey || symbol}-${useFallbackUrl ? 'fallback' : 'primary'}`; | ||
|
|
||
| // Show custom two-letter fallback if no symbol or error | ||
| if (!symbol || !imageUri || hasError) { | ||
| return ( | ||
|
|
@@ -72,15 +103,15 @@ const PerpsTokenLogo: React.FC<PerpsTokenLogoProps> = ({ | |
| </View> | ||
| )} | ||
| <Image | ||
| key={recyclingKey || symbol} // Use recyclingKey for proper recycling | ||
| key={imageKey} | ||
| source={{ uri: imageUri }} | ||
| style={imageStyle} | ||
| onLoadStart={handleLoadStart} | ||
| onLoadEnd={handleLoadEnd} | ||
| onError={handleError} | ||
| onError={handleImageError} | ||
| contentFit="contain" | ||
| cachePolicy="memory-disk" // Persistent caching across app sessions | ||
| recyclingKey={recyclingKey || symbol} // For FlashList optimization | ||
| recyclingKey={imageKey} // For FlashList optimization | ||
| transition={0} // Disable transition for faster rendering | ||
| priority="high" // High priority loading | ||
| placeholder={null} // No placeholder for cleaner loading | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EAS Update URL missing Expo project ID
High Severity
The EAS Update URL
https://u.expo.dev/is incomplete - it's missing the required Expo project UUID. According toota.config.js, the URL format ishttps://u.expo.dev/${PROJECT_ID}wherePROJECT_IDcomes from theEXPO_PROJECT_IDenvironment variable. This appears to be accidentally committed output from runningscripts/update-expo-channel.jswithout the environment variable set. Without the project ID, OTA updates won't be able to locate the correct Expo project, causing update checks to fail. These configuration changes also appear unrelated to the PR's stated purpose of implementing Perps icon fallback.🔬 Verification Test
Test code:
Command run:
Output:
Why this proves the bug: The output shows that without the
EXPO_PROJECT_IDenvironment variable, the URL defaults tohttps://u.expo.dev/(incomplete), which matches what was committed to the Android and iOS config files. The Expo Update URL requires a project UUID suffix to function correctly.Additional Locations (1)
ios/Expo.plist#L7-L9