From 5d701b30c3aeeb0202f4acc350fa410bca32a82f Mon Sep 17 00:00:00 2001 From: Stevan Borus Date: Mon, 5 Oct 2026 13:25:44 +0200 Subject: [PATCH] fix(web): scrolling a large diff no longer stalls while collapsed files load The progressive loader swaps each placeholder for the full parsed file as its patch arrives, even when the file is collapsed. Pierre highlights any item that has content, so every collapsed file that loaded mid-scroll was syntax-highlighted in a worker and its whole result, about 4 MB for a 300-line file, was deserialized on the main thread during the scroll. A collapsed file now hands the viewer a header-only stub and only gets its content once expanded. --- apps/web/src/components/DiffPanel.tsx | 12 ++-- apps/web/src/lib/diffRendering.test.ts | 79 ++++++++++++++++++++++++++ apps/web/src/lib/diffRendering.ts | 24 ++++++++ 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 50df89109e05..d0d9354ed2a9 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -35,6 +35,7 @@ import { useTheme } from "../hooks/useTheme"; import { buildFileDiffContentVersion, buildFileDiffIdentityKey, + getCollapsedFileDiffStub, getDiffCollapseIconClassName, getDiffLineStat, getRenderablePatch, @@ -478,15 +479,16 @@ export default function DiffPanel({ renderableFileEntries .filter(({ fileDiff }) => !lazySource || readyFilePaths.has(resolveFileDiffPath(fileDiff))) .map(({ fileDiff, fileKey, fileVersion }) => { + const pending = fileDiff.cacheKey?.endsWith(":pending") === true; + // Header-only placeholders use the viewer's collapsed geometry until their patch arrives. + const collapsed = collapsedDiffFileKeys.has(fileKey) || pending; return { - fileDiff, + fileDiff: + lazySource && collapsed && !pending ? getCollapsedFileDiffStub(fileDiff) : fileDiff, filePath: resolveFileDiffPath(fileDiff), fileKey, fileVersion, - // Header-only placeholders use the viewer's collapsed geometry until their patch arrives. - collapsed: - collapsedDiffFileKeys.has(fileKey) || - fileDiff.cacheKey?.endsWith(":pending") === true, + collapsed, }; }), [collapsedDiffFileKeys, renderableFileEntries, lazySource, readyFilePaths], diff --git a/apps/web/src/lib/diffRendering.test.ts b/apps/web/src/lib/diffRendering.test.ts index 7b8857f32a12..bf3d9e43d037 100644 --- a/apps/web/src/lib/diffRendering.test.ts +++ b/apps/web/src/lib/diffRendering.test.ts @@ -6,6 +6,7 @@ import { buildFileDiffIdentityKey, buildFileDiffRenderKey, buildPatchCacheKey, + getCollapsedFileDiffStub, getDiffLineStat, getRenderablePatch, resolveFileDiffPath, @@ -426,3 +427,81 @@ describe("a file whose name a patch header cannot carry plainly", () => { expect(resolveFileDiffPreviousPath(file)).toBe("old\tname.ts"); }); }); + +describe("getCollapsedFileDiffStub", () => { + const renamedPatch = [ + "diff --git a/old-name.ts b/new-name.ts", + "similarity index 50%", + "rename from old-name.ts", + "rename to new-name.ts", + "--- a/old-name.ts", + "+++ b/new-name.ts", + "@@ -1,2 +1,2 @@", + " const shared = 1;", + "-const before = 2;", + "+const after = 3;", + "diff --git a/other.ts b/other.ts", + "--- a/other.ts", + "+++ b/other.ts", + "@@ -1 +1 @@", + "-old", + "+new", + ].join("\n"); + + const parseFiles = () => { + const parsed = getRenderablePatch(renamedPatch, "collapsed-stub"); + if (parsed?.kind !== "files") throw new Error("expected parsed files"); + const [renamed, other] = parsed.files; + if (!renamed || !other) throw new Error("expected two files"); + return { renamed, other }; + }; + + it("drops the content and keeps the header fields", () => { + const { renamed } = parseFiles(); + expect(renamed.hunks.length).toBeGreaterThan(0); + + const stub = getCollapsedFileDiffStub(renamed); + + expect(stub.hunks).toEqual([]); + expect(stub.additionLines).toEqual([]); + expect(stub.deletionLines).toEqual([]); + expect(stub.splitLineCount).toBe(0); + expect(stub.unifiedLineCount).toBe(0); + expect(stub.name).toBe(renamed.name); + expect(stub.prevName).toBe(renamed.prevName); + expect(stub.prevName).toBe("old-name.ts"); + expect(stub.type).toBe(renamed.type); + expect(stub.lang).toBe(renamed.lang); + expect(stub.newObjectId).toBe(renamed.newObjectId); + expect(stub.prevObjectId).toBe(renamed.prevObjectId); + }); + + it("marks the stub collapsed rather than pending", () => { + const { renamed } = parseFiles(); + + const stub = getCollapsedFileDiffStub(renamed); + + expect(stub.cacheKey).toBe(`${renamed.cacheKey}:collapsed`); + expect(stub.cacheKey?.endsWith(":pending")).toBe(false); + }); + + it("returns the same stub for the same input and a different one for another", () => { + const { renamed, other } = parseFiles(); + + const first = getCollapsedFileDiffStub(renamed); + const second = getCollapsedFileDiffStub(renamed); + const otherStub = getCollapsedFileDiffStub(other); + + expect(second).toBe(first); + expect(otherStub).not.toBe(first); + expect(otherStub.name).toBe("other.ts"); + }); + + it("keeps the file's identity key", () => { + const { renamed } = parseFiles(); + + const stub = getCollapsedFileDiffStub(renamed); + + expect(buildFileDiffIdentityKey(stub)).toBe(buildFileDiffIdentityKey(renamed)); + }); +}); diff --git a/apps/web/src/lib/diffRendering.ts b/apps/web/src/lib/diffRendering.ts index c99349a7dafd..349e6988ac61 100644 --- a/apps/web/src/lib/diffRendering.ts +++ b/apps/web/src/lib/diffRendering.ts @@ -222,6 +222,30 @@ export function resolveFileDiffPreviousPath(fileDiff: FileDiffMetadata): string return fileDiffPath(fileDiff.prevName ?? fileDiff.name ?? ""); } +const collapsedFileDiffStubs = new WeakMap(); + +/** + * Header-only stand-in for a collapsed file, so the viewer gets no content and never highlights + * lines it does not display. The `:collapsed` suffix keeps it apart from a `:pending` placeholder, + * which disables the expand control. + */ +export function getCollapsedFileDiffStub(fileDiff: FileDiffMetadata): FileDiffMetadata { + const cached = collapsedFileDiffStubs.get(fileDiff); + if (cached) return cached; + const stub: FileDiffMetadata = { + ...fileDiff, + hunks: [], + additionLines: [], + deletionLines: [], + splitLineCount: 0, + unifiedLineCount: 0, + isPartial: true, + cacheKey: `${fileDiff.cacheKey ?? resolveFileDiffPath(fileDiff)}:collapsed`, + }; + collapsedFileDiffStubs.set(fileDiff, stub); + return stub; +} + /** * Stable across re-renders of the same file, distinct for every block in a * patch. A type change (regular file to symlink) arrives as a deletion and an