From 5508833a6420dca33a90bda3b9a6ee0e70d9b9b4 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Mon, 3 Aug 2026 15:55:45 -0700 Subject: [PATCH 1/4] diffs: add CodeView.removeItem API Kinda dumb we never had this before lol, but here we are --- .../app/(diffs)/docs/CodeView/content.mdx | 18 +++--- .../app/(diffs)/docs/VanillaAPI/content.mdx | 8 +-- packages/diffs/src/components/CodeView.ts | 17 ++++++ packages/diffs/src/react/CodeView.tsx | 14 +++++ .../diffs/test/CodeView.removeItem.test.ts | 59 +++++++++++++++++++ .../test/reactCodeViewEditorOptions.test.ts | 54 +++++++++++++++++ 6 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 packages/diffs/test/CodeView.removeItem.test.ts diff --git a/apps/docs/app/(diffs)/docs/CodeView/content.mdx b/apps/docs/app/(diffs)/docs/CodeView/content.mdx index 45cc087d9..a2e1f026d 100644 --- a/apps/docs/app/(diffs)/docs/CodeView/content.mdx +++ b/apps/docs/app/(diffs)/docs/CodeView/content.mdx @@ -36,8 +36,8 @@ regardless of scale, so its data model does not depend on traditional immutability or deep equality checks, which can quickly become expensive. - Every item needs a stable unique `id`. That id is how `scrollTo`, line - selection, `getItem`, `updateItem`, and reconciliation find the correct - records. + selection, `getItem`, `removeItem`, `updateItem`, and reconciliation find the + correct records. - Items are either `{ type: 'file', file }` or `{ type: 'diff', fileDiff }`. - If you keep the same item id but change its content or annotations, you must increment the `version` so `CodeView` can make an efficient targeted updates @@ -187,14 +187,14 @@ do not switch between them without remounting with a new `key`. | Mode | Use | Item prop | Item updates | | ---------- | -------------------------------------------------- | ----------------------- | ------------------------------------------------------------------------------------------------- | | Controlled | React state owns the complete item list | `items` | Publish a new `items` array. Append-only changes are optimized; other changes reconcile the list. | -| Imperative | The viewer instance owns the item list after mount | optional `initialItems` | Use the ref APIs: `addItems`, `getItem`, and `updateItem`. | +| Imperative | The viewer instance owns the item list after mount | optional `initialItems` | Use the ref APIs: `addItems`, `getItem`, `removeItem`, and `updateItem`. | Use controlled mode when item data already lives naturally in React state and the list is small enough that mutating arrays or items is cheap. Use imperative mode for very large or streaming surfaces where routing every item update through React would be expensive. In imperative mode, omit `items`, optionally seed the viewer with `initialItems`, and use the `CodeViewHandle` to add new -items or update existing ones. +items, remove items, or update existing ones. ### Editing Item Annotations @@ -229,12 +229,13 @@ remapping rules, stable metadata IDs, and annotation-content lifetime guidance. - In React, pass `initialItems` instead of `items` for imperative item ownership. `initialItems` seeds the viewer once; later item changes should go through the ref. -- In React, `addItems` and `updateItem` require imperative item ownership and - throw if the viewer is controlled with `items`. +- In React, `addItems`, `removeItem`, and `updateItem` require imperative item + ownership and throw if the viewer is controlled with `items`. - In React, use `selectedLines` and `onSelectedLinesChange` when selection needs to live in component state. - In React, use the ref for `scrollTo`, `setSelectedLines`, `getSelectedLines`, - `clearSelectedLines`, `getItem`, `updateItem`, `addItems`, and `getInstance`. + `clearSelectedLines`, `getItem`, `updateItem`, `addItems`, `removeItem`, and + `getInstance`. - `renderCustomHeader`, `renderHeaderPrefix`, `renderHeaderFilenameSuffix`, `renderHeaderMetadata`, `renderAnnotation`, and `renderGutterUtility` receive the whole `CodeViewItem`, which makes it easy to branch on `item.type`. @@ -242,7 +243,8 @@ remapping rules, stable metadata IDs, and annotation-content lifetime guidance. update over time. - In Vanilla JS, call `setup(root)` once with the scrollable container. - In Vanilla JS, use `setItems`, `addItem`, or `addItems` to populate the - viewer, and `getItem` / `updateItem` for item-level imperative changes. + viewer, and `getItem`, `removeItem`, or `updateItem` for item-level imperative + changes. - Shared callbacks receive the normal file/diff payload plus a `context` argument containing the current viewer item and instance. - `onPostRender` receives `(node, instance, phase, context)`. Its `unmount` diff --git a/apps/docs/app/(diffs)/docs/VanillaAPI/content.mdx b/apps/docs/app/(diffs)/docs/VanillaAPI/content.mdx index d102a0c4b..d498178ed 100644 --- a/apps/docs/app/(diffs)/docs/VanillaAPI/content.mdx +++ b/apps/docs/app/(diffs)/docs/VanillaAPI/content.mdx @@ -31,8 +31,8 @@ rendered `File` or `FileDiff` with `edit()`. (`onMergeConflictResolve` / `onMergeConflictAction`). The `CodeView` tab above is the quick-start version. For the deeper guide on -`setup`, `setItems`, `addItems`, `getItem`, `updateItem`, selection, and -`scrollTo`, see [CodeView](#codeview). +`setup`, `setItems`, `addItems`, `getItem`, `removeItem`, `updateItem`, +selection, and `scrollTo`, see [CodeView](#codeview). ### Props @@ -63,8 +63,8 @@ default; set `disableErrorHandling: true` when you want errors to rethrow. `CodeView` forwards many of those same options to each rendered item, while adding CodeView-specific controls like `layout`, `itemMetrics`, `stickyHeaders`, `pointerEventsOnScroll`, and `smoothScrollSettings`. Its class instance also -exposes item-level methods such as `addItems`, `getItem`, and `updateItem`. See -[CodeView](#codeview) for the dedicated guide. +exposes item-level methods such as `addItems`, `getItem`, `removeItem`, and +`updateItem`. See [CodeView](#codeview) for the dedicated guide. Header customization and collapsing behavior: diff --git a/packages/diffs/src/components/CodeView.ts b/packages/diffs/src/components/CodeView.ts index 596062a82..be2dadb82 100644 --- a/packages/diffs/src/components/CodeView.ts +++ b/packages/diffs/src/components/CodeView.ts @@ -1558,6 +1558,23 @@ export class CodeView { this.syncItemEditors(); } + public removeItem(itemId: string): boolean { + const item = this.idToItem.get(itemId); + if (item == null) { + console.error(`CodeView.removeItem: unknown item id "${itemId}"`); + return false; + } + + const nextItems: CodeViewItem[] = []; + for (const current of this.items) { + if (current !== item) { + nextItems.push(current.item); + } + } + this.setItems(nextItems); + return true; + } + public setItems(items: readonly CodeViewItem[]): void { if (items.length === 0) { // An empty controlled list removes every item, so end active edit diff --git a/packages/diffs/src/react/CodeView.tsx b/packages/diffs/src/react/CodeView.tsx index a64afdb34..7aee20bdc 100644 --- a/packages/diffs/src/react/CodeView.tsx +++ b/packages/diffs/src/react/CodeView.tsx @@ -133,6 +133,7 @@ export type CodeViewProps = export interface CodeViewHandle { addItems(items: readonly CodeViewItem[]): void; getItem(id: string): CodeViewItem | undefined; + removeItem(id: string): boolean; updateItem(item: CodeViewItem): boolean; updateItemId(oldId: string, newId: string): boolean; scrollTo(target: CodeViewScrollTarget): void; @@ -514,6 +515,19 @@ function CodeViewInner( return instance.getItem(id); } }, + removeItem(id) { + const { controlled, instance } = cachedDataRef.current; + assertUncontrolledCodeViewAction(controlled, 'removeItem'); + if (instance == null) { + console.error( + 'CodeView.removeItem: no valid instance to remove item from', + id + ); + return false; + } + + return instance.removeItem(id); + }, updateItem(item) { const { controlled, instance } = cachedDataRef.current; assertUncontrolledCodeViewAction(controlled, 'updateItem'); diff --git a/packages/diffs/test/CodeView.removeItem.test.ts b/packages/diffs/test/CodeView.removeItem.test.ts new file mode 100644 index 000000000..862cffc0f --- /dev/null +++ b/packages/diffs/test/CodeView.removeItem.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, spyOn, test } from 'bun:test'; + +import { CodeView } from '../src/components/CodeView'; +import { + createRoot, + installDom, + makeFileItem, + renderItems, + wait, +} from './domHarness'; + +describe('CodeView.removeItem', () => { + test('removes an item while preserving the remaining order', async () => { + const { cleanup } = installDom(); + const viewer = new CodeView(); + try { + viewer.setup(createRoot()); + await renderItems(viewer, [ + makeFileItem('first', 5), + makeFileItem('middle', 5), + makeFileItem('last', 5), + ]); + + expect(viewer.removeItem('middle')).toBe(true); + viewer.render(true); + + expect(viewer.getItem('middle')).toBeUndefined(); + expect(viewer.getRenderedItems().map((item) => item.id)).toEqual([ + 'first', + 'last', + ]); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + + test('returns false for an unknown item id', async () => { + const { cleanup } = installDom(); + const consoleError = spyOn(console, 'error').mockImplementation(() => {}); + const viewer = new CodeView(); + try { + viewer.setup(createRoot()); + await renderItems(viewer, [makeFileItem('existing', 5)]); + + expect(viewer.removeItem('missing')).toBe(false); + expect(viewer.getItem('existing')).toBeDefined(); + expect(consoleError).toHaveBeenCalledWith( + 'CodeView.removeItem: unknown item id "missing"' + ); + } finally { + consoleError.mockRestore(); + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); +}); diff --git a/packages/diffs/test/reactCodeViewEditorOptions.test.ts b/packages/diffs/test/reactCodeViewEditorOptions.test.ts index 7ae8729b5..fab26842c 100644 --- a/packages/diffs/test/reactCodeViewEditorOptions.test.ts +++ b/packages/diffs/test/reactCodeViewEditorOptions.test.ts @@ -667,6 +667,60 @@ describe('React CodeView editor factory', () => { } }); + test('removes initial items through the imperative handle', async () => { + const { cleanup } = installCodeViewDom(); + const cleanupActEnvironment = installReactActEnvironment(); + const container = document.createElement('div'); + document.body.appendChild(container); + const handle = createRef>(); + let root: Root | undefined; + + try { + root = createReactRoot(container); + await renderRoot( + root, + createCodeViewElement({ + initialItems: [makeFileItem('first'), makeFileItem('last')], + ref: handle, + }) + ); + + expect(handle.current?.removeItem('first')).toBe(true); + expect(handle.current?.getItem('first')).toBeUndefined(); + expect(handle.current?.getItem('last')).toBeDefined(); + } finally { + await unmountRoot(root); + cleanupActEnvironment(); + cleanup(); + } + }); + + test('rejects imperative item removal in controlled mode', async () => { + const { cleanup } = installCodeViewDom(); + const cleanupActEnvironment = installReactActEnvironment(); + const container = document.createElement('div'); + document.body.appendChild(container); + const handle = createRef>(); + let root: Root | undefined; + + try { + root = createReactRoot(container); + await renderRoot( + root, + createCodeViewElement({ items: [makeFileItem('a')], ref: handle }) + ); + + expect(() => handle.current?.removeItem('a')).toThrow( + 'CodeView.removeItem cannot be used when CodeView is controlled. Use initialItems for imperative item updates.' + ); + expect(handle.current?.getItem('a')).toBeDefined(); + } finally { + await unmountRoot(root); + cleanupActEnvironment(); + cleanup(); + } + }); + test('retains an item editor across virtualization', async () => { const { cleanup } = installCodeViewDom(); const cleanupActEnvironment = installReactActEnvironment(); From 104555fe62a46d4c870a827da86cc9c0a98aa0fd Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Mon, 3 Aug 2026 16:13:08 -0700 Subject: [PATCH 2/4] Hack fix for PR review --- packages/diffs/src/components/CodeView.ts | 8 ++++++- packages/diffs/test/CodeView.edit.test.ts | 27 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/diffs/src/components/CodeView.ts b/packages/diffs/src/components/CodeView.ts index be2dadb82..54c973071 100644 --- a/packages/diffs/src/components/CodeView.ts +++ b/packages/diffs/src/components/CodeView.ts @@ -1583,7 +1583,13 @@ export class CodeView { // reset()/cleanUp() calls stay silent — those are teardowns, not item // data updates. const completions: CodeViewItemEditChange[] = []; - for (const record of this.itemEditors.values()) { + for (const [id, record] of this.itemEditors) { + const item = this.idToItem.get(id); + record.editor.cleanUp(); + if (item?.type === 'diff') { + item.instance.completeEditSession(); + finishEditSessionForDiff(item.item.fileDiff); + } const { lastChange } = record.state; if (lastChange != null) { completions.push(lastChange); diff --git a/packages/diffs/test/CodeView.edit.test.ts b/packages/diffs/test/CodeView.edit.test.ts index aab58d30c..64efb1dbc 100644 --- a/packages/diffs/test/CodeView.edit.test.ts +++ b/packages/diffs/test/CodeView.edit.test.ts @@ -1204,6 +1204,33 @@ describe('CodeView item edit mode', () => { } }); + test('finalizes session hunks when removing the only item', async () => { + const { cleanup } = installDom(); + const { createEditor } = createEditorHarness(); + const viewer = new CodeView({ createEditor }); + const edited = makeSessionDiffItem('edited'); + if (edited.type !== 'diff') { + throw new Error('Expected a diff edit-session item.'); + } + try { + viewer.setup(createRoot()); + await renderItems(viewer, [edited]); + + revertLineTen(edited, viewer); + expect(edited.fileDiff.hunks).toHaveLength(2); + expect(edited.fileDiff.editSessionDirty).toBe(true); + + expect(viewer.removeItem(edited.id)).toBe(true); + + expect(edited.fileDiff.editSessionDirty).toBeUndefined(); + expect(edited.fileDiff.hunks).toHaveLength(1); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + test('ending a session reconciles the item layout height', async () => { const { cleanup } = installDom(); const { createEditor } = createEditorHarness(); From f00f212e6aef9d7c920fad77cc4c123c9f32e1f9 Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Mon, 3 Aug 2026 18:35:13 -0700 Subject: [PATCH 3/4] Proper fix for handling empty items in CodeView --- packages/diffs/src/components/CodeView.ts | 68 ++++++++------- .../src/utils/areManagedSnapshotsEqual.ts | 1 + packages/diffs/test/CodeView.edit.test.ts | 82 +++++++++++++++++-- .../test/CodeView.scrollAnchoring.test.ts | 12 ++- .../test/CodeView.workerPoolReady.test.ts | 68 ++++++++++++++- 5 files changed, 185 insertions(+), 46 deletions(-) diff --git a/packages/diffs/src/components/CodeView.ts b/packages/diffs/src/components/CodeView.ts index 54c973071..72fc4b444 100644 --- a/packages/diffs/src/components/CodeView.ts +++ b/packages/diffs/src/components/CodeView.ts @@ -1576,38 +1576,14 @@ export class CodeView { } public setItems(items: readonly CodeViewItem[]): void { - if (items.length === 0) { - // An empty controlled list removes every item, so end active edit - // sessions the way reconcile removals do: publish each session's final - // contents (from its last change) through onItemEditComplete. Direct - // reset()/cleanUp() calls stay silent — those are teardowns, not item - // data updates. - const completions: CodeViewItemEditChange[] = []; - for (const [id, record] of this.itemEditors) { - const item = this.idToItem.get(id); - record.editor.cleanUp(); - if (item?.type === 'diff') { - item.instance.completeEditSession(); - finishEditSessionForDiff(item.item.fileDiff); - } - const { lastChange } = record.state; - if (lastChange != null) { - completions.push(lastChange); - } - } - this.reset(); - // Fired after reset so a handler that calls back into setItems/addItems - // runs against clean state (mirrors syncItemEditors' post-loop firing). - for (const { item, file, lineAnnotations } of completions) { - this.options.onItemEditComplete?.(item, file, lineAnnotations); - } - } else if (this.items.length === 0) { + let removedItemsById: Readonly> | undefined; + if (this.items.length === 0) { this.appendItemsInternal(items); } else if (!this.tryAppendItems(items)) { - this.reconcileItems(items); + removedItemsById = this.reconcileItems(items); } this.syncSelection(); - this.syncItemEditors(); + this.syncItemEditors(removedItemsById); } /** @@ -2108,7 +2084,9 @@ export class CodeView { * attachItemEditor, so this only reconciles editors CodeView is already * holding. */ - private syncItemEditors(): void { + private syncItemEditors( + removedItems?: Readonly> + ): void { if (this.itemEditors.size === 0) { return; } @@ -2116,7 +2094,8 @@ export class CodeView { const completions: CodeViewItemEditChange[] = []; for (const [id, record] of this.itemEditors) { const item = this.idToItem.get(id); - if (item != null && this.isItemInEditMode(item)) { + const removedItem = removedItems?.get(id); + if (removedItem == null && item != null && this.isItemInEditMode(item)) { continue; } // cleanUp is idempotent, so editors already detached by their released @@ -2129,10 +2108,15 @@ export class CodeView { // so finish the session here (idempotent: the dirty marker clears on // the first run). A live item goes through its instance, which also // preserves expansion state and invalidates layout; removed items fall - // back to a plain metadata recompute from the last change's snapshot. - const itemSnapshot = item?.item ?? record.state.lastChange?.item; + // back to the snapshot captured with the editor's last change. + const { lastChange } = record.state; + const itemSnapshot = + removedItem == null + ? (item?.item ?? lastChange?.item) + : (lastChange?.item ?? removedItem.item); if (itemSnapshot?.type === 'diff') { if ( + removedItem == null && item != null && item.type === 'diff' && item.instance.completeEditSession() @@ -2142,13 +2126,14 @@ export class CodeView { } finishEditSessionForDiff(itemSnapshot.fileDiff); } - const { lastChange } = record.state; if (lastChange != null) { // Prefer the current item record (it carries the update that ended // the session, e.g. edit: false); the snapshot from the last change // covers sessions ended by removing the item. completions.push( - item == null ? lastChange : { ...lastChange, item: item.item } + removedItem != null || item == null + ? lastChange + : { ...lastChange, item: item.item } ); } } @@ -2504,7 +2489,9 @@ export class CodeView { * records, rebuilds the lookup maps, and marks layout dirty whenever order, * membership, or versioned item data changes. */ - private reconcileItems(items: readonly CodeViewItem[]): void { + private reconcileItems( + items: readonly CodeViewItem[] + ): Readonly> | undefined { const { items: previousItems, idToItem: previousById } = this; const removedItems = new Set(previousItems); const nextItems: CodeViewContextItem[] = []; @@ -2516,6 +2503,7 @@ export class CodeView { VirtualizedFileDiff | VirtualizedFile, CodeViewContextItem > = new Map(); + const removedItemsById: CodeViewItemMap = new Map(); let firstDirtyIndex: number | undefined; for (let index = 0; index < items.length; index++) { @@ -2555,7 +2543,7 @@ export class CodeView { if (firstDirtyIndex == null) { if (removedItems.size === 0) { - return; + return undefined; } firstDirtyIndex = Math.max(nextItems.length - 1, 0); } @@ -2567,6 +2555,7 @@ export class CodeView { if (removedItem == null || !removedItems.has(removedItem)) { continue; } + removedItemsById.set(removedItem.item.id, removedItem); this.releaseRenderedItem(removedItem); const dirtyIndex = Math.max(nextItems.length - 1, 0); firstDirtyIndex = Math.min(firstDirtyIndex ?? dirtyIndex, dirtyIndex); @@ -2585,6 +2574,7 @@ export class CodeView { this.markLayoutDirtyFromIndex(firstDirtyIndex); this.scrollDirty = true; this.render(); + return removedItemsById.size > 0 ? removedItemsById : undefined; } /** @@ -3515,6 +3505,12 @@ export class CodeView { private updateStickyPositioning(): void { const stickyBounds = this.getStickyBounds(); if (stickyBounds == null) { + // No rendered slice means no sticky scaffold: clear the spacer so an + // emptied viewer sheds the offset height captured from the last + // rendered layout instead of keeping phantom space in the container. + if (this.renderState.firstIndex === -1) { + this.stickyOffset.style.height = ''; + } return; } const { stickyTop, stickyBottom } = stickyBounds; diff --git a/packages/diffs/src/utils/areManagedSnapshotsEqual.ts b/packages/diffs/src/utils/areManagedSnapshotsEqual.ts index d84350260..c89779ce2 100644 --- a/packages/diffs/src/utils/areManagedSnapshotsEqual.ts +++ b/packages/diffs/src/utils/areManagedSnapshotsEqual.ts @@ -39,6 +39,7 @@ function areRenderedItemsEqual( previousItem.id !== nextItem.id || previousItem.type !== nextItem.type || previousItem.element !== nextItem.element || + previousItem.instance !== nextItem.instance || previousItem.version !== nextItem.version ) { return false; diff --git a/packages/diffs/test/CodeView.edit.test.ts b/packages/diffs/test/CodeView.edit.test.ts index 64efb1dbc..6ec7a535e 100644 --- a/packages/diffs/test/CodeView.edit.test.ts +++ b/packages/diffs/test/CodeView.edit.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from 'bun:test'; -import { CodeView } from '../src/components/CodeView'; +import { + CodeView, + type CodeViewCoordinator, + type CodeViewSlotSnapshot, +} from '../src/components/CodeView'; import { Editor } from '../src/editor/editor'; import type { CodeViewCreateEditorOptions, @@ -1231,6 +1235,49 @@ describe('CodeView item edit mode', () => { } }); + test('finalizes the last-change snapshot after a version update', async () => { + const { cleanup } = installDom(); + const { editors, createEditor } = createEditorHarness(); + const completions: CodeViewItem[] = []; + const viewer = new CodeView({ + createEditor, + onItemEditComplete(item) { + completions.push(item); + }, + }); + const edited = makeSessionDiffItem('edited'); + const replacement = makeSessionDiffItem('edited'); + replacement.version = 1; + const kept = makeEditFileItem('kept', false); + if (edited.type !== 'diff') { + throw new Error('Expected a diff edit-session item.'); + } + try { + viewer.setup(createRoot()); + await renderItems(viewer, [edited, kept]); + + revertLineTen(edited, viewer); + editors[0].emitChange({ name: 'edited.txt', contents: 'changed' }); + await renderItems(viewer, [replacement, kept]); + + // The version bump reuses the item record, so the editor and its + // session survive the update: no new editor, no completion yet. + expect(editors).toHaveLength(1); + expect(completions).toHaveLength(0); + + expect(viewer.removeItem(edited.id)).toBe(true); + + expect(completions).toHaveLength(1); + expect(completions[0]).toBe(edited); + expect(edited.fileDiff.editSessionDirty).toBeUndefined(); + expect(edited.fileDiff.hunks).toHaveLength(1); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + test('ending a session reconciles the item layout height', async () => { const { cleanup } = installDom(); const { createEditor } = createEditorHarness(); @@ -1417,24 +1464,47 @@ describe('CodeView item edit mode', () => { const { cleanup } = installDom(); const { editors, createEditor } = createEditorHarness(); const completions: Array<{ id: string; contents: string }> = []; + const snapshots: Array | undefined> = []; + const replacement = makeEditFileItem('a', false); + const onItemEditComplete = ( + item: CodeViewItem, + file: FileContents + ) => { + completions.push({ id: item.id, contents: file.contents }); + viewer.addItems([replacement]); + }; const viewer = new CodeView({ createEditor, - onItemEditComplete(item, file) { - completions.push({ id: item.id, contents: file.contents }); - }, + onItemEditComplete, }); + const coordinator: CodeViewCoordinator = { + hasAnnotationRenderer: false, + hasGutterRenderer: false, + hasHeaderRenderers: true, + onSnapshotChange(snapshot) { + snapshots.push(snapshot); + }, + }; try { + viewer.setSlotCoordinator(coordinator); viewer.setup(createRoot()); await renderItems(viewer, [makeEditFileItem('a')]); + const initialElement = viewer.getRenderedItems()[0]?.element; // setItems([]) is a removal like any other controlled update, so the - // session completes with its last-change snapshot even though the - // internal path is a full reset. + // session completes with its last-change snapshot. editors[0].emitChange({ name: 'a.ts', contents: 'unsaved' }); await renderItems(viewer, []); expect(completions).toEqual([{ id: 'a', contents: 'unsaved' }]); expect(editors[0].fullCleanUps).toBeGreaterThanOrEqual(1); + expect(viewer.getItem(replacement.id)).toBe(replacement); + const renderedReplacement = viewer.getRenderedItems()[0]; + expect(renderedReplacement?.element).toBe(initialElement); + expect(snapshots).toHaveLength(2); + expect(snapshots[1]?.items?.[0]?.instance).toBe( + renderedReplacement?.instance + ); } finally { viewer.cleanUp(); await wait(0); diff --git a/packages/diffs/test/CodeView.scrollAnchoring.test.ts b/packages/diffs/test/CodeView.scrollAnchoring.test.ts index 8b0542fe1..cffb051eb 100644 --- a/packages/diffs/test/CodeView.scrollAnchoring.test.ts +++ b/packages/diffs/test/CodeView.scrollAnchoring.test.ts @@ -232,9 +232,15 @@ describe('CodeView scroll anchoring', () => { expect((container as HTMLElement).style.height).toBe( `${SCROLL_REBASE_CONTAINER_HEIGHT}px` ); - - viewer.setItems([]); - expect((container as HTMLElement).style.height).toBe(''); + const stickyOffset = (container as HTMLElement).firstElementChild; + expect(stickyOffset).toBeInstanceOf(HTMLElement); + expect((stickyOffset as HTMLElement).style.height).not.toBe(''); + + await renderItems(viewer, []); + expect((container as HTMLElement).style.height).toBe('0px'); + // An emptied viewer sheds the sticky spacer height along with its + // items; a stale spacer would keep phantom space in the container. + expect((stickyOffset as HTMLElement).style.height).toBe(''); await renderItems(viewer, secondItems); diff --git a/packages/diffs/test/CodeView.workerPoolReady.test.ts b/packages/diffs/test/CodeView.workerPoolReady.test.ts index 31afc7f96..bf28c141e 100644 --- a/packages/diffs/test/CodeView.workerPoolReady.test.ts +++ b/packages/diffs/test/CodeView.workerPoolReady.test.ts @@ -100,6 +100,15 @@ class FakeWorkerPoolManager { } } + public markWaiting(): void { + this.initialized = false; + this.failed = false; + const stats = this.getStats(); + for (const callback of Array.from(this.statSubscribers)) { + callback(stats); + } + } + // Mirror WorkerPoolManager's init-failure state: it reverts to 'waiting' with // workersFailed: true rather than ever reaching 'initialized'. public markFailed(): void { @@ -222,6 +231,11 @@ describe('CodeView worker pool readiness', () => { try { viewer.setup(createRoot({ height: 1000 })); + // The empty mount already kicked initialization; model terminate() + // discarding that startup so the pool sits idle in 'waiting' again by + // the time the items render. + workerManager.markWaiting(); + const initializeCallsAfterSetup = workerManager.initializeCallCount; viewer.setItems([makeFileItem('file:waiting-pool', 3)]); viewer.render(true); @@ -229,7 +243,9 @@ describe('CodeView worker pool readiness', () => { // Rendering must kick initialization rather than block forever, which // then drives the pool to 'initialized' and lets the item render. - expect(workerManager.initializeCallCount).toBeGreaterThan(0); + expect(workerManager.initializeCallCount).toBeGreaterThan( + initializeCallsAfterSetup + ); expect(viewer.getRenderedItems().map((item) => item.id)).toEqual([ 'file:waiting-pool', ]); @@ -271,4 +287,54 @@ describe('CodeView worker pool readiness', () => { cleanup(); } }); + + test('an empty mount kicks initialization for a waiting pool', async () => { + const { cleanup } = installDom(); + const workerManager = new FakeWorkerPoolManager(); + const viewer = new CodeView( + { disableFileHeader: true }, + workerManager.asWorkerPoolManager() + ); + + try { + viewer.setup(createRoot({ height: 1000 })); + + // Mount renders through the same readiness gate as everything else, so + // even an empty viewer starts pool initialization immediately instead + // of deferring startup latency to the first non-empty render. + expect(workerManager.initializeCallCount).toBe(1); + expect(workerManager.statSubscriberCount).toBe(1); + + workerManager.markInitialized(); + expect(workerManager.statSubscriberCount).toBe(0); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + + test('an empty mount does not re-initialize a failed pool', async () => { + const { cleanup } = installDom(); + const workerManager = new FakeWorkerPoolManager(); + workerManager.markFailed(); + const viewer = new CodeView( + { disableFileHeader: true }, + workerManager.asWorkerPoolManager() + ); + + try { + viewer.setup(createRoot({ height: 1000 })); + + // Pool failure is sticky: the readiness gate treats a failed pool as + // ready (renderers fall back to synchronous highlighting), so mounting + // must not restart it. + expect(workerManager.initializeCallCount).toBe(0); + expect(workerManager.statSubscriberCount).toBe(0); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); }); From 3c96566f1ef6bde03414a9d540d98e02b766bbbf Mon Sep 17 00:00:00 2001 From: Amadeus Demarzi Date: Mon, 3 Aug 2026 19:07:04 -0700 Subject: [PATCH 4/4] Fix onSelectedLinesChange being fired on item removal There was some sloppiness from before with how selection callbacks were being fired if the item was removed and had selections. --- packages/diffs/src/components/CodeView.ts | 7 ++-- .../diffs/test/CodeView.removeItem.test.ts | 42 ++++++++++++++++++- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/packages/diffs/src/components/CodeView.ts b/packages/diffs/src/components/CodeView.ts index 72fc4b444..2d3a89d65 100644 --- a/packages/diffs/src/components/CodeView.ts +++ b/packages/diffs/src/components/CodeView.ts @@ -1503,8 +1503,8 @@ export class CodeView { this.markItemLayoutDirty(item); this.scrollDirty = true; this.render(); - this.syncSelection(); this.syncItemEditors(); + this.syncSelection(); return true; } @@ -1554,8 +1554,8 @@ export class CodeView { public addItems(inputs: readonly CodeViewItem[]): void { this.appendItemsInternal(inputs); - this.syncSelection(); this.syncItemEditors(); + this.syncSelection(); } public removeItem(itemId: string): boolean { @@ -1582,8 +1582,8 @@ export class CodeView { } else if (!this.tryAppendItems(items)) { removedItemsById = this.reconcileItems(items); } - this.syncSelection(); this.syncItemEditors(removedItemsById); + this.syncSelection(); } /** @@ -2003,6 +2003,7 @@ export class CodeView { const item = this.idToItem.get(this.selectedLines.id); if (item == null) { this.selectedLines = null; + this.options.onSelectedLinesChange?.(null); return; } diff --git a/packages/diffs/test/CodeView.removeItem.test.ts b/packages/diffs/test/CodeView.removeItem.test.ts index 862cffc0f..049e447c8 100644 --- a/packages/diffs/test/CodeView.removeItem.test.ts +++ b/packages/diffs/test/CodeView.removeItem.test.ts @@ -1,6 +1,9 @@ import { describe, expect, spyOn, test } from 'bun:test'; -import { CodeView } from '../src/components/CodeView'; +import { + CodeView, + type CodeViewLineSelection, +} from '../src/components/CodeView'; import { createRoot, installDom, @@ -36,6 +39,43 @@ describe('CodeView.removeItem', () => { } }); + test('emits a null selection change when removing the selected item', async () => { + const { cleanup } = installDom(); + const changes: (CodeViewLineSelection | null)[] = []; + const viewer = new CodeView({ + onSelectedLinesChange(selection) { + changes.push(selection); + }, + }); + try { + viewer.setup(createRoot()); + await renderItems(viewer, [ + makeFileItem('kept', 5), + makeFileItem('selected', 5), + ]); + viewer.setSelectedLines( + { id: 'selected', range: { start: 2, end: 3 } }, + { notify: false } + ); + + // Removing an unrelated item leaves the selection alone and stays + // silent. + expect(viewer.removeItem('kept')).toBe(true); + expect(changes).toEqual([]); + expect(viewer.getSelectedLines()?.id).toBe('selected'); + + // Removing the selected item clears it and tells the consumer, so + // controlled selection state can't write the dead selection back. + expect(viewer.removeItem('selected')).toBe(true); + expect(changes).toEqual([null]); + expect(viewer.getSelectedLines()).toBeNull(); + } finally { + viewer.cleanUp(); + await wait(0); + cleanup(); + } + }); + test('returns false for an unknown item id', async () => { const { cleanup } = installDom(); const consoleError = spyOn(console, 'error').mockImplementation(() => {});