Skip to content

Conversation

@gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 31, 2025

Description

fixes trade/limit routing borked on refresh

How to test

  • play around with spot swapper i.e refresh, change pairs, refresh, change tab to limit
  • play around with limit swapper i.e refresh, change pairs, refresh, change tab to spot

Screenshots

https://jam.dev/c/423c869f-2785-47b4-a6e8-6cf866e07a17

Summary by CodeRabbit

  • Bug Fixes

    • Resolved crashes that could occur when using URL parameters with limit orders before assets loaded.
    • Improved default asset initialization to properly handle cached and persisted data.
  • Refactor

    • Optimized initialization timing and readiness checks for asset-dependent features.

✏️ Tip: You can customize this high-level summary in your review settings.

PR #11520 introduced async asset loading which caused trade and limit order
URL routing to break on page refresh. 

Issue being investigated: LimitOrderInput crashes with 'invalid AssetId' even
on fresh /limit visits because limitOrderInput defaults are not being set.

Changes so far:
1. AppContext detects trade/limit routes with URL params and skips defaults
2. LimitOrder guards rendering until URL params are initialized

Need to investigate: Why /limit (no params) still crashes
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Walkthrough

The PR adds URL parameter readiness checks to prevent crashes in trade and limit order flows. It defers default asset initialization until assets are loaded, introduces location-based route detection, and uses refs to track one-time setup operations.

Changes

Cohort / File(s) Change Summary
URL Param Guard in Limit Order
src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
Introduces hasUrlParams computation from chainId and assetSubId, adds guard to prevent rendering when URL params are present but assets haven't initialized yet. Guard placed in two locations to prevent crashes from using defaultAsset with empty assetId.
Deferred Default Asset Initialization
src/context/AppProvider/AppContext.tsx
Refactors asset defaults initialization from immediate to deferred using refs (hasSetTradeDefaults, hasSetLimitDefaults). Adds location-based route detection (useLocation, matchPath) to determine trade/limit route URL parameters and compute hasTradeRouteParams. Moves default asset wiring (tradeInput, tradeRampInput, limitOrderInput) to separate effect. Adds new selectors (selectTradeInputBuyAsset, selectLimitOrderInputBuyAsset). Updates rendering readiness condition to account for trade/limit route params and input initialization.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • shapeshift/web#10934: Centralizes trade route URL parameter generation via useTradeNavigation hook, complementing this PR's param detection and guarded initialization logic.

Suggested reviewers

  • NeOMakinG
  • 0xApotheosis
  • premiumjibles

Poem

🐰 The hopping refactor hops with care,
Deferred defaults float through the air,
URL guards stand watch and wait,
Till assets load, before it's too late,
With refs and effects, all set to thrive,
Our trades stay safe and alive!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: trade routing' is directly related to the main objective of fixing trade/limit routing that broke on refresh, matching the PR's core purpose.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch push-snvonrzoqsuo

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3ec9533 and 0d0840b.

📒 Files selected for processing (2)
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
  • src/context/AppProvider/AppContext.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Never assume a library is available - always check imports/package.json first
Prefer composition over inheritance
Write self-documenting code with clear variable and function names
Keep functions small and focused on a single responsibility
Avoid deep nesting - use early returns instead
Prefer procedural and easy to understand code
Never expose, log, or commit secrets, API keys, or credentials
Validate all inputs, especially user inputs
Handle errors gracefully with meaningful messages
Don't silently catch and ignore exceptions
Log errors appropriately for debugging
Provide fallback behavior when possible
Use appropriate data structures for the task
Never add code comments unless explicitly requested
When modifying code, do not add comments that reference previous implementations or explain what changed. Comments should only describe the current logic and functionality.
Use meaningful names for branches, variables, and functions
Always run yarn lint --fix and yarn type-check after making changes
Avoid let variable assignments - prefer const with inline IIFE switch statements or extract to functions for conditional logic

