Skip to content

Commit bcdd179

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 fead02c commit bcdd179

File tree

1 file changed

+37
-57
lines changed

1 file changed

+37
-57
lines changed

src/browser/components/shared/DiffRenderer.tsx

Lines changed: 37 additions & 57 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,74 @@ 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(content: string, oldStart: number, newStart: number): HighlightedChunk[] {
410+
const lines = splitDiffLines(content);
411+
return groupDiffLines(lines, oldStart, newStart).map((chunk) => ({
412+
type: chunk.type,
413+
lines: chunk.lines.map((line, i) => ({
414+
html: escapeHtml(line),
415+
oldLineNumber: chunk.oldLineNumbers[i],
416+
newLineNumber: chunk.newLineNumbers[i],
417+
originalIndex: chunk.startIndex + i,
418+
})),
419+
usedFallback: true,
420+
}));
421+
}
422+
407423
/**
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).
424+
* Hook to highlight diff content. Returns plain-text immediately, then upgrades
425+
* to syntax-highlighted when ready. Never returns null (no loading flash).
415426
*/
416427
function useHighlightedDiff(
417428
content: string,
418429
language: string,
419430
oldStart: number,
420431
newStart: number,
421432
themeMode: ThemeMode
422-
): HighlightedChunk[] | null {
433+
): HighlightedChunk[] {
423434
const cacheKey = getDiffCacheKey(content, language, oldStart, newStart, themeMode);
424435
const cachedResult = highlightedDiffCache.get(cacheKey);
425436

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)
437+
// Sync fallback: plain-text chunks for instant render
438+
const plainText = useMemo(
439+
() => createPlainTextChunks(content, oldStart, newStart),
440+
[content, oldStart, newStart]
441+
);
442+
443+
const [chunks, setChunks] = useState<HighlightedChunk[]>(cachedResult ?? plainText);
429444
const hasRealHighlightRef = React.useRef(false);
430445

431446
useEffect(() => {
432-
// Already in cache - sync state and skip async work
433447
const cached = highlightedDiffCache.get(cacheKey);
434448
if (cached) {
435449
setChunks(cached);
436-
if (language !== "text") {
437-
hasRealHighlightRef.current = true;
438-
}
450+
if (language !== "text") hasRealHighlightRef.current = true;
439451
return;
440452
}
441453

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-
}
454+
// Keep syntax-highlighted version when toggling to language="text"
455+
if (language === "text" && hasRealHighlightRef.current) return;
447456

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

451460
let cancelled = false;
452-
453-
async function highlight() {
461+
void (async () => {
454462
const lines = splitDiffLines(content);
455463
const diffChunks = groupDiffLines(lines, oldStart, newStart);
456464
const highlighted = await Promise.all(
457465
diffChunks.map((chunk) => highlightDiffChunk(chunk, language, themeMode))
458466
);
459-
460467
if (!cancelled) {
461468
highlightedDiffCache.set(cacheKey, highlighted);
462469
setChunks(highlighted);
463-
if (language !== "text") {
464-
hasRealHighlightRef.current = true;
465-
}
470+
if (language !== "text") hasRealHighlightRef.current = true;
466471
}
467-
}
472+
})();
473+
return () => { cancelled = true; };
474+
}, [cacheKey, content, language, oldStart, newStart, themeMode, plainText]);
468475

469-
void highlight();
470-
return () => {
471-
cancelled = true;
472-
};
473-
}, [cacheKey, content, language, oldStart, newStart, themeMode]);
474-
475-
// Return cached result directly if available (sync path), else state (async path)
476476
return cachedResult ?? chunks;
477477
}
478478

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

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-
530521
// Get first and last line types for padding background colors
531522
const firstLineType = highlightedChunks[0]?.type;
532523
const lastLineType = highlightedChunks[highlightedChunks.length - 1]?.type;
@@ -825,8 +816,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
825816
// Build lineData from highlighted chunks (memoized to prevent repeated parsing)
826817
// Includes raw content for review note submission
827818
const lineData = React.useMemo(() => {
828-
if (!highlightedChunks) return [];
829-
830819
const data: Array<{
831820
index: number;
832821
type: DiffLineType;
@@ -934,15 +923,6 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
934923
return index >= start && index <= end;
935924
};
936925

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-
946926
// Get first and last line types for padding background colors
947927
const firstLineType = highlightedLineData[0]?.type;
948928
const lastLineType = highlightedLineData[highlightedLineData.length - 1]?.type;

0 commit comments

Comments
 (0)