diff --git a/apps/desktop/src/app/right-sidebar/review/file-tree.test.tsx b/apps/desktop/src/app/right-sidebar/review/file-tree.test.tsx new file mode 100644 index 0000000000000..3e8f4cf126c72 --- /dev/null +++ b/apps/desktop/src/app/right-sidebar/review/file-tree.test.tsx @@ -0,0 +1,130 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { HermesReviewFile } from '@/global' +import { I18nProvider } from '@/i18n' +import { $sidebarWorkspaceNodeOpen } from '@/store/layout' +import { $reviewFiles, $reviewOpen } from '@/store/review' + +import { ReviewFileTree } from './file-tree' + +const ROW_HEIGHT = 24 +const VIEWPORT_HEIGHT = 600 + +const file = (path: string): HermesReviewFile => ({ + added: 1, + path, + removed: 0, + staged: false, + status: '?' +}) + +// The issue's repro shape: a .NET publish/ folder with tens of thousands of +// untracked files and no .gitignore. +function filesUnderPublish(count: number): HermesReviewFile[] { + return Array.from({ length: count }, (_, i) => file(`publish/file-${String(i).padStart(4, '0')}.so`)) +} + +function topLevelFiles(count: number): HermesReviewFile[] { + return Array.from({ length: count }, (_, i) => file(`file-${String(i).padStart(4, '0')}.ts`)) +} + +function renderTree() { + return render( + + + + ) +} + +describe('ReviewFileTree', () => { + beforeEach(() => { + $reviewOpen.set(true) + $reviewFiles.set([]) + $sidebarWorkspaceNodeOpen.set({}) + + // jsdom has no layout: report the real row height for virtualized rows and + // a viewport for the scroller so the virtualizer mounts a deterministic + // window (instead of measuring everything as 0px). + vi.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockImplementation(function (this: HTMLElement) { + if (this.hasAttribute?.('data-index')) { + return ROW_HEIGHT + } + + if (this.hasAttribute?.('data-suppress-pane-reveal-side')) { + return VIEWPORT_HEIGHT + } + + return 0 + }) + vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(() => 240) + + // The virtualizer observes the scroller and rows; jsdom ships no observer, + // so install a no-op one (initialRect + fixed row heights drive the mount). + vi.stubGlobal( + 'ResizeObserver', + class { + constructor(_callback: ResizeObserverCallback) {} + disconnect = vi.fn() + observe = vi.fn() + unobserve = vi.fn() + } as unknown as typeof ResizeObserver + ) + }) + + afterEach(() => { + cleanup() + vi.unstubAllGlobals() + vi.restoreAllMocks() + $reviewFiles.set([]) + $sidebarWorkspaceNodeOpen.set({}) + $reviewOpen.set(false) + }) + + it('virtualizes heavy trees: only the visible window is mounted', () => { + $reviewFiles.set(topLevelFiles(5000)) + + const { container } = renderTree() + + const mounted = container.querySelectorAll('[data-index]') + expect(mounted.length).toBeGreaterThan(0) + // 5,000 files would be ~5,000 rows in the DOM without virtualization. + expect(mounted.length).toBeLessThan(100) + + // The scroller still accounts for the full 5,000 × 24px list height. + const spacer = container.querySelector('[style*="120000px"]') + expect(spacer).not.toBeNull() + + // The window starts at the top, so the first rows are mounted. + expect(screen.getByText('file-0000.ts')).toBeTruthy() + }) + + it('starts heavy trees collapsed and reveals children on expand, still bounded', () => { + $reviewFiles.set(filesUnderPublish(5000)) + + const { container } = renderTree() + + // Collapsed by default: just the publish/ folder row, no file rows yet. + expect(screen.getByText('publish')).toBeTruthy() + expect(container.querySelectorAll('[data-index]').length).toBe(1) + + fireEvent.click(screen.getByText('publish')) + + // Children appear as virtualized rows — a handful, not 5,000. + const mounted = container.querySelectorAll('[data-index]') + expect(mounted.length).toBeGreaterThan(1) + expect(mounted.length).toBeLessThan(100) + expect(screen.getByText('file-0000.so')).toBeTruthy() + }) + + it('keeps rendering small trees in full (animated path untouched)', () => { + $reviewFiles.set([file('a.ts'), file('b.ts'), file('src/c.ts')]) + + renderTree() + + expect(screen.getByText('a.ts')).toBeTruthy() + expect(screen.getByText('b.ts')).toBeTruthy() + expect(screen.getByText('src')).toBeTruthy() + expect(screen.getByText('c.ts')).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/app/right-sidebar/review/file-tree.tsx b/apps/desktop/src/app/right-sidebar/review/file-tree.tsx index d9be7d3f7d295..ebb01c4bdda7c 100644 --- a/apps/desktop/src/app/right-sidebar/review/file-tree.tsx +++ b/apps/desktop/src/app/right-sidebar/review/file-tree.tsx @@ -1,6 +1,15 @@ import { useStore } from '@nanostores/react' +import { useVirtualizer } from '@tanstack/react-virtual' import { AnimatePresence, motion } from 'motion/react' -import { type CSSProperties, type ReactNode, useEffect, useMemo, useRef, useState } from 'react' +import { + type CSSProperties, + type ReactNode, + type RefObject, + useEffect, + useMemo, + useRef, + useState +} from 'react' import { Button } from '@/components/ui/button' import { Codicon } from '@/components/ui/codicon' @@ -40,7 +49,14 @@ import { $currentCwd } from '@/store/session' import { pickRevealLabel } from '../file-actions' -import { buildReviewFlatList, buildReviewTree, type ReviewTreeNode } from './tree-data' +import { + buildReviewFlatList, + buildReviewTree, + countAllNodes, + flattenReviewRows, + type ReviewFlatRow, + type ReviewTreeNode +} from './tree-data' const INDENT = 12 @@ -78,14 +94,19 @@ const ROW_TRANSITION = { type: 'spring', stiffness: 1100, damping: 48, mass: 0.3 // batch of rows doesn't fly in. const ROW_INSTANT = { duration: 0 } as const -// Past this many changed files, drop the per-row motion (AnimatePresence + -// layout springs on every node is the heaviest cost) and lean on CSS -// content-visibility so off-screen rows skip layout/paint. +// Past this many visible rows, drop the animated list and virtualize: only the +// rows in the viewport (plus a small overscan) are mounted, so a folder with +// tens of thousands of untracked files no longer balloons the renderer into +// hundreds of thousands of DOM nodes. const HEAVY_LIST_CAP = 60 -// Reserve a stable row height (h-6 = 1.5rem) so the scrollbar stays correct -// while off-screen rows are skipped. -const ROW_CV_STYLE: CSSProperties = { containIntrinsicSize: 'auto 1.5rem', contentVisibility: 'auto' } +// Uniform row height (h-6 = 1.5rem). Every review row is this exact height, so +// the virtualizer's size estimate is exact and per-row measurement just keeps +// it honest under app zoom. +const ROW_HEIGHT = 24 + +// Rows mounted above and below the viewport while scrolling. +const OVERSCAN_ROWS = 12 export function ReviewFileTree() { const files = useStore($reviewFiles) @@ -95,7 +116,25 @@ export function ReviewFileTree() { const tree = useMemo(() => (mode === 'tree' ? buildReviewTree(files) : buildReviewFlatList(files)), [files, mode]) - const heavy = tree.length > HEAVY_LIST_CAP + // Heavy is decided by the TOTAL node count, not the top-level row count: the + // classic blow-up is ONE folder holding tens of thousands of untracked files, + // which is a single top-level node but must still take the virtualized path. + const heavy = useMemo(() => countAllNodes(tree) > HEAVY_LIST_CAP, [tree]) + + // Visible rows for the virtualized path. Heavy trees start fully collapsed + // (folders default closed) so even the first mount is a handful of rows; the + // user expands a folder to reveal its children, still virtualized. + const nodeOpen = useStore($sidebarWorkspaceNodeOpen) + + const rows = useMemo(() => { + if (!heavy) { + return [] + } + + return flattenReviewRows(tree, id => nodeOpen[`review:${id}`] ?? false) + }, [heavy, nodeOpen, tree]) + + const scrollerRef = useRef(null) // The Pane keeps this tree mounted while collapsed, so opening it doesn't // remount (AnimatePresence `initial={false}` can't help). The first refresh @@ -124,40 +163,21 @@ export function ReviewFileTree() { }, [open, loading]) return ( -
- +
+ {heavy ? ( + + ) : ( + + )}
) } -function ReviewNodeList({ - animate, - depth, - motion: useMotion, - nodes -}: { - animate: boolean - depth: number - motion: boolean - nodes: ReviewTreeNode[] -}) { - // Heavy lists: plain rows + content-visibility, no motion. - if (!useMotion) { - return ( - <> - {nodes.map(node => ( -
- {node.isDir ? ( - - ) : ( - - )} -
- ))} - - ) - } - +function ReviewNodeList({ animate, depth, nodes }: { animate: boolean; depth: number; nodes: ReviewTreeNode[] }) { return ( {nodes.map(node => ( @@ -170,7 +190,7 @@ function ReviewNodeList({ transition={animate ? ROW_TRANSITION : ROW_INSTANT} > {node.isDir ? ( - + ) : ( )} @@ -180,6 +200,66 @@ function ReviewNodeList({ ) } +// Virtualized heavy list: the scroller mounts only the rows intersecting the +// viewport (plus overscan), so a folder with tens of thousands of changed +// files never materializes every row in the DOM. Rows are absolutely +// positioned inside a spacer sized to the full list, which keeps the +// scrollbar honest. Folders render as `leaf` rows — their children come from +// the flattened row list, not inline — so expanding one just grows the list. +function VirtualizedReviewList({ + rows, + scrollRef +}: { + rows: ReviewFlatRow[] + scrollRef: RefObject +}) { + const virtualizer = useVirtualizer({ + count: rows.length, + estimateSize: () => ROW_HEIGHT, + getItemKey: index => rows[index]?.node.id ?? index, + getScrollElement: () => scrollRef.current, + // jsdom-friendly default; the real rect takes over on first observe. + initialRect: { height: 600, width: 240 }, + overscan: OVERSCAN_ROWS + }) + + const virtualItems = virtualizer.getVirtualItems() + const totalSize = virtualizer.getTotalSize() + + return ( +
+ {virtualItems.map(virtualItem => { + const row = rows[virtualItem.index] + + if (!row) { + return null + } + + return ( +
+ {row.node.isDir ? ( + + ) : ( + + )} +
+ ) + })} +
+ ) +} + // Depth-0 rows align their icon to the panel header's dither glyph: the tree // body has px-1 (4px) and the header glyph sits at px-2.5 (10px) + the label's // pl-2 (8px) = 18px, so the base inset is 18 − 4 = 14px. @@ -191,19 +271,22 @@ function rowStyle(depth: number): CSSProperties { function ReviewDirRow({ animate, + defaultOpen = true, depth, - motion: useMotion, + leaf = false, node }: { animate: boolean + defaultOpen?: boolean depth: number - motion: boolean + /** Virtualized rows render their children from the flattened row list, not inline. */ + leaf?: boolean node: ReviewTreeNode }) { const nodeOpen = useStore($sidebarWorkspaceNodeOpen) const id = `review:${node.id}` - const open = nodeOpen[id] ?? true - const toggle = () => toggleWorkspaceNodeCollapsed(id) + const open = nodeOpen[id] ?? defaultOpen + const toggle = () => toggleWorkspaceNodeCollapsed(id, defaultOpen) return ( <> @@ -222,8 +305,8 @@ function ReviewDirRow({ {!open && }
- {open && node.children && ( - + {!leaf && open && node.children && ( + )} ) diff --git a/apps/desktop/src/app/right-sidebar/review/tree-data.test.ts b/apps/desktop/src/app/right-sidebar/review/tree-data.test.ts index 95751f3d68a5c..583f0d05b6d23 100644 --- a/apps/desktop/src/app/right-sidebar/review/tree-data.test.ts +++ b/apps/desktop/src/app/right-sidebar/review/tree-data.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import type { HermesReviewFile } from '@/global' -import { buildReviewTree } from './tree-data' +import { buildReviewTree, countAllNodes, flattenReviewRows } from './tree-data' const file = (path: string, added = 1, removed = 0): HermesReviewFile => ({ path, @@ -43,3 +43,66 @@ describe('buildReviewTree', () => { expect(tree[0].children?.map(n => n.name).sort()).toEqual(['b', 'other.ts']) }) }) + +describe('countAllNodes', () => { + it('counts every node including descendants', () => { + const tree = buildReviewTree([file('src/a.ts'), file('src/b.ts'), file('readme.md')], false) + + // src + its two files + readme. + expect(countAllNodes(tree)).toBe(4) + }) + + it('counts one per top-level leaf', () => { + const tree = buildReviewTree([file('a.ts'), file('b.ts')], false) + + expect(countAllNodes(tree)).toBe(2) + }) + + it('counts a single deep folder holding thousands of files as heavy', () => { + const files = Array.from({ length: 40_000 }, (_, i) => file(`publish/lib-${i}.so`)) + const tree = buildReviewTree(files) + + // One top-level node, but the total is what matters for virtualization. + expect(tree.length).toBe(1) + expect(countAllNodes(tree)).toBe(40_001) + }) +}) + +describe('flattenReviewRows', () => { + it('flattens top-level leaves in order', () => { + const tree = buildReviewTree([file('b.ts'), file('a.ts')], false) + + const rows = flattenReviewRows(tree, () => true) + + expect(rows.map(r => r.node.id)).toEqual(['a.ts', 'b.ts']) + expect(rows.map(r => r.depth)).toEqual([0, 0]) + }) + + it('includes a directory children only while it is open', () => { + const tree = buildReviewTree([file('src/a.ts'), file('readme.md')], false) + + const collapsed = flattenReviewRows(tree, () => false) + expect(collapsed.map(r => r.node.id)).toEqual(['src', 'readme.md']) + + const expanded = flattenReviewRows(tree, () => true) + expect(expanded.map(r => r.node.id)).toEqual(['src', 'src/a.ts', 'readme.md']) + expect(expanded[1].depth).toBe(1) + }) + + it('recurses into nested open directories with increasing depth', () => { + const tree = buildReviewTree([file('a/b/c.ts')], false) + + const rows = flattenReviewRows(tree, () => true) + + expect(rows.map(r => r.node.id)).toEqual(['a', 'a/b', 'a/b/c.ts']) + expect(rows.map(r => r.depth)).toEqual([0, 1, 2]) + }) + + it('stops descending into a collapsed nested directory', () => { + const tree = buildReviewTree([file('a/b/c.ts')], false) + + const rows = flattenReviewRows(tree, id => id !== 'a/b') + + expect(rows.map(r => r.node.id)).toEqual(['a', 'a/b']) + }) +}) diff --git a/apps/desktop/src/app/right-sidebar/review/tree-data.ts b/apps/desktop/src/app/right-sidebar/review/tree-data.ts index 97a39cb081c29..ba4e348b35906 100644 --- a/apps/desktop/src/app/right-sidebar/review/tree-data.ts +++ b/apps/desktop/src/app/right-sidebar/review/tree-data.ts @@ -124,3 +124,50 @@ export function buildReviewTree(files: HermesReviewFile[], compact = true): Revi return finalize(root) } + +// A row in the virtualized review list: the node plus the indentation depth it +// renders at (directory nesting level). +export interface ReviewFlatRow { + node: ReviewTreeNode + depth: number +} + +// Total node count including every descendant — the cheap upper bound used to +// decide whether the tree needs virtualization. A single folder holding tens of +// thousands of untracked files is one top-level node but must still count as +// heavy. +export function countAllNodes(nodes: ReviewTreeNode[]): number { + let total = 0 + + for (const node of nodes) { + total += 1 + + if (node.children) { + total += countAllNodes(node.children) + } + } + + return total +} + +// Flatten the tree into the rows currently visible: a directory contributes +// its children only while open (per `isOpen`, which receives node ids), and +// every row carries its nesting depth. The virtualized scroller mounts only +// the rows in this list, so an open folder with tens of thousands of files +// never materializes every row in the DOM. +export function flattenReviewRows( + nodes: ReviewTreeNode[], + isOpen: (id: string) => boolean, + depth = 0, + rows: ReviewFlatRow[] = [] +): ReviewFlatRow[] { + for (const node of nodes) { + rows.push({ depth, node }) + + if (node.isDir && node.children && isOpen(node.id)) { + flattenReviewRows(node.children, isOpen, depth + 1, rows) + } + } + + return rows +}