From 9a1259082428571bdbac98503f86f312d2c498c4 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:44:28 -0700 Subject: [PATCH] perf(web): the pull request timeline mounts only the events near the viewport The timeline tab rendered every event of a pull request at once. A large pull request has thousands of commits, comments, and review threads, so opening the tab mounted hundreds of thousands of DOM nodes and stalled the page for seconds. Render the timeline through LegendList, which mounts only the rows near the viewport; the connector line moves to the list's content container. Co-Authored-By: Claude Opus 5.5 --- .../pullRequest/PullRequestDetailPanel.tsx | 10 +- .../PullRequestTimelineTab.browser.test.tsx | 89 ++++++++++++ .../pullRequest/PullRequestTimelineTab.tsx | 137 ++++++++++-------- .../pullRequest/pullRequestDetail.logic.ts | 2 +- 4 files changed, 172 insertions(+), 66 deletions(-) create mode 100644 apps/web/src/components/pullRequest/PullRequestTimelineTab.browser.test.tsx diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 135ae2970ceb9..5695c9860c470 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -413,16 +413,16 @@ export function PullRequestDetailPanel({ Partial> >({}); const condensed = condensedByTab[tab] ?? false; - const scrollerByTab = useRef>>({}); + const scrollerByTab = useRef>>({}); // Fresh ref callbacks would detach and reattach every scroller on each render. const [scrollerRefs] = useState(() => ({ - summary: (node: HTMLDivElement | null) => { + summary: (node: HTMLElement | null) => { scrollerByTab.current.summary = node; }, - timeline: (node: HTMLDivElement | null) => { + timeline: (node: HTMLElement | null) => { scrollerByTab.current.timeline = node; }, - code: (node: HTMLDivElement | null) => { + code: (node: HTMLElement | null) => { scrollerByTab.current.code = node; }, })); @@ -432,7 +432,7 @@ export function PullRequestDetailPanel({ // so applying a delta afterward would compensate twice. const compensationRef = useRef<{ tab: PullRequestDetailTab; - scroller: HTMLDivElement; + scroller: HTMLElement; target: number; } | null>(null); // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- restores the scroller's scrollTop before paint after the header condenses diff --git a/apps/web/src/components/pullRequest/PullRequestTimelineTab.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestTimelineTab.browser.test.tsx new file mode 100644 index 0000000000000..4e8f1e4319c5c --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestTimelineTab.browser.test.tsx @@ -0,0 +1,89 @@ +import { + EnvironmentId, + ProjectId, + SourceControlProviderKind, + type PullRequestDetailView, +} from "@t3tools/contracts"; +import { expect, it } from "vite-plus/test"; + +import { render } from "../../../test/render"; +import "../../index.css"; +import { PullRequestTimelineTab } from "./PullRequestTimelineTab"; + +const COMMITS = 2000; + +const detail: PullRequestDetailView = { + provider: SourceControlProviderKind.make("github"), + projectId: ProjectId.make("project"), + projectTitle: "Project", + workspaceRoot: "/workspace", + repository: "owner/repo", + number: 1, + title: "Huge pull request", + body: "", + url: "https://github.com/owner/repo/pull/1", + author: { login: "author", name: null, avatarUrl: null }, + viewer: "author", + state: "open", + isDraft: false, + mergeability: "mergeable", + additions: 1, + deletions: 0, + changedFiles: 1, + headBranch: "feature", + baseBranch: "main", + createdAt: "2026-09-01T00:00:00Z", + updatedAt: "2026-09-01T00:00:00Z", + mergedAt: null, + closedAt: null, + reviewers: [], + labels: [], + checks: [], + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreads: [], + commits: Array.from({ length: COMMITS }, (_, index) => ({ + oid: `${index.toString(16).padStart(7, "0")}${"0".repeat(33)}`, + messageHeadline: `Commit number ${index}`, + committedDate: new Date(Date.UTC(2026, 8, 1, 0, index)).toISOString(), + })), + mergeCapabilities: { merge: false, squash: false, rebase: false }, + capabilities: { + diff: true, + comment: true, + search: true, + actions: [], + mergeMethods: [], + review: { inlineComment: false, reply: false, resolve: false, verdicts: [] }, + reviewers: { request: false, listCandidates: false }, + edit: { changeRequest: false, comment: false }, + }, + viewerPermissions: { + actions: [], + comment: false, + resolve: false, + verdicts: [], + requestReviewers: false, + }, +}; + +it("mounts only the timeline rows near the viewport of a huge pull request", async () => { + await using screen = await render( +
+ {}} + detail={detail} + environmentId={EnvironmentId.make("environment")} + reference={detail} + order="oldest" + onOpenCommit={() => {}} + onRefresh={() => {}} + /> +
, + ); + await expect.element(screen.getByText("Commit number 0")).toBeVisible(); + const mounted = screen.container.querySelectorAll("button").length; + expect(mounted).toBeGreaterThan(0); + expect(mounted).toBeLessThan(COMMITS / 10); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx index b522094e90c98..1f41f2381148f 100644 --- a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx @@ -34,6 +34,7 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { buildPullRequestTimeline, groupPullRequestTimelineConversations, + type PullRequestTimelineRow, isPullRequestVerdictStale, newestPullRequestCommitAt, pullRequestReviewOutcome, @@ -56,6 +57,7 @@ import { pullRequestReviewOutcomeToneClassName, } from "./pullRequestPresentation"; import { PullRequestGlyph } from "./pullRequestIcons"; +import { LegendList, type LegendListRef } from "@legendapp/list/react"; interface ReactionSurface { readonly canReact: boolean; @@ -634,7 +636,7 @@ export function PullRequestTimelineTab({ onOpenCommit, onRefresh, }: { - scrollerRef: (node: HTMLDivElement | null) => void; + scrollerRef: (node: HTMLElement | null) => void; detail: PullRequestDetailView; environmentId: EnvironmentId; threadRef?: ScopedThreadRef | null; @@ -666,66 +668,81 @@ export function PullRequestTimelineTab({ void readLocalApi()?.shell.openExternal(url); }; - return ( -
-
-
- - {rows.map((row) => { - if (row.kind === "comments") { - return ( - - ); - } - const event = row.event; - if (event.thread !== undefined) { - return ( - - ); - } - if (event.kind === "commit") { - return ; - } - const outcome = pullRequestReviewOutcome(event.reviewState); - if (outcome !== null) { - return ( - - ); - } - return ; - })} -
+ const rowKey = (row: PullRequestTimelineRow) => { + if (row.kind === "comments") return `comments:${row.events[0]?.id ?? "empty"}`; + const event = row.event; + // A thread's resolution or dismissal remounts it, so its disclosure starts in the new state. + return event.thread === undefined + ? event.id + : `thread:${event.id}:${event.thread.isResolved}:${pullRequestReviewOutcome(event.reviewState) === "dismissed"}`; + }; + const renderRow = ({ item: row }: { item: PullRequestTimelineRow }) => { + if (row.kind === "comments") { + return ( + + ); + } + const event = row.event; + if (event.thread !== undefined) { + return ( + + ); + } + if (event.kind === "commit") return ; + const outcome = pullRequestReviewOutcome(event.reviewState); + if (outcome !== null) { + return ( + + ); + } + return ; + }; - {events.length === 0 ? ( -
- -

No activity yet.

-
- ) : null} + if (events.length === 0) { + return ( +
+
+ +

No activity yet.

+
-
+ ); + } + + // Virtualized: a large pull request has thousands of events, and mounting every one stalls + // the page. The connector line is drawn by the content container, behind the rows. + return ( + + ref={(list: LegendListRef | null) => scrollerRef(list?.getScrollableNode() ?? null)} + data={rows} + extraData={{ editable, reactions, newestCommitAt }} + keyExtractor={rowKey} + renderItem={renderRow} + estimatedItemSize={140} + className="scrollbar-gutter-both h-full px-4" + contentContainerClassName="relative mx-auto w-full max-w-3xl before:absolute before:bottom-10 before:left-[15px] before:top-6 before:w-px before:bg-border/45" + ListHeaderComponent={
} + ListFooterComponent={
} + /> ); } diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index d03f57bc44a36..f422dfbc0e3bd 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -483,7 +483,7 @@ export interface PullRequestTimelineThread readonly replies: ReadonlyArray; } -type PullRequestTimelineRow = +export type PullRequestTimelineRow = | { readonly kind: "event"; readonly event: PullRequestTimelineEvent } | { readonly kind: "comments"; readonly events: ReadonlyArray };