Files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid useEffect where practical - use it only when necessary and following best practices
Avoid 'any' types - use specific type annotations instead
For default values with user overrides, use computed values (useMemo) instead of useEffect - pattern: userSelected ?? smartDefault ?? fallback
When function parameters are unused due to interface requirements, refactor the interface or implementation to remove them rather than prefixing with underscore
Sanitize data before displaying to prevent XSS
Memoize aggressively - wrap component variables in useMemo and callbacks in useCallback where possible
For static JSX icon elements (e.g., <TbCopy />) that don't depend on state/props, define them as constants outside the component to avoid re-renders instead of using useMemo
Account for light/dark mode using useColorModeValue hook
Account for responsive mobile designs in all UI components
When applying styles, use the existing standards and conventions of the codebase
Use Chakra UI components and conventions
All copy/text must use translation keys - never hardcode strings
Use the translation hook: useTranslate() from react-polyglot
Use useFeatureFlag('FlagName') hook to access feature flag values in components
Prefer type over interface for type definitions
Use strict typing - avoid any
Use Nominal types for domain identifiers (e.g., WalletId, AccountId)
Import types from @shapeshiftoss/caip for chain/account/asset IDs
Use useAppSelector for Redux state
Use useAppDispatch for Redux actions
Memoize expensive computations with useMemo
Memoize callbacks with useCallback

