Skip to content

[diffs] Optimize diffs virtualized initial render #606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SlexAxton wants to merge 1 commit into from
Closed

[diffs] Optimize diffs virtualized initial render #606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions packages/diffs/src/components/File.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { upsertHostThemeStyle } from '../utils/hostTheme';
import { prerenderHTMLIfNecessary } from '../utils/prerenderHTMLIfNecessary';
import { setPreNodeProperties } from '../utils/setWrapperNodeProps';
import type { WorkerPoolManager } from '../worker';
import { DiffsContainerLoaded } from './web-components';
import { DiffsContainerLoaded, ensureDiffsShadowRoot } from './web-components';

const EMPTY_STRINGS: string[] = [];

Expand Down Expand Up @@ -572,9 +572,7 @@ export class File<LAnnotation = undefined> {
this.cleanChildNodes();

if (this.placeHolder == null) {
const shadowRoot =
this.fileContainer.shadowRoot ??
this.fileContainer.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(this.fileContainer, false);
this.placeHolder = document.createElement('div');
this.placeHolder.dataset.placeholder = '';
shadowRoot.appendChild(this.placeHolder);
Expand Down Expand Up @@ -726,8 +724,7 @@ export class File<LAnnotation = undefined> {
themeType: ThemeTypes,
baseThemeType?: 'light' | 'dark'
): void {
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
const effectiveThemeType = baseThemeType ?? themeType;
if (
this.themeCSSStyle?.parentNode === shadowRoot &&
Expand Down Expand Up @@ -1017,6 +1014,7 @@ export class File<LAnnotation = undefined> {
const { file } = this;
if (file == null) return;
this.cleanupErrorWrapper();
const shadowRoot = ensureDiffsShadowRoot(container);
this.placeHolder?.remove();
this.placeHolder = undefined;
const headerHTML = toHtml(headerAST);
Expand All @@ -1028,9 +1026,9 @@ export class File<LAnnotation = undefined> {
return;
}
if (this.headerElement != null) {
container.shadowRoot?.replaceChild(newHeader, this.headerElement);
shadowRoot.replaceChild(newHeader, this.headerElement);
} else {
container.shadowRoot?.prepend(newHeader);
shadowRoot.prepend(newHeader);
}
this.headerElement = newHeader;
this.lastRenderedHeaderHTML = headerHTML;
Expand Down Expand Up @@ -1136,20 +1134,20 @@ export class File<LAnnotation = undefined> {
parentNode.appendChild(this.fileContainer);
}
if (this.spriteSVG == null) {
const shadowRoot = ensureDiffsShadowRoot(this.fileContainer);
const fragment = document.createElement('div');
fragment.innerHTML = SVGSpriteSheet;
const firstChild = fragment.firstChild;
if (firstChild instanceof SVGElement) {
this.spriteSVG = firstChild;
this.fileContainer.shadowRoot?.appendChild(this.spriteSVG);
shadowRoot.appendChild(this.spriteSVG);
}
}
return this.fileContainer;
}

private getOrCreatePreNode(container: HTMLElement): HTMLPreElement {
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
// If we haven't created a pre element yet, lets go ahead and do that
if (this.pre == null) {
this.pre = document.createElement('pre');
Expand All @@ -1160,7 +1158,7 @@ export class File<LAnnotation = undefined> {
// If we have a new parent container for the pre element, lets go ahead and
// move it into the new container
else if (this.pre.parentNode !== shadowRoot) {
container.shadowRoot?.appendChild(this.pre);
shadowRoot.appendChild(this.pre);
this.appliedPreAttributes = undefined;
}

Expand Down Expand Up @@ -1211,8 +1209,7 @@ export class File<LAnnotation = undefined> {
pre.remove();
this.pre = undefined;
this.appliedPreAttributes = undefined;
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
this.errorWrapper ??= document.createElement('div');
this.errorWrapper.dataset.errorWrapper = '';
this.errorWrapper.innerHTML = '';
Expand Down
82 changes: 54 additions & 28 deletions packages/diffs/src/components/FileDiff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ import { parseDiffFromFile } from '../utils/parseDiffFromFile';
import { prerenderHTMLIfNecessary } from '../utils/prerenderHTMLIfNecessary';
import { setPreNodeProperties } from '../utils/setWrapperNodeProps';
import type { WorkerPoolManager } from '../worker';
import { DiffsContainerLoaded } from './web-components';
import { DiffsContainerLoaded, ensureDiffsShadowRoot } from './web-components';

export interface FileDiffRenderProps<LAnnotation> {
fileDiff?: FileDiffMetadata;
Expand Down Expand Up @@ -401,6 +401,7 @@ export class FileDiff<LAnnotation = undefined> {
lineAnnotations: DiffLineAnnotation<LAnnotation>[]
): void {
this.lineAnnotations = lineAnnotations;
this.hunksRenderer.setLineAnnotations(lineAnnotations);
}

private canPartiallyRender(
Expand Down Expand Up @@ -611,7 +612,11 @@ export class FileDiff<LAnnotation = undefined> {
this.renderGutterUtility();
this.injectUnsafeCSS();
this.interactionManager.setup(this.pre);
this.resizeManager.setup(this.pre, overflow === 'wrap');
if (overflow === 'wrap' || this.lineAnnotations.length > 0) {
this.resizeManager.setup(this.pre, overflow === 'wrap');
} else {
this.resizeManager.cleanUp();
}
if (overflow === 'scroll' && diffStyle === 'split') {
this.scrollSyncManager.setup(
this.pre,
Expand Down Expand Up @@ -730,8 +735,6 @@ export class FileDiff<LAnnotation = undefined> {
}
this.hunksRenderer.setOptions(this.getHunksRendererOptions(this.options));

this.hunksRenderer.setLineAnnotations(this.lineAnnotations);

const {
diffStyle = 'split',
disableErrorHandling = false,
Expand Down Expand Up @@ -850,7 +853,11 @@ export class FileDiff<LAnnotation = undefined> {
this.renderGutterUtility();

this.interactionManager.setup(pre);
this.resizeManager.setup(pre, overflow === 'wrap');
if (overflow === 'wrap' || this.lineAnnotations.length > 0) {
this.resizeManager.setup(pre, overflow === 'wrap');
} else {
this.resizeManager.cleanUp();
}
if (overflow === 'scroll' && diffStyle === 'split') {
this.scrollSyncManager.setup(
pre,
Expand Down Expand Up @@ -927,12 +934,12 @@ export class FileDiff<LAnnotation = undefined> {
this.cleanChildNodes();

if (this.placeHolder == null) {
const shadowRoot =
this.fileContainer.shadowRoot ??
this.fileContainer.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(this.fileContainer, false);
this.placeHolder = document.createElement('div');
this.placeHolder.dataset.placeholder = '';
this.placeHolder.style.cssText = `contain: strict; height: ${height}px;`;
shadowRoot.appendChild(this.placeHolder);
return true;
}
this.placeHolder.style.setProperty('height', `${height}px`);
return true;
Expand Down Expand Up @@ -1103,25 +1110,32 @@ export class FileDiff<LAnnotation = undefined> {
if (parentNode != null && this.fileContainer.parentNode !== parentNode) {
parentNode.appendChild(this.fileContainer);
}
if (this.spriteSVG == null) {
const fragment = document.createElement('div');
fragment.innerHTML = SVGSpriteSheet;
const firstChild = fragment.firstChild;
if (firstChild instanceof SVGElement) {
this.spriteSVG = firstChild;
this.fileContainer.shadowRoot?.appendChild(this.spriteSVG);
}
}
return this.fileContainer;
}

private ensureSpriteSVG(container: HTMLElement): void {
if (this.spriteSVG != null) {
return;
}
const shadowRoot = container.shadowRoot;
if (shadowRoot == null) {
return;
}
const fragment = document.createElement('div');
fragment.innerHTML = SVGSpriteSheet;
const firstChild = fragment.firstChild;
if (firstChild instanceof SVGElement) {
this.spriteSVG = firstChild;
shadowRoot.appendChild(this.spriteSVG);
}
}

protected getFileContainer(): HTMLElement | undefined {
return this.fileContainer;
}

private getOrCreatePreNode(container: HTMLElement): HTMLPreElement {
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
// If we haven't created a pre element yet, lets go ahead and do that
if (this.pre == null) {
this.pre = document.createElement('pre');
Expand Down Expand Up @@ -1167,6 +1181,8 @@ export class FileDiff<LAnnotation = undefined> {
container: HTMLElement
): void {
this.cleanupErrorWrapper();
const shadowRoot = ensureDiffsShadowRoot(container);
this.ensureSpriteSVG(container);
this.placeHolder?.remove();
this.placeHolder = undefined;
const { fileDiff } = this;
Expand All @@ -1179,9 +1195,9 @@ export class FileDiff<LAnnotation = undefined> {
return;
}
if (this.headerElement != null) {
container.shadowRoot?.replaceChild(newHeader, this.headerElement);
shadowRoot.replaceChild(newHeader, this.headerElement);
} else {
container.shadowRoot?.prepend(newHeader);
shadowRoot.prepend(newHeader);
}
this.headerElement = newHeader;
this.lastRenderedHeaderHTML = headerHTML;
Expand Down Expand Up @@ -1312,8 +1328,7 @@ export class FileDiff<LAnnotation = undefined> {
themeType: ThemeTypes,
baseThemeType?: 'light' | 'dark'
): void {
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
const effectiveThemeType = baseThemeType ?? themeType;
if (
this.themeCSSStyle?.parentNode === shadowRoot &&
Expand Down Expand Up @@ -1346,14 +1361,26 @@ export class FileDiff<LAnnotation = undefined> {
(this.options.hunkSeparators ?? 'line-info') === 'line-info';
const rowSpan = overflow === 'wrap' ? result.rowCount : undefined;
this.cleanupErrorWrapper();
if (this.fileContainer != null) {
this.ensureSpriteSVG(this.fileContainer);
}
this.applyPreNodeAttributes(pre, result);

let shouldReplace = false;
// Create code elements and insert HTML content
const codeElements: HTMLElement[] = [];
const unifiedAST = this.hunksRenderer.renderCodeAST('unified', result);
const deletionsAST = this.hunksRenderer.renderCodeAST('deletions', result);
const additionsAST = this.hunksRenderer.renderCodeAST('additions', result);
const unifiedAST =
result.unifiedContentAST != null
? this.hunksRenderer.renderCodeAST('unified', result)
: undefined;
const deletionsAST =
result.deletionsContentAST != null
? this.hunksRenderer.renderCodeAST('deletions', result)
: undefined;
const additionsAST =
result.additionsContentAST != null
? this.hunksRenderer.renderCodeAST('additions', result)
: undefined;
if (unifiedAST != null) {
shouldReplace =
this.codeUnified == null ||
Expand Down Expand Up @@ -2086,8 +2113,7 @@ export class FileDiff<LAnnotation = undefined> {
pre.remove();
this.pre = undefined;
this.appliedPreAttributes = undefined;
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
this.errorWrapper ??= document.createElement('div');
this.errorWrapper.dataset.errorWrapper = '';
this.errorWrapper.innerHTML = '';
Expand Down
4 changes: 2 additions & 2 deletions packages/diffs/src/components/FileStream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { getHighlighterThemeStyles } from '../utils/getHighlighterThemeStyles';
import { getOrCreateCodeNode } from '../utils/getOrCreateCodeNode';
import { upsertHostThemeStyle } from '../utils/hostTheme';
import { setPreNodeProperties } from '../utils/setWrapperNodeProps';
import { ensureDiffsShadowRoot } from './web-components';

export interface FileStreamOptions extends BaseCodeOptions {
lang?: SupportedLanguages;
Expand Down Expand Up @@ -336,8 +337,7 @@ export class FileStream {
themeType: ThemeTypes,
baseThemeType?: 'light' | 'dark'
): void {
const shadowRoot =
container.shadowRoot ?? container.attachShadow({ mode: 'open' });
const shadowRoot = ensureDiffsShadowRoot(container);
const effectiveThemeType = baseThemeType ?? themeType;
if (
this.themeCSSStyle?.parentNode === shadowRoot &&
Expand Down
45 changes: 33 additions & 12 deletions packages/diffs/src/components/VirtualizedFileDiff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,21 +133,27 @@ export class VirtualizedFileDiff<
if (lineIndexAttr == null) continue;

const lineIndex = parseLineIndex(lineIndexAttr, diffStyle);
let measuredHeight = line.getBoundingClientRect().height;
let hasMetadata = false;
// Annotations or noNewline metadata increase the size of the their
// attached line
if (
const metadataElement =
line.nextElementSibling instanceof HTMLElement &&
('lineAnnotation' in line.nextElementSibling.dataset ||
'noNewline' in line.nextElementSibling.dataset)
) {
if ('noNewline' in line.nextElementSibling.dataset) {
hasMetadata = true;
? line.nextElementSibling
: undefined;

if (metadataElement == null) {
if (this.heightCache.delete(lineIndex)) {
hasLineHeightChange = true;
}
measuredHeight +=
line.nextElementSibling.getBoundingClientRect().height;
continue;
Comment on lines +143 to +147

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Measure non-metadata rows in wrap-mode height reconciliation

When overflow is wrap, rows can become taller from soft wrapping even without a sibling metadata row. This branch deletes cached heights and immediately continues whenever metadataElement is missing, so wrapped rows are never measured and computeApproximateSize() falls back to a single-line estimate. In large wrapped diffs this underestimates file height, which causes virtualized ranges/placeholders to drift and visible jumpiness while scrolling.

Useful? React with 👍 / 👎.

}

const hasMetadata = 'noNewline' in metadataElement.dataset;
// In scroll mode normal rows use the configured line height. Measuring
// only rows with attached metadata avoids forcing layout for every
// visible line while still correcting rows whose height can change.
const measuredHeight =
line.getBoundingClientRect().height +
metadataElement.getBoundingClientRect().height;
const expectedHeight = this.getLineHeight(lineIndex, hasMetadata);

if (measuredHeight === expectedHeight) {
Expand Down Expand Up @@ -175,6 +181,10 @@ export class VirtualizedFileDiff<
}
}

public getEstimatedBottom(): number | undefined {
return this.top == null ? undefined : this.top + this.height;
}

public onRender = (dirty: boolean): boolean => {
if (this.fileContainer == null) {
return false;
Expand Down Expand Up @@ -366,14 +376,14 @@ export class VirtualizedFileDiff<
if (!isSetup) {
this.computeApproximateSize();
this.virtualizer.connect(fileContainer, this);
this.top ??= this.virtualizer.getOffsetInScrollContainer(fileContainer);
this.top ??= this.getInitialTop(fileContainer);
this.isVisible = this.virtualizer.isInstanceVisible(
this.top,
this.height
);
this.isSetup = true;
} else {
this.top ??= this.virtualizer.getOffsetInScrollContainer(fileContainer);
this.top ??= this.getInitialTop(fileContainer);
}

if (!this.isVisible) {
Expand All @@ -396,6 +406,17 @@ export class VirtualizedFileDiff<
});
}

// Use the previous virtualized sibling's estimated bottom to avoid forcing
// layout for every file during the initial sequential render. If another
// element, such as patch metadata, breaks the sequence we fall back to a real
// DOM measurement for that file.
private getInitialTop(fileContainer: HTMLElement): number {
return (
this.virtualizer.getEstimatedOffsetAfterPrevious?.(fileContainer) ??
this.virtualizer.getOffsetInScrollContainer(fileContainer)
);
}

private getDiffStyle(): 'split' | 'unified' {
return this.options.diffStyle ?? 'split';
}
Expand Down
Loading
Loading