diff --git a/apps/web/src/components/files/FileBrowserPanel.browser.test.tsx b/apps/web/src/components/files/FileBrowserPanel.browser.test.tsx index 7b8a4e32804c..c7456f141c3b 100644 --- a/apps/web/src/components/files/FileBrowserPanel.browser.test.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.browser.test.tsx @@ -14,7 +14,15 @@ const entries: readonly ProjectEntry[] = [ { path: "README.md", kind: "file" }, ]; const loadingDirectories: ReadonlySet = new Set(); -const directory = { +const directory: { + entries: readonly ProjectEntry[]; + load: () => Promise; + refresh: () => void; + ready: boolean; + error: null; + isPending: boolean; + loadingDirectories: ReadonlySet; +} = { entries, load: () => Promise.resolve(), refresh: () => {}, @@ -46,6 +54,15 @@ function findTreeRow(path: string): HTMLElement | null { return null; } +function withDirectoryListing(listing: readonly ProjectEntry[]): Disposable { + directory.entries = listing; + return { + [Symbol.dispose]: () => { + directory.entries = entries; + }, + }; +} + describe("FileBrowserPanel", () => { it("copies with the pointer and returns to the row after keyboard dismissal", async () => { await using _screen = await render( @@ -118,6 +135,108 @@ describe("FileBrowserPanel", () => { await vi.waitFor(() => expect(onOpenFile).toHaveBeenCalledTimes(1)); }); + it("keeps an expanded folder open when a very large listing rebuilds the tree", async () => { + using _listing = withDirectoryListing(entries); + const renderPanel = () => ( + {}} + workspaceMutationId={null} + /> + ); + await using screen = await render(renderPanel()); + await vi.waitFor(() => expect(findTreeRow("src/")).not.toBeNull()); + findTreeRow("src/")!.click(); + await vi.waitFor(() => expect(findTreeRow("src/")!.getAttribute("aria-expanded")).toBe("true")); + + directory.entries = [ + ...entries, + // Out of sort order, because inserting in order hides the cost of each insert. + ...Array.from({ length: 2_000 }, (_, index): ProjectEntry => { + const name = String((index * 7_919) % 2_000).padStart(4, "0"); + return { path: `src/generated-${name}`, kind: "directory" }; + }), + ]; + await screen.rerender(renderPanel()); + + await vi.waitFor(() => expect(findTreeRow("src/generated-0000/")).not.toBeNull()); + expect(findTreeRow("src/")!.getAttribute("aria-expanded")).toBe("true"); + }); + + it("keeps the selected file through a very large listing without reopening it", async () => { + const onOpenFile = vi.fn(); + const renderPanel = () => ( + + ); + using _listing = withDirectoryListing([...entries, { path: "src/aaa.ts", kind: "file" }]); + await using screen = await render(renderPanel()); + await vi.waitFor(() => + expect(findTreeRow("src/aaa.ts")?.getAttribute("aria-selected")).toBe("true"), + ); + + directory.entries = [ + ...directory.entries, + ...Array.from({ length: 2_000 }, (_, index): ProjectEntry => { + const name = String((index * 7_919) % 2_000).padStart(4, "0"); + return { path: `src/generated-${name}.ts`, kind: "file" }; + }), + ]; + await screen.rerender(renderPanel()); + + await vi.waitFor(() => expect(findTreeRow("src/generated-0000.ts")).not.toBeNull()); + expect(findTreeRow("src/aaa.ts")?.getAttribute("aria-selected")).toBe("true"); + expect(findTreeRow("src/")?.getAttribute("aria-expanded")).toBe("true"); + expect(onOpenFile).not.toHaveBeenCalled(); + }); + + it("keeps an open search accurate when a very large folder listing arrives", async () => { + const renderPanel = () => ( + {}} + workspaceMutationId={null} + /> + ); + using _listing = withDirectoryListing([ + ...entries, + { path: "lib", kind: "directory" }, + { path: "lib/generated-existing.ts", kind: "file" }, + ]); + await using screen = await render(renderPanel()); + const search = page.getByRole("searchbox", { name: "Search repo files" }); + await userEvent.fill(search, "generated-"); + await vi.waitFor(() => expect(findTreeRow("lib/generated-existing.ts")).not.toBeNull()); + + directory.entries = [ + ...directory.entries, + ...Array.from({ length: 2_000 }, (_, index): ProjectEntry => { + const name = String((index * 7_919) % 2_000).padStart(4, "0"); + return { path: `src/generated-${name}.ts`, kind: "file" }; + }), + ]; + await screen.rerender(renderPanel()); + + await vi.waitFor(() => expect(findTreeRow("src/generated-0000.ts")).not.toBeNull()); + expect(findTreeRow("lib/generated-existing.ts")).not.toBeNull(); + await expect.element(search).toHaveValue("generated-"); + }); + it("mod+f from a tree row, or the panel find command, focuses the file search", async () => { await using _screen = await render( 0) model.batch(updates); }, [ready, entryKinds, model, treePaths]); diff --git a/apps/web/src/components/files/fileTreePathReconciliation.test.ts b/apps/web/src/components/files/fileTreePathReconciliation.test.ts new file mode 100644 index 000000000000..9e532b43fba9 --- /dev/null +++ b/apps/web/src/components/files/fileTreePathReconciliation.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vite-plus/test"; + +import { buildFileTreePathUpdates, shouldRebuildFileTree } from "./fileTreePathReconciliation"; + +const folder = (name: string, count: number) => + Array.from({ length: count }, (_, index) => `${name}/entry-${index}`); + +const decide = (previous: readonly string[], next: readonly string[]) => + shouldRebuildFileTree(previous, next, buildFileTreePathUpdates(previous, next)); + +describe("buildFileTreePathUpdates", () => { + it("removes a folder once and skips the paths inside it", () => { + expect( + buildFileTreePathUpdates(["src/", "src/a/", "src/a/b/", "src/a/c.ts"], ["src/"]), + ).toEqual([{ type: "remove", path: "src/a/", recursive: true }]); + }); + + it("does not treat a sibling that shares a name prefix as inside a removed folder", () => { + expect(buildFileTreePathUpdates(["src/a/", "src/ab/", "src/a.ts"], [])).toEqual([ + { type: "remove", path: "src/a/", recursive: true }, + { type: "remove", path: "src/ab/", recursive: true }, + { type: "remove", path: "src/a.ts" }, + ]); + }); +}); + +describe("shouldRebuildFileTree", () => { + it("rebuilds when many paths land in a folder that dominates the tree", () => { + const existing = folder("big", 50_000); + expect(decide(existing, [...existing, ...addTo("big", 301)])).toBe(true); + }); + + it("counts a folder's subfolders as siblings, whose paths end in a slash", () => { + const existing = folder("big", 50_000).map((path) => `${path}/`); + const added = addTo("big", 301).map((path) => `${path}/`); + expect(decide(existing, [...existing, ...added])).toBe(true); + expect(decide(existing, existing.slice(301))).toBe(true); + }); + + it("batches a few hundred paths into a small folder beside a large unrelated one", () => { + const existing = folder("big", 50_000); + expect(decide(existing, [...existing, ...folder("small", 301)])).toBe(false); + }); + + it("batches many operations spread thinly across many small folders", () => { + const existing = folder("big", 50_000); + const added = Array.from({ length: 1_000 }, (_, index) => `small-${index}/file.ts`); + expect(decide(existing, [...existing, ...added])).toBe(false); + }); +}); + +function addTo(name: string, count: number) { + return Array.from({ length: count }, (_, index) => `${name}/added-${index}`); +} diff --git a/apps/web/src/components/files/fileTreePathReconciliation.ts b/apps/web/src/components/files/fileTreePathReconciliation.ts index 5bcccb2cd2a1..b57dfd013b8d 100644 --- a/apps/web/src/components/files/fileTreePathReconciliation.ts +++ b/apps/web/src/components/files/fileTreePathReconciliation.ts @@ -1,26 +1,70 @@ import type { FileTreeBatchOperation } from "@pierre/trees"; +// A batched operation reworks its folder's siblings, while a rebuild reworks every path. On flat +// folders of 50,000 and 332,000 entries a rebuild won somewhere between 200 and 500 operations. +const REBUILD_COST_IN_FOLDER_OPERATIONS = 300; + +function parentOf(path: string): string { + const end = path.endsWith("/") ? path.length - 1 : path.length; + return path.slice(0, Math.max(0, path.lastIndexOf("/", end - 1))); +} + +function countByParent(paths: readonly string[]): Map { + const counts = new Map(); + for (const path of paths) { + const parent = parentOf(path); + counts.set(parent, (counts.get(parent) ?? 0) + 1); + } + return counts; +} + +export function shouldRebuildFileTree( + previousPaths: readonly string[], + nextPaths: readonly string[], + updates: readonly FileTreeBatchOperation[], +): boolean { + // Up to this many operations, the estimate below can't exceed the rebuild, so skip counting. + if (updates.length <= REBUILD_COST_IN_FOLDER_OPERATIONS) return false; + const previousCounts = countByParent(previousPaths); + const nextCounts = countByParent(nextPaths); + let batchWork = 0; + for (const update of updates) { + if (update.type !== "add" && update.type !== "remove") continue; + const parent = parentOf(update.path); + batchWork += Math.max(previousCounts.get(parent) ?? 0, nextCounts.get(parent) ?? 0); + } + const rebuildWork = Math.max(previousPaths.length, nextPaths.length); + return batchWork > REBUILD_COST_IN_FOLDER_OPERATIONS * rebuildWork; +} + function pathDepth(path: string): number { return path.split("/").filter(Boolean).length; } +function isInsideRemovedDirectory(path: string, removedRoots: ReadonlySet): boolean { + for (let slash = path.indexOf("/"); slash !== -1; slash = path.indexOf("/", slash + 1)) { + if (removedRoots.has(path.slice(0, slash + 1))) return true; + } + return false; +} + export function buildFileTreePathUpdates( previousPaths: readonly string[], nextPaths: readonly string[], ): FileTreeBatchOperation[] { const previous = new Set(previousPaths); const next = new Set(nextPaths); - const removedDirectoryRoots: string[] = []; + const removedDirectoryRoots = new Set(); const updates: FileTreeBatchOperation[] = []; const removedPaths = previousPaths .filter((path) => !next.has(path)) .toSorted((left, right) => pathDepth(left) - pathDepth(right)); for (const path of removedPaths) { - if (removedDirectoryRoots.some((directory) => path.startsWith(directory))) continue; + if (isInsideRemovedDirectory(path, removedDirectoryRoots)) continue; const recursive = path.endsWith("/"); updates.push({ type: "remove", path, ...(recursive ? { recursive: true } : {}) }); - if (recursive) removedDirectoryRoots.push(path); + if (recursive) removedDirectoryRoots.add(path); } const addedPaths = nextPaths