Skip to content

Commit 2eaad1f

Browse files
committed
fix: eliminate Processing... flash on diff rendering
Diffs showed a brief "Processing..." flash even on cache hits due to: - setChunks(null) being called on any cache miss - useState initializer only running on mount - Cache key changing on visibility toggle Fix: Generate plain-text chunks synchronously as immediate fallback, then upgrade to syntax-highlighted version when ready. The hook now never returns null, eliminating the loading state entirely. Net -20 LoC.
1 parent 1bc6a00 commit 2eaad1f

File tree

1 file changed

+40
-54
lines changed

1 file changed

+40
-54
lines changed

src/browser/components/shared/DiffRenderer.tsx

Lines changed: 40 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* ReviewPanel uses SelectableDiffRenderer for interactive line selection.
55
*/
66

7-
import React, { useEffect, useState } from "react";
7+
import React, { useEffect, useMemo, useState } from "react";
88
import { cn } from "@/common/lib/utils";
99
import { getLanguageFromPath } from "@/common/utils/git/languageDetector";
1010
import { useOverflowDetection } from "@/browser/hooks/useOverflowDetection";
@@ -13,6 +13,7 @@ import { Tooltip, TooltipTrigger, TooltipContent } from "../ui/tooltip";
1313
import { groupDiffLines } from "@/browser/utils/highlighting/diffChunking";
1414
import { useTheme, type ThemeMode } from "@/browser/contexts/ThemeContext";
1515
import {
16+
escapeHtml,
1617
highlightDiffChunk,
1718
type HighlightedChunk,
1819
} from "@/browser/utils/highlighting/highlightDiffChunk";
@@ -404,75 +405,80 @@ function getDiffCacheKey(
404405
return `${contentHash}:${oldStart}:${newStart}:${language}:${themeMode}`;
405406
}
406407

408+
/** Synchronous plain-text chunks for instant rendering (no "Processing..." flash) */
409+
function createPlainTextChunks(
410+
content: string,
411+
oldStart: number,
412+
newStart: number
413+
): HighlightedChunk[] {
414+
const lines = splitDiffLines(content);
415+
return groupDiffLines(lines, oldStart, newStart).map((chunk) => ({
416+
type: chunk.type,
417+
lines: chunk.lines.map((line, i) => ({
418+
html: escapeHtml(line),
419+
oldLineNumber: chunk.oldLineNumbers[i],
420+
newLineNumber: chunk.newLineNumbers[i],
421+
originalIndex: chunk.startIndex + i,
422+
})),
423+
usedFallback: true,
424+
}));
425+
}
426+
407427
/**
408-
* Hook to pre-process and highlight diff content in chunks.
409-
* Results are cached at the module level for synchronous cache hits,
410-
* eliminating "Processing" flash when re-rendering the same diff.
411-
*
412-
* When language="text" (highlighting disabled), keeps existing highlighted
413-
* chunks rather than downgrading to plain text. This prevents flicker when
414-
* hunks scroll out of viewport (enableHighlighting=false).
428+
* Hook to highlight diff content. Returns plain-text immediately, then upgrades
429+
* to syntax-highlighted when ready. Never returns null (no loading flash).
415430
*/
416431
function useHighlightedDiff(
417432
content: string,
418433
language: string,
419434
oldStart: number,
420435
newStart: number,
421436
themeMode: ThemeMode
422-
): HighlightedChunk[] | null {
437+
): HighlightedChunk[] {
423438
const cacheKey = getDiffCacheKey(content, language, oldStart, newStart, themeMode);
424439
const cachedResult = highlightedDiffCache.get(cacheKey);
425440

426-
// State for async highlighting results (initialized from cache if available)
427-
const [chunks, setChunks] = useState<HighlightedChunk[] | null>(cachedResult ?? null);
428-
// Track if we've highlighted this content with real syntax (not plain text)
441+
// Sync fallback: plain-text chunks for instant render
442+
const plainText = useMemo(
443+
() => createPlainTextChunks(content, oldStart, newStart),
444+
[content, oldStart, newStart]
445+
);
446+
447+
const [chunks, setChunks] = useState<HighlightedChunk[]>(cachedResult ?? plainText);
429448
const hasRealHighlightRef = React.useRef(false);
430449

431450
useEffect(() => {
432-
// Already in cache - sync state and skip async work
433451
const cached = highlightedDiffCache.get(cacheKey);
434452
if (cached) {
435453
setChunks(cached);
436-
if (language !== "text") {
437-
hasRealHighlightRef.current = true;
438-
}
454+
if (language !== "text") hasRealHighlightRef.current = true;
439455
return;
440456
}
441457

442-
// When highlighting is disabled (language="text") but we've already
443-
// highlighted with real syntax, keep showing that version
444-
if (language === "text" && hasRealHighlightRef.current) {
445-
return;
446-
}
458+
// Keep syntax-highlighted version when toggling to language="text"
459+
if (language === "text" && hasRealHighlightRef.current) return;
447460

448-
// Reset to loading state for new uncached content
449-
setChunks(null);
461+
// Show plain-text immediately, then upgrade async
462+
setChunks(plainText);
450463

451464
let cancelled = false;
452-
453-
async function highlight() {
465+
void (async () => {
454466
const lines = splitDiffLines(content);
455467
const diffChunks = groupDiffLines(lines, oldStart, newStart);
456468
const highlighted = await Promise.all(
457469
diffChunks.map((chunk) => highlightDiffChunk(chunk, language, themeMode))
458470
);
459-
460471
if (!cancelled) {
461472
highlightedDiffCache.set(cacheKey, highlighted);
462473
setChunks(highlighted);
463-
if (language !== "text") {
464-
hasRealHighlightRef.current = true;
465-
}
474+
if (language !== "text") hasRealHighlightRef.current = true;
466475
}
467-
}
468-
469-
void highlight();
476+
})();
470477
return () => {
471478
cancelled = true;
472479
};
473-
}, [cacheKey, content, language, oldStart, newStart, themeMode]);
480+
}, [cacheKey, content, language, oldStart, newStart, themeMode, plainText]);
474481

