Improve review-tool performance.

This commit is contained in:
Greyson Parrelli
2026-09-08 14:13:53 -04:00
parent 8b3aadbd48
commit 2a0e03cc41
5 changed files with 739 additions and 245 deletions
+525 -157
View File
@@ -15,13 +15,10 @@ import {
Hunk,
getChangeKey,
getCollapsedLinesCountBetween,
markEdits,
tokenize,
useSourceExpansion,
type ChangeData,
type FileData,
type HunkData,
type HunkTokens,
type ViewType,
} from 'react-diff-view';
@@ -34,7 +31,8 @@ import {
newSourceOf,
splitLines,
} from '../lib/interdiff';
import { languageForFile, refractorAdapter } from '../lib/language';
import { useTokens } from '../lib/highlight';
import { languageForFile } from '../lib/language';
import { CommentThread } from './CommentThread';
import { Composer } from './Composer';
import { Icon } from './Icon';
@@ -161,6 +159,268 @@ function portTopOf(port: Element): number {
return port.getBoundingClientRect().top;
}
// ---- The render window -----------------------------------------------------
//
// Three hundred files of diff is a quarter of a million DOM nodes and the better
// part of a minute of Prism, and the reader is looking at one file of it. So a
// file's diff is mounted only while its card is somewhere near the viewport, and
// taken out again once the reader is several screens past it.
//
// What never leaves is the card: its header, its viewed checkbox, its comment
// count, and the `file-<path>` anchor the file rail and every jump scroll to.
// Only the expensive, invisible part comes and goes, and it leaves a gap of its
// own height behind, so the scrollbar keeps meaning what it meant and scrolling
// back finds the file where it was.
// How near "near" is, in scroll ports. Mounting reaches further out than
// unmounting keeps: with one boundary, a reader parked on it would pay the mount
// cost over and over, which is worse than never having unmounted at all.
const MOUNT_MARGIN = 1.5;
const KEEP_MARGIN = 4;
// Height of one diff row — the CSS's --diff-line-height, which the gutter and
// code cells are laid out to. Only ever used to guess at a file nobody has
// scrolled to yet.
const ROW_HEIGHT = 24;
// What a file's diff is probably worth in pixels. Used for a card that has never
// been on screen, and so has nothing measured to go on; once it has, the
// measurement replaces this. Wrapped lines make it a floor rather than an answer,
// which is the right way round — a gap that is too short grows as you reach it,
// and the browser's scroll anchoring absorbs that; one that is too tall leaves a
// hole.
function estimateHeight(file: FileData, viewType: ViewType): number {
let rows = 0;
for (const hunk of file.hunks) {
rows += 1; // the @@ decoration above it
if (viewType === 'unified') {
rows += hunk.changes.length;
continue;
}
// Split view puts deletions alongside the insertions that replaced them, so
// a rewritten block is as tall as its taller side rather than both together.
let normal = 0;
let deletes = 0;
let inserts = 0;
for (const change of hunk.changes) {
if (change.type === 'normal') normal++;
else if (change.type === 'delete') deletes++;
else inserts++;
}
rows += normal + Math.max(deletes, inserts);
}
return rows * ROW_HEIGHT;
}
// Roughly what a file card's header and margin cost, for sizing the first
// screenful before anything has been measured.
const CARD_CHROME = 48;
// useFileWindow decides which files are close enough to the reader to be worth
// rendering, and remembers how tall the rest were when they last were.
//
// The observers are rooted on the scroll port rather than on the viewport. With
// `root: null` the port's own clipping is applied to a target *before* the root
// margin is, so a margin measured in viewports buys nothing at all inside a
// nested scroller — every card past the port's bottom edge reads as equally far
// away.
function useFileWindow(files: FileData[], viewType: ViewType) {
// The first screenful, worked out during render rather than after it. The
// observers cannot report until a frame has been laid out, and without a seed
// that frame is a column of empty cards — a flicker on every load, and on
// every change of base ref.
const seed = useMemo(() => {
const budget = window.innerHeight * (MOUNT_MARGIN + 1);
const out = new Set<string>();
let used = 0;
for (const file of files) {
if (used > budget) break;
out.add(filePath(file));
used += CARD_CHROME + estimateHeight(file, viewType);
}
return out;
}, [files, viewType]);
const [near, setNear] = useState<ReadonlySet<string>>(seed);
// A new change set replaces the window rather than adding to it. Assigning
// during render (rather than in an effect) is what keeps the seed on the very
// first paint of the new diff.
const seeded = useRef(seed);
if (seeded.current !== seed) {
seeded.current = seed;
setNear(seed);
}
// Element identity is what the observers hand back, so the path has to be
// looked up from it. Heights are keyed by path instead: they have to outlive
// the element they were measured from, which is the whole point of keeping
// them.
const pathOfEl = useRef(new WeakMap<Element, string>());
const heights = useRef(new Map<string, number>());
const cards = useRef(new Map<string, Element>());
const bodies = useRef(new Map<string, Element>());
const observers = useRef<{
mount: IntersectionObserver;
keep: IntersectionObserver;
size: ResizeObserver;
} | null>(null);
// Built from the first card to register, because the margins are measured
// against the scroll port and there is no port to measure until a card is in
// one.
const observersFor = useCallback((el: Element) => {
if (observers.current) return observers.current;
const port = scrollPortOf(el);
const root =
port === document.scrollingElement || port === document.documentElement
? null
: port;
const collect = (entries: IntersectionObserverEntry[], wanted: boolean) => {
let hit: string[] | null = null;
for (const entry of entries) {
if (entry.isIntersecting !== wanted) continue;
const path = pathOfEl.current.get(entry.target);
if (path != null) (hit ??= []).push(path);
}
return hit;
};
observers.current = {
mount: new IntersectionObserver(
(entries) => {
const arrived = collect(entries, true);
if (!arrived) return;
setNear((prev) => {
const fresh = arrived.filter((p) => !prev.has(p));
if (fresh.length === 0) return prev;
const next = new Set(prev);
for (const path of fresh) next.add(path);
return next;
});
},
{ root, rootMargin: `${MOUNT_MARGIN * 100}% 0px` },
),
keep: new IntersectionObserver(
(entries) => {
const left = collect(entries, false);
if (!left) return;
setNear((prev) => {
const gone = left.filter((p) => prev.has(p));
if (gone.length === 0) return prev;
const next = new Set(prev);
for (const path of gone) next.delete(path);
return next;
});
},
{ root, rootMargin: `${KEEP_MARGIN * 100}% 0px` },
),
// borderBoxSize rather than offsetHeight: the callback runs right after
// layout, and asking an element for its height there would dirty it again.
size: new ResizeObserver((entries) => {
for (const entry of entries) {
const path = pathOfEl.current.get(entry.target);
if (path == null) continue;
const box = entry.borderBoxSize?.[0];
const height = box
? box.blockSize
: (entry.target as HTMLElement).offsetHeight;
if (height > 0) heights.current.set(path, height);
}
}),
};
return observers.current;
}, []);
useEffect(
() => () => {
const o = observers.current;
observers.current = null;
o?.mount.disconnect();
o?.keep.disconnect();
o?.size.disconnect();
},
[],
);
// One ref callback per file, cached: a fresh function each render would have
// React detach and reattach every card on every render, which is exactly the
// work this hook exists to spare it.
const cardCache = useRef(new Map<string, (el: HTMLElement | null) => void>());
const cardRef = useCallback(
(path: string) => {
let fn = cardCache.current.get(path);
if (!fn) {
fn = (el: HTMLElement | null) => {
const before = cards.current.get(path);
if (before && before !== el) {
observers.current?.mount.unobserve(before);
observers.current?.keep.unobserve(before);
cards.current.delete(path);
}
if (!el) return;
pathOfEl.current.set(el, path);
cards.current.set(path, el);
const o = observersFor(el);
o.mount.observe(el);
o.keep.observe(el);
};
cardCache.current.set(path, fn);
}
return fn;
},
[observersFor],
);
const bodyCache = useRef(new Map<string, (el: HTMLElement | null) => void>());
const bodyRef = useCallback(
(path: string) => {
let fn = bodyCache.current.get(path);
if (!fn) {
fn = (el: HTMLElement | null) => {
const before = bodies.current.get(path);
if (before && before !== el) {
observers.current?.size.unobserve(before);
bodies.current.delete(path);
}
if (!el) return;
pathOfEl.current.set(el, path);
bodies.current.set(path, el);
observersFor(el).size.observe(el);
};
bodyCache.current.set(path, fn);
}
return fn;
},
[observersFor],
);
// A different diff, or the same one laid out the other way, is a different set
// of heights. Keeping the old ones would leave gaps sized for a page that no
// longer exists.
useEffect(() => {
heights.current.clear();
}, [files, viewType]);
// How tall a gap this file's absent diff should leave. Measured if it has ever
// been rendered; guessed, once, if not.
const gapHeight = useCallback(
(file: FileData, path: string) => {
let height = heights.current.get(path);
if (height == null) {
height = estimateHeight(file, viewType);
heights.current.set(path, height);
}
return height;
},
[viewType],
);
return { near, cardRef, bodyRef, gapHeight };
}
export function DiffView({
files,
comments,
@@ -179,6 +439,7 @@ export function DiffView({
draft,
}: Props) {
const commentsByFile = useCommentsByFile(comments, outdated);
const { near, cardRef, bodyRef, gapHeight } = useFileWindow(files, viewType);
if (files.length === 0) {
return (
@@ -214,6 +475,10 @@ export function DiffView({
changed={changed.has(path)}
previous={snapshots.get(path) ?? null}
reveal={reveal?.file === path ? reveal.seq : null}
near={near.has(path)}
gap={near.has(path) ? 0 : gapHeight(file, path)}
cardRef={cardRef(path)}
bodyRef={bodyRef(path)}
onSetViewed={onSetViewed}
onStartDraft={onStartDraft}
onCancelDraft={onCancelDraft}
@@ -253,6 +518,10 @@ interface DragState {
// resized — and without this each of those costs a re-render of the entire
// change set. Every prop is either a primitive or an identity held stable for
// exactly that purpose; see useCommentsByFile, and `submitDraft` in App.
//
// What it renders is only ever the card: the header, and either the diff or the
// gap standing in for it. Everything the diff costs lives in FileBody, which is
// mounted only while the reader is somewhere near this file.
const FileView = memo(function FileView({
file,
base,
@@ -263,6 +532,10 @@ const FileView = memo(function FileView({
changed,
previous,
reveal,
near,
gap,
cardRef,
bodyRef,
onSetViewed,
onStartDraft,
onCancelDraft,
@@ -279,6 +552,14 @@ const FileView = memo(function FileView({
changed: boolean;
previous: string | null;
reveal: number | null;
// Whether the render window has this file close enough to the reader to be
// worth drawing — see useFileWindow.
near: boolean;
// How much room its absent diff should hold open while it isn't. Zero when the
// diff is there to hold its own.
gap: number;
cardRef: (el: HTMLElement | null) => void;
bodyRef: (el: HTMLElement | null) => void;
onSetViewed: Props['onSetViewed'];
onStartDraft: (d: DraftTarget) => void;
onCancelDraft: () => void;
@@ -290,7 +571,6 @@ const FileView = memo(function FileView({
// and read one you leave open — but a file already marked viewed opens folded,
// and the checkbox folds it for you (see toggleViewed).
const [collapsed, setCollapsed] = useState(viewed);
const [drag, setDrag] = useState<DragState | null>(null);
// Which diff this file is showing: everything since the base ref, or only what
// has moved since you last marked it viewed. A file with a snapshot behind it
@@ -369,14 +649,29 @@ const FileView = memo(function FileView({
// the same arrays from one render to the next.
const { line: lineComments, file: fileComments, stale: staleComments } = comments;
// Whether the diff itself belongs in the document. Two things outrank the
// render window. A draft being typed here must survive the reader scrolling
// away from it — unmounting the composer would throw away what they wrote —
// and a file a jump has asked for is about to be scrolled to, so it has to be
// rendered before the scroll can find anything in it.
const pinned = draft != null || reveal != null;
const active = !collapsed && (near || pinned);
// Fetch the base-side source so collapsed context can be expanded on demand.
// Added files have no base version, so expansion is disabled for them.
//
// Gated on the file being near, because a change set of three hundred files
// would otherwise open with three hundred `git show`s for context nobody has
// scrolled to. A file that has gone stale is fetched wherever it sits: its
// header offers a since-viewed diff, and it can't say how big that is without
// the file's contents.
const [oldSource, setOldSource] = useState<string | null>(null);
useEffect(() => {
if (file.type === 'add') {
setOldSource(null);
return;
}
if (!near && !pinned && !hasPrevious) return;
let canceled = false;
api.fileContent(base, file.oldPath).then((s) => {
if (!canceled) setOldSource(s);
@@ -384,7 +679,7 @@ const FileView = memo(function FileView({
return () => {
canceled = true;
};
}, [base, file.oldPath, file.type]);
}, [base, file.oldPath, file.type, near, pinned, hasPrevious]);
// The file as it stands now, rebuilt from the base source and the patch. It is
// what a viewed mark snapshots, and the "after" side of the since-viewed diff.
@@ -404,12 +699,206 @@ const FileView = memo(function FileView({
// says so rather than showing a scope the code below isn't rendering.
const showingSince = scope === 'since' && sinceHunks != null;
// Everything downstream — expansion, highlighting, anchoring — works off the
// hunks actually being shown and the source their old side belongs to. For the
// since-viewed diff that source is the snapshot, which is a complete copy of
// the file, so context expansion and whole-file tokenizing both still work.
const baseHunks = showingSince ? sinceHunks : file.hunks;
const expandSource = showingSince ? previous : oldSource;
const openCount = comments.open;
// The header's +/ always describe the whole change, whichever scope is on
// screen: they are how big this file's part of the review is, and having them
// shrink when you toggle would make the file look like it had been reverted.
// What moved since you last looked is spelled out on the toggle instead.
const additions = countChanges(file.hunks, 'insert');
const deletions = countChanges(file.hunks, 'delete');
const sinceAdditions = sinceHunks ? countChanges(sinceHunks, 'insert') : 0;
const sinceDeletions = sinceHunks ? countChanges(sinceHunks, 'delete') : 0;
// Marking a file viewed folds it away, and unmarking brings it back — the
// reason you'd touch the checkbox is that you're done with (or returning to)
// this file, so the fold is the point.
//
// The contents go with the mark, so that when this file next changes there is
// something to diff against. `newSource` is null only for a binary file or one
// whose base contents haven't arrived; the mark is still worth making then, it
// just won't be able to show you what moved.
const toggleViewed = () => {
onSetViewed(path, !viewed, newSource ?? undefined);
fold(!viewed);
};
return (
<section
className={`file${viewed ? ' is-viewed' : ''}`}
id={`file-${path}`}
ref={cardRef}
>
<header
className={`file-head${collapsed ? ' is-collapsed' : ''}${
viewed ? ' is-viewed' : ''
}`}
>
<button
className="file-collapse"
onClick={() => fold(!collapsed)}
aria-label={collapsed ? 'Expand' : 'Collapse'}
>
<Icon name={collapsed ? 'chevron-right' : 'chevron-down'} />
</button>
<span className="file-path">{path}</span>
<button
className="icon-btn"
onClick={() => navigator.clipboard?.writeText(path)}
title="Copy path"
aria-label="Copy path"
>
<Icon name="copy" />
</button>
{file.type !== 'modify' && (
<span className={`file-status file-status-${file.type}`}>
{statusLabel(file.type)}
</span>
)}
{file.type === 'rename' && (
<span className="file-rename muted"> {file.oldPath}</span>
)}
{changed && !viewed && (
<span
className="file-status file-status-changed"
title="This file's diff changed since you marked it viewed, so the mark came off"
>
changed since viewed
</span>
)}
<span className="file-head-right">
{hasPrevious && !file.isBinary && (
<span className="segmented scope-toggle" role="group" aria-label="Diff scope">
<button
className={showingSince ? 'is-active' : ''}
onClick={() => setScope('since')}
disabled={sinceHunks == null}
aria-pressed={showingSince}
title={
sinceHunks == null
? 'Loading the version you last viewed…'
: `Only what moved since you marked this file viewed — +${sinceAdditions} ${sinceDeletions}`
}
>
<Icon name="clock" size={14} />
<span className="btn-label">since viewed</span>
</button>
<button
className={showingSince ? '' : 'is-active'}
onClick={() => setScope('full')}
aria-pressed={!showingSince}
title={`This file's whole diff against ${base}`}
>
<Icon name="file-diff" size={14} />
<span className="btn-label">full diff</span>
</button>
</span>
)}
<span className="file-stat file-stat-add">+{additions}</span>
<span className="file-stat file-stat-del">{deletions}</span>
<DiffStat additions={additions} deletions={deletions} />
<label
className={`viewed-check${viewed ? ' is-on' : ''}`}
title={
viewed
? 'Mark as not viewed (expands the file)'
: 'Mark as viewed (collapses the file)'
}
>
<input type="checkbox" checked={viewed} onChange={toggleViewed} />
Viewed
</label>
<button
className="icon-btn has-label"
onClick={() => onStartDraft({ level: 'file', file: path })}
title={
openCount > 0
? `${openCount} open comment${openCount === 1 ? '' : 's'} — add another`
: 'Comment on this file'
}
>
<Icon name="comment" />
{openCount > 0 && openCount}
</button>
</span>
</header>
{!collapsed &&
(active ? (
<FileBody
file={file}
path={path}
viewType={viewType}
showingSince={showingSince}
// Everything downstream — expansion, highlighting, anchoring — works
// off the hunks actually being shown and the source their old side
// belongs to. For the since-viewed diff that source is the snapshot,
// which is a complete copy of the file, so context expansion and
// whole-file tokenizing both still work.
baseHunks={showingSince ? sinceHunks! : file.hunks}
expandSource={showingSince ? previous : oldSource}
sinceIsEmpty={showingSince && sinceHunks!.length === 0}
lineComments={lineComments}
fileComments={fileComments}
staleComments={staleComments}
draft={draft}
bodyRef={bodyRef}
onStartDraft={onStartDraft}
onCancelDraft={onCancelDraft}
onSubmitDraft={onSubmitDraft}
onChanged={onChanged}
/>
) : (
// The diff is out of the render window. Hold its room open so the
// scrollbar keeps its meaning and scrolling back lands where it left.
<div className="file-gap" style={{ height: gap }} aria-hidden="true" />
))}
</section>
);
});
// A file's diff, and everything it costs: the table, the Prism pass behind it,
// and the threads pinned to its lines. Separate from FileView so it can be left
// out of the document while the reader is elsewhere in the change set — see
// useFileWindow — and so the header, the anchor and the viewed mark stay put
// when it is.
function FileBody({
file,
path,
viewType,
showingSince,
baseHunks,
expandSource,
sinceIsEmpty,
lineComments,
fileComments,
staleComments,
draft,
bodyRef,
onStartDraft,
onCancelDraft,
onSubmitDraft,
onChanged,
}: {
file: FileData;
path: string;
viewType: ViewType;
showingSince: boolean;
baseHunks: HunkData[];
expandSource: string | null;
// The since-viewed diff came out empty: the mark came off for a rename or a
// mode change rather than for anything in the contents.
sinceIsEmpty: boolean;
lineComments: Comment[];
fileComments: Comment[];
staleComments: Comment[];
draft: DraftTarget | null;
bodyRef: (el: HTMLElement | null) => void;
onStartDraft: (d: DraftTarget) => void;
onCancelDraft: () => void;
onSubmitDraft: Props['onSubmitDraft'];
onChanged: () => void;
}) {
const [drag, setDrag] = useState<DragState | null>(null);
const [hunks, expandRange] = useSourceExpansion(baseHunks, expandSource);
const canExpand = expandSource != null;
@@ -434,29 +923,29 @@ const FileView = memo(function FileView({
// A wholly added or deleted file needs no base source: its hunks already carry
// every line, so tokenizing them is exact. Otherwise we wait for the fetch
// rather than highlight a fragment — a beat of plain text beats wrong colors.
const tokens: HunkTokens | undefined = useMemo(() => {
//
// The work itself happens in a worker (see lib/highlight), so a file large
// enough for it to matter colours in a moment after it renders rather than
// holding the frame while it does.
const language = useMemo(() => {
const lang = languageForFile(path);
if (!lang) return undefined;
if (!lang) return null;
// The since-viewed diff always has its whole old side to hand — that's what a
// snapshot is — so the "added file" exemption below only applies to the full
// diff, where an added file genuinely has no base.
const wholeAdd = !showingSince && file.type === 'add';
const whole = wholeAdd ? undefined : (expandSource ?? undefined);
if (!wholeAdd && whole === undefined) return undefined;
if (whole !== undefined && whole.length > MAX_HIGHLIGHT_BYTES) return undefined;
try {
return tokenize(hunks, {
highlight: true,
refractor: refractorAdapter,
language: lang,
oldSource: whole,
// Word-level marks inside a changed line, the way GitHub shows them.
enhancers: [markEdits(hunks, { type: 'block' })],
});
} catch {
return undefined;
}
}, [hunks, path, expandSource, file.type, showingSince]);
if (!wholeAdd && expandSource == null) return null;
if (expandSource != null && expandSource.length > MAX_HIGHLIGHT_BYTES) return null;
return lang;
}, [path, expandSource, file.type, showingSince]);
const wholeSource = useMemo(
() =>
!showingSince && file.type === 'add' ? undefined : (expandSource ?? undefined),
[showingSince, file.type, expandSource],
);
const tokens = useTokens(hunks, language, wholeSource);
// Map "side:line" -> react-diff-view change key, so we can attach widgets. In
// the since-viewed diff the old side is a snapshot this browser made up, so
@@ -606,127 +1095,8 @@ const FileView = memo(function FileView({
return () => window.removeEventListener('mouseup', onUp);
}, [drag, lineKeyToChangeKey, onStartDraft, path]);
const openCount = comments.open;
// The header's +/ always describe the whole change, whichever scope is on
// screen: they are how big this file's part of the review is, and having them
// shrink when you toggle would make the file look like it had been reverted.
// What moved since you last looked is spelled out on the toggle instead.
const additions = countChanges(file.hunks, 'insert');
const deletions = countChanges(file.hunks, 'delete');
const sinceAdditions = sinceHunks ? countChanges(sinceHunks, 'insert') : 0;
const sinceDeletions = sinceHunks ? countChanges(sinceHunks, 'delete') : 0;
// Marking a file viewed folds it away, and unmarking brings it back — the
// reason you'd touch the checkbox is that you're done with (or returning to)
// this file, so the fold is the point.
//
// The contents go with the mark, so that when this file next changes there is
// something to diff against. `newSource` is null only for a binary file or one
// whose base contents haven't arrived; the mark is still worth making then, it
// just won't be able to show you what moved.
const toggleViewed = () => {
onSetViewed(path, !viewed, newSource ?? undefined);
fold(!viewed);
};
return (
<section className={`file${viewed ? ' is-viewed' : ''}`} id={`file-${path}`}>
<header
className={`file-head${collapsed ? ' is-collapsed' : ''}${
viewed ? ' is-viewed' : ''
}`}
>
<button
className="file-collapse"
onClick={() => fold(!collapsed)}
aria-label={collapsed ? 'Expand' : 'Collapse'}
>
<Icon name={collapsed ? 'chevron-right' : 'chevron-down'} />
</button>
<span className="file-path">{path}</span>
<button
className="icon-btn"
onClick={() => navigator.clipboard?.writeText(path)}
title="Copy path"
aria-label="Copy path"
>
<Icon name="copy" />
</button>
{file.type !== 'modify' && (
<span className={`file-status file-status-${file.type}`}>
{statusLabel(file.type)}
</span>
)}
{file.type === 'rename' && (
<span className="file-rename muted"> {file.oldPath}</span>
)}
{changed && !viewed && (
<span
className="file-status file-status-changed"
title="This file's diff changed since you marked it viewed, so the mark came off"
>
changed since viewed
</span>
)}
<span className="file-head-right">
{hasPrevious && !file.isBinary && (
<span className="segmented scope-toggle" role="group" aria-label="Diff scope">
<button
className={showingSince ? 'is-active' : ''}
onClick={() => setScope('since')}
disabled={sinceHunks == null}
aria-pressed={showingSince}
title={
sinceHunks == null
? 'Loading the version you last viewed…'
: `Only what moved since you marked this file viewed — +${sinceAdditions} ${sinceDeletions}`
}
>
<Icon name="clock" size={14} />
<span className="btn-label">since viewed</span>
</button>
<button
className={showingSince ? '' : 'is-active'}
onClick={() => setScope('full')}
aria-pressed={!showingSince}
title={`This file's whole diff against ${base}`}
>
<Icon name="file-diff" size={14} />
<span className="btn-label">full diff</span>
</button>
</span>
)}
<span className="file-stat file-stat-add">+{additions}</span>
<span className="file-stat file-stat-del">{deletions}</span>
<DiffStat additions={additions} deletions={deletions} />
<label
className={`viewed-check${viewed ? ' is-on' : ''}`}
title={
viewed
? 'Mark as not viewed (expands the file)'
: 'Mark as viewed (collapses the file)'
}
>
<input type="checkbox" checked={viewed} onChange={toggleViewed} />
Viewed
</label>
<button
className="icon-btn has-label"
onClick={() => onStartDraft({ level: 'file', file: path })}
title={
openCount > 0
? `${openCount} open comment${openCount === 1 ? '' : 's'} — add another`
: 'Comment on this file'
}
>
<Icon name="comment" />
{openCount > 0 && openCount}
</button>
</span>
</header>
{!collapsed && (
<>
<div className="file-body" ref={bodyRef}>
{(fileComments.length > 0 ||
(draft?.level === 'file' && draft.file === path)) && (
<div className="file-level-comments">
@@ -745,9 +1115,9 @@ const FileView = memo(function FileView({
<div className="scope-note">
<Icon name="file-diff" size={14} />
<span>
{sinceHunks?.length
? 'Showing what changed since you last marked this file viewed. The left side is the file as you read it then.'
: "Nothing in this file's contents moved since you last viewed it — the mark came off for something else, a rename or a mode change."}
{sinceIsEmpty
? "Nothing in this file's contents moved since you last viewed it — the mark came off for something else, a rename or a mode change."
: 'Showing what changed since you last marked this file viewed. The left side is the file as you read it then.'}
</span>
{unplacedComments > 0 && (
<span className="muted">
@@ -822,11 +1192,9 @@ const FileView = memo(function FileView({
}}
</Diff>
)}
</>
)}
</section>
</div>
);
});
}
// Lines revealed per click on a directional expander, as on GitHub.
const CHUNK = 20;
+51
View File
@@ -0,0 +1,51 @@
import { useEffect, useState } from 'react';
import type { HunkData, HunkTokens } from 'react-diff-view';
import type { HighlightRequest, HighlightResponse } from './highlight.worker';
// Inlined rather than emitted as a second file: the built bundle is embedded in
// the playpen binary by name (see src/review/assets.zig), and one name is worth
// keeping.
import HighlightWorker from './highlight.worker?worker&inline';
// One worker for the page. Highlighting is serial on it by construction, which
// is what we want: three files scrolling into view at once should colour in one
// after another rather than fight each other for a core.
let worker: Worker | null = null;
function highlighter(): Worker {
return (worker ??= new HighlightWorker());
}
let nextId = 0;
// useTokens highlights a file's hunks in the background, returning undefined
// until the answer arrives — and again whenever the question changes, because
// tokens cut for one set of hunks describe the wrong lines of another. A file
// waiting on its colours renders in plain text, which is what it does today
// while its base source is still being fetched.
//
// Pass a null language for anything that shouldn't be highlighted at all: a file
// type Prism doesn't know, or one whose base source hasn't landed yet.
export function useTokens(
hunks: HunkData[],
language: string | null,
oldSource: string | undefined,
): HunkTokens | undefined {
const [tokens, setTokens] = useState<HunkTokens | undefined>(undefined);
useEffect(() => {
setTokens(undefined);
if (language == null) return;
const id = ++nextId;
const w = highlighter();
const onMessage = ({ data }: MessageEvent<HighlightResponse>) => {
if (data.id !== id) return;
setTokens(data.tokens ?? undefined);
};
w.addEventListener('message', onMessage);
w.postMessage({ id, hunks, oldSource, language } satisfies HighlightRequest);
return () => w.removeEventListener('message', onMessage);
}, [hunks, language, oldSource]);
return tokens;
}
+59
View File
@@ -0,0 +1,59 @@
/// <reference lib="webworker" />
// Prism, off the main thread.
//
// Highlighting a diff is the single most expensive thing this page does, and
// almost none of that cost is Prism itself: tokenizing an eight-thousand-line
// file takes refractor about a tenth of a second, and react-diff-view another
// two seconds to cut the result up and hand each hunk its lines. On the main
// thread that is a two-second freeze — on load, and again on every context
// expansion, since expanding changes the hunks and the whole file is re-cut.
//
// So it happens here instead. The page renders the diff in plain text the moment
// it has it and the colours arrive a beat later, which is what it already did
// for any file whose base contents were still being fetched.
//
// What comes back has to be structure-cloned across, and that is not free — it
// is the one part of the cost the page still pays — but it is a fifth of doing
// the work here, and it does not block anything that is trying to scroll.
import { markEdits, tokenize, type HunkData, type HunkTokens } from 'react-diff-view';
import { refractorAdapter } from './language';
export interface HighlightRequest {
id: number;
hunks: HunkData[];
// The file's whole base-side source, so Prism sees the constructs that open
// above the first visible line. Undefined for a wholly added file, whose hunks
// already carry every line there is.
oldSource: string | undefined;
language: string;
}
export interface HighlightResponse {
id: number;
// Null when tokenizing threw — a language Prism mis-handles, or hunks that
// don't line up with the source. The page falls back to plain text.
tokens: HunkTokens | null;
}
const ctx = self as unknown as DedicatedWorkerGlobalScope;
ctx.addEventListener('message', (event: MessageEvent<HighlightRequest>) => {
const { id, hunks, oldSource, language } = event.data;
let tokens: HunkTokens | null = null;
try {
tokens = tokenize(hunks, {
highlight: true,
refractor: refractorAdapter,
language,
oldSource,
// Word-level marks inside a changed line, the way GitHub shows them.
enhancers: [markEdits(hunks, { type: 'block' })],
});
} catch {
tokens = null;
}
ctx.postMessage({ id, tokens } satisfies HighlightResponse);
});
+1
View File
@@ -0,0 +1 @@
/// <reference types="vite/client" />
+15
View File
@@ -13,6 +13,21 @@ const apiTarget = `http://127.0.0.1:${apiPort}`;
export default defineConfig({
plugins: [react()],
resolve: {
// The syntax highlighter runs in a worker (see src/lib/highlight.ts), and one
// package deep under refractor — decode-named-character-reference, pulled in
// by the markup language to decode HTML entities — ships a browser build
// that calls `document.createElement` at module scope. A worker has no
// document, so importing refractor there threw before a line was highlighted.
//
// The package publishes a document-free build for exactly this case and
// lists it first in its export map, under `worker`. Asking for that
// condition is what picks it up. It resolves for the main bundle too, which
// is what we want: it is the same function with a lookup table in place of
// the DOM's entity parser, and one implementation beats two.
conditions: ['worker'],
},
// Relative asset URLs, because the page is served from a tab's path
// (`/t/tab3/`) and not from the server root. With an absolute base the
// browser would ask for `/assets/app.js` and get the tab router instead.