From 2a0e03cc41f4aa548d038aa863f5360e75aae983 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 8 Sep 2026 14:13:53 -0400 Subject: [PATCH] Improve review-tool performance. --- web/src/components/DiffView.tsx | 858 +++++++++++++++++++++++--------- web/src/lib/highlight.ts | 51 ++ web/src/lib/highlight.worker.ts | 59 +++ web/src/vite-env.d.ts | 1 + web/vite.config.ts | 15 + 5 files changed, 739 insertions(+), 245 deletions(-) create mode 100644 web/src/lib/highlight.ts create mode 100644 web/src/lib/highlight.worker.ts create mode 100644 web/src/vite-env.d.ts diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index da42582..ad5027f 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -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-` 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(); + 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>(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()); + const heights = useRef(new Map()); + const cards = useRef(new Map()); + const bodies = useRef(new Map()); + + 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 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 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(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(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 ( +
+
+ + {path} + + {file.type !== 'modify' && ( + + {statusLabel(file.type)} + + )} + {file.type === 'rename' && ( + ← {file.oldPath} + )} + {changed && !viewed && ( + + changed since viewed + + )} + + {hasPrevious && !file.isBinary && ( + + + + + )} + +{additions} + −{deletions} + + + + +
+ + {!collapsed && + (active ? ( + + ) : ( + // 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. +
+ ); +}); + +// 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(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,227 +1095,106 @@ 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 ( -
-
- - {path} - - {file.type !== 'modify' && ( - - {statusLabel(file.type)} +
+ {(fileComments.length > 0 || + (draft?.level === 'file' && draft.file === path)) && ( +
+ {fileComments.length > 0 && ( + + )} + {draft?.level === 'file' && draft.file === path && ( + + )} +
+ )} + {staleComments.length > 0 && ( + + )} + {showingSince && ( +
+ + + {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.'} - )} - {file.type === 'rename' && ( - ← {file.oldPath} - )} - {changed && !viewed && ( - - changed since viewed - - )} - - {hasPrevious && !file.isBinary && ( - - - + {unplacedComments > 0 && ( + + {unplacedComments} comment + {unplacedComments === 1 ? '' : 's'} elsewhere in this file — switch + to the full diff to see them in place. )} - +{additions} - −{deletions} - - - - -
- - {!collapsed && ( - <> - {(fileComments.length > 0 || - (draft?.level === 'file' && draft.file === path)) && ( -
- {fileComments.length > 0 && ( - - )} - {draft?.level === 'file' && draft.file === path && ( - - )} -
- )} - {staleComments.length > 0 && ( - - )} - {showingSince && ( -
- - - {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."} - - {unplacedComments > 0 && ( - - {unplacedComments} comment - {unplacedComments === 1 ? '' : 's'} elsewhere in this file — switch - to the full diff to see them in place. - - )} -
- )} - {file.isBinary ? ( -
Binary file not shown.
- ) : ( - - {(renderHunks) => { - const out: ReactElement[] = []; - renderHunks.forEach((hunk, i) => { - const prev: HunkData | null = i > 0 ? renderHunks[i - 1] : null; - const collapsed = getCollapsedLinesCountBetween(prev, hunk); - // Ranges are [start, end) — end is EXCLUSIVE, matching - // react-diff-view's expandFromRawCode (slice semantics). - const start = prev ? prev.oldStart + prev.oldLines : 1; - out.push( - - - , - ); - out.push(); - }); - // Trailing gap: lines after the last hunk to end of file. - const last = renderHunks[renderHunks.length - 1]; - if (last && canExpand && totalOldLines != null) { - const start = last.oldStart + last.oldLines; - const collapsed = totalOldLines - start + 1; - if (collapsed > 0) { - out.push( - - - , - ); - } - } - return out; - }} - - )} - + )} -
+ {file.isBinary ? ( +
Binary file not shown.
+ ) : ( + + {(renderHunks) => { + const out: ReactElement[] = []; + renderHunks.forEach((hunk, i) => { + const prev: HunkData | null = i > 0 ? renderHunks[i - 1] : null; + const collapsed = getCollapsedLinesCountBetween(prev, hunk); + // Ranges are [start, end) — end is EXCLUSIVE, matching + // react-diff-view's expandFromRawCode (slice semantics). + const start = prev ? prev.oldStart + prev.oldLines : 1; + out.push( + + + , + ); + out.push(); + }); + // Trailing gap: lines after the last hunk to end of file. + const last = renderHunks[renderHunks.length - 1]; + if (last && canExpand && totalOldLines != null) { + const start = last.oldStart + last.oldLines; + const collapsed = totalOldLines - start + 1; + if (collapsed > 0) { + out.push( + + + , + ); + } + } + return out; + }} + + )} + ); -}); +} // Lines revealed per click on a directional expander, as on GitHub. const CHUNK = 20; diff --git a/web/src/lib/highlight.ts b/web/src/lib/highlight.ts new file mode 100644 index 0000000..6e06fb2 --- /dev/null +++ b/web/src/lib/highlight.ts @@ -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(undefined); + + useEffect(() => { + setTokens(undefined); + if (language == null) return; + + const id = ++nextId; + const w = highlighter(); + const onMessage = ({ data }: MessageEvent) => { + 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; +} diff --git a/web/src/lib/highlight.worker.ts b/web/src/lib/highlight.worker.ts new file mode 100644 index 0000000..a229363 --- /dev/null +++ b/web/src/lib/highlight.worker.ts @@ -0,0 +1,59 @@ +/// + +// 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) => { + 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); +}); diff --git a/web/src/vite-env.d.ts b/web/src/vite-env.d.ts new file mode 100644 index 0000000..11f02fe --- /dev/null +++ b/web/src/vite-env.d.ts @@ -0,0 +1 @@ +/// diff --git a/web/vite.config.ts b/web/vite.config.ts index e2bd474..79f8d05 100644 --- a/web/vite.config.ts +++ b/web/vite.config.ts @@ -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.