475-
// Return cached result directly if available (sync path), else state (async path)
476482
return cachedResult ?? chunks;
477483
}
478484

@@ -518,15 +524,6 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
518524
return calculateLineNumberWidths(lines);
519525
}, [highlightedChunks, showLineNumbers]);
520526

521-
// Show loading state while highlighting
522-
if (!highlightedChunks) {
523-
return (
524-
<DiffContainer fontSize={fontSize} maxHeight={maxHeight} className={className}>
525-
<div style={{ opacity: 0.5, padding: "8px" }}>Processing...</div>
526-
</DiffContainer>
527-
);
528-
}
529-
530527
// Get first and last line types for padding background colors
531528
const firstLineType = highlightedChunks[0]?.type;
532529
const lastLineType = highlightedChunks[highlightedChunks.length - 1]?.type;
@@ -825,8 +822,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
825822
// Build lineData from highlighted chunks (memoized to prevent repeated parsing)
826823
// Includes raw content for review note submission
827824
const lineData = React.useMemo(() => {
828-
if (!highlightedChunks) return [];
829-
830825
const data: Array<{
831826
index: number;
832827
type: DiffLineType;
@@ -934,15 +929,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
934929
return index >= start && index <= end;
935930
};
936931

937-
// Show loading state while highlighting
938-
if (!highlightedChunks || highlightedLineData.length === 0) {
939-
return (
940-
<DiffContainer fontSize={fontSize} maxHeight={maxHeight} className={className}>
941-
<div style={{ opacity: 0.5, padding: "8px" }}>Processing...</div>
942-
</DiffContainer>
943-
);
944-
}
945-
946932
// Get first and last line types for padding background colors
947933
const firstLineType = highlightedLineData[0]?.type;
948934
const lastLineType = highlightedLineData[highlightedLineData.length - 1]?.type;

0 commit comments

Comments
 (0)