feat: react compiler PoC #11006
Merged
feat: react compiler PoC #11006
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughSwap Vite React SWC plugin for Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Repo as Repository
participant Build as Vite/Babel
participant Lint as ESLint
participant App as Application
rect rgb(214,238,247)
Dev->>Repo: Update `package.json` & `vite.config.mts` to use `@vitejs/plugin-react` v5 and add `babel-plugin-react-compiler`
end
rect rgb(247,240,214)
Dev->>Repo: Remove `eslint-plugin-react-memo` and delete many `eslint-disable` comments
Dev->>Repo: Insert selective `'use no memo'` markers in components/hooks
end
rect rgb(232,247,232)
Build->>App: Babel/react-compiler runs during build (may alter component transforms)
Lint->>Repo: ESLint now enforces react-memo rules (previous suppressions removed)
App->>Build: Components with `'use no memo'` act as opt-outs for compiler transforms
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
gomesalexandre
commented
Nov 6, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit 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.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
package.json(2 hunks)src/components/Graph/PrimaryChart/PrimaryChart.tsx(4 hunks)src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx(11 hunks)src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsx(2 hunks)vite.config.mts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
**/*.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/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsxvite.config.mtspackage.json
**/*.{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/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.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/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
🧠 Learnings (42)
📓 Common learnings
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.
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: 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: 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: 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-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : Components receiving props are wrapped with `memo`
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsxvite.config.mts
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for conditional values and simple transformations
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for derived values and computed properties
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use `useMemo` for expensive computations, object/array creations, and filtered data
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.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/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.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/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-08-06T09:46:50.860Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/components/CategoryCard.tsx:27-196
Timestamp: 2025-08-06T09:46:50.860Z
Learning: In src/pages/Explore/components/CategoryCard.tsx, NeOMakinG indicated that wrapping the CategoryCard component with React.memo would be "quite useless in this case", suggesting that memoization is not beneficial for this particular component despite it receiving props, likely due to its specific usage patterns or context.
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : NEVER use `// eslint-disable-next-line react-hooks/exhaustive-deps` unless absolutely necessary
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.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/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsx
📚 Learning: 2025-09-16T09:32:21.333Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10490
File: src/components/Layout/Header/NavBar/NavigationDropdown.tsx:96-103
Timestamp: 2025-09-16T09:32:21.333Z
Learning: In the shapeshift/web codebase, gomesalexandre confirms that using `item.icon && <Icon as={item.icon} boxSize={4} />` for Chakra UI MenuItem.icon prop is valid and acceptable - the boolean short-circuit pattern works fine with Chakra components and doesn't need to be changed to explicit undefined.
Applied to files:
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.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/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsxsrc/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.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/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.tsxsrc/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-21T22:15:25.918Z
Learnt from: kaladinlight
Repo: shapeshift/web PR: 10326
File: src/hooks/useActionCenterSubscribers/useFetchBasePortfolio.ts:31-59
Timestamp: 2025-08-21T22:15:25.918Z
Learning: In the ShapeShift web codebase, the maintainer prefers inferred return types for React hooks over explicit return types, particularly for complex hooks that can have intricate return signatures.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-08-22T13:00:23.679Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/Fox/FoxEcosystemPage.tsx:28-44
Timestamp: 2025-08-22T13:00:23.679Z
Learning: In the shapeshift/web codebase, explicit React component type annotations (like React.FC) are not required or preferred. The team doesn't prioritize adding these type annotations to functional components.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.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/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-11-03T22:31:30.786Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10985
File: packages/swapper/src/swappers/PortalsSwapper/getPortalsTradeQuote/getPortalsTradeQuote.ts:0-0
Timestamp: 2025-11-03T22:31:30.786Z
Learning: In packages/swapper/src/swappers/PortalsSwapper, the rate and quote files intentionally use different approaches for calculating buyAmountBeforeSlippageCryptoBaseUnit: getPortalsTradeRate.tsx uses minOutputAmount / (1 - buffer) for conservative estimates, while getPortalsTradeQuote.ts uses outputAmount / (1 - buffer) for final quote display. This difference is validated by on-chain simulation testing and is intentional.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-08-19T22:09:40.304Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10278
File: src/components/AssetHeader/QuickBuy.tsx:109-116
Timestamp: 2025-08-19T22:09:40.304Z
Learning: In React components with dynamic arrays of interactive elements (like quick buy buttons), inline event handlers with eslint-disable can be more pragmatic than memoized factories that still create new functions on each render. The performance impact is often negligible and the inline approach is clearer about what's happening.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
Repo: shapeshift/web PR: 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/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns across WalletConnect modal components, including side-effects-during-render for error handling (showErrorToast + handleReject calls before return null), rather than introducing isolated refactors that would create inconsistency in the codebase.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-09-12T12:00:33.924Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10461
File: src/plugins/walletConnectToDapps/components/modals/SendTransactionConfirmation.tsx:42-50
Timestamp: 2025-09-12T12:00:33.924Z
Learning: gomesalexandre prefers maintaining consistency with existing code patterns in WalletConnect modals, including side-effects-during-render for error handling (showErrorToast + handleReject), rather than introducing isolated refactors that would make the codebase inconsistent.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-08-29T07:07:49.332Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-10-01T07:42:40.195Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10596
File: src/components/Layout/Header/NavBar/WalletConnectedMenu.tsx:77-99
Timestamp: 2025-10-01T07:42:40.195Z
Learning: In WalletConnectedMenu.tsx's handleReconnectWallet handler, gomesalexandre prefers throwing an error for unsupported wallet types in the default case rather than gracefully handling with a fallback. His reasoning: "if we have a problem here, we have bigger problems" - only supported wallets (KeepKey, Ledger, MetaMask, Coinbase, Phantom) should reach the reconnect flow when disconnected/locked, so encountering an unsupported type indicates a larger architectural issue that should be surfaced explicitly rather than masked with graceful degradation.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/swapper/**/*.{ts,tsx} : ALWAYS use TradeQuoteError enum for error codes in swapper errors
Applied to files:
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
📚 Learning: 2025-08-06T09:47:29.865Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:174-178
Timestamp: 2025-08-06T09:47:29.865Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG prefers keeping complex height calculations inline rather than extracting them to useMemo, prioritizing perceived performance/execution speed over code readability, even when the calculation would only run when the dependency (isSearching) changes.
Applied to files:
src/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-06T11:44:30.697Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:81-83
Timestamp: 2025-08-06T11:44:30.697Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG questioned the suggestion to memoize a simple static carouselOptions object ({ loop: true }), indicating that static objects that never change should remain as module-level constants rather than being memoized inside components, as memoization would add unnecessary overhead for truly static values.
Applied to files:
src/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-06T11:02:09.501Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:54-72
Timestamp: 2025-08-06T11:02:09.501Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG indicated that memoizing the ExploreCard component would be unnecessary, questioning "why would we memoize that", suggesting that like CategoryCard, this component doesn't benefit from memoization despite receiving props, likely due to its specific usage patterns in the carousel with static content.
Applied to files:
src/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-10-17T07:51:58.374Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10783
File: src/context/WalletProvider/NewWalletViews/NewWalletViewsSwitch.tsx:344-349
Timestamp: 2025-10-17T07:51:58.374Z
Learning: In the shapeshift/web codebase, Chakra UI's ModalContent component supports the containerProps prop. When using the useModalRegistration hook from ModalStackProvider, spreading {...modalContentProps} directly onto ModalContent is correct and properly applies z-index and pointer-events through containerProps.sx. Do not suggest extracting sx from modalContentProps.containerProps.sx.
Applied to files:
src/components/Graph/PrimaryChart/PrimaryChart.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS use React.lazy for code splitting
Applied to files:
vite.config.mts
📚 Learning: 2025-08-03T22:10:11.424Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/naming-conventions.mdc:0-0
Timestamp: 2025-08-03T22:10:11.424Z
Learning: Applies to **/*.tsx : Flag default exports for components
Applied to files:
vite.config.mts
📚 Learning: 2025-08-08T20:16:12.898Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10222
File: package.json:202-202
Timestamp: 2025-08-08T20:16:12.898Z
Learning: In shapeshift/web, the semver package must be included in dependencies (not devDependencies) because hdwallet packages have transient dependencies that require semver but don't ship it themselves. This ensures semver is available at runtime for hdwallet functionality.
Applied to files:
package.json
🧬 Code graph analysis (2)
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx (11)
src/components/MultiHopTrade/components/TradeInput/components/TradeSettingsMenu.tsx (1)
TradeSettingsMenu(14-24)src/state/slices/tradeInputSlice/tradeInputSlice.ts (1)
tradeInput(35-53)packages/swapper/src/swappers/ArbitrumBridgeSwapper/getTradeQuote/getTradeQuote.ts (1)
isArbitrumBridgeTradeQuoteOrRate(24-26)packages/caip/src/assetId/assetId.ts (2)
AssetId(17-17)fromAssetId(143-178)src/components/AssetSelection/AssetSelection.tsx (1)
TradeAssetSelect(40-129)src/components/MultiHopTrade/components/SharedTradeInput/SharedTradeInputBody.tsx (1)
SharedTradeInputBody(38-156)src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx (1)
HighlightedTokens(38-330)src/components/MultiHopTrade/components/TradeAssetInput.tsx (1)
TradeAssetInput(78-86)src/components/Acknowledgement/WarningAcknowledgement.tsx (1)
WarningAcknowledgement(4-5)src/components/MultiHopTrade/components/SharedTradeInput/SharedTradeInput.tsx (1)
SharedTradeInput(43-105)src/components/MultiHopTrade/components/TradeInput/components/CollapsibleQuoteList.tsx (1)
CollapsibleQuoteList(20-53)
src/components/Graph/PrimaryChart/PrimaryChart.tsx (3)
packages/types/src/market.ts (1)
HistoryData(21-24)src/components/Amount/Amount.tsx (1)
Amount(21-42)src/components/Text/Text.tsx (1)
RawText(15-17)
⏰ 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: Install and Cache
🔇 Additional comments (7)
vite.config.mts (1)
82-93: LGTM! React Compiler configuration is correct.The switch from
@vitejs/plugin-react-swcto@vitejs/plugin-reactwithbabel-plugin-react-compilerenables the React Compiler in annotation mode. This allows incremental adoption via the'use memo'directive.src/components/Graph/PrimaryChart/PrimaryChart.tsx (2)
127-150: Approve systematic memoization removal.The conversion from
useCallbackto a regular function forrenderTooltipaligns with the React Compiler PoC approach. The compiler should automatically memoize this function to prevent unnecessary re-renders of the Tooltip component.
93-125: I need to gather more information about the React Compiler setup and the full context of the PrimaryChart component.Now let me check the project configuration for React Compiler setup:
I need to verify the actual project configuration and component code:
Based on my research, I can now provide a comprehensive analysis. The critical issue is that React Compiler is designed to work by default without any configuration, however if targeting React versions below 19, it requires special configuration.
The presence of
'use memo'on line 80 suggests the developer may be preparing for React Compiler support, but this directive only has meaning if compilationMode is set to 'annotation', which only compiles "use memo" functions.Regarding the performance concerns: d3.extent returns min and max in a single pass over the input, and extent is not just a wrapper around min and max. However, calling extent is more efficient than calling both min and max separately. The code is indeed calling both (
minPrice/maxPricecomputed separately, then used in domain), which is redundant.Without access to the repository files to verify whether
babel-plugin-react-compileris installed and properly configured inbabel.config.jsorconfig-overrides.js, I cannot definitively confirm that React Compiler is active in this project.
React Compiler not configured; redundant d3 computations need memoization.
The
'use memo'directive alone cannot optimize these values without React Compiler being installed and configured. Babel plugins must be defined in either a babel section in package.json or in a .babelrc file, and react-app-rewired projects use these files for customization. Without explicit verification thatbabel-plugin-react-compileris installed and configured, the removeduseMemocalls will causeextent(),min(),max(), andscaleLinear()to re-execute on every render.Additionally, the code redundantly calculates
minPrice/maxPriceviaMath.min(...data.map())andMath.max(...data.map()), then separately callsmin()andmax()again for the domain—this should be refactored to useextent()once.src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx (2)
170-177: Verify React Compiler handles complex boolean expressions.The
isLoadingcalculation is now a direct boolean expression instead ofuseMemo. While simpler, this expression evaluates multiple selector results and boolean conditions on every render.Confirm the React Compiler successfully memoizes this computed boolean to avoid unnecessary re-evaluations.
247-250: Approve removal of manual memoization for handlers.The conversion of
setBuyAsset,setSellAsset,handleSwitchAssets, and other handlers fromuseCallbackto inline functions aligns with the React Compiler PoC. The compiler should automatically stabilize these function references.Also applies to: 326-332, 337-350, 352-367
src/pages/ThorChainLP/components/AddLiquidity/AddLiquidityInput.tsx (2)
1-1: Inconsistent React Compiler adoption strategy confirmed—clarify intent and align implementation.Your observation is accurate. The codebase shows three distinct adoption patterns:
- AddLiquidityInput.tsx: Conservative approach—retains top-level
eslint-disableplus alluseMemo/useCallbackpatterns intact despite'use memo'directive- PrimaryChart.tsx: Moderate approach—has
'use memo'directive and top-leveleslint-disablebut removed all manual memoizations- TradeInput.tsx: Progressive approach—has
'use memo'directive, removed manual memoizations, and replaced with inline eslint-disable comments explaining "React Compiler now handles memoization automagically"Given the project's established practice of aggressive memoization (documented in learnings and
.cursor/rules/react-best-practices.mdc), AddLiquidityInput's conservative stance conflicts with the trajectory of TradeInput.tsx. Either remove the manual memoizations here to match TradeInput's approach, or clarify whether AddLiquidityInput requires explicit memoization for performance-critical reasons (performance profiling should validate this).
164-164: The React Compiler directive is correctly implemented.The
'use memo'directive is placed as the first statement in the function body with correct syntax (single quotes), exactly matching the React Compiler documentation requirements. No changes needed.
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
Outdated
Show resolved
Hide resolved
gomesalexandre
commented
Nov 6, 2025
| "@types/ssri": "^7.1.1", | ||
| "@types/uuid": "^9.0.5", | ||
| "@vitejs/plugin-react": "^4.3.4", | ||
| "@vitejs/plugin-react-swc": "^3.8.0", |
Contributor
Author
There was a problem hiding this comment.
Not compatible with compiler just yet, see https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react-swc/CHANGELOG.md vs https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/CHANGELOG.md
What we lose here is dev server cold start and HMR speeds, though in effect haven't really noticed a diff with cold start.
HMR speed isn't an issue either, with one large caveat: cold start (basically yarn dev) can take a long time i.e more than 1:30 to be in an HMR-ready state.
Once there, HMR does work as fast as before however.
gomesalexandre
commented
Nov 6, 2025
gomesalexandre
commented
Nov 6, 2025
gomesalexandre
commented
Nov 6, 2025
gomesalexandre
commented
Nov 6, 2025
src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx
Outdated
Show resolved
Hide resolved
gomesalexandre
commented
Nov 6, 2025
premiumjibles
requested changes
Nov 10, 2025
Collaborator
There was a problem hiding this comment.
Exciting times 🔥 . Can't wait to not think about useMemo and useCallback everywhere. Code changes mostly look sane. Couple of potentially misguided concerns on IIFE's so correct me if my assumptions are incorrect. Few things popped up in testing. See below.
-
Tested changing accounts ✔️
-
Tested swap buy/sell - 🚫 - My quotes seem to bork when i swap buy/sell asset which doesn't happen on dev. But seems more like it just doesn't like when i'm selling UTXO's https://jam.dev/c/d0541011-db44-4998-a19e-f74db70d43da
-
Change card config ✔️
-
Change slippage ✔️
-
Enter custom address ✔️
-
Tested put limit order - 🚫 - Didn't seem to get a notification on this branch but fine on develop https://jam.dev/c/e6fc3b4b-1a81-4440-9c93-3a2d2451aec1. (here's develop, this is local develop not deployed develop )
| })() | ||
|
|
||
| const warningAcknowledgementMessage = useMemo(() => { | ||
| const warningAcknowledgementMessage = (() => { |
Collaborator
There was a problem hiding this comment.
If i'm not mistaken, IIFE's will still run every render even with react compiler.
So to keep react compiler happy we'd need to convert it to straight assignments eg...
const recommendedMinimumCryptoBaseUnit = (activeQuote as ThorTradeQuote)
?.recommendedMinimumCryptoBaseUnit
const warningAcknowledgementMessage = !recommendedMinimumCryptoBaseUnit
? translate('warningAcknowledgement.unsafeTrade')
: translate('warningAcknowledgement.thorTrade', {
symbol: sellAsset.symbol,
recommendedMin: fromBaseUnit(recommendedMinimumCryptoBaseUnit, sellAsset.precision),
})
Contributor
Author
There was a problem hiding this comment.
Indeed would! Nice catch ser. FYI, reverted as discussed - now enabled the compiler across the board without changes to existing codebase memoization:
| ]) | ||
|
|
||
| const footerContent = useMemo(() => { | ||
| const footerContent = (() => { |
Collaborator
There was a problem hiding this comment.
Similar here. Probably better to just inline this anyway? Not sure why it needs a function wrapper
Contributor
Author
|
Deprioritizing this one to finish near intents and moving to draft with new plan of action when I'm bakk:
While it will touch the whole codebase, risks are actually way less then touching memoization in existing parts of the codebase, and we can bring those incrementally if we wish to, then. |
gomesalexandre
commented
Nov 14, 2025
Contributor
Author
There was a problem hiding this comment.
tl;dr is react compiler can get pretty strict and/or stubborn and will either:
- escape hatch from using react compiler altogether (in which case, there's actually no diff from develop)
- compile in a way that just doesn't work nicely depending on how the component and its children tree (including npm deps)
The former is obviously not an issue (although makes the compiler useless) but the latter is a bugfest which warrants use no memo - all the use no memo directives in this PR have been added following spotted bugs during full runtime smoke test
gomesalexandre
commented
Nov 14, 2025
| "jest", | ||
| "prettier", | ||
| "simple-import-sort", | ||
| "eslint-plugin-react-memo", |
premiumjibles
approved these changes
Nov 16, 2025
Collaborator
premiumjibles
left a comment
premiumjibles
left a comment
There was a problem hiding this comment.
Approved if the below are false alarms
Smoke test results (unsure which of these are intermittent so ignore anything you can't reproduce):
- TCY page crashing (works in develop + prod) 🚫
- Can't load pools page (works in develop + prod) 🚫
- Mobile web smoke test looks all good (apart from same issues above) ✔️
- for proper mobile test are we able to pop this into gome env? I'm having some issues with expo at the moment
Added 'use no memo' directive to TopAssetsCarousel component to opt it out of React Compiler optimization. The component's useTopAssets hook conditionally calls different query hooks based on category (via CATEGORY_TO_QUERY_HOOK), which violates the Rules of Hooks. This pattern causes React Compiler's useMemoCache to allocate different cache sizes on different renders, resulting in: - "Expected a constant size argument for each invocation of useMemoCache" error - "React has detected a change in the order of Hooks" error - Trade page crashes The 'use no memo' directive follows the same opt-out pattern used for 13 other components in the React Compiler integration (PR #11006). TODO: Refactor useTopAssets to call all hooks unconditionally to enable compiler optimization in the future. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Added 'use no memo' directives to TCY components to opt out of React Compiler optimization. Root Cause: The StakeInput component had a stale closure issue where form validation state (isValid from react-hook-form) and React Query state (isEstimatedFeesDataError) weren't updating through React Compiler's memoization boundaries. This caused the Confirm button to show red error state on initial Stake tab load. The bug occurred because: - Form renders with empty values on initial mount - React Query's estimateFees is disabled (enabled: false) - isEstimatedFeesDataError starts as undefined - React Compiler aggressively memoizes this initial state - Form validation runs AFTER first render - Button's isValid prop gets stale value from compiler memo - Button shows red because it thinks form is invalid Tab switching fixed it temporarily by forcing component unmount/remount, confirming the state initialization race condition. Components opted out: - StakeInput.tsx (critical - has the confirm button with complex state) - UnstakeInput.tsx (consistency - similar structure to StakeInput) - useSendThorTx.tsx (60+ line estimateFeesArgs useMemo, complex state) - Widget.tsx (tab container managing switching state) This follows the same opt-out pattern used for 13+ other components in PR #11006. TODO: Investigate if useSendThorTx can be refactored to avoid the giant useMemo chain and enable compiler optimization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Completes the React Compiler opt-out chain for TCY components by adding 'use no memo' to the parent form provider components. Root Cause Analysis: The previous commit added 'use no memo' to StakeInput/UnstakeInput (child components), but the parent components Stake.tsx and Unstake.tsx (which contain FormProvider and useForm hooks) were still being compiled by React Compiler. React Compiler's optimization was interfering with React Hook Form's Proxy-based formState reactivity system. The Proxy needs proper initialization timing to track which properties (like isValid) are accessed and make them reactive. Why it failed: 1. Stake.tsx creates useForm with empty defaultValues 2. React Compiler memoizes the methods object aggressively 3. FormProvider wraps children 4. StakeInput calls useFormContext() to access formState.isValid 5. Proxy doesn't initialize correctly due to compiler optimization 6. isValid = false (WRONG - empty string should pass validation) Why tab switching "fixed" it: - Tab switch triggers re-render of component tree - FormProvider re-initializes without stale memoization - Proxy works correctly on second mount - isValid = true (CORRECT) The Fix: Added 'use no memo' to Stake.tsx and Unstake.tsx to prevent React Compiler from optimizing the form initialization layer, allowing React Hook Form's Proxy to initialize correctly on first mount. This completes the full opt-out chain: - Widget.tsx ✅ (tab container) - Stake.tsx ✅ (form provider) - NEW - StakeInput.tsx ✅ (form consumer) - Unstake.tsx ✅ (form provider) - NEW - UnstakeInput.tsx ✅ (form consumer) - useSendThorTx.tsx ✅ (complex hook) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/pages/TCY/components/Stake/Stake.tsx (1)
47-47: Document the reason for the compiler opt-out.The
'use no memo'directive correctly opts this component out of React compiler optimizations. However, without documentation, future maintainers won't know why this component requires the opt-out or what specific runtime issues were observed.Consider adding a comment explaining the specific runtime issue:
+ // Compiler opt-out: [describe the observed issue, e.g., "causes re-render loop with MemoryRouter"] 'use no memo'To verify this opt-out is still necessary, test the component with the directive removed in a preview build. If the original issue is resolved in future compiler versions, this opt-out can be removed.
src/pages/TCY/components/Widget.tsx (1)
17-17: Clarify compiler opt-out scope and necessity.The
'use no memo'directive opts out React compiler optimizations for this component. Since the component still uses explicituseMemoforTabHeader(lines 20-24), clarify whether:
- The directive only disables automatic component memoization (making the explicit
useMemostill effective), or- The directive disables all compiler optimizations (making the explicit
useMemothe only optimization present)Additionally, document what specific runtime issue required this opt-out.
Consider adding a comment:
+ // Compiler opt-out: [describe the observed issue] + // Note: Explicit useMemo for TabHeader is still active 'use no memo'Verify this opt-out is still necessary by testing without the directive in a preview build.
📜 Review details
Configuration used: CodeRabbit 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.
📒 Files selected for processing (8)
src/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsx(1 hunks)src/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.ts(1 hunks)src/lib/utils/thorchain/hooks/useSendThorTx.tsx(1 hunks)src/pages/TCY/components/Stake/Stake.tsx(1 hunks)src/pages/TCY/components/Stake/StakeInput.tsx(1 hunks)src/pages/TCY/components/Unstake/Unstake.tsx(1 hunks)src/pages/TCY/components/Unstake/UnstakeInput.tsx(1 hunks)src/pages/TCY/components/Widget.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/pages/TCY/components/Unstake/Unstake.tsx
- src/lib/utils/thorchain/hooks/useSendThorTx.tsx
- src/pages/TCY/components/Stake/StakeInput.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/TCY/components/Unstake/UnstakeInput.tsx
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/plugins/walletConnectToDapps/components/header/WalletConnectToDappsHeaderButton.tsx:0-0
Timestamp: 2025-09-08T22:00:48.005Z
Learning: gomesalexandre dismissed an aria-label accessibility suggestion with "meh" in PR #10418 for WalletConnectToDappsHeaderButton.tsx, consistent with the team's pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10418
File: src/Routes/RoutesCommon.tsx:231-267
Timestamp: 2025-09-03T21:17:27.699Z
Learning: gomesalexandre prefers to keep PR diffs focused and reasonable in size, deferring tangential improvements (like Mixpanel privacy enhancements) to separate efforts rather than expanding the scope of feature PRs.
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: gomesalexandre
Repo: shapeshift/web PR: 10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:69-74
Timestamp: 2025-09-10T15:35:36.547Z
Learning: gomesalexandre dismissed alt text accessibility suggestion with "meh" in PR #10458 for EIP155SignTypedDataConfirmation.tsx Image component, consistent with team pattern of deferring minor a11y improvements to follow-up PRs rather than expanding feature PR scope.
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.
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10572
File: src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx:9-9
Timestamp: 2025-09-18T09:19:59.425Z
Learning: gomesalexandre dismissed a technical suggestion about using location from useBrowserRouter instead of mixing useLocation with useBrowserRouter.navigate with "meh" in PR #10572 for StakingPositionsByProvider.tsx, consistent with his pattern of preferring functional solutions over architectural purity when the current approach works adequately.
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: 10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/TransactionContent.tsx:40-140
Timestamp: 2025-09-12T12:04:24.562Z
Learning: gomesalexandre prefers to defer error handling improvements in new/preparatory code until real-world error cases are encountered, rather than implementing speculative error handling without concrete failure scenarios to test against. This aligns with keeping PR scope focused and evidence-based development practices.
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: 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: 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-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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.ts
📚 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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/pages/TCY/components/Widget.tsxsrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsxsrc/pages/TCY/components/Stake/Stake.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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/pages/TCY/components/Stake/Stake.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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsx
📚 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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/pages/TCY/components/Widget.tsxsrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.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/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.tssrc/pages/TCY/components/Widget.tsxsrc/pages/TCY/components/Stake/Stake.tsx
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.ts
📚 Learning: 2025-08-13T17:10:05.685Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10249
File: src/state/slices/actionSlice/types.ts:91-94
Timestamp: 2025-08-13T17:10:05.685Z
Learning: In the ActionGenericTransactionMetadata type in src/state/slices/actionSlice/types.ts, the thorMemo field is typed as `string | null` (rather than the more common `string | undefined` pattern) because it receives its value from the memo field returned by useSendThorTx(), which is also typed as `string | null`. This maintains type consistency across the data flow.
Applied to files:
src/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.ts
📚 Learning: 2025-08-06T09:46:50.860Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/components/CategoryCard.tsx:27-196
Timestamp: 2025-08-06T09:46:50.860Z
Learning: In src/pages/Explore/components/CategoryCard.tsx, NeOMakinG indicated that wrapping the CategoryCard component with React.memo would be "quite useless in this case", suggesting that memoization is not beneficial for this particular component despite it receiving props, likely due to its specific usage patterns or context.
Applied to files:
src/pages/TCY/components/Widget.tsxsrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsxsrc/pages/TCY/components/Stake/Stake.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/pages/TCY/components/Widget.tsxsrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsxsrc/pages/TCY/components/Stake/Stake.tsx
📚 Learning: 2025-08-06T09:47:29.865Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:174-178
Timestamp: 2025-08-06T09:47:29.865Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG prefers keeping complex height calculations inline rather than extracting them to useMemo, prioritizing perceived performance/execution speed over code readability, even when the calculation would only run when the dependency (isSearching) changes.
Applied to files:
src/pages/TCY/components/Widget.tsxsrc/pages/TCY/components/Stake/Stake.tsx
📚 Learning: 2025-08-06T11:44:30.697Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:81-83
Timestamp: 2025-08-06T11:44:30.697Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG questioned the suggestion to memoize a simple static carouselOptions object ({ loop: true }), indicating that static objects that never change should remain as module-level constants rather than being memoized inside components, as memoization would add unnecessary overhead for truly static values.
Applied to files:
src/pages/TCY/components/Widget.tsxsrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsxsrc/pages/TCY/components/Stake/Stake.tsx
📚 Learning: 2025-08-06T11:02:09.501Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10191
File: src/pages/Explore/Explore.tsx:54-72
Timestamp: 2025-08-06T11:02:09.501Z
Learning: In src/pages/Explore/Explore.tsx, NeOMakinG indicated that memoizing the ExploreCard component would be unnecessary, questioning "why would we memoize that", suggesting that like CategoryCard, this component doesn't benefit from memoization despite receiving props, likely due to its specific usage patterns in the carousel with static content.
Applied to files:
src/pages/TCY/components/Widget.tsxsrc/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsxsrc/pages/TCY/components/Stake/Stake.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/pages/TCY/components/Widget.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/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsx
📚 Learning: 2025-09-18T09:19:59.425Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10572
File: src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx:9-9
Timestamp: 2025-09-18T09:19:59.425Z
Learning: gomesalexandre dismissed a technical suggestion about using location from useBrowserRouter instead of mixing useLocation with useBrowserRouter.navigate with "meh" in PR #10572 for StakingPositionsByProvider.tsx, consistent with his pattern of preferring functional solutions over architectural purity when the current approach works adequately.
Applied to files:
src/pages/TCY/components/Stake/Stake.tsx
⏰ 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: Install and Cache
src/components/MultiHopTrade/components/TradeInput/components/TopAssetsCarousel.tsx
Show resolved
Hide resolved
src/components/MultiHopTrade/components/TradeInput/hooks/useTopAssets.ts
Show resolved
Hide resolved
Contributor
Author
Are you consistently able to repro @premiumjibles ? Can't seem to repro, although I can notice a default error state on the stake button, which gets fixed when going to unstake and back https://jam.dev/c/9843f9ea-361e-479b-9b86-aeef72556e25
That one I can't repro either: https://jam.dev/c/a0753111-7351-47e6-b72f-d380af32e00b That said, noticed we're missing complete copy (in develop too) - created #11110 for it |
Collaborator
|
@gomesalexandre mine is still borked but checked out develop and still borked so not your branch. Something funky with my local so all good. Confirmed mobile app works as normal ✔️ |
0xApotheosis
approved these changes
Nov 19, 2025
Member
0xApotheosis
left a comment
0xApotheosis
left a comment
There was a problem hiding this comment.
Diff very sane. Gave the app a runtime check and confirmed all major flows work as expected, I'll leave the in-depth testing for ops in https://discord.com/channels/554694662431178782/1440121180215443547
Member
|
Merging, as per pre-release thread: https://discord.com/channels/554694662431178782/1440121180215443547/1442339962077843496 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Disregard the below. This takes the opposite approach of enabling the compiler across the board, but disabling it in specific components following runtime testing and spotted issues.
The plan will be to eventually fix said issues so that we don't need to
use no memo.Also removes the
require-memoeslint-rule, since it's effectively not required touseMemo()/useCallback()from now on (although, if a component hasuse no memowe still should).Lil' PoC of using react compiler in app, namely in:-<AddLiquidityInput />-<PrimaryChart />-<TradeInput />Those 3 have been chosen as they're very memo-heavy, and low risk for the two first ones, in theory higher risk for the latter, but effectively low, as we're already memoizing all the things here.Issue (if applicable)
closes #11004
Risk
Very high. Do a full app smoke test, including mobile web, and mobile app.
Testing
Engineering
☝🏽
Do the above both in dev server as well as with
yarn build:web && yarn preview:devOperations
Screenshots (if applicable)
https://jam.dev/c/4187b0c1-8aa4-4c51-a331-d0203cba8ba1
https://jam.dev/c/9668852a-2200-41ca-a1f1-5e671493035b
https://jam.dev/c/46755895-bbf2-4c94-8719-c936a68d6ad3
https://jam.dev/c/64ccb806-9284-4cd2-a9e8-444b3b986c17
https://jam.dev/c/ed482ff2-dcc3-40ad-9d30-e31892c9ad5a
Summary by CodeRabbit
Chores
Style
Impact
✏️ Tip: You can customize this high-level summary in your review settings.