Skip to content

Farm page re-renders its entire pool list and deposit modal on every 5s event-poll tick with no row-level memoization for pools #88

Description

@prodbycorne

Problem

Farm (src/app/farm/page.tsx) calls useSorobanEvents(poolContractIds, [...]) (line 454), which on every successful 5-second poll tick invalidates QUERY_KEYS.POOLS and/or QUERY_KEYS.USER_POSITION whenever it detects a relevant event. Each invalidation triggers usePools()/useAllUserPositions() refetches, which produce new array references for pools/userPositions, which flow through useMemo-wrapped availablePools/myPositions (lines 493-532) — but critically, availablePools.map((farm) => (...)) (lines 574-627) renders each pool row as an inline JSX block directly inside Farm, not as its own memoized component. Unlike EarningRow (which is wrapped in React.memo with a custom comparator, earningRowPropsAreEqual, specifically to avoid needless re-renders), the 'Farm Pools' list has no equivalent per-row memoization, so every pool row re-renders whenever any pool's data changes, even for users with many pools who are only interested in one, and even when the event that triggered invalidation was for a pool the user isn't looking at.

Additionally, DepositModal is a full component defined at module scope inside page.tsx and instantiated once per Farm render with farm={selectedFarm} — since Farm itself re-renders on every pools/positions refetch (its own state, not memoized against unrelated prop changes), the modal component (and its useStellarBalance/useLockAssetsFeePreview/useLockAssets hook instances) re-mount's internal hook tree re-runs on every parent re-render even while isOpen={false}, wasting render cycles on hidden UI with live query hooks.

Acceptance Criteria

  • Extract each pool row in the 'Farm Pools' list into its own memoized component (mirroring EarningRow's pattern) so only the rows whose underlying pool/position data actually changed re-render.
  • Verify React.memo/dependency correctness with React DevTools Profiler or an automated render-count assertion before and after the change, on a mocked scenario with 20+ pools where only one pool's event fires.
  • Consider whether DepositModal's live query hooks (useStellarBalance, useLockAssetsFeePreview) should be conditionally mounted (e.g. only run when isOpen is true) to avoid unnecessary background fetches for a closed modal.
  • Document the expected render-count budget for the Farm page under a realistic pool count and add a regression check (e.g. a React Profiler-based test or a simple render-count spy in farm/page.test.tsx).

Relevant Files

  • src/app/farm/page.tsx — availablePools.map(...) (~L574-627) renders unmemoized pool rows; DepositModal (~L86-429) instantiated with live hooks regardless of isOpen
  • src/app/farm/EarningRow.tsx — the existing React.memo pattern this page's pool-list rendering should follow
  • src/hooks/useSorobanEvents.ts — the 5s poll that triggers the invalidations driving these re-renders

Additional Notes

Re-read src/app/farm/page.tsx and src/app/farm/EarningRow.tsx directly. Confirmed:

  • The availablePools.map((farm) => (...)) block renders a Flex per row inline in Farm's JSX, with no React.memo wrapper and no extraction into its own component — every prop consumed inside that block (farm.name, farm.earned, farm.stake, farm.dailyRate, farm.totalStakedLiquidity, plus the closure over isConnected, isNetworkMismatch, and handleDepositClick) is recomputed and re-rendered together on every Farm re-render, exactly as described.
  • EarningRow.tsx confirms the existing pattern to mirror: it defines earningRowPropsAreEqual as a named custom comparator function and passes it as the second argument to React.memo(...) at the component's closing line — this is a straightforward, already-proven pattern in this exact file tree, reinforcing that this is a mechanical fix, not a design problem.
  • DepositModal is defined as a plain function (function DepositModal({ isOpen, ... })) at module scope in page.tsx, confirmed instantiated once via <DepositModal isOpen={isDepositOpen} ... /> — its internal useEffect (gated on [isOpen, farm?.id]) does reset internal state when isOpen toggles, but this doesn't address the underlying issue: React still executes DepositModal's full function body, including its hook calls (useStellarBalance, useLockAssetsFeePreview, useLockAssets), on every parent re-render regardless of isOpen's value, since Chakra's Modal doesn't unmount children/render-prop content by default when closed (isOpen=false only visually hides it via the underlying Modal/Portal, unless unmountOnExit-equivalent behavior — Chakra's v2 Modal actually unmounts by default unless Modal's own internals differ; verify at implementation time whether Chakra v2's Modal truly keeps children mounted while closed, since if it doesn't, the "modal hooks re-run while hidden" claim may only apply to the brief render pass before Chakra's own conditional unmount kicks in — this is worth confirming empirically with React DevTools before writing the fix, rather than assuming the original body's framing is fully accurate).

Edge cases to cover

  • A user with exactly one pool: memoization gains are negligible here, so the regression test should specifically use a 20+ pool mock (per the existing acceptance criteria) to make the improvement observable; a 1-2 pool test would pass trivially before and after the fix and wouldn't catch a regression.
  • handleDepositClick and other inline closures passed to each row (onClick={() => handleDepositClick(farm)}) must be stable or the custom comparator needs to explicitly ignore function-identity changes (mirroring how earningRowPropsAreEqual likely already handles this for EarningRow) — otherwise wrapping the row in React.memo without addressing closure identity will not prevent re-renders at all, silently defeating the fix.
  • isConnected/isNetworkMismatch are read from context/hooks at the Farm level and passed into every row — if either changes, all rows should re-render (that's correct, not a bug), so the memo comparator must distinguish "this specific pool's data changed" from "a cross-cutting flag every row legitimately depends on changed."

Implementation sketch

  1. Extract the availablePools.map(...) body into a new FarmPoolRow component (mirroring EarningRow's file/export shape) accepting { farm, isConnected, isNetworkMismatch, onDeposit } as props.
  2. Write a farmPoolRowPropsAreEqual comparator (mirroring earningRowPropsAreEqual) comparing farm.id/farm.earned/farm.stake/farm.dailyRate/farm.totalStakedLiquidity plus the shared flags, ignoring the onDeposit callback's identity (or wrap handleDepositClick in useCallback keyed on stable deps at the Farm level).
  3. Wrap FarmPoolRow in React.memo(FarmPoolRow, farmPoolRowPropsAreEqual).
  4. For DepositModal: first verify empirically (React DevTools Profiler, "why did this render" or a console.count in useStellarBalance) whether its hooks actually re-run while isOpen=false, then either lazy-mount it ({isDepositOpen && <DepositModal .../>}) or add early-return guards inside its data-fetching hooks keyed on isOpen (e.g. pass enabled: isOpen to whatever React Query options useStellarBalance/useLockAssetsFeePreview accept, if they're React Query-based).

Test plan

  • React DevTools Profiler or a render-count spy (per acceptance criteria) in farm/page.test.tsx: mock 20+ pools, fire a useSorobanEvents-style invalidation for one pool's QUERY_KEYS.POOLS/QUERY_KEYS.USER_POSITION, assert only that pool's FarmPoolRow re-renders.
  • Unit: farmPoolRowPropsAreEqual comparator given identical vs. changed farm objects.
  • Before/after render-count assertion for DepositModal's hook calls specifically while isOpen=false, once the empirical Chakra-mounting question above is resolved — this determines whether the fix is "lazy-mount" or "add enabled guards," and the test should assert whichever mechanism is actually implemented.

Cross-references

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial CampaignCampaign: Official CampaignOfficial Campaign | FWC26Campaign: Official Campaign | FWC26farmFarming/staking flow — deposit, lock, unlock, creditsperformanceRendering performance, caching, or bundle sizevery hardExtremely hard — deep expertise, careful design, and significant time required

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions