Skip to content

refactor: use lazy useEnsData for treasury vote ENS resolution#572

Draft
rickstaa wants to merge 1 commit intomainfrom
refactor/lazy-ens-treasury-votes
Draft

refactor: use lazy useEnsData for treasury vote ENS resolution#572
rickstaa wants to merge 1 commit intomainfrom
refactor/lazy-ens-treasury-votes

Conversation

@rickstaa
Copy link
Member

@rickstaa rickstaa commented Mar 3, 2026

Summary

  • Replaces the blocking ENS cache pattern (module-level Map + Promise.all in useEffect) in feat: improve ens caching of treasury votes table #561 with the existing useEnsData SWR hook used across the rest of the app
  • Votes render instantly with truncated addresses while ENS names resolve lazily per-row, matching the OrchestratorList pattern
  • Removes ~60 lines of custom ENS cache code, the async decorateVotes function, and the votesLoading state

What changed

  • TreasuryVoteTable/index.tsx — removed ENS cache, replaced useEffect/decorateVotes with synchronous useMemo
  • Views/VoteItem.tsxMobileVoteView and DesktopVoteView now call useEnsData() per-row for lazy ENS
  • Views/DesktopVoteTable.tsx — removed ensName from Vote type and onSelect signature
  • TreasuryVotePopover.tsx — resolves ENS internally via useEnsData() instead of receiving it as a prop

Why

The previous approach blocked the entire vote table render behind Promise.all() of ENS lookups — even on revisits, every voter's ENS name had to re-resolve before anything displayed. This made the table feel slow despite having the vote data ready.

Replaces #561

Test plan

  • Visit a treasury proposal page — votes table should render immediately with truncated addresses
  • ENS names should pop in lazily as they resolve (no spinner for ENS)
  • Click the history icon — popover should show voter's ENS name
  • Mobile view should show ENS names lazily in vote cards
  • Navigating away and back should show cached ENS names instantly (SWR cache)

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 3, 2026 15:07
@rickstaa rickstaa requested a review from ECWireless as a code owner March 3, 2026 15:07
@vercel
Copy link
Contributor

vercel bot commented Mar 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
explorer-arbitrum-one Ready Ready Preview, Comment Mar 3, 2026 3:12pm

Request Review

Replace async useEffect that blocked rendering behind ENS lookups with
synchronous useMemo for vote merging. ENS names now resolve lazily
per-row via the existing useEnsData hook, matching the pattern used
by OrchestratorList and other components.

Co-authored-By: ECWireless <elliott@coopallc.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors treasury proposal vote rendering to resolve ENS names lazily via the existing useEnsData SWR hook, removing the previous blocking ENS fetch/caching approach so vote rows can render immediately with truncated addresses while ENS resolves per-row.

Changes:

  • Removed the custom ENS cache + async decorateVotes flow from TreasuryVoteTable, replacing it with synchronous vote decoration.
  • Updated vote row components to call useEnsData() per voter for lazy ENS display.
  • Simplified the Vote type and selection/popover wiring to no longer pass ensName through props.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
components/Treasury/TreasuryVoteTable/index.tsx Removes ENS blocking logic and decorates votes synchronously from vote + event data.
components/Treasury/TreasuryVoteTable/Views/VoteItem.tsx Resolves ENS lazily per row (mobile + desktop vote views) and updates onSelect payload.
components/Treasury/TreasuryVoteTable/Views/DesktopVoteTable.tsx Removes ensName from Vote and adjusts the voter column/accessor + onSelect signature.
components/Treasury/TreasuryVoteTable/TreasuryVotePopover.tsx Resolves ENS internally via useEnsData() instead of receiving it via props.
Comments suppressed due to low confidence (1)

components/Treasury/TreasuryVoteTable/Views/DesktopVoteTable.tsx:45

  • react-table sorting uses the column accessor value. With accessor: "voter", the accessor returns an object, so sorting the “Voter” column will effectively compare "[object Object]" for every row (i.e., sorting won’t work correctly). Use an accessor that returns a primitive (e.g., voter id string) and keep the custom Cell for rendering EthAddressBadge.
      {
        Header: "Voter",
        accessor: "voter",
        id: "voter",
        Cell: ({ row }) => (
          <Box css={{ minWidth: 120 }}>
            <EthAddressBadge value={row.original.voter.id} />
          </Box>
        ),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +50 to +63
return votesList.map((vote) => {
const events = eventsList
.filter((event) => event.voter.id === vote.voter.id)
.sort((a, b) => b.timestamp - a.timestamp);

const latestEvent = events[0];

return {
...vote,
reason: latestEvent?.reason || vote.reason || "",
transactionHash: latestEvent?.transaction.id ?? "",
timestamp: latestEvent?.timestamp,
};
}) as Vote[];
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useVotes currently does an eventsList.filter(...).sort(...) for every vote, and only uses events[0]. This is O(votes × events log events) and will get expensive as vote/event counts grow. Consider building a single Map<voterId, latestEvent> in one pass over eventsList (or sorting eventsList once) and then decorating votes via a lookup, avoiding per-vote sorts/filters.

Copilot uses AI. Check for mistakes.
timestamp: latestEvent?.timestamp,
};
}) as Vote[];
}, [treasuryVotesData, treasuryVoteEventsData]);
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useMemo dependency array is [treasuryVotesData, treasuryVoteEventsData], but Apollo often returns new wrapper objects even when the underlying lists are unchanged. That can cause this memo to recompute on most renders. Depending on treasuryVotesData?.treasuryVotes and treasuryVoteEventsData?.treasuryVoteEvents (or votesList/eventsList references) would make the memoization effective.

Suggested change
}, [treasuryVotesData, treasuryVoteEventsData]);
}, [treasuryVotesData?.treasuryVotes, treasuryVoteEventsData?.treasuryVoteEvents]);

Copilot uses AI. Check for mistakes.
timestamp: latestEvent?.timestamp,
};
}) as Vote[];
}, [treasuryVotesData, treasuryVoteEventsData]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}, [treasuryVotesData, treasuryVoteEventsData]);
}, [treasuryVotesData?.treasuryVotes, treasuryVoteEventsData?.treasuryVoteEvents]);

useMemo dependency array uses wrapper objects instead of actual data arrays, causing unnecessary recomputation on every render

Fix on Vercel

Comment on lines +50 to +55
return votesList.map((vote) => {
const events = eventsList
.filter((event) => event.voter.id === vote.voter.id)
.sort((a, b) => b.timestamp - a.timestamp);

const latestEvent = events[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return votesList.map((vote) => {
const events = eventsList
.filter((event) => event.voter.id === vote.voter.id)
.sort((a, b) => b.timestamp - a.timestamp);
const latestEvent = events[0];
// Build a map of voter ID to latest event in a single pass
// First sort events by timestamp descending, then build map (first event per voter is latest)
const sortedEvents = [...eventsList].sort(
(a, b) => b.timestamp - a.timestamp
);
const latestEventByVoter = new Map();
for (const event of sortedEvents) {
if (!latestEventByVoter.has(event.voter.id)) {
latestEventByVoter.set(event.voter.id, event);
}
}
return votesList.map((vote) => {
const latestEvent = latestEventByVoter.get(vote.voter.id);

The useVotes hook has O(votes × events log events) performance issue from filtering and sorting events for each vote independently

Fix on Vercel

() => [
{
Header: "Voter",
accessor: "ensName",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Voter" column accessor changed from a string field to an object, breaking sort functionality in react-table

Fix on Vercel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants