fix: reapply optimise search #10318
fix: reapply optimise search #10318
Conversation
📝 WalkthroughWalkthroughReplaces local filterAssetsBySearchTerm with a library-grade searchAssets, adds asset-search types/config/utils, moves isEthAddress to a new utils file, implements an AssetSearch web worker plus useAssetSearchWorker hook, updates TradeAssetSearch/SearchTermAssetList to use worker-driven search, and configures Vite to emit ES-module workers. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TAS as TradeAssetSearch
participant Hook as useAssetSearchWorker
participant W as AssetSearchWorker
participant L as SearchTermAssetList
U->>TAS: Type search term
TAS->>Hook: handleSearchChange(term)
Note over Hook: debounce (200ms)
Hook->>W: postMessage(search { requestId, term, chain/wallet flags })
W->>W: pre-filter by chain/wallet support
W->>W: run searchAssets(term, assets)
W-->>Hook: postMessage(searchResult { requestId, assetIds })
Hook-->>TAS: update workerSearchState (results/isSearching)
TAS->>L: render with props { assets, workerSearchState }
alt Worker ready
L->>L: map assetIds -> full assets
L-->>U: render results
else Worker failed / not ready
L->>L: run searchAssets fallback on main thread
L-->>U: render results
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
premiumjibles
force-pushed
the
reapply-optimise-search
branch
from
a39a261 to
4dd5b6e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/components/StakingVaults/PositionTable.tsx (3)
188-209: Normalize the address and removeanyto avoid missed matches and improve typing.
- Bug:
isEthAddress(filterValue)is checked on the raw input but comparison uses.toLowerCase()on the left side only. Upper/mixed-case inputs won’t match.- Typing:
filterValue: anyis unnecessary; propsearchQueryis already a string.- Consistency: Reuse the shared match-sorter config to avoid drift with the new asset search module.
Apply this diff to normalize and improve types:
- const filterRowsBySearchTerm = useCallback( - (rows: AggregatedOpportunitiesByAssetIdReturn[], filterValue: any) => { + const filterRowsBySearchTerm = useCallback( + (rows: AggregatedOpportunitiesByAssetIdReturn[], filterValue: string) => { if (!filterValue) return rows - if (typeof filterValue !== 'string') { - return [] - } - const search = filterValue.trim().toLowerCase() - if (isEthAddress(filterValue)) { - return rows.filter( - row => fromAssetId(row.assetId).assetReference.toLowerCase() === filterValue, - ) + const normalized = filterValue.trim().toLowerCase() + if (isEthAddress(normalized)) { + return rows.filter( + row => fromAssetId(row.assetId).assetReference.toLowerCase() === normalized, + ) } const assetIds = rows.map(row => row.assetId) const rowAssets = assets.filter(asset => assetIds.includes(asset.assetId)) - const matchedAssets = matchSorter(rowAssets, search, { - keys: ['name', 'symbol'], - threshold: matchSorter.rankings.CONTAINS, - }).map(asset => asset.assetId) + const matchedAssets = matchSorter( + rowAssets, + normalized, + ASSET_SEARCH_MATCH_SORTER_CONFIG, + ).map(asset => asset.assetId) const results = rows.filter(row => matchedAssets.includes(row.assetId)) return results }, [assets], )Add the shared config import at the top of the file:
import { ASSET_SEARCH_MATCH_SORTER_CONFIG } from '@/lib/assetSearch'
172-181: Icon button lacks onClick; add handler for accessibility and UX.The expander icon renders an interactive control without an action. Add onClick and aria-expanded to reflect state and enable activation via the button itself.
Apply this diff:
Cell: ({ row }: { row: RowProps }) => ( <Flex justifyContent='flex-end' width='full'> <IconButton variant='ghost' size='md' aria-label={translate('common.table.expandRow')} + aria-expanded={row.isExpanded} + onClick={() => row.toggleRowExpanded()} isActive={row.isExpanded} icon={row.isExpanded ? <ArrowUpIcon /> : <ArrowDownIcon />} /> </Flex> ),
188-209: Consider reusing the centralized search utility to eliminate duplication.This local search replicates logic now provided by the new asset search module. If feasible, replace this with
searchAssetsto ensure consistent behavior, shared config, and easier tweaks going forward. I can draft a refactor if you confirm the desired output shape here (assetIds vs rows).src/pages/TransactionHistory/hooks/useSearch.tsx (1)
15-33: Debounce is recreated on every keystroke — ineffective and racy; memoize and cancelCreating a new debounced function inside handleInputChange defeats debouncing and may schedule stale trailing calls. Also, whitespace-only queries should be treated as empty input to avoid returning odd results.
Apply this diff to use a memoized debounced function and cancel pending calls when clearing input:
const handleInputChange = useCallback( (inputValue: string) => { setSearchTerm(inputValue) - if (inputValue === '') { + const trimmed = inputValue.trim() + if (trimmed === '') { setMatchingAssets(null) + debouncedSetMatchingAssets.cancel() } else { - debounce( - () => { - setMatchingAssets(searchAssets(inputValue, assets)) - }, - 500, - { - leading: true, - }, - )() + debouncedSetMatchingAssets(trimmed) } }, - [assets], + [assets, debouncedSetMatchingAssets], )Add the following outside the selected lines to define and clean up the debounced function:
// Add to imports: import { useCallback, useEffect, useMemo, useRef, useState } from 'react' // Add below state declarations: const debouncedSetMatchingAssets = useMemo( () => debounce( (term: string) => { setMatchingAssets(searchAssets(term, assets)) }, 500, { leading: true }, // trailing defaults to true ), [assets], ) useEffect(() => { return () => debouncedSetMatchingAssets.cancel() }, [debouncedSetMatchingAssets])Also applies to: 21-29, 23-23
src/components/TradeAssetSearch/TradeAssetSearch.tsx (3)
25-30: Prune unused import after removing local assets selection.After removing the local
assetsselector, also remove this unused import to keep things tidy.import { selectAssetsSortedByMarketCap, - selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName, selectPortfolioFungibleAssetsSortedByBalance, selectPortfolioTotalUserCurrencyBalance, selectWalletConnectedChainIds, } from '@/state/slices/selectors'
151-154: Avoid magic number; use the NUM_QUICK_ACCESS_ASSETS constant.This makes the intent explicit and DRY.
if (activeChainId !== 'All') { - return popularAssets.slice(0, 6) + return popularAssets.slice(0, NUM_QUICK_ACCESS_ASSETS) }
287-299: Follow-up to rename: switch conditional tohasSearchQuery.Keeps naming consistent with Line 121 change.
- {isSearching ? ( + {hasSearchQuery ? ( <SearchTermAssetList activeChainId={activeChainId} searchString={searchString} onAssetClick={handleAssetClick} onImportClick={handleImportIntent} isLoading={isPopularAssetIdsLoading} assetFilterPredicate={assetFilterPredicate} allowWalletUnsupportedAssets={!hasWallet || allowWalletUnsupportedAssets} workerSearchState={workerSearchState} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
src/components/AssetSearch/AssetSearch.tsx(2 hunks)src/components/StakingVaults/PositionTable.tsx(1 hunks)src/components/TradeAssetSearch/TradeAssetSearch.tsx(7 hunks)src/components/TradeAssetSearch/components/SearchTermAssetList.tsx(5 hunks)src/components/TradeAssetSearch/helpers/filterAssetsBySearchTerm/filterAssetsBySearchTerm.ts(0 hunks)src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts(1 hunks)src/components/TradeAssetSearch/workers/assetSearch.worker.ts(1 hunks)src/lib/address/utils.ts(0 hunks)src/lib/assetSearch/config.ts(1 hunks)src/lib/assetSearch/index.ts(1 hunks)src/lib/assetSearch/types.ts(1 hunks)src/lib/assetSearch/utils.test.ts(2 hunks)src/lib/assetSearch/utils.ts(1 hunks)src/lib/utils/ethAddress.ts(1 hunks)src/pages/TransactionHistory/hooks/useSearch.tsx(2 hunks)vite.config.mts(1 hunks)
💤 Files with no reviewable changes (2)
- src/lib/address/utils.ts
- src/components/TradeAssetSearch/helpers/filterAssetsBySearchTerm/filterAssetsBySearchTerm.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/StakingVaults/PositionTable.tsxsrc/lib/utils/ethAddress.tssrc/lib/assetSearch/index.tssrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.tssrc/lib/assetSearch/config.tssrc/components/AssetSearch/AssetSearch.tsxsrc/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/StakingVaults/PositionTable.tsxsrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/components/AssetSearch/AssetSearch.tsxsrc/components/TradeAssetSearch/TradeAssetSearch.tsx
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/StakingVaults/PositionTable.tsxsrc/lib/utils/ethAddress.tsvite.config.mtssrc/lib/assetSearch/index.tssrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.tssrc/lib/assetSearch/config.tssrc/components/AssetSearch/AssetSearch.tsxsrc/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.ts
**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/StakingVaults/PositionTable.tsxsrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/components/AssetSearch/AssetSearch.tsxsrc/components/TradeAssetSearch/TradeAssetSearch.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/StakingVaults/PositionTable.tsxsrc/lib/utils/ethAddress.tssrc/lib/assetSearch/index.tssrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.tssrc/lib/assetSearch/config.tssrc/components/AssetSearch/AssetSearch.tsxsrc/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.ts
**/use*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
🧠 Learnings (5)
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/index.ts : Export unique functions/types from packages/swapper/src/index.ts if needed.
Applied to files:
src/lib/assetSearch/index.ts
📚 Learning: 2025-08-03T22:10:11.424Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/naming-conventions.mdc:0-0
Timestamp: 2025-08-03T22:10:11.424Z
Learning: Applies to **/*.{ts,tsx} : ALWAYS use named exports for components and functions
Applied to files:
src/lib/assetSearch/index.ts
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Applied to files:
src/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/components/AssetSearch/AssetSearch.tsxsrc/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.ts
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx:97-108
Timestamp: 2025-08-14T17:54:32.563Z
Learning: In ReusableLpStatus component (src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx), the txAssets dependency is stable from first render because poolAsset, baseAsset, actionSide, and action are all defined first render, making the current txAssetsStatuses initialization pattern safe without needing useEffect synchronization.
Applied to files:
src/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/components/AssetSearch/AssetSearch.tsxsrc/components/TradeAssetSearch/TradeAssetSearch.tsx
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
PR: shapeshift/web#10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.
Applied to files:
src/lib/assetSearch/types.tssrc/lib/assetSearch/utils.ts
🧬 Code Graph Analysis (9)
src/pages/TransactionHistory/hooks/useSearch.tsx (1)
src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)
src/components/TradeAssetSearch/workers/assetSearch.worker.ts (2)
src/lib/assetSearch/types.ts (3)
SearchableAsset(3-8)AssetSearchWorkerInboundMessage(42-45)AssetSearchWorkerOutboundMessage(47-47)src/lib/assetSearch/utils.ts (2)
filterAssetsByChainSupport(22-51)searchAssets(53-66)
src/components/TradeAssetSearch/components/SearchTermAssetList.tsx (6)
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (1)
WorkerSearchState(15-20)packages/caip/src/assetId/assetId.ts (2)
AssetId(17-17)toAssetId(59-130)packages/utils/src/getAssetNamespaceFromChainId.ts (1)
getAssetNamespaceFromChainId(7-33)packages/utils/src/makeAsset/makeAsset.ts (2)
MinimalAsset(8-8)makeAsset(16-53)src/lib/utils/index.ts (1)
isSome(98-99)src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)
src/lib/assetSearch/types.ts (1)
packages/caip/src/assetId/assetId.ts (1)
AssetId(17-17)
src/components/AssetSearch/AssetSearch.tsx (1)
src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)
src/lib/assetSearch/utils.ts (4)
packages/caip/src/assetId/assetId.ts (3)
AssetId(17-17)isNft(181-187)fromAssetId(143-178)src/lib/assetSearch/types.ts (1)
SearchableAsset(3-8)src/lib/assetSearch/config.ts (1)
ASSET_SEARCH_MATCH_SORTER_CONFIG(3-6)src/lib/utils/ethAddress.ts (1)
isEthAddress(1-1)
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (4)
packages/caip/src/assetId/assetId.ts (1)
AssetId(17-17)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/common-selectors.ts (1)
selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName(207-237)src/lib/assetSearch/types.ts (1)
AssetSearchWorkerOutboundMessage(47-47)
src/components/TradeAssetSearch/TradeAssetSearch.tsx (2)
src/state/slices/common-selectors.ts (1)
selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName(207-237)src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (1)
useAssetSearchWorker(29-168)
src/lib/assetSearch/utils.test.ts (2)
src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)packages/caip/src/assetId/assetId.ts (1)
fromAssetId(143-178)
🔇 Additional comments (21)
vite.config.mts (2)
112-114: ESM worker format addition is correct and aligns with module workers.Setting worker.format = 'es' is the right call for bundling module workers in Vite. This should address the shared worker bundling/build issues mentioned in the PR description.
112-114: No Worker/SharedWorker constructors found—no action neededI searched the entire codebase for any
new Worker(...)ornew SharedWorker(...)instantiations (including dynamic URL imports) and found none. Since there are currently no Worker constructors, there’s nothing to update here. If you add any in the future, be sure to include{ type: 'module' }in the options.src/components/StakingVaults/PositionTable.tsx (1)
23-23: Import path update looks good.Switching to '@/lib/utils/ethAddress' aligns with the new utility location and reduces bundle footprint from viem.
src/lib/assetSearch/config.ts (1)
1-6: Solid, minimal, and typed match-sorter config.Good use of
as constto preserve literal types and centralize search behavior. This aids consistency across main-thread and worker-based searches.src/lib/assetSearch/index.ts (1)
1-3: Barrel re-exports are clean and enable ergonomic imports.This organization promotes consistency and improves DX without affecting tree-shaking.
src/pages/TransactionHistory/hooks/useSearch.tsx (1)
5-5: Centralizing search via shared lib is the right moveImporting searchAssets from '@/lib/assetSearch' keeps search behavior consistent across the app.
src/components/TradeAssetSearch/workers/assetSearch.worker.ts (1)
16-26: Trim & Short-Circuit Empty Queries in assetSearch.worker.tsTo prevent accidental “full-list” responses on whitespace and improve error visibility:
• Rename and trim the incoming query:
- Extract
rawSearchStringfrommsg.payload- Derive
searchString = (rawSearchString ?? '').trim()
• Short-circuit empty searches:- If
searchString === '', return an empty array instead of callingsearchAssets
• Surface errors with context:- In your
catch (e)block, callconsole.error('assetSearch.worker: search failed', e)(or replace with your app’s structured logger once you’ve confirmed its import path)Diff example:
--- a/src/components/TradeAssetSearch/workers/assetSearch.worker.ts +++ b/src/components/TradeAssetSearch/workers/assetSearch.worker.ts @@ 16,26 - const handleSearch = (msg: AssetSearchWorkerInboundMessage & { type: 'search' }): void => { - const { searchString, activeChainId, allowWalletUnsupportedAssets, walletConnectedChainIds } = - msg.payload + const handleSearch = (msg: AssetSearchWorkerInboundMessage & { type: 'search' }): void => { + const { + searchString: rawSearchString, + activeChainId, + allowWalletUnsupportedAssets, + walletConnectedChainIds, + } = msg.payload + const searchString = (rawSearchString ?? '').trim() - const preFiltered = filterAssetsByChainSupport(ASSETS, { + const preFiltered = filterAssetsByChainSupport(ASSETS, { activeChainId, allowWalletUnsupportedAssets, walletConnectedChainIds, @@ - const filtered = searchAssets(searchString, preFiltered) + const filtered = + searchString === '' + ? [] + : searchAssets(searchString, preFiltered) @@ 45,53 - } catch { + } catch (e) { + // TODO: switch to structured logger once available + console.error('assetSearch.worker: search failed', e) postMessage({ type: 'searchResult', requestId: data.requestId,Please verify:
- That
ASSETSremains the correct source (noassetsCacheimport is available here).- Whether your codebase provides a shared logger utility you can import instead of
console.error.src/components/AssetSearch/AssetSearch.tsx (1)
17-17: Consolidated search import is correctSwitching to searchAssets aligns this component with the shared search logic.
src/components/TradeAssetSearch/components/SearchTermAssetList.tsx (4)
9-10: Props and shared search fallback look good
- Adding workerSearchState to props cleanly decouples worker lifecycle from rendering.
- Using searchAssets as a main-thread fallback is appropriate.
Also applies to: 14-15, 32-33, 43-44
93-101: Efficient assetIdMap constructionBuilding a lookup map once per assetsForChain change is a good optimization for mapping worker results.
102-128: Custom token synthesis: solid guards and deduping
- Properly skips incomplete metadata.
- Ensures no duplicates by checking assetIdMap first.
- Uses makeAsset to enrich minimal assets.
166-171: Loading state correctly reflects worker activityIncluding workerSearchState.isSearching prevents UI flicker while the worker processes queries.
src/components/TradeAssetSearch/TradeAssetSearch.tsx (3)
231-244: LGTM on input binding and memoization.Binding
value/onChangeto the worker state and memoizinginputPropswith correct deps is solid. Autofocus guard for coarse pointers is a nice touch.
296-296: LGTM on worker-driven results propagation.Passing
workerSearchStatetoSearchTermAssetListaligns the UI with worker lifecycle and enables graceful fallback.
246-307: Verify Shared ErrorBoundary AvailabilityI wasn’t able to locate any existing
ErrorBoundaryor fallback component in the codebase. Please confirm whether you already have a shared implementation. If not, I recommend:
- Creating a new
ErrorBoundary(class-based or via [react-error-boundary]) in, for example,
src/components/ErrorBoundary/that:
• ImplementscomponentDidCatch/getDerivedStateFromErrorfor structured logging
• Renders a user-friendly fallback which uses youruseErrorToasthook
• (Optionally) Exposes a reset/retry callback- Then wrapping the return value in
TradeAssetSearch.tsx:-import React from 'react' +import { ErrorBoundary, ErrorFallback } from 'components/ErrorBoundary' return ( - <> + <ErrorBoundary FallbackComponent={ErrorFallback}> + <> … - </> + </> + </ErrorBoundary> )Let me know if you’d like me to draft the shared
ErrorBoundaryand fallback.src/lib/assetSearch/utils.ts (2)
53-66: LGTM on search flow and address fast-path.
- Defensive checks are fine.
- Address branch delegates to
filterAssetsByEthAddress.matchSorterwith the provided config is appropriate.
10-11: Explicitly excluding NFTs viaisSearchableAssetreads well.No issues here.
src/lib/assetSearch/types.ts (1)
3-8: SearchableAsset typing is clear and minimal.Good, descriptive shape and correct CAIP types.
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (3)
56-67: LGTM on requestId de-dupe and ready-state gating.Matching
requestIdand ignoring out-of-date results is the right way to avoid races.
122-131: Good on effectiveallowWalletUnsupportedAssetsderivation.
!hasWallet || allowWalletUnsupportedAssetscaptures the desired semantics and reduces branching downstream.
69-76: Unable to locate shared logger utility – please adviseI searched for existing
logger/createLogger/getLoggerutilities and didn’t find any. To proceed:• Confirm whether there’s a shared structured-logging facility (e.g.
@/lib/logger) we should import.
• If not, let me know and I can wire up the preferred logger.
• In bothworker.onerrorand thecatchblock, also setisSearching: falseto stop the spinner on failure.Suggested diff once logger is available:
- worker.onerror = error => { - console.error('Worker error:', error) - setWorkerSearchState(prev => ({ ...prev, workerState: 'failed' })) - } - } catch (error) { - console.error('Failed to initialize worker:', error) - setWorkerSearchState(prev => ({ ...prev, workerState: 'failed' })) - } + worker.onerror = error => { + logger.error({ error }, 'AssetSearchWorker error') + setWorkerSearchState(prev => ({ ...prev, workerState: 'failed', isSearching: false })) + } + } catch (error) { + logger.error({ error }, 'Failed to initialize AssetSearchWorker') + setWorkerSearchState(prev => ({ ...prev, workerState: 'failed', isSearching: false })) + }
premiumjibles
force-pushed
the
reapply-optimise-search
branch
from
4dd5b6e to
c535f33
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
src/components/StakingVaults/PositionTable.tsx (2)
195-199: Fix case-sensitivity: mixed/checksummed ETH addresses won’t match current lowercased comparison.
isEthAddress(filterValue)accepts mixed-case checksummed inputs, but you compareassetReference.toLowerCase()to the originalfilterValue. Use the already-lowercasedsearchor lowercase both sides to ensure matches.Apply:
- if (isEthAddress(filterValue)) { - return rows.filter( - row => fromAssetId(row.assetId).assetReference.toLowerCase() === filterValue, - ) - } + if (isEthAddress(filterValue)) { + return rows.filter(row => { + const ref = fromAssetId(row.assetId).assetReference + return ref.toLowerCase() === search + }) + }
202-206: Optional: reuse central match-sorter config to avoid drift across features.To keep search behavior identical to the new asset-search utilities, import and use
ASSET_SEARCH_MATCH_SORTER_CONFIGinstead of re-declaring keys/threshold here.Apply:
- const matchedAssets = matchSorter(rowAssets, search, { - keys: ['name', 'symbol'], - threshold: matchSorter.rankings.CONTAINS, - }).map(asset => asset.assetId) + const matchedAssets = matchSorter( + rowAssets, + search, + ASSET_SEARCH_MATCH_SORTER_CONFIG, + ).map(asset => asset.assetId)And add the import:
+ import { ASSET_SEARCH_MATCH_SORTER_CONFIG } from '@/lib/assetSearch'src/pages/TransactionHistory/hooks/useSearch.tsx (1)
15-33: Debounce is recreated per keystroke — leads to extra trailing calls and stale resultsThe debounced function is instantiated inside the input handler, so each keypress creates a new debouncer, defeating debouncing and risking out-of-order updates. Memoize the debounced search once, pass args to it, trim input, and cancel on unmount.
-import { useCallback, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { searchAssets } from '@/lib/assetSearch' @@ const [matchingAssets, setMatchingAssets] = useState<Asset[] | null>(null) - const handleInputChange = useCallback( - (inputValue: string) => { - setSearchTerm(inputValue) - if (inputValue === '') { - setMatchingAssets(null) - } else { - debounce( - () => { - setMatchingAssets(searchAssets(inputValue, assets)) - }, - 500, - { - leading: true, - }, - )() - } - }, - [assets], - ) + const debouncedSearch = useMemo( + () => + debounce((term: string, list: Asset[]) => { + setMatchingAssets(searchAssets(term, list)) + }, 500, { leading: true, trailing: true }), + [], + ) + + useEffect(() => { + return () => debouncedSearch.cancel() + }, [debouncedSearch]) + + const handleInputChange = useCallback( + (inputValue: string) => { + setSearchTerm(inputValue) + const term = inputValue.trim() + if (!term) { + setMatchingAssets(null) + debouncedSearch.cancel() + return + } + debouncedSearch(term, assets) + }, + [assets, debouncedSearch], + )src/lib/assetSearch/utils.test.ts (1)
22-26: Test fixture inconsistency: BTC asset chainId doesn’t match assetId’s chainThe BTC object uses chainId 'eip155:1' (Ethereum) while its assetId is a bip122 (Bitcoin) CAIP-19. This mismatch can cause misleading outcomes in chain-sensitive tests.
- { - chainId: 'eip155:1', - assetId: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + { + chainId: 'bip122:000000000019d6689c085ae165831e93', + assetId: 'bip122:000000000019d6689c085ae165831e93/slip44:0',If intentional, consider adding an inline comment clarifying why the inconsistency is required for the test.
src/components/AssetSearch/AssetSearch.tsx (2)
87-94: Derive chain list from supportedAssets to avoid empty chains in UIComputing chainIds from the full assets prop can display chains with zero eligible assets after support filtering. Derive from supportedAssets instead.
- const chainIds: ChainId[] = useMemo(() => { - const unsortedChainIds = uniq(assets.map(asset => asset.chainId)) + const chainIds: ChainId[] = useMemo(() => { + const unsortedChainIds = uniq(supportedAssets.map(asset => asset.chainId)) const filteredChainIds = allowWalletUnsupportedAssets ? unsortedChainIds : unsortedChainIds.filter(chainId => walletConnectedChainIds.includes(chainId)) const sortedChainIds = sortChainIdsByDisplayName(filteredChainIds) return sortedChainIds - }, [allowWalletUnsupportedAssets, assets, walletConnectedChainIds]) + }, [allowWalletUnsupportedAssets, supportedAssets, walletConnectedChainIds])
30-41: Confirm error boundary coverage at usage sitePer our TSX guideline, components should be wrapped by an error boundary with a user-friendly fallback. If this component isn’t already rendered under a boundary at the page/route level, consider adding one.
I can provide a minimal ErrorBoundary wrapper component and integrate it at the route level if needed.
src/components/TradeAssetSearch/TradeAssetSearch.tsx (2)
151-155: Use the constant instead of a magic number.Keep the slice size in sync with
NUM_QUICK_ACCESS_ASSETS.- return popularAssets.slice(0, 6) + return popularAssets.slice(0, NUM_QUICK_ACCESS_ASSETS)
72-75: Sync internalactiveChainIdwhenselectedChainIdprop changes.
useState(selectedChainId)only seeds once. If the parent updatesselectedChainId, the component won’t reflect it. Add a sync effect.Add after Line 75:
+ // Keep local state in sync with prop updates + useEffect(() => { + setActiveChainId(selectedChainId) + }, [selectedChainId])src/components/TradeAssetSearch/components/SearchTermAssetList.tsx (1)
67-70: Trim contract address input before querying custom tokens.Prevents unnecessary requests for whitespace-only input and matches UI trim semantics.
const { data: customTokens, isLoading: isLoadingCustomTokens } = useGetCustomTokensQuery({ - contractAddress: searchString, + contractAddress: searchString.trim(), chainIds, })
♻️ Duplicate comments (14)
src/lib/utils/ethAddress.ts (1)
1-1: Clarify that this is a heuristic (no EIP-55 checksum) to prevent misuse in validation flows.Apply:
+/** + * Lightweight heuristic for ETH address detection used in search/filtering. + * NOTE: Does NOT validate EIP-55 checksum. Do not use for signing/sending flows. + */ export const isEthAddress = (address: string): boolean => /^0x[0-9A-Fa-f]{40}$/.test(address)Optionally, we can add a tiny checksum validator and a
normalizeHexAddress()helper if needed.src/lib/assetSearch/types.ts (1)
10-40: Simplify worker message API: drop unusedinitand adopt discriminated unionsWe confirmed that the
initmessage type isn’t sent or handled anywhere in the worker or its hook, making it safe to remove. Switching to a single discriminated-union style for inbound and outbound messages will:
- Eliminate the now-unused
InitMessagevariant.- Provide stronger type narrowing on
typefor both consumers and implementors.- Reduce API noise by collapsing separate interfaces into concise union members.
Proposed replacement in src/lib/assetSearch/types.ts:
-export interface AssetSearchWorkerMessages { - InitMessage: { - type: 'init' - payload: { assets: SearchableAsset[] } - } - UpdateAssetsMessage: { - type: 'updateAssets' - payload: { assets: SearchableAsset[] } - } - SearchMessage: { - type: 'search' - requestId: number - payload: { - searchString: string - activeChainId: ChainId | 'All' - allowWalletUnsupportedAssets: boolean | undefined - walletConnectedChainIds: ChainId[] - } - } - SearchResultMessage: { - type: 'searchResult' - requestId: number - payload: { assetIds: AssetId[] } - } -} - -export type AssetSearchWorkerInboundMessage = - | AssetSearchWorkerMessages['InitMessage'] - | AssetSearchWorkerMessages['UpdateAssetsMessage'] - | AssetSearchWorkerMessages['SearchMessage'] - -export type AssetSearchWorkerOutboundMessage = - AssetSearchWorkerMessages['SearchResultMessage'] +export type ActiveChainFilter = ChainId | 'All' + +export type AssetSearchWorkerInboundMessage = + | { type: 'updateAssets'; payload: { assets: SearchableAsset[] } } + | { + type: 'search' + requestId: number + payload: { + searchString: string + activeChainId: ActiveChainFilter + allowWalletUnsupportedAssets?: boolean + walletConnectedChainIds: ChainId[] + } + } + +export type AssetSearchWorkerOutboundMessage = { + type: 'searchResult' + requestId: number + payload: { assetIds: AssetId[] } +}Feel free to apply this refactor to streamline the API and improve type safety.
src/lib/assetSearch/utils.test.ts (1)
5-5: Tests migrated to searchAssets — add ETH address branch coveragesearchAssets has a special-case ETH address path. Add a test to assert that providing a checksummed or uppercased address returns the ERC20 asset.
describe('searchAssets', () => { + it('returns ERC20 by contract address (case-insensitive)', () => { + const dai = assets.find(a => a.symbol === 'DAI')! + const addressUpper = fromAssetId(dai.assetId).assetReference.toUpperCase() + const returnedAssets = searchAssets(addressUpper, assets) + expect(returnedAssets[0].symbol).toBe('DAI') + })src/components/AssetSearch/AssetSearch.tsx (1)
80-84: Trim the search string before invoking searchAssetsPrevents whitespace-only queries from triggering a search and keeps UX consistent with other callers.
- useEffect(() => { - if (filteredAssets) { - setSearchTermAssets(searching ? searchAssets(searchString, filteredAssets) : filteredAssets) - } - }, [searchString, searching, filteredAssets]) + useEffect(() => { + if (!filteredAssets) return + const term = searchString.trim() + setSearchTermAssets(term ? searchAssets(term, filteredAssets) : filteredAssets) + }, [searchString, filteredAssets])src/components/TradeAssetSearch/workers/assetSearch.worker.ts (1)
13-15: Use camelCase for mutable cache and guard against missing payloadUPPER_SNAKE_CASE suggests a constant; this is mutable worker state. Also, default to [] to avoid assigning undefined if a malformed message is received.
-// Internal state -let ASSETS: SearchableAsset[] = [] +// Internal state +let assetsCache: SearchableAsset[] = [] @@ - msg.payload + msg.payload @@ - const preFiltered = filterAssetsByChainSupport(ASSETS, { + const preFiltered = filterAssetsByChainSupport(assetsCache, { @@ - ASSETS = data.payload.assets + assetsCache = data.payload.assets ?? []Also applies to: 41-43
src/components/TradeAssetSearch/TradeAssetSearch.tsx (4)
25-31: Prune unused import after removing local assets selector.Once the redundant selector is removed, this named import is no longer used here.
selectAssetsSortedByMarketCap, - selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName, selectPortfolioFungibleAssetsSortedByBalance, selectPortfolioTotalUserCurrencyBalance, selectWalletConnectedChainIds,
76-79: Remove redundant assets selector (hook already selects assets).The hook internally selects assets, so this local selector does duplicated work and increases render churn. Drop it.
- const assets = useAppSelector( - selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName, - )
106-115: Don’t passassetsinto the worker hook; it’s not part of the API and creates drift.The hook signature doesn’t accept
assets. Extra properties in an object literal can fail excess property checks and mislead readers.const assetWorkerParams = useMemo( () => ({ - assets, activeChainId, allowWalletUnsupportedAssets, walletConnectedChainIds, hasWallet, }), - [activeChainId, allowWalletUnsupportedAssets, assets, hasWallet, walletConnectedChainIds], + [activeChainId, allowWalletUnsupportedAssets, hasWallet, walletConnectedChainIds], )
121-122: RenameisSearchingto reflect UI query state, not worker state.This boolean means “input has non-empty text”, which differs from
workerSearchState.isSearching. Rename to avoid confusion.- const isSearching = useMemo(() => searchString.length > 0, [searchString]) + const hasSearchQuery = useMemo(() => searchString.length > 0, [searchString])- {isSearching ? ( + {hasSearchQuery ? ( <SearchTermAssetList activeChainId={activeChainId} searchString={searchString} onAssetClick={handleAssetClick} onImportClick={handleImportIntent} isLoading={isPopularAssetIdsLoading} assetFilterPredicate={assetFilterPredicate} allowWalletUnsupportedAssets={!hasWallet || allowWalletUnsupportedAssets} workerSearchState={workerSearchState} /> ) : (Also applies to: 287-297
src/components/TradeAssetSearch/components/SearchTermAssetList.tsx (1)
130-144: Trim search string for consistent behavior across worker and fallback.Avoid whitespace-only searches and keep parity with the debounced, trimmed worker input.
const searchTermAssets = useMemo(() => { - const filteredAssets: Asset[] = (() => { + const term = searchString.trim() + const filteredAssets: Asset[] = (() => { // Main thread search due to dead worker if (workerSearchState.workerState === 'failed') { - return searchAssets(searchString, assetsForChain) + return term ? searchAssets(term, assetsForChain) : [] } // Use the results from the worker if (workerSearchState.workerState === 'ready' && workerSearchState.searchResults) { return workerSearchState.searchResults.map(assetId => assetIdMap[assetId]).filter(isSome) } return [] })()- }, [ + }, [ customAssets, workerSearchState.workerState, workerSearchState.searchResults, - searchString, + searchString, assetsForChain, assetIdMap, portfolioUserCurrencyBalances, ])(Note: dependency array remains correct since it still tracks
searchString.)src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (2)
10-11: Use canonical selectors import path for consistency.Align with other modules that import from
@/state/slices/selectors.-import { selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName } from '@/state/slices/common-selectors' +import { selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName } from '@/state/slices/selectors'
86-97: Treat empty asset sets as a valid ready state; always post updates.Zero assets shouldn’t block search readiness. Post an empty update and mark as ready; only mark “initializing” when the worker isn’t constructed.
- useEffect(() => { - if (workerRef.current && assets.length) { - workerRef.current.postMessage({ - type: 'updateAssets', - payload: { assets: assets.map(a => pick(a, ['assetId', 'name', 'symbol', 'chainId'])) }, - }) - setWorkerSearchState(prev => ({ ...prev, workerState: 'ready' })) - } else { - setWorkerSearchState(prev => ({ ...prev, workerState: 'initializing' })) - } - }, [assets]) + useEffect(() => { + if (workerRef.current) { + workerRef.current.postMessage({ + type: 'updateAssets', + payload: { assets: assets.map(a => pick(a, ['assetId', 'name', 'symbol', 'chainId'])) }, + }) + setWorkerSearchState(prev => ({ ...prev, workerState: 'ready' })) + } else { + setWorkerSearchState(prev => ({ ...prev, workerState: 'initializing' })) + } + }, [assets])src/lib/assetSearch/utils.ts (2)
12-20: Guard against non-address input in ETH address filter.Make the helper resilient and avoid accidental matches when callers forget to pre-check.
export const filterAssetsByEthAddress = <T extends { assetId: AssetId }>( address: string, assets: T[], ): T[] => { - const searchLower = address.toLowerCase() + if (!isEthAddress(address)) return [] + const searchLower = address.toLowerCase() return assets.filter( asset => fromAssetId(asset.assetId).assetReference.toLowerCase() === searchLower, ) }
22-51: DefaultactiveChainIdto'All'to avoid a hidden “false” branch.Eliminates the footgun where undefined returns
falseand filters everything out.export const filterAssetsByChainSupport = <T extends { assetId: AssetId; chainId: ChainId }>( assets: T[], options: { activeChainId?: ChainId | 'All' allowWalletUnsupportedAssets?: boolean walletConnectedChainIds: ChainId[] }, ): T[] => { - const { activeChainId, allowWalletUnsupportedAssets, walletConnectedChainIds } = options + const { + activeChainId = 'All', + allowWalletUnsupportedAssets, + walletConnectedChainIds, + } = options return assets.filter(asset => { // Always filter out NFTs if (!isSearchableAsset(asset.assetId)) return false if (activeChainId === 'All') { if (allowWalletUnsupportedAssets) return true return walletConnectedChainIds.includes(asset.chainId) } - if ( - activeChainId && - !allowWalletUnsupportedAssets && - !walletConnectedChainIds.includes(activeChainId) - ) { + if (!allowWalletUnsupportedAssets && !walletConnectedChainIds.includes(activeChainId)) { return false } - return activeChainId ? asset.chainId === activeChainId : false + return asset.chainId === activeChainId }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
src/components/AssetSearch/AssetSearch.tsx(2 hunks)src/components/StakingVaults/PositionTable.tsx(1 hunks)src/components/TradeAssetSearch/TradeAssetSearch.tsx(7 hunks)src/components/TradeAssetSearch/components/SearchTermAssetList.tsx(5 hunks)src/components/TradeAssetSearch/helpers/filterAssetsBySearchTerm/filterAssetsBySearchTerm.ts(0 hunks)src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts(1 hunks)src/components/TradeAssetSearch/workers/assetSearch.worker.ts(1 hunks)src/lib/address/utils.ts(0 hunks)src/lib/assetSearch/config.ts(1 hunks)src/lib/assetSearch/index.ts(1 hunks)src/lib/assetSearch/types.ts(1 hunks)src/lib/assetSearch/utils.test.ts(2 hunks)src/lib/assetSearch/utils.ts(1 hunks)src/lib/utils/ethAddress.ts(1 hunks)src/pages/TransactionHistory/hooks/useSearch.tsx(2 hunks)vite.config.mts(1 hunks)
💤 Files with no reviewable changes (2)
- src/components/TradeAssetSearch/helpers/filterAssetsBySearchTerm/filterAssetsBySearchTerm.ts
- src/lib/address/utils.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/lib/assetSearch/config.tssrc/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/lib/assetSearch/utils.tssrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/lib/assetSearch/index.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/AssetSearch/AssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.tssrc/lib/utils/ethAddress.ts
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/lib/assetSearch/config.tsvite.config.mtssrc/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/lib/assetSearch/utils.tssrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/lib/assetSearch/index.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/AssetSearch/AssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.tssrc/lib/utils/ethAddress.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/lib/assetSearch/config.tssrc/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/lib/assetSearch/utils.tssrc/pages/TransactionHistory/hooks/useSearch.tsxsrc/lib/assetSearch/index.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/AssetSearch/AssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.tssrc/lib/utils/ethAddress.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/AssetSearch/AssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsx
**/use*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/pages/TransactionHistory/hooks/useSearch.tsxsrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/AssetSearch/AssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsx
🧠 Learnings (12)
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Applied to files:
src/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.tssrc/components/AssetSearch/AssetSearch.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsxsrc/lib/assetSearch/types.ts
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Applied to files:
src/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.test.tssrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/AssetSearch/AssetSearch.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-17T21:53:03.806Z
Learnt from: 0xApotheosis
PR: shapeshift/web#10290
File: scripts/generateAssetData/color-map.json:41-47
Timestamp: 2025-08-17T21:53:03.806Z
Learning: In the ShapeShift web codebase, native assets (using CAIP-19 slip44 namespace like eip155:1/slip44:60, bip122:.../slip44:..., cosmos:.../slip44:...) are manually hardcoded and not generated via the automated asset generation script. Only ERC20/BEP20 tokens go through the asset generation process. The validation scripts should only validate generated assets, not manually added native assets.
Applied to files:
src/components/TradeAssetSearch/workers/assetSearch.worker.tssrc/lib/assetSearch/utils.test.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter is expected to have an assetId property. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property (signature: (state, { assetId })), not a raw AssetId string. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/lib/assetSearch/utils.tssrc/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-11T09:45:51.174Z
Learnt from: gomesalexandre
PR: shapeshift/web#10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:175-180
Timestamp: 2025-08-11T09:45:51.174Z
Learning: gomesalexandre prefers truthy checks over explicit boolean comparisons (e.g., `walletSupportsSellAssetChain` instead of `walletSupportsSellAssetChain === true`) when dealing with tri-state values (boolean | null) in TypeScript, as the falsy behavior for null/undefined is intentional and acceptable.
Applied to files:
src/lib/assetSearch/utils.ts
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
PR: shapeshift/web#10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.
Applied to files:
src/lib/assetSearch/utils.tssrc/lib/assetSearch/types.ts
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/index.ts : Export unique functions/types from packages/swapper/src/index.ts if needed.
Applied to files:
src/lib/assetSearch/index.ts
📚 Learning: 2025-08-03T22:10:11.424Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/naming-conventions.mdc:0-0
Timestamp: 2025-08-03T22:10:11.424Z
Learning: Applies to **/*.{ts,tsx} : ALWAYS use named exports for components and functions
Applied to files:
src/lib/assetSearch/index.ts
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx:97-108
Timestamp: 2025-08-14T17:54:32.563Z
Learning: In ReusableLpStatus component (src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx), the txAssets dependency is stable from first render because poolAsset, baseAsset, actionSide, and action are all defined first render, making the current txAssetsStatuses initialization pattern safe without needing useEffect synchronization.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/AssetSearch/AssetSearch.tsxsrc/components/TradeAssetSearch/components/SearchTermAssetList.tsx
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
PR: shapeshift/web#10215
File: src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts:65-67
Timestamp: 2025-08-10T21:09:25.643Z
Learning: In the MultiHopTrade components, `selectInputBuyAsset` and `selectInputSellAsset` selectors from `tradeInputSlice` always return defined values because they have default values in the initial state (BTC for buyAsset, ETH for sellAsset, with fallback to defaultAsset). Null checks for these assets are unnecessary when using these selectors.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
🧬 Code Graph Analysis (9)
src/components/TradeAssetSearch/workers/assetSearch.worker.ts (2)
src/lib/assetSearch/types.ts (3)
SearchableAsset(3-8)AssetSearchWorkerInboundMessage(42-45)AssetSearchWorkerOutboundMessage(47-47)src/lib/assetSearch/utils.ts (2)
filterAssetsByChainSupport(22-51)searchAssets(53-66)
src/lib/assetSearch/utils.ts (4)
packages/caip/src/assetId/assetId.ts (3)
AssetId(17-17)isNft(181-187)fromAssetId(143-178)src/lib/assetSearch/types.ts (1)
SearchableAsset(3-8)src/lib/assetSearch/config.ts (1)
ASSET_SEARCH_MATCH_SORTER_CONFIG(3-6)src/lib/utils/ethAddress.ts (1)
isEthAddress(1-1)
src/pages/TransactionHistory/hooks/useSearch.tsx (2)
src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)src/state/slices/assetsSlice/assetsSlice.ts (1)
assets(42-64)
src/components/TradeAssetSearch/TradeAssetSearch.tsx (2)
src/state/slices/common-selectors.ts (1)
selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName(207-237)src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (1)
useAssetSearchWorker(29-168)
src/lib/assetSearch/utils.test.ts (2)
src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)packages/caip/src/assetId/assetId.ts (1)
fromAssetId(143-178)
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (4)
packages/caip/src/assetId/assetId.ts (1)
AssetId(17-17)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/common-selectors.ts (1)
selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName(207-237)src/lib/assetSearch/types.ts (1)
AssetSearchWorkerOutboundMessage(47-47)
src/components/AssetSearch/AssetSearch.tsx (1)
src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)
src/components/TradeAssetSearch/components/SearchTermAssetList.tsx (5)
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (1)
WorkerSearchState(15-20)packages/utils/src/getAssetNamespaceFromChainId.ts (1)
getAssetNamespaceFromChainId(7-33)packages/utils/src/makeAsset/makeAsset.ts (2)
MinimalAsset(8-8)makeAsset(16-53)src/lib/utils/index.ts (1)
isSome(98-99)src/lib/assetSearch/utils.ts (1)
searchAssets(53-66)
src/lib/assetSearch/types.ts (1)
packages/caip/src/assetId/assetId.ts (1)
AssetId(17-17)
🔇 Additional comments (9)
vite.config.mts (1)
112-114: ESM worker format is the right call for SharedWorker/new Worker module builds.Setting worker.format = 'es' fixes Vite’s default IIFE worker emission and aligns with module workers. Good change.
src/components/StakingVaults/PositionTable.tsx (1)
23-23: Import path change looks good; keep usage consistent with the new utility’s semantics.Switching to '@/lib/utils/ethAddress' is consistent with the extraction.
src/lib/assetSearch/types.ts (1)
47-47: No action required: barrel already re-exports the worker message typeThe barrel file
src/lib/assetSearch/index.tsincludesexport * from './types'which brings
AssetSearchWorkerOutboundMessageinto the public API, so no additional re-export is needed.src/lib/assetSearch/index.ts (1)
1-3: Barrel looks good — cohesive, named exports onlyThe barrel cleanly re-exports config, types, and utils, aligning with our named-exports guideline and simplifying consumers. No issues.
src/lib/assetSearch/utils.test.ts (1)
62-85: LGTM on behavior assertions and order sensitivitySymbol/name/assetId queries and closest-match precedence look correct and stable against the match-sorter configuration.
src/components/TradeAssetSearch/components/SearchTermAssetList.tsx (3)
93-101: LGTM: assetIdMap for O(1) lookups and dedupe.Good use of a local map to resolve worker results and skip duplicates efficiently.
102-129: LGTM: Safe custom-asset creation with completeness checks and dedupe.The completeness guard,
toAssetIdnormalization, andmakeAssetusage are sound.
166-171: LGTM: Loading state integrates worker searching with existing flags.Including
workerSearchState.isSearchingaligns UX with background work.src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (1)
126-131: No change needed forallowWalletUnsupportedAssetscoercionThe
AssetSearchWorkerInboundMessagetype already definesallowWalletUnsupportedAssets?: booleanso passing
undefinedis both valid and intentional. The worker’sfilterAssetsByChainSupporthelper also accepts an optional boolean, treatingundefinedasfalseunder the hood—mirroring the behavior ofBoolean(undefined)without any side effects.• Payload interface: allows
boolean | undefined.
• Worker logic: handles missing (undefined) the same asfalse.No refactor is required here.
Likely an incorrect or invalid review comment.
premiumjibles
force-pushed
the
reapply-optimise-search
branch
from
c535f33 to
d42b37e
Compare
premiumjibles
force-pushed
the
reapply-optimise-search
branch
from
d42b37e to
96d0e5a
Compare
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 (3)
src/components/StakingVaults/PositionTable.tsx (2)
188-210: Fix ETH-address search: case normalization bug and unsafeanyparameter
- Bug: You compare a lowercased asset reference to the raw
filterValue, causing uppercase/Checksummed inputs to miss. Normalize the input once and reuse it.- Type hygiene: Replace
anywithunknownand early-return on non-string.- Minor: Trim once and reuse; avoid re-trimming in branches.
- const filterRowsBySearchTerm = useCallback( - (rows: AggregatedOpportunitiesByAssetIdReturn[], filterValue: any) => { - if (!filterValue) return rows - if (typeof filterValue !== 'string') { - return [] - } - const search = filterValue.trim().toLowerCase() - if (isEthAddress(filterValue)) { - return rows.filter( - row => fromAssetId(row.assetId).assetReference.toLowerCase() === filterValue, - ) - } + const filterRowsBySearchTerm = useCallback( + (rows: AggregatedOpportunitiesByAssetIdReturn[], filterValue: unknown) => { + if (!filterValue) return rows + if (typeof filterValue !== 'string') return [] + const term = filterValue.trim() + const termLower = term.toLowerCase() + if (!term) return rows + if (isEthAddress(term)) { + return rows.filter( + row => fromAssetId(row.assetId).assetReference.toLowerCase() === termLower, + ) + } - const assetIds = rows.map(row => row.assetId) - const rowAssets = assets.filter(asset => assetIds.includes(asset.assetId)) - const matchedAssets = matchSorter(rowAssets, search, { + const assetIdSet = new Set(rows.map(r => r.assetId)) + const rowAssets = assets.filter(asset => assetIdSet.has(asset.assetId)) + const matchedAssets = matchSorter(rowAssets, termLower, { keys: ['name', 'symbol'], threshold: matchSorter.rankings.CONTAINS, }).map(asset => asset.assetId) const results = rows.filter(row => matchedAssets.includes(row.assetId)) return results }, [assets], )
117-186: Optional: Reuse centralized search utility to avoid driftYou’re essentially duplicating logic that exists in
src/lib/assetSearch/utils.ts(normalization + matchSorter config + ETH routing). Consider delegating tosearchAssetsto keep behavior consistent across the app.+import { searchAssets } from '@/lib/assetSearch/utils' @@ - const assetIdSet = new Set(rows.map(r => r.assetId)) - const rowAssets = assets.filter(asset => assetIdSet.has(asset.assetId)) - const matchedAssets = matchSorter(rowAssets, termLower, { - keys: ['name', 'symbol'], - threshold: matchSorter.rankings.CONTAINS, - }).map(asset => asset.assetId) - const results = rows.filter(row => matchedAssets.includes(row.assetId)) - return results + const assetIdSet = new Set(rows.map(r => r.assetId)) + const rowAssets = assets.filter(asset => assetIdSet.has(asset.assetId)) + const matchedAssetIds = new Set(searchAssets(term, rowAssets).map(a => a.assetId)) + return rows.filter(row => matchedAssetIds.has(row.assetId))src/components/TradeAssetSearch/TradeAssetSearch.tsx (1)
146-149: Use the existing constant for slice sizeSmall consistency/readability win: use
NUM_QUICK_ACCESS_ASSETSinstead of hard-coded6.- if (activeChainId !== 'All') { - return popularAssets.slice(0, 6) - } + if (activeChainId !== 'All') { + return popularAssets.slice(0, NUM_QUICK_ACCESS_ASSETS) + }
♻️ Duplicate comments (6)
src/lib/assetSearch/utils.ts (3)
12-20: Guard against non-address input and normalize onceReturn early if
addressisn’t a valid ETH address and normalize with a singletrim().toLowerCase()call. This makes the helper resilient and faster on invalid input.export const filterAssetsByEthAddress = <T extends { assetId: AssetId }>( address: string, assets: T[], ): T[] => { - const searchLower = address.toLowerCase() + const addr = address?.trim() + if (!addr || !isEthAddress(addr)) return [] + const searchLower = addr.toLowerCase() return assets.filter( asset => fromAssetId(asset.assetId).assetReference.toLowerCase() === searchLower, ) }
22-51: DefaultactiveChainIdto 'All' to avoid falsy-footgun; simplify final returnCurrent code returns
falsewhenactiveChainIdis omitted. Defaulting to'All'yields predictable behavior and lets you simplify the last return.): T[] => { - const { activeChainId, allowWalletUnsupportedAssets, walletConnectedChainIds } = options + const { + activeChainId = 'All', + allowWalletUnsupportedAssets, + walletConnectedChainIds, + } = options @@ - return activeChainId ? asset.chainId === activeChainId : false + return asset.chainId === activeChainId }) }
53-66: Normalize/trimsearchTermonce; route ETH search with trimmed valueCentralize normalization so callers get consistent behavior (including whitespace-only input).
): T[] => { if (!assets) return [] - if (!searchTerm) return assets + const term = searchTerm?.trim() ?? '' + if (!term) return assets - if (isEthAddress(searchTerm)) { - return filterAssetsByEthAddress(searchTerm, assets) + if (isEthAddress(term)) { + return filterAssetsByEthAddress(term, assets) } - return matchSorter(assets, searchTerm, config) + return matchSorter(assets, term, config) }src/components/TradeAssetSearch/TradeAssetSearch.tsx (1)
116-117: Name and semantics: derive from trimmed input and avoid confusion with worker state
isSearchingcurrently means “has any characters” and diverges for whitespace-only input. Also, it’s easy to confuse withworkerSearchState.isSearching. Rename and derive fromtrim().- const isSearching = useMemo(() => searchString.length > 0, [searchString]) + const hasSearchQuery = useMemo(() => searchString.trim().length > 0, [searchString])And update usages:
- {isSearching ? ( + {hasSearchQuery ? (src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (2)
10-11: Resolved: using the canonical selectors import path.Good catch aligning to '@/state/slices/selectors'; this removes churn vs '@/state/slices/common-selectors'.
69-76: Prefer structured logging over console.error for worker errors.Per project rules, replace console.error with the project’s structured logger; keep the state transition to Failed unchanged. Non-blocking given team preferences noted in past PRs, but aligns with the repo guidelines.
+// import your logger (adjust path if different) +import { logger } from '@/lib/logger' @@ - worker.onerror = error => { - console.error('Worker error:', error) + worker.onerror = error => { + logger.error({ error }, 'assetSearch worker error') setWorkerSearchState(prev => ({ ...prev, workerState: 'failed' })) } @@ - } catch (error) { - console.error('Failed to initialize worker:', error) + } catch (error) { + logger.error({ error }, 'assetSearch worker init failed') setWorkerSearchState(prev => ({ ...prev, workerState: 'failed' })) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/StakingVaults/PositionTable.tsx(1 hunks)src/components/TradeAssetSearch/TradeAssetSearch.tsx(5 hunks)src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts(1 hunks)src/lib/address/utils.ts(0 hunks)src/lib/assetSearch/utils.ts(1 hunks)src/lib/utils/ethAddress.ts(1 hunks)vite.config.mts(1 hunks)
💤 Files with no reviewable changes (1)
- src/lib/address/utils.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/StakingVaults/PositionTable.tsxsrc/lib/utils/ethAddress.tssrc/lib/assetSearch/utils.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsx
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/StakingVaults/PositionTable.tsxvite.config.mtssrc/lib/utils/ethAddress.tssrc/lib/assetSearch/utils.ts
**/*.{tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/StakingVaults/PositionTable.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/components/StakingVaults/PositionTable.tsxsrc/lib/utils/ethAddress.tssrc/lib/assetSearch/utils.ts
**/use*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
🧠 Learnings (18)
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
PR: shapeshift/web#10215
File: src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts:65-67
Timestamp: 2025-08-10T21:09:25.643Z
Learning: In the MultiHopTrade components, `selectInputBuyAsset` and `selectInputSellAsset` selectors from `tradeInputSlice` always return defined values because they have default values in the initial state (BTC for buyAsset, ETH for sellAsset, with fallback to defaultAsset). Null checks for these assets are unnecessary when using these selectors.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/lib/assetSearch/utils.ts
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter is expected to have an assetId property. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/lib/assetSearch/utils.ts
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property (signature: (state, { assetId })), not a raw AssetId string. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/components/TradeAssetSearch/hooks/useAssetSearchWorker.tssrc/lib/assetSearch/utils.ts
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsxsrc/lib/assetSearch/utils.ts
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx:97-108
Timestamp: 2025-08-14T17:54:32.563Z
Learning: In ReusableLpStatus component (src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx), the txAssets dependency is stable from first render because poolAsset, baseAsset, actionSide, and action are all defined first render, making the current txAssetsStatuses initialization pattern safe without needing useEffect synchronization.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsx
📚 Learning: 2025-08-13T13:16:11.290Z
Learnt from: NeOMakinG
PR: shapeshift/web#10271
File: src/components/AssetHeader/AssetActions.tsx:20-20
Timestamp: 2025-08-13T13:16:11.290Z
Learning: Project shapeshift/web: NeOMakinG confirmed that the project does not support server-side rendering (SSR), so concerns about guarding navigator/window access for SSR safety are not applicable to this codebase.
Applied to files:
src/components/TradeAssetSearch/TradeAssetSearch.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : Console.error without structured logging is an anti-pattern
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : ALWAYS use structured logging for errors
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : Poor error logging is an anti-pattern
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-13T17:06:16.397Z
Learnt from: gomesalexandre
PR: shapeshift/web#10249
File: src/hooks/useActionCenterSubscribers/useThorchainLpWithdrawActionSubscriber.tsx:76-78
Timestamp: 2025-08-13T17:06:16.397Z
Learning: gomesalexandre prefers to keep error handling simple with console.error rather than implementing structured logging, especially when broader error handling patterns in the codebase (like THOR error handling) need systematic improvement rather than piecemeal fixes.
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-07-29T10:35:22.059Z
Learnt from: NeOMakinG
PR: shapeshift/web#10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-07T11:22:16.983Z
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : ALWAYS log errors for debugging
Applied to files:
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts
📚 Learning: 2025-08-21T00:49:17.261Z
Learnt from: premiumjibles
PR: shapeshift/web#10318
File: vite.config.mts:112-114
Timestamp: 2025-08-21T00:49:17.261Z
Learning: The shapeshift/web codebase does not currently contain any Worker or SharedWorker instantiations in the src directory, so concerns about missing `{ type: 'module' }` flags in worker creation are not applicable.
Applied to files:
vite.config.mts
📚 Learning: 2025-08-11T09:45:51.174Z
Learnt from: gomesalexandre
PR: shapeshift/web#10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:175-180
Timestamp: 2025-08-11T09:45:51.174Z
Learning: gomesalexandre prefers truthy checks over explicit boolean comparisons (e.g., `walletSupportsSellAssetChain` instead of `walletSupportsSellAssetChain === true`) when dealing with tri-state values (boolean | null) in TypeScript, as the falsy behavior for null/undefined is intentional and acceptable.
Applied to files:
src/lib/assetSearch/utils.ts
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
PR: shapeshift/web#10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.
Applied to files:
src/lib/assetSearch/utils.ts
🧬 Code Graph Analysis (3)
src/components/TradeAssetSearch/TradeAssetSearch.tsx (1)
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (1)
useAssetSearchWorker(29-168)
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (4)
packages/caip/src/assetId/assetId.ts (1)
AssetId(17-17)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/common-selectors.ts (1)
selectAssetsSortedByMarketCapUserCurrencyBalanceCryptoPrecisionAndName(207-237)src/lib/assetSearch/types.ts (1)
AssetSearchWorkerOutboundMessage(47-47)
src/lib/assetSearch/utils.ts (4)
packages/caip/src/assetId/assetId.ts (3)
AssetId(17-17)isNft(181-187)fromAssetId(143-178)src/lib/assetSearch/types.ts (1)
SearchableAsset(3-8)src/lib/assetSearch/config.ts (1)
ASSET_SEARCH_MATCH_SORTER_CONFIG(3-6)src/lib/utils/ethAddress.ts (1)
isEthAddress(1-1)
🔇 Additional comments (8)
src/lib/utils/ethAddress.ts (1)
1-1: LGTM: Minimal, fast heuristic fits search use-casesThe literal regex is compiled once and the function is properly typed. No changes needed here.
src/components/StakingVaults/PositionTable.tsx (1)
23-23: Import path migration is correctSwitching to '@/lib/utils/ethAddress' aligns with the new utility location and reduces accidental deep imports.
vite.config.mts (1)
112-114: ESM worker output enabled — aligns with bundler worker modulesSetting
worker.format: 'es'is the correct lever for Vite’s ES-module workers and matches thenew AssetSearchWorker()pattern. No further config required here.src/components/TradeAssetSearch/TradeAssetSearch.tsx (1)
102-110: Parameter object shape is clean and memoized correctlyThe worker params reflect the hook’s API and the dependency array is tight. Good separation of concerns.
src/components/TradeAssetSearch/hooks/useAssetSearchWorker.ts (4)
49-57: StrictMode-safe worker instantiation guard looks correct.The early return on an existing ref avoids double-instantiation under React StrictMode. Nice.
78-84: Clean termination on unmount looks good.The worker is properly terminated and the ref nulled to prevent leaks. LGTM.
56-67: RequestId-based response matching is correct.Ignoring out-of-order responses by checking the ref wins races cleanly.
142-156: Event handler state transitions are clean and descriptive.handleSearchChange uses a clear name and sets isSearching/searchResults predictably based on trimmed input. Good API for consumers.
|
Changed to draft as it's on hold |
Sorry @NeOMakinG meant to remove that, not on hold anymore. We can get it better with just a simple debounce after some testing but it's still not quite as smooth as this so I think it's still worth getting this in. |
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
https://jam.dev/c/3e8f7c9f-0cd0-450d-95ed-a9cd49747e91
Already tested that previously, the only diff is the build one, so happy with this one!
Description
This includes one commit which is a straight reapply of this PR https://github.com/shapeshift/web/pulls?q=is%3Apr+is%3Aclosed as well as a commit to fix vite build with the shared worker.
I had to configure to make sure vite properly bundles the worker. I also pulled a util the worker was using into it's own file, otherwise the bundle imported the full viem suite which made the bundle huge.
I made sure the build pipeline passes here https://github.com/shapeshift/web/tree/jib
Issue (if applicable)
closes #
Risk
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores