-
Notifications
You must be signed in to change notification settings - Fork 1
feat(arcade): update components for arcade native #34
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?
Conversation
| } | ||
| } | ||
| return coverMap; | ||
| }, [gamesList.length]); // Depend on lightweight list length! |
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.
Bug: useMemo depends on different array than computation uses
The gameCovers useMemo computes from the games array but its dependency array uses gamesList.length. These are two separate arrays from the arcade context. While they're currently updated together, using a different data source in the dependency than what's used in the computation is incorrect and could cause stale map data if the arrays ever diverge or if maintenance changes one without the other.
| // Increment version to trigger re-renders | ||
| setVersion(v => v + 1); | ||
| } | ||
| }, [baseGames.length]); |
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.
Bug: useEffect only updates games when count changes
The useEffect has [baseGames.length] as its dependency and the condition checks baseGames.length !== gamesRef.current.length. This means if game properties change (names, icons, covers, etc.) but the total game count remains the same, the refs won't update and consumers will see stale game data.
| return ( | ||
| <View className={`${sizeClass} ${bgClass} rounded items-center justify-center`}> | ||
| <Text className={`${textClass} font-bold ${textSize}`}> | ||
| {title[0].toUpperCase()} |
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.
Bug: GameIcon crashes if title is empty string
The expression title[0].toUpperCase() will throw a TypeError if title is an empty string, since title[0] returns undefined and calling .toUpperCase() on undefined crashes. This component is used in the side drawer where title comes from game names in the data. If a game has an empty name, the app would crash when rendering the fallback icon.
| // Convert games | ||
| const newGames = baseGames.map((game, index) => ({ | ||
| ...game, | ||
| id: parseInt(game.id) || index, |
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.
Bug: Game ID zero incorrectly replaced by array index
The expression parseInt(game.id) || index uses the logical OR operator with a falsy check. If a game has id: "0", parseInt("0") returns 0, which is falsy in JavaScript, causing the expression to fall back to index. This incorrectly assigns the array position as the ID instead of preserving the valid ID of zero.
| } | ||
|
|
||
| return { idMap, slugMap }; | ||
| }, [games.length]); // Only depend on length! |
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.
Bug: Stale closure bug in game lookup maps
The useMemo creating lookup maps depends on games.length but iterates over games inside. If game data changes without the count changing (e.g., a game's name or properties are updated), the maps won't be rebuilt and will return stale game objects. The byId and bySlug lookups will return outdated information.
| } catch (err) { | ||
| console.error('❌ ArcadeProvider: Failed to load games', err); | ||
| baseGames = []; | ||
| } |
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.
Bug: React hook called inside try-catch block
The useGames() hook is called inside a try-catch block, which violates React's rules of hooks. Hooks must be called unconditionally at the top level of a component - they cannot be wrapped in try-catch, conditionals, or loops. While this code will execute, the try-catch won't actually catch errors thrown by the hook during React's render cycle. If useGames() throws, it will propagate up and cause rendering issues. The error handling approach here gives a false sense of safety without actually providing it.
| symbol: "CREDITS", | ||
| decimals: 18, | ||
| project: "arcade", | ||
| }, |
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.
Bug: Metadata type mismatch for credits token field
The Metadata type definition (lines 12-19) does not include a project field, but the credits token metadata at lines 96 and 112 uses project: "arcade" instead of address. The type defines address and project as optional, but the inconsistency between how tokens and credits set these fields could cause issues if consuming code expects the address field to be populated for all tokens, including credits.
Additional Locations (1)
| }; | ||
|
|
||
| fetchFirstTokenImages(); | ||
| }, [client, isReady, tokenContracts.length, fetchingImages, hasFetchedImages]); |
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.
Bug: Missing cleanup causes state updates on unmounted component
The useEffect launches an async function fetchFirstTokenImages that calls setFirstTokenImages, setFetchingImages, and setHasFetchedImages after await Promise.all(promises) completes. There's no cleanup function or abort mechanism, so if the component unmounts while the network requests are pending, these state updates will execute on an unmounted component, causing React warnings and potential memory leaks.
|
|
||
| // Get images | ||
| const images = editionSocials?.images || gameSocials?.images || []; | ||
| const validImages = images.filter((i: string) => !!i && i.trim() !== ''); |
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.
Bug: Filter call on potentially non-array value will crash
The expressions editionSocials?.videos || gameSocials?.videos || [] and similar for images only fall back to an empty array when the value is falsy. If the API returns a truthy non-array value for videos or images (such as a string URL instead of an array), calling .filter() on it will throw a TypeError. Using Array.isArray() check or ensuring array coercion would prevent this crash.
| version, // Include version so consumers can depend on it | ||
| }), [version]); // Only depend on version number! | ||
|
|
||
| console.log('🎮 ArcadeProvider: Providing', gamesRef.current.length, 'real games (v', version, ')'); |
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.
Bug: Debug console.log statements left in code
Multiple debug console.log statements with emoji prefixes (🎮, 💰) have been left in the production code. These statements in ArcadeProvider and MarketplaceProvider will log on every render and component update, cluttering the console output in production builds.
Additional Locations (1)
| }; | ||
|
|
||
| fetchFirstTokenImages(); | ||
| }, [client, isReady, tokenContracts.length, fetchingImages, hasFetchedImages]); |
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.
Bug: useEffect dependency mismatch causes stale token data
The useEffect that fetches first token images depends on tokenContracts.length but iterates over tokenContracts inside. If individual token contracts change (e.g., updated contract addresses or properties) while the array length stays the same, the effect won't re-run and will process stale data. The dependency should include the actual data being used.
Add Status filter section above Owner filter in side drawer to filter tokens by listing status.
- Replace useTokenBalances with useTokens for NFT fetching - Add getTokenName and getTokenImageUri helpers to parse metadata - Display proper token names instead of just token IDs - Handle metadata in both JSON and base64 formats
| const state = getTraitValuesState?.(traitName); | ||
| if (!state || state.values.length === 0) { | ||
| // Set loading state | ||
| setLoadingTraitNames(new Set([...loadingTraitNames, traitName])); |
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.
Bug: Stale closure causes lost loading states in setLoadingTraitNames
The setLoadingTraitNames call uses loadingTraitNames directly from the closure (new Set([...loadingTraitNames, traitName])) instead of the functional update pattern. When a user rapidly expands multiple traits or clicks "Load more" on multiple traits in quick succession, the loadingTraitNames value captured at callback creation time becomes stale. This causes later updates to overwrite earlier ones, losing loading states for some traits. The fix requires using the functional form: setLoadingTraitNames(prev => new Set([...prev, traitName])).
Note
Replaces mocks with live Torii-backed contexts (games, collections, tokens), adds efficient game lookup and image sanitization, and updates major UI (header, side drawer, marketplace, inventory, about) with new cards, icons, and theming.
context/arcade.tsx(games, editions, lightweightgamesList, versioning).context/collection.tsx(token contracts, first-token image fetch, SVG sanitization, game mapping, status).context/token.tsx(Cartridge balances, session username) and scaffold marketplace context.useGameLookupfor efficient game ID/slug lookups; export viahooks/index.ts.GameCard,GameIcon,BackButton; refactorItemCardwith collection/NFT variants and image sanitization.Header(game cover/accent color),SideDrawer(profile/connect, filters with owner search and traits, token detail view), andInventoryScene(connect state, tokens with icons/changes, NFTs grid).Marketplaceto filter by edition/game and use collection cards; enhanceAbout(media, socials, details).Thumbnailtoexpo-image.LordsColorIcon,USDCIcon,ArcadeBrandIcon,ArcadeTextIcon,ItemsListIcon,ScarecrowIcon,VerifiedBadgeIcon; tweakHamburgerIcon.useGameLookupfor controller-based theming.image-sanitization(sanitizeSvgDataUri,sanitizeImageUri) and integrate across components.Written by Cursor Bugbot for commit a3e28c2. This will update automatically on new commits. Configure here.