**/*.{ts,tsx}: Use Result<T, E> pattern for error handling in swappers and APIs; ALWAYS use Ok() and Err() from @sniptt/monads; AVOID throwing within swapper API implementations
ALWAYS use custom error classes from @shapeshiftoss/errors with meaningful error codes for internationalization and relevant details in error objects
ALWAYS wrap async op...

Files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
**/*.{tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

**/*.{tsx,jsx}: ALWAYS wrap React components in error boundaries and provide user-friendly fallback components with error logging
ALWAYS use useErrorToast hook for displaying errors with translated error messages and handle different error types appropriately

Use PascalCase for React component names and match the component name to the file name

Files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)

**/*.{js,jsx,ts,tsx}: Use camelCase for variables, functions, and methods with descriptive names that explain the purpose
Use verb prefixes for functions that perform actions (e.g., fetch, validate, execute, update, calculate)
Use UPPER_SNAKE_CASE for constants and configuration values with descriptive names
Use handle prefix for event handlers with descriptive names in camelCase
Use descriptive boolean variable names with is, has, can, should prefixes
Use named exports for components, functions, and utilities instead of default exports
Use descriptive import names and avoid renaming imports unless necessary
Avoid non-descriptive variable names like data, item, obj, and single-letter variable names except in loops
Avoid abbreviations in names unless they are widely understood
Avoid generic function names like fn, func, or callback

Files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

**/*.{jsx,tsx}: ALWAYS use useMemo for expensive computations, object/array creations, and filtered data
ALWAYS use useMemo for derived values and computed properties
ALWAYS use useMemo for conditional values and simple transformations
ALWAYS use useCallback for event handlers and functions passed as props
ALWAYS use useCallback for any function that could be passed as a prop or dependency
ALWAYS include all dependencies in useEffect, useMemo, useCallback dependency arrays
NEVER use // eslint-disable-next-line react-hooks/exhaustive-deps unless absolutely necessary, and 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
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly in async operations
ALWAYS provide user-friendly error messages in error handling
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components using React.lazy for code splitting
ALWAYS use Suspense wrapper for lazy loaded components
USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places
Wrap components receiving props with memo for performance optimization

Files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)

Ensure TypeScript types are explicit and proper; avoid use of any type

Files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
🧠 Learnings (29)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11170
File: patches/@shapeshiftoss+bitcoinjs-lib+7.0.0-shapeshift.0.patch:9-19
Timestamp: 2025-11-25T21:43:10.838Z
Learning: In shapeshift/web, gomesalexandre will not expand PR scope to fix latent bugs in unused API surface (like bitcoinjs-lib patch validation methods) when comprehensive testing proves the actual used code paths work correctly, preferring to avoid costly hdwallet/web verdaccio publish cycles and full regression testing for conceptual issues with zero runtime impact.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11536
File: src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx:252-265
Timestamp: 2025-12-27T16:02:59.817Z
Learning: When fixing critical bugs in shapeshift/web, gomesalexandre prefers to keep changes minimal and focused on correctness rather than combining bug fixes with code quality improvements like extracting duplicated logic, even when duplication is present.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10490
File: src/pages/Trade/tabs/LimitTab.tsx:0-0
Timestamp: 2025-09-16T09:36:40.347Z
Learning: gomesalexandre declined to add error boundaries to the LimitTab route element in PR #10490, stating "not concerned about error boundaries as part of this PR", consistent with his preference to keep PR scope focused on the main navigation/UI changes rather than expanding into error handling infrastructure.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/EIP712MessageDisplay.tsx:21-24
Timestamp: 2025-09-12T13:16:27.004Z
Learning: gomesalexandre declined to add error boundaries to WalletConnect modals in PR #10461, stating "no error boundaries in this pr ser", consistent with his preference to keep PR scope focused and defer tangential improvements to separate efforts.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10232
File: packages/unchained-client/openapitools.json:61-61
Timestamp: 2025-08-08T10:23:16.843Z
Learning: In shapeshift/web, for temporary “monkey patch” PRs (e.g., packages/unchained-client/openapitools.json using jsDelivr CDN refs like cosmos/mayachain), gomesalexandre is fine with branch-based URLs and does not want SHA pinning. Treat this as a scoped exception to their general preference for pinned dependencies/refs.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10231
File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14
Timestamp: 2025-08-08T15:00:22.321Z
Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/ContractInteractionBreakdown.tsx:0-0
Timestamp: 2025-09-13T16:45:18.813Z
Learning: gomesalexandre prefers aggressively deleting unused/obsolete code files ("ramboing") rather than fixing technical issues in code that won't be used, demonstrating his preference for keeping codebases clean and PR scope focused.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/types.ts:7-7
Timestamp: 2025-09-10T15:34:29.604Z
Learning: gomesalexandre is comfortable relying on transitive dependencies (like abitype through ethers/viem) rather than explicitly declaring them in package.json, preferring to avoid package.json bloat when the transitive dependency approach works reliably in practice.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10503
File: .env:56-56
Timestamp: 2025-09-16T13:17:02.938Z
Learning: gomesalexandre prefers to enable feature flags globally in the base .env file when the intent is to activate features everywhere, even when there are known issues like crashes, demonstrating his preference for intentional global feature rollouts over cautious per-environment enablement.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10783
File: src/context/ModalStackProvider/useModalRegistration.ts:30-41
Timestamp: 2025-10-16T11:14:40.657Z
Learning: gomesalexandre prefers to add lint rules (like typescript-eslint/strict-boolean-expressions for truthiness checks on numbers) to catch common issues project-wide rather than relying on code review to catch them.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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.
📚 Learning: 2025-08-08T11:41:36.971Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : USE local state for component-level state; LIFT state up when needed across multiple components; USE Context for avoiding prop drilling; USE Redux only for global state shared across multiple places

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to **/*.{ts,tsx} : Use `useAppSelector` for Redux state

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 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/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-08-22T13:03:36.919Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/Explore/Explore.tsx:110-113
Timestamp: 2025-08-22T13:03:36.919Z
Learning: In src/Routes/RoutesCommon.tsx, the /fox-ecosystem route is already properly gated using disable: !getConfig().VITE_FEATURE_RFOX_FOX_ECOSYSTEM_PAGE, which prevents direct navigation when the VITE_FEATURE_RFOX_FOX_ECOSYSTEM_PAGE flag is false.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 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/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-08-22T13:02:58.824Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/hooks/useRfoxRewardDistributionActionSubscriber.tsx:33-41
Timestamp: 2025-08-22T13:02:58.824Z
Learning: In src/pages/RFOX/hooks/useRfoxRewardDistributionActionSubscriber.tsx, NeOMakinG declined optimizing useMemo dependencies to depend on lifetimeRewardDistributionsQuery.data instead of the entire query object, indicating indifference toward this type of performance optimization.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-19T16:59:50.569Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11012
File: src/context/WalletProvider/Vultisig/components/Connect.tsx:24-59
Timestamp: 2025-11-19T16:59:50.569Z
Learning: In src/context/WalletProvider/*/components/Connect.tsx files across the ShapeShift web codebase, the established pattern for handling null/undefined adapter from getAdapter() is to simply check `if (adapter) { ... }` without an else clause. All wallet Connect components (Coinbase, Keplr, Phantom, Ledger, MetaMask, WalletConnectV2, KeepKey, Vultisig) follow this pattern—they reset loading state after the if block but do not show error messages when adapter is null. This is an intentional design decision and should be maintained for consistency.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-07-29T10:27:23.424Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/react-best-practices.mdc:8-14
Timestamp: 2025-07-29T10:27:23.424Z
Learning: The ShapeShift team practices aggressive memoization in React components as documented in .cursor/rules/react-best-practices.mdc. They use useMemo for all transformations, derived values, and conditional values, and useCallback for all event handlers and functions that could be passed as props. This approach was adopted after experiencing performance issues ("had hard time") and is their current established practice, though they acknowledge it may evolve in the future.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Implement filterBuyAssetsBySellAssetId method to filter assets by supported chain IDs in the buy property

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:57.909Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-11-24T21:20:57.909Z
Learning: Applies to packages/swapper/src/swappers/*/*.ts : Implement filterAssetIdsBySellable method to filter assets by supported chain IDs in the sell property

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-09-04T10:18:34.140Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10427
File: src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx:40-40
Timestamp: 2025-09-04T10:18:34.140Z
Learning: In the shapeshift/web codebase, src/state/slices/selectors.ts uses wildcard exports (`export * from './[sliceName]/selectors'`) to re-export all selectors from individual slice selector files, making them available through the barrel import. This means selectors like selectTxByFilter from txHistorySlice/selectors are properly accessible via '@/state/slices/selectors' even though they don't appear in explicit named exports.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:04.979Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:20:04.979Z
Learning: Applies to src/state/slices/**/*.ts : Export selectors from slice using inline `selectors` property

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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/context/AppProvider/AppContext.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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/context/AppProvider/AppContext.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 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/context/AppProvider/AppContext.tsx
📚 Learning: 2025-08-11T09:46:41.060Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:167-172
Timestamp: 2025-08-11T09:46:41.060Z
Learning: In the shapeshift/web repository, the display cache logic for trade quotes (using `selectUserAvailableTradeQuotes` and `selectUserUnavailableTradeQuotes`) is intentionally kept the same between `TradeInput.tsx` and `TradeQuotes.tsx` components. The `hasQuotes` computation in `TradeInput.tsx` uses these display cache selectors by design, matching the pattern used in `TradeQuotes.tsx`.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-09-02T00:02:44.889Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10380
File: src/components/AssetSearch/components/GroupedAssetRow.tsx:116-0
Timestamp: 2025-09-02T00:02:44.889Z
Learning: premiumjibles prefers avoiding store.getState() calls in favor of useAppSelector where reactivity is needed, to prevent accidentally breaking reactive updates in components.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-11-24T21:20:44.637Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-11-24T21:20:44.637Z
Learning: Applies to **/*.{jsx,tsx} : ALWAYS use Suspense wrapper for lazy loaded components

Applied to files:

  • src/context/AppProvider/AppContext.tsx
📚 Learning: 2025-12-27T16:02:52.792Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 11536
File: src/components/MultiHopTrade/components/TradeConfirm/hooks/useTradeExecution.tsx:252-265
Timestamp: 2025-12-27T16:02:52.792Z
Learning: When reviewing bug fixes, especially in shapeshift/web, prefer minimal changes that fix correctness over introducing broader refactors or quality-of-life improvements (e.g., extracting duplicated logic) unless such improvements are essential to the fix. Apply this guideline broadly to TSX files and related components, not just the specific location, to keep changes focused and maintainable.

Applied to files:

  • src/context/AppProvider/AppContext.tsx
  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.

Applied to files:

  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-08-22T12:58:26.590Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx:108-111
Timestamp: 2025-08-22T12:58:26.590Z
Learning: In the RFOX GenericTransactionDisplayType flow in src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx, the txHash is always guaranteed to be present according to NeOMakinG, so defensive null checks for txLink are not needed in this context.

Applied to files:

  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
📚 Learning: 2025-08-22T12:58:36.070Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx:33-55
Timestamp: 2025-08-22T12:58:36.070Z
Learning: In RewardDistributionNotification component (src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx), NeOMakinG confirmed that runeAsset is expected to always be defined when the component renders, so defensive guards against undefined runeAsset are not needed.

Applied to files:

  • src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx
🧬 Code graph analysis (1)
src/context/AppProvider/AppContext.tsx (5)
src/Routes/RoutesCommon.tsx (2)
  • TRADE_ROUTE_ASSET_SPECIFIC (36-37)
  • LIMIT_ORDER_ROUTE_ASSET_SPECIFIC (38-39)
src/lib/asset-service/service/AssetService.ts (3)
  • assetsById (39-41)
  • assets (48-50)
  • assetIds (45-47)
src/state/slices/assetsSlice/assetsSlice.ts (1)
  • assets (42-74)
packages/caip/src/constants.ts (3)
  • btcAssetId (4-4)
  • ethAssetId (10-10)
  • foxAssetId (39-39)
packages/swapper/src/swappers/ChainflipSwapper/constants.ts (1)
  • usdcAsset (51-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Call / Static
🔇 Additional comments (6)
src/components/MultiHopTrade/components/LimitOrder/LimitOrder.tsx (2)

90-92: LGTM on the URL params detection.

The hasUrlParams computation correctly identifies when URL parameters exist by checking both chainId and assetSubId, aligning with the route pattern in LIMIT_ORDER_ROUTE_ASSET_SPECIFIC.


207-211: Guard logic looks correct, but initialization may occur with missing assets.

The guard prevents rendering when URL params exist but initialization hasn't completed, which addresses the crash described in the PR. However, setIsInitialized(true) (line 131) executes unconditionally regardless of whether routeBuyAsset and routeSellAsset were successfully resolved.

If the URL contains invalid asset IDs, the component will pass this guard (since isInitialized becomes true) but may still render with uninitialized Redux state.

Is this intentional behavior? If invalid URL params should fall back to defaults rather than rendering with potentially missing assets, consider gating setIsInitialized(true) on asset availability:

-    setIsInitialized(true)
+    // Only mark as initialized if we have valid assets from URL or can fall back to defaults
+    if (routeBuyAsset || routeSellAsset || (!chainId && !assetSubId)) {
+      setIsInitialized(true)
+    }
src/context/AppProvider/AppContext.tsx (4)

79-90: Route detection logic is sound.

The hasTradeRouteParams computation correctly identifies trade/limit routes with URL parameters using matchPath. The early chainId check on line 84 provides a quick exit before the more expensive limit route match.


141-179: Ref-based one-time defaults pattern aligns with codebase conventions.

The use of refs (hasSetTradeDefaults, hasSetLimitDefaults) to track one-time setup is consistent with patterns used elsewhere in the codebase (per retrieved learnings about ref-based single-mount dedupe).

One observation: based on retrieved learnings, selectInputBuyAsset from tradeInputSlice always returns defined values because they have default values in initial state (BTC for buyAsset, ETH for sellAsset). However, these defaults use defaultAsset which has an empty assetId: ''.

The condition !tradeInputBuyAsset.assetId (line 162) relies on this empty string being falsy, which correctly identifies uninitialized state. This is a subtle but correct implementation detail.


117-119: Documentation comment is helpful for future maintainers.

The comment clearly explains why trade input defaults were moved out of queryFn and into a separate effect, addressing the React Query caching edge case.


355-359: Non-trade routes wait unnecessarily for trade input initialization, potentially causing a blank render on app load.

The readiness condition requires both tradeInputBuyAsset.assetId and limitOrderInputBuyAsset.assetId to be populated on all routes except those with trade-specific URL parameters. However, routes like /wallet, /accounts, /explore, /markets, etc. don't use trade functionality and shouldn't block on this initialization.

Consider detecting whether the current route is a trade route (e.g., /trade/* or /limit/*) and only requiring trade input initialization for those routes, allowing other routes to render immediately after assets load.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gomesalexandre gomesalexandre marked this pull request as ready for review December 31, 2025 13:32
@gomesalexandre gomesalexandre requested a review from a team as a code owner December 31, 2025 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants