feat: reference link#505
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR refactors market filter state management by introducing a persisted Zustand store for filter preferences, separating them from transient UI state. It adds URL state parsing to apply search parameters on page load and reorganizes network type definitions into a dedicated module. Changes
Sequence DiagramsequenceDiagram
participant Browser
participant MarketPage
participant Markets as Markets Component
participant Parser as URL Parser
participant Store as Filter Store
participant Hook as useFilteredMarkets
participant Display as Market Display
Browser->>MarketPage: Load with ?network=arb&...
MarketPage->>MarketPage: Await searchParams promise
MarketPage->>Markets: Pass initialSearchParams
Markets->>Parser: parseMarketFilterUrlState(searchParams)
Parser->>Parser: Resolve network from query params
Parser-->>Markets: MarketFilterUrlState
Markets->>Store: useMarketFilterPreferences()
Note over Markets: useLayoutEffect on signature change
Markets->>Store: Apply parsed network to store
Store-->>Markets: Persisted filters updated
Markets->>Hook: render with hook
Hook->>Store: Read selectedNetwork, selectedCollaterals, selectedLoanAssets
Store-->>Hook: Current persisted selections
Hook->>Hook: filterMarkets(...)
Hook-->>Display: Filtered market list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
src/hooks/useFilteredMarkets.ts (1)
25-25: Consider selecting individual slices.
useMarketFilterPreferences()without a selector returns the whole store, so the hook re-runs whenever any field/action reference of that store changes. Selecting only the three fields you read here would tighten re-render anduseMemoinvalidation:const selectedNetwork = useMarketFilterPreferences((s) => s.selectedNetwork); const selectedCollaterals = useMarketFilterPreferences((s) => s.selectedCollaterals); const selectedLoanAssets = useMarketFilterPreferences((s) => s.selectedLoanAssets);Not blocking.
Also applies to: 153-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useFilteredMarkets.ts` at line 25, The hook is subscribing to the entire store via useMarketFilterPreferences(), causing unnecessary re-renders; change it to select only the three fields you consume by replacing persistedFilters with three selectors (e.g., call useMarketFilterPreferences((s) => s.selectedNetwork), useMarketFilterPreferences((s) => s.selectedCollaterals), and useMarketFilterPreferences((s) => s.selectedLoanAssets)) where persistedFilters is used (also apply the same selector-based change for the other occurrences around the block that spans the area referenced as lines 153-166) so the component only re-runs when those specific values change.src/utils/supported-networks.ts (1)
13-23: Optional: tighten array typing.Consider
as const(or explicitreadonly SupportedNetworks[]) so callers can't mutateALL_SUPPORTED_NETWORKSand the literal types are preserved.Diff
-export const ALL_SUPPORTED_NETWORKS = [ +export const ALL_SUPPORTED_NETWORKS = [ SupportedNetworks.Mainnet, ... SupportedNetworks.Monad, -]; +] as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/supported-networks.ts` around lines 13 - 23, The exported ALL_SUPPORTED_NETWORKS array should be made immutable and preserve literal types to prevent mutation and improve type safety; update the declaration of ALL_SUPPORTED_NETWORKS (the exported constant in supported-networks.ts) to either use a const assertion (e.g., append "as const") or annotate it as "readonly SupportedNetworks[]" so callers cannot mutate it and TypeScript retains literal/readonly semantics, then run type checks to ensure any consumers expecting a mutable array are adjusted to accept a readonly array if you chose the readonly annotation.src/utils/networks.ts (1)
196-198:getNetworkConfigstill uses an unsafe cast.The
as NetworkConfigmasks a possibleundefinedif a chain id ever slips through (e.g., new enum member added without anetworksentry). Now that the param isSupportedNetworkId, you could either rely on a completeRecord<SupportedNetworkId, NetworkConfig>lookup, or throw explicitly when not found. Not blocking — just a foot-gun for future additions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/networks.ts` around lines 196 - 198, The getNetworkConfig function currently masks missing entries with an unsafe cast; change it to perform a safe lookup and fail fast: replace the casted find with a real check — e.g., const cfg = networks.find(n => n.network === chainId); if (!cfg) throw new Error(`Missing NetworkConfig for ${chainId}`); return cfg; — or alternatively convert the networks collection to a Record<SupportedNetworkId, NetworkConfig> and return networks[chainId] (still throwing if undefined). Update getNetworkConfig to reference the networks symbol and remove the "as NetworkConfig" cast.src/features/markets/markets-view.tsx (1)
41-43: Subscribing to the whole preferences store causes extra re-renders.
useMarketFilterPreferences()without a selector re-renders this component on every preference change. SinceapplySelectionswrites to the same store insideuseLayoutEffect, the effect re-runs (saved by the signature guard, so no infinite loop, just wasted work).Optional: subscribe via narrow selectors, or use
useShallowfor the actions you actually need.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/markets/markets-view.tsx` around lines 41 - 43, The component is subscribing to the entire preferences store via useMarketFilterPreferences(), causing needless re-renders when unrelated preferences change; change the hook usage to subscribe only to the specific pieces or actions you need (e.g., select just the preferences read by this component or the applySelections action) or use a shallow selector hook like useShallow so updates are scoped; update the call sites where useMarketFilterPreferences() is used (and any place that calls applySelections inside the useLayoutEffect) to pass a selector that returns only the required fields or actions to avoid the redundant re-rendering loop.src/features/markets/market-filter-url-state.ts (2)
32-34:REF_NETWORK_ALIASESis currently a one-entry map.Mostly a heads-up: only
etherlinkis reachable via?ref=. If that's the intended scope (only etherlink links are special), fine. If you expect to add more ref networks soon, a comment noting why this is narrower thanNETWORK_ALIASESwould save the next reader some digging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/markets/market-filter-url-state.ts` around lines 32 - 34, REF_NETWORK_ALIASES currently contains only the single mapping "etherlink" => SupportedNetworks.Etherlink which may be surprising; update the declaration for REF_NETWORK_ALIASES (the const in market-filter-url-state.ts) to either expand it with additional ref network entries if expected, or add a concise inline comment explaining why it intentionally mirrors only the "etherlink" ref (and how it differs from NETWORK_ALIASES) so future readers know this map is intentionally narrow; reference the const name REF_NETWORK_ALIASES and the SupportedNetworks enum in your change.
101-116: Mixed clear + value reduces to clear; worth a comment.
?loan=symbol:usdc&loan=allbecomes[](full reset) becauserawValues.some(isClear)short-circuits the whole list. Probably the right call, but a one-liner comment here would prevent future "why did my selection vanish?" debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/markets/market-filter-url-state.ts` around lines 101 - 116, In parseTokenSelectorsParam, the current check using splitSearchParamValues + rawValues.some(...CLEAR_VALUES via normalizeParamValue) treats any mix of a clear token (e.g., "all") with actual selectors as a full reset (returns []); add a one-line comment above that if (rawValues.length === 0 || ...) branch documenting that "presence of any clear token among values yields full clear/reset (returns empty array) rather than merging values" so future readers understand why a mixed ?loan=symbol:usdc&loan=all becomes a reset; reference splitSearchParamValues, normalizeParamValue and CLEAR_VALUES in the comment for clarity.src/features/markets/market-filter-selection.ts (1)
18-46: LGTM — small, focused selection helpers.One minor thought:
marketFilterSelectionIncludesAssetsplits the key on every call. If it gets used inside hot per-market filter loops, consider precomputing aSet<string>of sub-keys at the predicate level. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/markets/market-filter-selection.ts` around lines 18 - 46, The predicate marketFilterSelectionIncludesAsset currently splits the composite selectionKey string on every call which is wasteful in hot loops; refactor by adding a helper that converts a selectionKey into a Set of sub-keys (e.g. parseMarketFilterSelectionKey or selectionKeyToSet) and update callers (where per-market filtering runs) to compute that Set once and use Set.has(toChainAssetKey(...)) instead of repeatedly calling marketFilterSelectionIncludesAsset; keep getMarketFilterAssetSelectionKey and resolveMarketFilterTokenSelectors unchanged except where they should feed or consume the new Set-based form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/page.tsx`:
- Around line 19-32: The redirect currently serializes and forwards the entire
resolvedSearchParams via serializeSearchParamsRecord, which forwards unrelated
query params; update the Page handler to first build a filtered params object
containing only keys present in MARKET_INTENT_QUERY_KEYS (use
MARKET_INTENT_QUERY_KEYS.has(key) against resolvedSearchParams keys), then call
serializeSearchParamsRecord(filteredParams) and pass that result to redirect;
reference the variables/functions Page, resolvedSearchParams,
MARKET_INTENT_QUERY_KEYS, serializeSearchParamsRecord, and redirect when making
the change.
In `@src/features/markets/markets-view.tsx`:
- Around line 54-84: The URL-derived selection memos (resolvedUrlLoanSelections
and resolvedUrlCollateralSelections) can resolve against empty
uniqueLoanAssets/uniqueCollaterals and produce [] too early, wiping URL
selections; to fix, treat empty item lists as "not ready" inside those useMemo
blocks: after the undefined/empty-checks and before resolving, return null
whenever loading is true OR uniqueLoanAssets/uniqueCollaterals are empty (e.g.,
uniqueLoanAssets.length === 0 / uniqueCollaterals.length === 0), so
resolveMarketFilterSelectionsFromUrlState only runs when items are populated;
alternatively compute uniqueLoanAssets/uniqueCollaterals with useMemo so they
are populated in the same render as loading flips to false to avoid the race
that causes applySelections/appliedUrlSignatureRef to prematurely apply an empty
selection.
In `@src/stores/useMarketFilterPreferences.ts`:
- Around line 5-58: Persisted composite keys in useMarketFilterPreferences
(selectedCollaterals/selectedLoanAssets) can become stale when token.networks
change because they store the output of getMarketFilterAssetSelectionKey; fix by
changing the persistence strategy: either store a stable canonical identifier
(e.g., token symbol or per-network chainId-address pairs) instead of the
composite key and re-derive the composite key at read-time, or implement a
stronger migrate function (migrate in the persist config) that loads persisted
selectedCollaterals/selectedLoanAssets, validates each entry against the current
token registry from src/utils/tokens.ts (or by using
getMarketFilterAssetSelectionKey for current networks), and drops or remaps
orphaned keys to current identifiers before returning the migrated state; update
DEFAULT_STATE handling in useMarketFilterPreferences and ensure
applySelections/setSelected* still operate on the canonical form or the
re-derived composite keys.
---
Nitpick comments:
In `@src/features/markets/market-filter-selection.ts`:
- Around line 18-46: The predicate marketFilterSelectionIncludesAsset currently
splits the composite selectionKey string on every call which is wasteful in hot
loops; refactor by adding a helper that converts a selectionKey into a Set of
sub-keys (e.g. parseMarketFilterSelectionKey or selectionKeyToSet) and update
callers (where per-market filtering runs) to compute that Set once and use
Set.has(toChainAssetKey(...)) instead of repeatedly calling
marketFilterSelectionIncludesAsset; keep getMarketFilterAssetSelectionKey and
resolveMarketFilterTokenSelectors unchanged except where they should feed or
consume the new Set-based form.
In `@src/features/markets/market-filter-url-state.ts`:
- Around line 32-34: REF_NETWORK_ALIASES currently contains only the single
mapping "etherlink" => SupportedNetworks.Etherlink which may be surprising;
update the declaration for REF_NETWORK_ALIASES (the const in
market-filter-url-state.ts) to either expand it with additional ref network
entries if expected, or add a concise inline comment explaining why it
intentionally mirrors only the "etherlink" ref (and how it differs from
NETWORK_ALIASES) so future readers know this map is intentionally narrow;
reference the const name REF_NETWORK_ALIASES and the SupportedNetworks enum in
your change.
- Around line 101-116: In parseTokenSelectorsParam, the current check using
splitSearchParamValues + rawValues.some(...CLEAR_VALUES via normalizeParamValue)
treats any mix of a clear token (e.g., "all") with actual selectors as a full
reset (returns []); add a one-line comment above that if (rawValues.length === 0
|| ...) branch documenting that "presence of any clear token among values yields
full clear/reset (returns empty array) rather than merging values" so future
readers understand why a mixed ?loan=symbol:usdc&loan=all becomes a reset;
reference splitSearchParamValues, normalizeParamValue and CLEAR_VALUES in the
comment for clarity.
In `@src/features/markets/markets-view.tsx`:
- Around line 41-43: The component is subscribing to the entire preferences
store via useMarketFilterPreferences(), causing needless re-renders when
unrelated preferences change; change the hook usage to subscribe only to the
specific pieces or actions you need (e.g., select just the preferences read by
this component or the applySelections action) or use a shallow selector hook
like useShallow so updates are scoped; update the call sites where
useMarketFilterPreferences() is used (and any place that calls applySelections
inside the useLayoutEffect) to pass a selector that returns only the required
fields or actions to avoid the redundant re-rendering loop.
In `@src/hooks/useFilteredMarkets.ts`:
- Line 25: The hook is subscribing to the entire store via
useMarketFilterPreferences(), causing unnecessary re-renders; change it to
select only the three fields you consume by replacing persistedFilters with
three selectors (e.g., call useMarketFilterPreferences((s) =>
s.selectedNetwork), useMarketFilterPreferences((s) => s.selectedCollaterals),
and useMarketFilterPreferences((s) => s.selectedLoanAssets)) where
persistedFilters is used (also apply the same selector-based change for the
other occurrences around the block that spans the area referenced as lines
153-166) so the component only re-runs when those specific values change.
In `@src/utils/networks.ts`:
- Around line 196-198: The getNetworkConfig function currently masks missing
entries with an unsafe cast; change it to perform a safe lookup and fail fast:
replace the casted find with a real check — e.g., const cfg = networks.find(n =>
n.network === chainId); if (!cfg) throw new Error(`Missing NetworkConfig for
${chainId}`); return cfg; — or alternatively convert the networks collection to
a Record<SupportedNetworkId, NetworkConfig> and return networks[chainId] (still
throwing if undefined). Update getNetworkConfig to reference the networks symbol
and remove the "as NetworkConfig" cast.
In `@src/utils/supported-networks.ts`:
- Around line 13-23: The exported ALL_SUPPORTED_NETWORKS array should be made
immutable and preserve literal types to prevent mutation and improve type
safety; update the declaration of ALL_SUPPORTED_NETWORKS (the exported constant
in supported-networks.ts) to either use a const assertion (e.g., append "as
const") or annotate it as "readonly SupportedNetworks[]" so callers cannot
mutate it and TypeScript retains literal/readonly semantics, then run type
checks to ensure any consumers expecting a mutable array are adjusted to accept
a readonly array if you chose the readonly annotation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bf710b03-757f-47e0-8364-f43eeeb1fba7
📒 Files selected for processing (16)
app/markets/page.tsxapp/page.tsxsrc/features/markets/components/filters/asset-filter.tsxsrc/features/markets/market-filter-selection.tssrc/features/markets/market-filter-url-state.test.tssrc/features/markets/market-filter-url-state.tssrc/features/markets/markets-view.tsxsrc/hooks/useFilteredMarkets.tssrc/stores/useMarketFilterPreferences.tssrc/stores/useMarketsFilters.tssrc/utils/chain-asset-key.tssrc/utils/marketFilters.tssrc/utils/networks.tssrc/utils/search-params.tssrc/utils/supported-networks.tssrc/utils/tokens.ts
| export type MarketFilterPreferencesState = { | ||
| selectedCollaterals: string[]; | ||
| selectedLoanAssets: string[]; | ||
| selectedNetwork: SupportedNetworks | null; | ||
| }; | ||
|
|
||
| type MarketFilterPreferencesActions = { | ||
| applySelections: (state: Partial<MarketFilterPreferencesState>) => void; | ||
| reset: () => void; | ||
| setSelectedCollaterals: (collaterals: string[]) => void; | ||
| setSelectedLoanAssets: (assets: string[]) => void; | ||
| setSelectedNetwork: (network: SupportedNetworks | null) => void; | ||
| }; | ||
|
|
||
| type MarketFilterPreferencesStore = MarketFilterPreferencesState & MarketFilterPreferencesActions; | ||
|
|
||
| const dedupeSelections = (values: string[]) => [...new Set(values)]; | ||
|
|
||
| const DEFAULT_STATE: MarketFilterPreferencesState = { | ||
| selectedCollaterals: [], | ||
| selectedLoanAssets: [], | ||
| selectedNetwork: null, | ||
| }; | ||
|
|
||
| export const useMarketFilterPreferences = create<MarketFilterPreferencesStore>()( | ||
| persist( | ||
| (set) => ({ | ||
| ...DEFAULT_STATE, | ||
|
|
||
| setSelectedCollaterals: (collaterals) => set({ selectedCollaterals: dedupeSelections(collaterals) }), | ||
| setSelectedLoanAssets: (assets) => set({ selectedLoanAssets: dedupeSelections(assets) }), | ||
| setSelectedNetwork: (network) => set({ selectedNetwork: network }), | ||
| applySelections: (state) => | ||
| set((currentState) => ({ | ||
| selectedNetwork: 'selectedNetwork' in state ? (state.selectedNetwork ?? null) : currentState.selectedNetwork, | ||
| selectedLoanAssets: | ||
| 'selectedLoanAssets' in state ? dedupeSelections(state.selectedLoanAssets ?? []) : currentState.selectedLoanAssets, | ||
| selectedCollaterals: | ||
| 'selectedCollaterals' in state ? dedupeSelections(state.selectedCollaterals ?? []) : currentState.selectedCollaterals, | ||
| })), | ||
| reset: () => set(DEFAULT_STATE), | ||
| }), | ||
| { | ||
| name: 'monarch_store_marketFilterPreferences', | ||
| version: 1, | ||
| migrate: (state) => { | ||
| if (!state || typeof state !== 'object') { | ||
| return DEFAULT_STATE; | ||
| } | ||
| return { ...DEFAULT_STATE, ...(state as Partial<MarketFilterPreferencesState>) }; | ||
| }, | ||
| }, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Persisted selection keys can drift when a token's network list changes.
selectedCollaterals / selectedLoanAssets store the composite key produced by getMarketFilterAssetSelectionKey, which joins every network in token.networks with |. If a token in src/utils/tokens.ts later gains or loses a network entry, the stored key will no longer match any current asset and the user's persisted selection silently becomes inert.
Worth considering one of:
- Persist a stable canonical identifier (e.g., symbol or a per-network
chainId-addressset) and re-derive the composite key on read. - Add a
migratestep that revalidates stored keys against the current token registry and drops orphans.
Otherwise the current version: 1 migrate just merges shape and won't catch this drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/useMarketFilterPreferences.ts` around lines 5 - 58, Persisted
composite keys in useMarketFilterPreferences
(selectedCollaterals/selectedLoanAssets) can become stale when token.networks
change because they store the output of getMarketFilterAssetSelectionKey; fix by
changing the persistence strategy: either store a stable canonical identifier
(e.g., token symbol or per-network chainId-address pairs) instead of the
composite key and re-derive the composite key at read-time, or implement a
stronger migrate function (migrate in the persist config) that loads persisted
selectedCollaterals/selectedLoanAssets, validates each entry against the current
token registry from src/utils/tokens.ts (or by using
getMarketFilterAssetSelectionKey for current networks), and drops or remaps
orphaned keys to current identifiers before returning the migrated state; update
DEFAULT_STATE handling in useMarketFilterPreferences and ensure
applySelections/setSelected* still operate on the canonical form or the
re-derived composite keys.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/markets/markets-view.tsx (1)
34-34: Subscribing to the whole persisted store re-runs the URL-sync effect on every filter change.
useMarketFilterPreferences()without a selector returns the full state, sopersistedFiltersgets a new reference whenever any persisted field changes. That makes theuseLayoutEffectdep array re-fire on every selection update. TheappliedUrlSignatureRefguard prevents re-application, but it's wasted work and easy to misread.Two small fixes worth considering:
- Use selectors so the component subscribes only to what it reads (e.g.
useMarketFilterPreferences((s) => s.selectedNetwork)and the same for setters/applySelections/reset).- In the layout effect, read the store imperatively via
useMarketFilterPreferences.getState()instead of closing overpersistedFilters, and drop it from the dep array.Same store-shape concern applies to
handleClearAll(line 181) — itsuseCallbackdeps churn for the same reason.Also applies to: 127-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/markets/markets-view.tsx` at line 34, The component is subscribing to the entire persisted store via useMarketFilterPreferences() which produces a new reference on any store change and causes the useLayoutEffect and callbacks like handleClearAll to re-run unnecessarily; fix it by replacing broad subscriptions with fine-grained selectors (e.g. useMarketFilterPreferences(s => s.selectedNetwork), etc.) for every value and setter the component reads (including applySelections and reset), or alternatively inside the useLayoutEffect read the latest values imperatively using useMarketFilterPreferences.getState() and remove persistedFilters from the effect deps, and similarly tighten handleClearAll's useCallback deps to only the actual selectors/setters used so the callbacks no longer churn on unrelated store updates (refer to useMarketFilterPreferences, persistedFilters, useLayoutEffect, appliedUrlSignatureRef, handleClearAll, applySelections, reset, and getState).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/features/markets/markets-view.tsx`:
- Line 34: The component is subscribing to the entire persisted store via
useMarketFilterPreferences() which produces a new reference on any store change
and causes the useLayoutEffect and callbacks like handleClearAll to re-run
unnecessarily; fix it by replacing broad subscriptions with fine-grained
selectors (e.g. useMarketFilterPreferences(s => s.selectedNetwork), etc.) for
every value and setter the component reads (including applySelections and
reset), or alternatively inside the useLayoutEffect read the latest values
imperatively using useMarketFilterPreferences.getState() and remove
persistedFilters from the effect deps, and similarly tighten handleClearAll's
useCallback deps to only the actual selectors/setters used so the callbacks no
longer churn on unrelated store updates (refer to useMarketFilterPreferences,
persistedFilters, useLayoutEffect, appliedUrlSignatureRef, handleClearAll,
applySelections, reset, and getState).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 78f52424-1ae0-44b6-beed-a70b948db831
📒 Files selected for processing (2)
src/features/markets/market-filter-url-state.tssrc/features/markets/markets-view.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/markets/market-filter-url-state.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
Release Notes