Skip to content

Commit 0932c71

Browse files
committed
fix(web): avoid rendering oversized diff lines
- Introduced `canRenderFileDiff` function to determine if a file diff can be displayed based on line length limits. - Updated `DiffPanel` to utilize this function, providing user feedback for large files. - Added tests for the new rendering logic to ensure proper functionality.
1 parent ada410b commit 0932c71

3 files changed

Lines changed: 65 additions & 11 deletions

File tree

apps/web/src/components/DiffPanel.tsx

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ import { readLocalApi } from "../localApi";
2727
import { resolvePathLinkTarget } from "../terminal-links";
2828
import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch";
2929
import { useTheme } from "../hooks/useTheme";
30-
import { buildPatchCacheKey } from "../lib/diffRendering";
31-
import { resolveDiffThemeName } from "../lib/diffRendering";
30+
import { buildPatchCacheKey, canRenderFileDiff, resolveDiffThemeName } from "../lib/diffRendering";
3231
import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
3332
import { selectProjectByRef, useStore } from "../store";
3433
import { createThreadSelectorByRef } from "../storeSelectors";
@@ -306,12 +305,22 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
306305
if (!renderablePatch || renderablePatch.kind !== "files") {
307306
return [];
308307
}
309-
return renderablePatch.files.toSorted((left, right) =>
310-
resolveFileDiffPath(left).localeCompare(resolveFileDiffPath(right), undefined, {
311-
numeric: true,
312-
sensitivity: "base",
313-
}),
314-
);
308+
return renderablePatch.files
309+
.map((fileDiff) => ({
310+
canRender: canRenderFileDiff(fileDiff),
311+
fileDiff,
312+
filePath: resolveFileDiffPath(fileDiff),
313+
}))
314+
.toSorted((left, right) => {
315+
const renderOrder = Number(!left.canRender) - Number(!right.canRender);
316+
if (renderOrder !== 0) {
317+
return renderOrder;
318+
}
319+
return left.filePath.localeCompare(right.filePath, undefined, {
320+
numeric: true,
321+
sensitivity: "base",
322+
});
323+
});
315324
}, [renderablePatch]);
316325

317326
useEffect(() => {
@@ -600,8 +609,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
600609
intersectionObserverMargin: 1200,
601610
}}
602611
>
603-
{renderableFiles.map((fileDiff) => {
604-
const filePath = resolveFileDiffPath(fileDiff);
612+
{renderableFiles.map(({ canRender, fileDiff, filePath }) => {
605613
const fileKey = buildFileDiffRenderKey(fileDiff);
606614
const themedFileKey = `${fileKey}:${resolvedTheme}`;
607615
return (
@@ -629,8 +637,17 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
629637
theme: resolveDiffThemeName(resolvedTheme),
630638
themeType: resolvedTheme as DiffThemeType,
631639
unsafeCSS: DIFF_PANEL_UNSAFE_CSS,
640+
collapsed: !canRender,
632641
}}
633642
/>
643+
644+
{!canRender && (
645+
<div className="px-3 py-3 text-[11px] leading-relaxed text-muted-foreground">
646+
<p>
647+
This file is too large to display. Open the file to inspect the change.
648+
</p>
649+
</div>
650+
)}
634651
</div>
635652
);
636653
})}

apps/web/src/lib/diffRendering.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it } from "vitest";
2-
import { buildPatchCacheKey } from "./diffRendering";
2+
import {
3+
buildPatchCacheKey,
4+
canRenderFileDiff,
5+
MAX_RENDERABLE_DIFF_LINE_LENGTH,
6+
} from "./diffRendering";
37

48
describe("buildPatchCacheKey", () => {
59
it("returns a stable cache key for identical content", () => {
@@ -29,3 +33,23 @@ describe("buildPatchCacheKey", () => {
2933
);
3034
});
3135
});
36+
37+
describe("diff render line limits", () => {
38+
it("rejects file diffs with pathological line lengths", () => {
39+
expect(
40+
canRenderFileDiff({
41+
additionLines: ["small"],
42+
deletionLines: ["x".repeat(MAX_RENDERABLE_DIFF_LINE_LENGTH + 1)],
43+
}),
44+
).toBe(false);
45+
});
46+
47+
it("allows file diffs within the line length limit", () => {
48+
expect(
49+
canRenderFileDiff({
50+
additionLines: ["x".repeat(MAX_RENDERABLE_DIFF_LINE_LENGTH)],
51+
deletionLines: ["small"],
52+
}),
53+
).toBe(true);
54+
});
55+
});

apps/web/src/lib/diffRendering.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1+
import type { FileDiffMetadata } from "@pierre/diffs";
2+
13
export const DIFF_THEME_NAMES = {
24
light: "pierre-light",
35
dark: "pierre-dark",
46
} as const;
57

68
export type DiffThemeName = (typeof DIFF_THEME_NAMES)[keyof typeof DIFF_THEME_NAMES];
79

10+
export const MAX_RENDERABLE_DIFF_LINE_LENGTH = 500_000;
11+
812
export function resolveDiffThemeName(theme: "light" | "dark"): DiffThemeName {
913
return theme === "dark" ? DIFF_THEME_NAMES.dark : DIFF_THEME_NAMES.light;
1014
}
@@ -37,3 +41,12 @@ export function buildPatchCacheKey(patch: string, scope = "diff-panel"): string
3741
).toString(36);
3842
return `${scope}:${normalizedPatch.length}:${primary}:${secondary}`;
3943
}
44+
45+
export function canRenderFileDiff(
46+
fileDiff: Pick<FileDiffMetadata, "additionLines" | "deletionLines">,
47+
): boolean {
48+
return (
49+
fileDiff.additionLines.every((line) => line.length <= MAX_RENDERABLE_DIFF_LINE_LENGTH) &&
50+
fileDiff.deletionLines.every((line) => line.length <= MAX_RENDERABLE_DIFF_LINE_LENGTH)
51+
);
52+
}

0 commit comments

Comments
 (0)