perf(wallet): fix React Compiler bailouts on Asset Details - #34827
Draft
juanmigdr wants to merge 1 commit into
Draft
perf(wallet): fix React Compiler bailouts on Asset Details#34827juanmigdr wants to merge 1 commit into
juanmigdr wants to merge 1 commit into
Conversation
… Asset Details The Asset Details chart/transactions subtree (PriceChartProvider, AssetOverviewContent) was re-rendering 6 times per visit instead of being memoized by React Compiler, because several components had Rules-of-React violations (refs read during render, throw/value-blocks inside try/catch) that made the compiler skip them entirely. Fixes the compiler bailouts across TokenDetails (Views + AssetOverview), AssetOverviewContent, TokenDetailsActions, and Transactions/index.js by moving ref writes into effects, converting a render-time-read ref to state, and rewriting a few try/catch bodies to avoid throw/optional-chaining/ternary value blocks the compiler can't yet handle. Verified against the actual React Compiler that every rewrite is necessary (reverting any one of them reintroduces its bailout) and that none of it is over-fixing. Two ref-based patterns are kept as intentional, documented bailouts because converting them would change behavior: the navigation-lock ref in TokenDetailsActions (a state-based lock would be async/batched and could let a double-tap through), and the perps-market reset in TokenDetails (a useEffect version raced with a child effect and broke tracking). Verified with React DevTools profiling: PriceChartProvider/Transactions render count dropped from 6 to 3 per visit, and every remaining render now maps to a real data change rather than an unmemoized render.
Contributor
PR template — items to address before "Ready for review"Warnings — informational, address before merging:
See docs/readme/ready-for-review.md for the full Definition of Ready for Review. |
Contributor
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag selection rationale:
Per SmokePerps tag description: also select SmokeWalletPlatform (Trending section) and SmokeConfirmations (Add Funds deposits are on-chain transactions) — both already selected. Performance Test Selection: |
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
The Asset Details screen's chart/transactions area (
PriceChartProviderand its subtree) was re-rendering 6 times per visit instead of getting memoized by React Compiler. The root cause: several components in that subtree had Rules-of-React violations (reading refs during render,throw/ternaries/optional-chaining insidetry/catch) that made the compiler skip optimizing them entirely.This PR fixes those compiler bailouts across
TokenDetails(both the Views and AssetOverview versions),AssetOverviewContent,TokenDetailsActions, andTransactions/index.js— mostly by moving ref writes into effects and rewriting a handful oftry/catchbodies to avoid patterns the compiler can't yet handle. I verified against the actual compiler output that every single rewrite is necessary (reverting any one of them reintroduces its specific bailout), so there's no unnecessary churn here.Two ref-based patterns are intentionally left as-is (documented in code comments) because "fixing" them would change behavior:
TokenDetailsActions— converting it to state would make the double-tap guard async/batched, which could let a rapid second tap through during a slow render.TokenDetails— auseEffectversion raced with a child component's own effect and brokeTOKEN_DETAILS_OPENEDtracking (caught by existing tests).Measured with React DevTools profiling (tap into the same token 5x, before/after):
PriceChartProvider/Transactionsrender count dropped from 6 to 3 per visit, and every remaining render now maps to a real data change (loading state, chart price direction resolving, perps market resolving) instead of a phantom re-render caused by the lack of memoization.Note: this doesn't move the Sentry
AssetDetailstrace metric itself, since that trace ends on first mount, before any of this re-render activity happens. The improvement is to post-mount jank/perceived loading smoothness, not the traced metric.Changelog
CHANGELOG entry: null
Related issues
Fixes: N/A — investigative performance work, no linked ticket.
Manual testing steps
Screenshots/Recordings
N/A — this is a render-behavior/perf fix with no visual/UI changes.
Before
After
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist