From b72f8b1fe327eea60d838fe6cb5375f1d8814135 Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Fri, 10 Apr 2026 15:25:07 -0400 Subject: [PATCH 1/3] Optimize core view performance across dashboard - Eliminate waterfalls: parallelize review page loader fetches, prefetch first page of diff files in route loader, eagerly preload diff chunks - Reduce bundle size: lazy load CommandPalette, GitHubAccessDialog, Agentation, and Markdown in pull request rows - Fix re-render cascades: extract StickyGroupHeader in pulls/issues pages, use ActiveFileStore with useSyncExternalStore for review file tree, extract ReviewToolbar and ReviewSidebar as memoized components, stabilize callbacks with ref patterns, stop history.replaceState on scroll - Add estimated diff height placeholders to prevent layout shift while diffs load on the review page - Add content-visibility to pull/issue group sections - Memo list row components (PullRequestRow, IssueRow, DetailTab) --- .../src/components/issues/issue-row.tsx | 9 +- .../components/layouts/dashboard-layout.tsx | 20 +- .../components/layouts/dashboard-topbar.tsx | 102 ++-- .../src/components/pulls/pull-request-row.tsx | 27 +- .../pulls/review/review-diff-pane.tsx | 12 +- .../pulls/review/review-file-diff-block.tsx | 24 + .../pulls/review/review-file-tree.tsx | 73 ++- .../components/pulls/review/review-page.tsx | 435 +++++++++++------- apps/dashboard/src/routes/__root.tsx | 12 +- .../$owner/$repo/review.$pullId.tsx | 36 +- .../src/routes/_protected/issues.tsx | 98 ++-- .../dashboard/src/routes/_protected/pulls.tsx | 98 ++-- 12 files changed, 634 insertions(+), 312 deletions(-) diff --git a/apps/dashboard/src/components/issues/issue-row.tsx b/apps/dashboard/src/components/issues/issue-row.tsx index f1becfd..62bffa6 100644 --- a/apps/dashboard/src/components/issues/issue-row.tsx +++ b/apps/dashboard/src/components/issues/issue-row.tsx @@ -1,6 +1,7 @@ import { CommentIcon, IssuesIcon } from "@diffkit/icons"; import { cn } from "@diffkit/ui/lib/utils"; import { Link } from "@tanstack/react-router"; +import { memo } from "react"; import { formatRelativeTime } from "#/lib/format-relative-time"; import type { IssueSummary } from "#/lib/github.types"; @@ -14,7 +15,11 @@ function getIssueStateProps(issue: IssueSummary) { return { color: "text-green-500" }; } -export function IssueRow({ issue }: { issue: IssueSummary }) { +export const IssueRow = memo(function IssueRow({ + issue, +}: { + issue: IssueSummary; +}) { const { color } = getIssueStateProps(issue); const href = `/${issue.repository.owner}/${issue.repository.name}/issues/${issue.number}`; @@ -55,4 +60,4 @@ export function IssueRow({ issue }: { issue: IssueSummary }) { )} ); -} +}); diff --git a/apps/dashboard/src/components/layouts/dashboard-layout.tsx b/apps/dashboard/src/components/layouts/dashboard-layout.tsx index ed514cc..1dcc87b 100644 --- a/apps/dashboard/src/components/layouts/dashboard-layout.tsx +++ b/apps/dashboard/src/components/layouts/dashboard-layout.tsx @@ -1,6 +1,6 @@ import { useQuery } from "@tanstack/react-query"; import { getRouteApi, Outlet } from "@tanstack/react-router"; -import { CommandPalette } from "#/components/navigation/command-palette"; +import { lazy, Suspense } from "react"; import { githubMyIssuesQueryOptions, githubMyPullsQueryOptions, @@ -9,7 +9,17 @@ import { useGitHubRevalidation } from "#/lib/use-github-revalidation"; import { useHasMounted } from "#/lib/use-has-mounted"; import { DashboardBottomBar } from "./dashboard-bottombar"; import { DashboardTopbar } from "./dashboard-topbar"; -import { GitHubAccessDialog } from "./github-access-dialog"; + +const CommandPalette = lazy(() => + import("#/components/navigation/command-palette").then((mod) => ({ + default: mod.CommandPalette, + })), +); +const GitHubAccessDialog = lazy(() => + import("./github-access-dialog").then((mod) => ({ + default: mod.GitHubAccessDialog, + })), +); const routeApi = getRouteApi("/_protected"); @@ -60,8 +70,10 @@ export function DashboardLayout() { - - + + + + ); } diff --git a/apps/dashboard/src/components/layouts/dashboard-topbar.tsx b/apps/dashboard/src/components/layouts/dashboard-topbar.tsx index e423b30..d50ed96 100644 --- a/apps/dashboard/src/components/layouts/dashboard-topbar.tsx +++ b/apps/dashboard/src/components/layouts/dashboard-topbar.tsx @@ -21,9 +21,9 @@ import { DropdownMenuShortcut, DropdownMenuTrigger, } from "@diffkit/ui/components/dropdown-menu"; -import { Link, useRouter } from "@tanstack/react-router"; +import { Link, useRouter, useRouterState } from "@tanstack/react-router"; import { useTheme } from "next-themes"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; import { signOutToLogin } from "#/lib/auth-actions"; import { preloadRouteOnce } from "#/lib/route-preload"; import { useGlobalShortcuts } from "#/lib/shortcuts"; @@ -73,7 +73,12 @@ export function DashboardTopbar({ const { theme, setTheme } = useTheme(); const [avatarLoadFailed, setAvatarLoadFailed] = useState(false); const openTabs = useTabs(); + // Store router in a ref — only used imperatively (navigate, preload), + // never read during render, so we avoid subscribing to state changes. const router = useRouter(); + const routerRef = useRef(router); + routerRef.current = router; + const pathname = useRouterState({ select: (s) => s.location.pathname }); const scrollRef = useRef(null); const [canScrollLeft, setCanScrollLeft] = useState(false); const [canScrollRight, setCanScrollRight] = useState(false); @@ -108,49 +113,63 @@ export function DashboardTopbar({ .slice(0, 2) .toUpperCase(); - const navItems: NavItem[] = [ - { to: "/", label: "Overview", icon: HomeIcon }, - { - to: "/pulls", - label: "Pull Requests", - icon: GitPullRequestIcon, - count: counts.pulls, - }, - { - to: "/issues", - label: "Issues", - icon: IssuesIcon, - count: counts.issues, - }, - { - to: "/reviews", - label: "Reviews", - icon: ReviewsIcon, - count: counts.reviews, - }, - ]; + const navItems = useMemo( + () => [ + { to: "/", label: "Overview", icon: HomeIcon }, + { + to: "/pulls", + label: "Pull Requests", + icon: GitPullRequestIcon, + count: counts.pulls, + }, + { + to: "/issues", + label: "Issues", + icon: IssuesIcon, + count: counts.issues, + }, + { + to: "/reviews", + label: "Reviews", + icon: ReviewsIcon, + count: counts.reviews, + }, + ], + [counts.pulls, counts.issues, counts.reviews], + ); useEffect(() => { if (!tabsReady) return; void Promise.allSettled( - primaryNavRoutes.map((to) => router.preloadRoute({ to })), + primaryNavRoutes.map((to) => routerRef.current.preloadRoute({ to })), ); - }, [router, tabsReady]); + }, [tabsReady]); useEffect(() => { if (!tabsReady || openTabs.length === 0) return; void Promise.allSettled( - openTabs.map((tab) => preloadRouteOnce(router, tab.url)), + openTabs.map((tab) => preloadRouteOnce(routerRef.current, tab.url)), ); - }, [router, tabsReady, openTabs]); + }, [tabsReady, openTabs]); function navigateToTab(tab: Tab | undefined) { if (!tab) return; - void router.navigate({ to: tab.url }); + void routerRef.current.navigate({ to: tab.url }); } + const handleCloseTab = useCallback( + (id: string, tabUrl: string) => { + const isActive = pathname === tabUrl; + removeTab(id); + if (isActive) { + void routerRef.current.navigate({ to: "/" }); + } + }, + [pathname], + ); + useGlobalShortcuts([ ...Array.from( { length: Math.min(openTabs.length, MAX_TAB_SHORTCUTS) }, @@ -167,7 +186,7 @@ export function DashboardTopbar({ enabled: tabsReady && openTabs.length > 1, onTrigger: () => { const currentIndex = openTabs.findIndex( - (tab) => tab.url === router.state.location.pathname, + (tab) => tab.url === routerRef.current.state.location.pathname, ); const nextIndex = currentIndex === -1 @@ -181,7 +200,7 @@ export function DashboardTopbar({ enabled: tabsReady && openTabs.length > 1, onTrigger: () => { const currentIndex = openTabs.findIndex( - (tab) => tab.url === router.state.location.pathname, + (tab) => tab.url === routerRef.current.state.location.pathname, ); const nextIndex = currentIndex === -1 ? 0 : (currentIndex + 1) % openTabs.length; @@ -331,14 +350,8 @@ export function DashboardTopbar({ key={tab.id} tab={tab} icon={Icon} - onClose={(id) => { - const isActive = - router.state.location.pathname === tab.url; - removeTab(id); - if (isActive) { - void router.navigate({ to: "/" }); - } - }} + onClose={handleCloseTab} + routerRef={routerRef} /> ); })} @@ -361,18 +374,19 @@ export function DashboardTopbar({ ); } -function DetailTab({ +const DetailTab = memo(function DetailTab({ tab, icon: Icon, onClose, + routerRef, }: { tab: Tab; icon: typeof GitPullRequestIcon; - onClose: (id: string) => void; + onClose: (id: string, tabUrl: string) => void; + routerRef: React.RefObject>; }) { - const router = useRouter(); const preloadTab = () => { - void preloadRouteOnce(router, tab.url); + void preloadRouteOnce(routerRef.current, tab.url); }; return ( @@ -407,7 +421,7 @@ function DetailTab({ onClick={(e) => { e.preventDefault(); e.stopPropagation(); - onClose(tab.id); + onClose(tab.id, tab.url); }} className="absolute inset-y-0 right-0 flex items-center rounded-r-md bg-surface-1 pl-1.5 pr-1.5 opacity-0 transition-opacity group-hover:opacity-100" aria-label={`Close ${tab.title}`} @@ -419,4 +433,4 @@ function DetailTab({ ); -} +}); diff --git a/apps/dashboard/src/components/pulls/pull-request-row.tsx b/apps/dashboard/src/components/pulls/pull-request-row.tsx index 7ade4fe..b6a86c3 100644 --- a/apps/dashboard/src/components/pulls/pull-request-row.tsx +++ b/apps/dashboard/src/components/pulls/pull-request-row.tsx @@ -6,12 +6,11 @@ import { GitPullRequestIcon, ViewIcon, } from "@diffkit/icons"; -import { Markdown } from "@diffkit/ui/components/markdown"; import { Spinner } from "@diffkit/ui/components/spinner"; import { cn } from "@diffkit/ui/lib/utils"; import { useQuery } from "@tanstack/react-query"; import { Link, useRouter } from "@tanstack/react-router"; -import { useState } from "react"; +import { lazy, memo, Suspense, useState } from "react"; import { formatRelativeTime } from "#/lib/format-relative-time"; import { type GitHubQueryScope, @@ -20,6 +19,12 @@ import { import type { PullSummary } from "#/lib/github.types"; import { preloadRouteOnce } from "#/lib/route-preload"; +const Markdown = lazy(() => + import("@diffkit/ui/components/markdown").then((mod) => ({ + default: mod.Markdown, + })), +); + function getPrStateProps(pr: PullSummary) { if (pr.isDraft) { return { icon: GitPullRequestDraftIcon, color: "text-muted-foreground" }; @@ -33,7 +38,7 @@ function getPrStateProps(pr: PullSummary) { return { icon: GitPullRequestIcon, color: "text-green-500" }; } -export function PullRequestRow({ +export const PullRequestRow = memo(function PullRequestRow({ pr, scope, }: { @@ -149,9 +154,17 @@ export function PullRequestRow({
- - {comment.body} - + + {comment.body.slice(0, 200)} +

+ } + > + + {comment.body} + +
))} @@ -161,4 +174,4 @@ export function PullRequestRow({ )} ); -} +}); diff --git a/apps/dashboard/src/components/pulls/review/review-diff-pane.tsx b/apps/dashboard/src/components/pulls/review/review-diff-pane.tsx index afd66df..daec247 100644 --- a/apps/dashboard/src/components/pulls/review/review-diff-pane.tsx +++ b/apps/dashboard/src/components/pulls/review/review-diff-pane.tsx @@ -169,7 +169,13 @@ export const ReviewDiffPane = memo( element.scrollIntoView({ block: "start" }); const filename = element.getAttribute("data-filename"); - if (filename) onActiveFileChange(filename); + if (filename) { + onActiveFileChange(filename); + const hash = `#${encodeFileId(filename)}`; + if (window.location.hash !== hash) { + history.replaceState(null, "", hash); + } + } setScrollTarget(null); }); return () => cancelAnimationFrame(frameId); @@ -244,10 +250,6 @@ export const ReviewDiffPane = memo( const filename = entry.target.getAttribute("data-filename"); if (filename) { onActiveFileChange(filename); - const hash = `#${encodeFileId(filename)}`; - if (window.location.hash !== hash) { - history.replaceState(null, "", hash); - } } } }, diff --git a/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx b/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx index c447ff5..a99be3f 100644 --- a/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx +++ b/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx @@ -30,6 +30,22 @@ type ReviewPatchDiffComponent = ComponentType>; const LARGE_PATCH_CHANGE_THRESHOLD = 400; const LARGE_PATCH_CHAR_THRESHOLD = 24_000; +const DIFF_LINE_HEIGHT = 20; +const DIFF_HUNK_SEPARATOR_HEIGHT = 28; + +function estimateDiffHeight( + patch: string | undefined, + diffStyle: "unified" | "split", +): number { + if (!patch) return 0; + const lines = patch.split("\n").length; + const hunkCount = patch.match(/^@@/gm)?.length ?? 0; + const effectiveLines = + diffStyle === "split" ? Math.ceil(lines * 0.75) : lines; + return ( + effectiveLines * DIFF_LINE_HEIGHT + hunkCount * DIFF_HUNK_SEPARATOR_HEIGHT + ); +} const PatchDiff: LazyExoticComponent = lazy(() => import.meta.env.SSR @@ -174,6 +190,14 @@ export const ReviewFileDiffBlock = memo(function ReviewFileDiffBlock({ isCollapsed={isCollapsed} onToggleCollapse={() => setIsCollapsed(!isCollapsed)} /> + {!isCollapsed && !isNearViewport && ( +
+ )} {!isCollapsed && isNearViewport && (
diff --git a/apps/dashboard/src/components/pulls/review/review-file-tree.tsx b/apps/dashboard/src/components/pulls/review/review-file-tree.tsx index 24ee48b..caa164a 100644 --- a/apps/dashboard/src/components/pulls/review/review-file-tree.tsx +++ b/apps/dashboard/src/components/pulls/review/review-file-tree.tsx @@ -1,18 +1,56 @@ import { FileIcon, FolderIcon } from "@diffkit/icons"; import { cn } from "@diffkit/ui/lib/utils"; -import { useState } from "react"; +import { memo, useCallback, useState, useSyncExternalStore } from "react"; import type { FileTreeNode } from "./review-types"; import { encodeFileId } from "./review-utils"; -export function ReviewFileTreeNode({ +/** + * Lightweight store so that only the old-active and new-active file nodes + * re-render when the active file changes — not the entire tree. + */ +export type ActiveFileStore = { + get: () => string | null; + set: (file: string | null) => void; + subscribe: (listener: () => void) => () => void; +}; + +export function createActiveFileStore( + initial: string | null = null, +): ActiveFileStore { + let value = initial; + const listeners = new Set<() => void>(); + return { + get: () => value, + set: (v) => { + if (v === value) return; + value = v; + for (const l of listeners) l(); + }, + subscribe: (l) => { + listeners.add(l); + return () => listeners.delete(l); + }, + }; +} + +function useIsActiveFile(store: ActiveFileStore, path: string): boolean { + const subscribe = useCallback( + (cb: () => void) => store.subscribe(cb), + [store], + ); + const getSnapshot = useCallback(() => store.get() === path, [store, path]); + return useSyncExternalStore(subscribe, getSnapshot); +} + +export const ReviewFileTreeNode = memo(function ReviewFileTreeNode({ node, depth, - activeFile, + activeFileStore, onFileClick, }: { node: FileTreeNode; depth: number; - activeFile: string | null; + activeFileStore: ActiveFileStore; onFileClick: (path: string) => void; }) { const [isOpen, setIsOpen] = useState(true); @@ -51,7 +89,7 @@ export function ReviewFileTreeNode({ key={child.path} node={child} depth={depth + 1} - activeFile={activeFile} + activeFileStore={activeFileStore} onFileClick={onFileClick} /> ))} @@ -61,7 +99,28 @@ export function ReviewFileTreeNode({ ); } - const isActive = activeFile === node.path; + return ( + + ); +}); + +const FileTreeLeaf = memo(function FileTreeLeaf({ + node, + depth, + activeFileStore, + onFileClick, +}: { + node: FileTreeNode; + depth: number; + activeFileStore: ActiveFileStore; + onFileClick: (path: string) => void; +}) { + const isActive = useIsActiveFile(activeFileStore, node.path); const fileId = encodeFileId(node.path); return ( @@ -96,4 +155,4 @@ export function ReviewFileTreeNode({ )} ); -} +}); diff --git a/apps/dashboard/src/components/pulls/review/review-page.tsx b/apps/dashboard/src/components/pulls/review/review-page.tsx index e4f3380..97afe11 100644 --- a/apps/dashboard/src/components/pulls/review/review-page.tsx +++ b/apps/dashboard/src/components/pulls/review/review-page.tsx @@ -15,10 +15,10 @@ import { import { getRouteApi, Link } from "@tanstack/react-router"; import { lazy, + memo, Suspense, useCallback, useDeferredValue, - useEffect, useMemo, useRef, useState, @@ -31,11 +31,18 @@ import { githubPullReviewCommentsQueryOptions, githubQueryKeys, } from "#/lib/github.query"; -import type { PullReviewComment } from "#/lib/github.types"; -import { useHasMounted } from "#/lib/use-has-mounted"; +import type { + PullDetail, + PullFileSummary, + PullReviewComment, +} from "#/lib/github.types"; import { useRegisterTab } from "#/lib/use-register-tab"; import type { ReviewDiffPaneHandle } from "./review-diff-pane"; -import { ReviewFileTreeNode } from "./review-file-tree"; +import { + type ActiveFileStore, + createActiveFileStore, + ReviewFileTreeNode, +} from "./review-file-tree"; import { ReviewSubmitPopover } from "./review-submit-popover"; import type { ActiveCommentForm, @@ -47,8 +54,9 @@ import { buildFileTree } from "./review-utils"; const routeApi = getRouteApi("/_protected/$owner/$repo/review/$pullId"); const PULL_FILES_PAGE_SIZE = 50; +const reviewDiffPaneImport = import("./review-diff-pane"); const ReviewDiffPane = lazy(() => - import("./review-diff-pane").then((mod) => ({ + reviewDiffPaneImport.then((mod) => ({ default: mod.ReviewDiffPane, })), ); @@ -59,12 +67,12 @@ export function ReviewPage() { const { owner, repo, pullId } = routeApi.useParams(); const pullNumber = Number(pullId); const scope = { userId: user.id }; - const hasMounted = useHasMounted(); const queryClient = useQueryClient(); const input = { owner, repo, pullNumber }; const diffPaneRef = useRef(null); - const [shouldLoadReviewComments, setShouldLoadReviewComments] = - useState(false); + + // Stable store for active file — updates bypass ReviewPage renders entirely + const activeFileStore = useRef(createActiveFileStore(null)).current; const pageQuery = useQuery({ ...githubPullPageQueryOptions(scope, input), @@ -78,10 +86,10 @@ export function ReviewPage() { refetchOnWindowFocus: false, }); + const firstFilesPage = loaderData?.firstFilesPage ?? null; const filesQuery = useInfiniteQuery({ queryKey: githubQueryKeys.pulls.files(scope, input), initialPageParam: 1, - enabled: hasMounted, queryFn: ({ pageParam }) => getPullFiles({ data: { @@ -91,13 +99,22 @@ export function ReviewPage() { }, }), getNextPageParam: (lastPage) => lastPage.nextPage ?? undefined, + ...(firstFilesPage + ? { + initialData: { + pages: [firstFilesPage], + pageParams: [1], + }, + } + : {}), refetchOnMount: false, refetchOnWindowFocus: false, }); + const hasDiffPayload = filesQuery.data !== undefined; const reviewCommentsQuery = useQuery({ ...githubPullReviewCommentsQueryOptions(scope, input), - enabled: shouldLoadReviewComments, + enabled: hasDiffPayload, refetchOnWindowFocus: false, }); @@ -109,7 +126,6 @@ export function ReviewPage() { [filesQuery.data], ); const reviewComments = reviewCommentsQuery.data ?? []; - const hasDiffPayload = filesQuery.data !== undefined; const [diffStyle, setDiffStyle] = useState<"unified" | "split">("unified"); const [pendingComments, setPendingComments] = useState([]); @@ -118,10 +134,7 @@ export function ReviewPage() { const [selectedLines, setSelectedLines] = useState( null, ); - const [activeFile, setActiveFile] = useState(null); - const [fileFilter, setFileFilter] = useState(""); const [isSubmitting, setIsSubmitting] = useState(false); - const deferredFileFilter = useDeferredValue(fileFilter); useRegisterTab( pr @@ -138,34 +151,21 @@ export function ReviewPage() { : null, ); - const fileTree = useMemo(() => buildFileTree(sidebarFiles), [sidebarFiles]); - - const filteredTree = useMemo(() => { - if (!deferredFileFilter) return fileTree; - const lower = deferredFileFilter.toLowerCase(); - - function filterNodes(nodes: FileTreeNode[]): FileTreeNode[] { - return nodes - .map((node) => { - if (node.type === "file") { - return node.name.toLowerCase().includes(lower) ? node : null; - } - - const filteredChildren = filterNodes(node.children); - return filteredChildren.length > 0 - ? { ...node, children: filteredChildren } - : null; - }) - .filter(Boolean) as FileTreeNode[]; - } - - return filterNodes(fileTree); - }, [deferredFileFilter, fileTree]); + // Diff pane → sidebar active file sync (no ReviewPage re-render) + const handleActiveFileChange = useCallback( + (filename: string) => { + activeFileStore.set(filename); + }, + [activeFileStore], + ); - const scrollToFile = useCallback((filename: string) => { - diffPaneRef.current?.scrollToFile(filename); - setActiveFile(filename); - }, []); + const scrollToFile = useCallback( + (filename: string) => { + diffPaneRef.current?.scrollToFile(filename); + activeFileStore.set(filename); + }, + [activeFileStore], + ); const annotationsByFile = useMemo(() => { const map = new Map[]>(); @@ -202,16 +202,6 @@ export function ReviewPage() { return { totalAdditions, totalDeletions }; }, [sidebarFiles]); - useEffect(() => { - if (!hasMounted || !hasDiffPayload || shouldLoadReviewComments) return; - - const timeoutId = window.setTimeout(() => { - setShouldLoadReviewComments(true); - }, 250); - - return () => window.clearTimeout(timeoutId); - }, [hasDiffPayload, hasMounted, shouldLoadReviewComments]); - const addPendingComment = useCallback((comment: PendingComment) => { setPendingComments((previous) => [...previous, comment]); setActiveCommentForm(null); @@ -299,6 +289,16 @@ export function ReviewPage() { [owner, pendingComments, pullNumber, queryClient, repo], ); + // Ref-stable onLoadMore — avoids busting ReviewDiffPane memo + const filesQueryRef = useRef(filesQuery); + filesQueryRef.current = filesQuery; + const handleLoadMore = useCallback(() => { + const q = filesQueryRef.current; + if (q.hasNextPage && !q.isFetchingNextPage) { + void q.fetchNextPage(); + } + }, []); + if (pageQuery.error) throw pageQuery.error; if (fileSummariesQuery.error) throw fileSummariesQuery.error; if (filesQuery.error) throw filesQuery.error; @@ -312,130 +312,38 @@ export function ReviewPage() { ); } - const stateConfig = getPrStateConfig(pr); - const StateIcon = stateConfig.icon; const sidebarFileCount = sidebarFiles.length; return (
-
- - - #{pr.number} - - -
- -
-
- -
- {pr.title} -
- -
-
- - - - {sidebarFileCount} - {" "} - {sidebarFileCount === 1 ? "file" : "files"} - - - +{diffStats.totalAdditions} - - - -{diffStats.totalDeletions} - -
- -
- -
- - -
- -
- - -
-
+ -
-
-
- - setFileFilter(event.target.value)} - className="ml-2 w-full bg-transparent text-xs outline-none placeholder:text-muted-foreground" - /> -
-
- -
- {filteredTree.map((node) => ( - - ))} -
- -
- {sidebarFileCount} {sidebarFileCount === 1 ? "file" : "files"}{" "} - changed -
-
+ - {hasMounted && hasDiffPayload ? ( + {hasDiffPayload ? ( }> { - if ( - filesQuery.hasNextPage && - !filesQuery.isFetchingNextPage - ) { - void filesQuery.fetchNextPage(); - } - }} + onLoadMore={handleLoadMore} activeCommentForm={activeCommentForm} selectedLines={selectedLines} - onActiveFileChange={setActiveFile} + onActiveFileChange={handleActiveFileChange} onStartComment={handleStartComment} onCancelComment={handleCancelComment} onAddComment={handleAddComment} @@ -471,6 +372,196 @@ export function ReviewPage() { ); } +// --------------------------------------------------------------------------- +// ReviewToolbar — memoized, only re-renders when its own props change +// --------------------------------------------------------------------------- + +const ReviewToolbar = memo(function ReviewToolbar({ + owner, + repo, + pullId, + pr, + sidebarFileCount, + diffStats, + diffStyle, + onSetDiffStyle, + pendingCount, + isSubmitting, + onSubmitReview, +}: { + owner: string; + repo: string; + pullId: string; + pr: PullDetail; + sidebarFileCount: number; + diffStats: { totalAdditions: number; totalDeletions: number }; + diffStyle: "unified" | "split"; + onSetDiffStyle: (style: "unified" | "split") => void; + pendingCount: number; + isSubmitting: boolean; + onSubmitReview: (body: string, event: ReviewEvent) => Promise; +}) { + const stateConfig = getPrStateConfig(pr); + const StateIcon = stateConfig.icon; + + return ( +
+ + + #{pr.number} + + +
+ +
+
+ +
+ {pr.title} +
+ +
+
+ + + + {sidebarFileCount} + {" "} + {sidebarFileCount === 1 ? "file" : "files"} + + + +{diffStats.totalAdditions} + + + -{diffStats.totalDeletions} + +
+ +
+ +
+ + +
+ +
+ + +
+
+ ); +}); + +// --------------------------------------------------------------------------- +// ReviewSidebar — owns file filter state, reads activeFile from store +// --------------------------------------------------------------------------- + +const ReviewSidebar = memo(function ReviewSidebar({ + sidebarFiles, + sidebarFileCount, + activeFileStore, + onFileClick, +}: { + sidebarFiles: PullFileSummary[]; + sidebarFileCount: number; + activeFileStore: ActiveFileStore; + onFileClick: (path: string) => void; +}) { + const [fileFilter, setFileFilter] = useState(""); + const deferredFileFilter = useDeferredValue(fileFilter); + + const fileTree = useMemo(() => buildFileTree(sidebarFiles), [sidebarFiles]); + + const filteredTree = useMemo(() => { + if (!deferredFileFilter) return fileTree; + const lower = deferredFileFilter.toLowerCase(); + + function filterNodes(nodes: FileTreeNode[]): FileTreeNode[] { + return nodes + .map((node) => { + if (node.type === "file") { + return node.name.toLowerCase().includes(lower) ? node : null; + } + + const filteredChildren = filterNodes(node.children); + return filteredChildren.length > 0 + ? { ...node, children: filteredChildren } + : null; + }) + .filter(Boolean) as FileTreeNode[]; + } + + return filterNodes(fileTree); + }, [deferredFileFilter, fileTree]); + + return ( +
+
+
+ + setFileFilter(event.target.value)} + className="ml-2 w-full bg-transparent text-xs outline-none placeholder:text-muted-foreground" + /> +
+
+ +
+ {filteredTree.map((node) => ( + + ))} +
+ +
+ {sidebarFileCount} {sidebarFileCount === 1 ? "file" : "files"} changed +
+
+ ); +}); + function ReviewDiffPanePlaceholder() { return
; } diff --git a/apps/dashboard/src/routes/__root.tsx b/apps/dashboard/src/routes/__root.tsx index f240664..30bff74 100644 --- a/apps/dashboard/src/routes/__root.tsx +++ b/apps/dashboard/src/routes/__root.tsx @@ -8,13 +8,17 @@ import { Scripts, } from "@tanstack/react-router"; import { TanStackRouterDevtoolsPanel } from "@tanstack/react-router-devtools"; -import { Agentation } from "agentation"; import { ThemeProvider } from "next-themes"; import { NuqsAdapter } from "nuqs/adapters/tanstack-router"; +import { lazy, Suspense } from "react"; import { ErrorScreen } from "#/components/layouts/error-screen"; import { buildSeo, buildWebSiteSchema } from "#/lib/seo"; import { siteConfig } from "#/lib/site-config"; +const Agentation = lazy(() => + import("agentation").then((mod) => ({ default: mod.Agentation })), +); + import appCss from "../styles.css?url"; export const Route = createRootRouteWithContext<{ @@ -78,7 +82,11 @@ function RootDocument({ children }: { children: React.ReactNode }) { {children} - {import.meta.env.DEV ? : null} + {import.meta.env.DEV ? ( + + + + ) : null} { @@ -18,15 +22,33 @@ export const Route = createFileRoute("/_protected/$owner/$repo/review/$pullId")( input, ); - const pageData = - context.queryClient.getQueryData(pageOptions.queryKey) ?? - (await context.queryClient.ensureQueryData(pageOptions)); + const cachedPageData = context.queryClient.getQueryData( + pageOptions.queryKey, + ); + const cachedFileSummaries = context.queryClient.getQueryData( + fileSummariesOptions.queryKey, + ); + + // Check if infinite query already has data + const filesQueryKey = githubQueryKeys.pulls.files(scope, input); + const cachedFilesData = context.queryClient.getQueryData(filesQueryKey); - const fileSummaries = - context.queryClient.getQueryData(fileSummariesOptions.queryKey) ?? - (await context.queryClient.ensureQueryData(fileSummariesOptions)); + const [pageData, fileSummaries, firstFilesPage] = await Promise.all([ + cachedPageData ?? context.queryClient.ensureQueryData(pageOptions), + cachedFileSummaries ?? + context.queryClient.ensureQueryData(fileSummariesOptions), + cachedFilesData + ? null + : getPullFiles({ + data: { + ...input, + page: 1, + perPage: PULL_FILES_PAGE_SIZE, + }, + }), + ]); - return { pageData, fileSummaries }; + return { pageData, fileSummaries, firstFilesPage }; }, head: ({ loaderData, match, params }) => { const pull = loaderData?.pageData?.detail; diff --git a/apps/dashboard/src/routes/_protected/issues.tsx b/apps/dashboard/src/routes/_protected/issues.tsx index 632ebe8..794bbf1 100644 --- a/apps/dashboard/src/routes/_protected/issues.tsx +++ b/apps/dashboard/src/routes/_protected/issues.tsx @@ -4,6 +4,7 @@ import { useQuery } from "@tanstack/react-query"; import { createFileRoute } from "@tanstack/react-router"; import { type ComponentType, + memo, type RefObject, useEffect, useRef, @@ -126,7 +127,7 @@ type IssueGroupData = { const ISSUE_GROUP_STICKY_TOP = -32; -function IssueMetricCard({ +const IssueMetricCard = memo(function IssueMetricCard({ href, icon: Icon, label, @@ -168,12 +169,12 @@ function IssueMetricCard({ {content} ); -} +}); -function IssueGroup({ +const IssueGroup = memo(function IssueGroup({ id, title, - icon: Icon, + icon, issues, scrollContainerRef, }: { @@ -184,6 +185,50 @@ function IssueGroup({ scrollContainerRef: RefObject; }) { const sectionRef = useRef(null); + + return ( +
+ + {issues.length > 0 && ( +
+ {issues.map((issue) => ( + + ))} +
+ )} +
+ ); +}); + +function StickyGroupHeader({ + sectionRef, + scrollContainerRef, + stickyTop: stickyTopOffset, + icon: Icon, + title, + count, + isEmpty, +}: { + sectionRef: RefObject; + scrollContainerRef: RefObject; + stickyTop: number; + icon: ComponentType<{ size?: number; strokeWidth?: number }>; + title: string; + count: number; + isEmpty: boolean; +}) { const headerRef = useRef(null); const [isStickyActive, setIsStickyActive] = useState(false); @@ -199,7 +244,7 @@ function IssueGroup({ const updateStickyState = () => { const scrollContainerRect = scrollContainer.getBoundingClientRect(); const sectionRect = section.getBoundingClientRect(); - const stickyTop = scrollContainerRect.top + ISSUE_GROUP_STICKY_TOP; + const stickyTop = scrollContainerRect.top + stickyTopOffset; const headerHeight = header.offsetHeight; const isStuck = sectionRect.top <= stickyTop && @@ -218,35 +263,26 @@ function IssueGroup({ scrollContainer.removeEventListener("scroll", updateStickyState); window.removeEventListener("resize", updateStickyState); }; - }, [scrollContainerRef]); + }, [scrollContainerRef, sectionRef, stickyTopOffset]); return ( -
-
-
-
- -
-

{title}

+
+
+
+
- - {issues.length} - +

{title}

- {issues.length > 0 && ( -
- {issues.map((issue) => ( - - ))} -
- )} -
+ + {count} + +
); } diff --git a/apps/dashboard/src/routes/_protected/pulls.tsx b/apps/dashboard/src/routes/_protected/pulls.tsx index b8ab169..5bb7845 100644 --- a/apps/dashboard/src/routes/_protected/pulls.tsx +++ b/apps/dashboard/src/routes/_protected/pulls.tsx @@ -10,6 +10,7 @@ import { useQuery } from "@tanstack/react-query"; import { createFileRoute } from "@tanstack/react-router"; import { type ComponentType, + memo, type RefObject, useEffect, useRef, @@ -148,7 +149,7 @@ type PullGroupData = { const PULL_GROUP_STICKY_TOP = -32; -function PullMetricCard({ +const PullMetricCard = memo(function PullMetricCard({ href, icon: Icon, label, @@ -190,12 +191,12 @@ function PullMetricCard({ {content} ); -} +}); -function PullGroup({ +const PullGroup = memo(function PullGroup({ id, title, - icon: Icon, + icon, pulls, scope, scrollContainerRef, @@ -208,6 +209,50 @@ function PullGroup({ scrollContainerRef: RefObject; }) { const sectionRef = useRef(null); + + return ( +
+ + {pulls.length > 0 && ( +
+ {pulls.map((pull) => ( + + ))} +
+ )} +
+ ); +}); + +function StickyGroupHeader({ + sectionRef, + scrollContainerRef, + stickyTop: stickyTopOffset, + icon: Icon, + title, + count, + isEmpty, +}: { + sectionRef: RefObject; + scrollContainerRef: RefObject; + stickyTop: number; + icon: ComponentType<{ size?: number; strokeWidth?: number }>; + title: string; + count: number; + isEmpty: boolean; +}) { const headerRef = useRef(null); const [isStickyActive, setIsStickyActive] = useState(false); @@ -223,7 +268,7 @@ function PullGroup({ const updateStickyState = () => { const scrollContainerRect = scrollContainer.getBoundingClientRect(); const sectionRect = section.getBoundingClientRect(); - const stickyTop = scrollContainerRect.top + PULL_GROUP_STICKY_TOP; + const stickyTop = scrollContainerRect.top + stickyTopOffset; const headerHeight = header.offsetHeight; const isStuck = sectionRect.top <= stickyTop && @@ -242,35 +287,26 @@ function PullGroup({ scrollContainer.removeEventListener("scroll", updateStickyState); window.removeEventListener("resize", updateStickyState); }; - }, [scrollContainerRef]); + }, [scrollContainerRef, sectionRef, stickyTopOffset]); return ( -
-
-
-
- -
-

{title}

+
+
+
+
- - {pulls.length} - +

{title}

- {pulls.length > 0 && ( -
- {pulls.map((pull) => ( - - ))} -
- )} -
+ + {count} + +
); } From 518122a087f2a6642ce887fabc1e8bd045e184ae Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Fri, 10 Apr 2026 15:28:42 -0400 Subject: [PATCH 2/3] Reduce diff files page size from 50 to 25 for faster initial load --- apps/dashboard/src/components/pulls/review/review-page.tsx | 2 +- .../src/routes/_protected/$owner/$repo/review.$pullId.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dashboard/src/components/pulls/review/review-page.tsx b/apps/dashboard/src/components/pulls/review/review-page.tsx index 97afe11..3f4d34d 100644 --- a/apps/dashboard/src/components/pulls/review/review-page.tsx +++ b/apps/dashboard/src/components/pulls/review/review-page.tsx @@ -53,7 +53,7 @@ import type { import { buildFileTree } from "./review-utils"; const routeApi = getRouteApi("/_protected/$owner/$repo/review/$pullId"); -const PULL_FILES_PAGE_SIZE = 50; +const PULL_FILES_PAGE_SIZE = 25; const reviewDiffPaneImport = import("./review-diff-pane"); const ReviewDiffPane = lazy(() => reviewDiffPaneImport.then((mod) => ({ diff --git a/apps/dashboard/src/routes/_protected/$owner/$repo/review.$pullId.tsx b/apps/dashboard/src/routes/_protected/$owner/$repo/review.$pullId.tsx index 70ab11c..d95bd50 100644 --- a/apps/dashboard/src/routes/_protected/$owner/$repo/review.$pullId.tsx +++ b/apps/dashboard/src/routes/_protected/$owner/$repo/review.$pullId.tsx @@ -8,7 +8,7 @@ import { } from "#/lib/github.query"; import { buildSeo, formatPageTitle, summarizeText } from "#/lib/seo"; -const PULL_FILES_PAGE_SIZE = 50; +const PULL_FILES_PAGE_SIZE = 25; export const Route = createFileRoute("/_protected/$owner/$repo/review/$pullId")( { From 1853fd654d4c1430b69a74fc0a9264ae93b750ca Mon Sep 17 00:00:00 2001 From: Alan Daniel Date: Fri, 10 Apr 2026 15:35:46 -0400 Subject: [PATCH 3/3] Isolate merge status polling from PullDetailPage to prevent full-page re-renders Move statusQuery into a dedicated MergeStatusSection component so polling only re-renders the merge card, not the header, body, timeline, or sidebar. Also guard Shiki theme registration to prevent duplicate registration warnings. --- .../pulls/detail/pull-detail-activity.tsx | 60 +++++++++++++++---- .../pulls/detail/pull-detail-page.tsx | 12 +--- .../pulls/review/review-file-diff-block.tsx | 4 +- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx index 3287835..04e7397 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-activity.tsx @@ -24,7 +24,7 @@ import { Markdown } from "@diffkit/ui/components/markdown"; import { Skeleton } from "@diffkit/ui/components/skeleton"; import { toast } from "@diffkit/ui/components/sonner"; import { cn } from "@diffkit/ui/lib/utils"; -import { useQueryClient } from "@tanstack/react-query"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; import { DetailActivityHeader, @@ -37,6 +37,10 @@ import { requestPullReviewers, updatePullBranch, } from "#/lib/github.functions"; +import { + type GitHubQueryScope, + githubPullStatusQueryOptions, +} from "#/lib/github.query"; import type { PullCheckRun, PullComment, @@ -44,26 +48,27 @@ import type { PullDetail, PullStatus, } from "#/lib/github.types"; +import { githubCachePolicy } from "#/lib/github-cache-policy"; import { checkPermissionWarning } from "#/lib/warning-store"; export function PullDetailActivitySection({ comments, commits, isFetching, - status, pr, owner, repo, pullNumber, + scope, }: { comments?: PullComment[]; commits?: PullCommit[]; isFetching: boolean; - status: PullStatus | null; pr: PullDetail; owner: string; repo: string; pullNumber: number; + scope: GitHubQueryScope; }) { return (
@@ -112,16 +117,12 @@ export function PullDetailActivitySection({ {!pr.isMerged && pr.state !== "closed" && (
- {status ? ( - - ) : ( - - )} +
)} @@ -132,6 +133,39 @@ export function PullDetailActivitySection({ ); } +// ── Merge status section — owns its own polling query ──────────────── + +function MergeStatusSection({ + scope, + owner, + repo, + pullNumber, +}: { + scope: GitHubQueryScope; + owner: string; + repo: string; + pullNumber: number; +}) { + const statusQuery = useQuery({ + ...githubPullStatusQueryOptions(scope, { owner, repo, pullNumber }), + refetchOnWindowFocus: "always", + refetchInterval: githubCachePolicy.status.staleTimeMs, + }); + + const status = statusQuery.data ?? null; + + if (!status) return ; + + return ( + + ); +} + function MergeStatusCard({ status, owner, diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-page.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-page.tsx index fb8f15e..d53ec15 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-page.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-page.tsx @@ -7,10 +7,8 @@ import { } from "#/components/details/detail-page"; import { githubPullPageQueryOptions, - githubPullStatusQueryOptions, githubViewerQueryOptions, } from "#/lib/github.query"; -import { githubCachePolicy } from "#/lib/github-cache-policy"; import { useHasMounted } from "#/lib/use-has-mounted"; import { useRegisterTab } from "#/lib/use-register-tab"; import { PullBodySection } from "./pull-body-section"; @@ -32,13 +30,6 @@ export function PullDetailPage() { enabled: hasMounted, }); - const statusQuery = useQuery({ - ...githubPullStatusQueryOptions(scope, { owner, repo, pullNumber }), - enabled: hasMounted && pageQuery.data?.detail != null, - refetchOnWindowFocus: "always", - refetchInterval: githubCachePolicy.status.staleTimeMs, - }); - const viewerQuery = useQuery({ ...githubViewerQueryOptions(scope), enabled: hasMounted, @@ -47,7 +38,6 @@ export function PullDetailPage() { const pr = pageQuery.data?.detail; const comments = pageQuery.data?.comments; const commits = pageQuery.data?.commits; - const status = statusQuery.data ?? null; const viewer = viewerQuery.data ?? null; useRegisterTab( @@ -91,11 +81,11 @@ export function PullDetailPage() { comments={comments} commits={commits} isFetching={pageQuery.isFetching} - status={status} pr={pr} owner={owner} repo={repo} pullNumber={pullNumber} + scope={scope} /> } diff --git a/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx b/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx index a99be3f..d0bdc33 100644 --- a/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx +++ b/apps/dashboard/src/components/pulls/review/review-file-diff-block.tsx @@ -57,7 +57,9 @@ const PatchDiff: LazyExoticComponent = lazy(() => })), ); -if (!import.meta.env.SSR) { +let themesRegistered = false; +if (!import.meta.env.SSR && !themesRegistered) { + themesRegistered = true; import("@pierre/diffs").then(({ registerCustomTheme }) => { registerCustomTheme("vercel-light", () => Promise.resolve(vercelLight)); registerCustomTheme("vercel-dark", () => Promise.resolve(vercelDark));