From 842c3738fda9ad4ff00ef1195039c3373817ba9a Mon Sep 17 00:00:00 2001 From: jacob314 Date: Tue, 21 Apr 2026 18:46:38 -0700 Subject: [PATCH 1/6] Fix scroll of a nested child excessive update bug. more fixes. refactor(sticky): optimize sticky node lookup and remove debug flag - Removed `debugRainbow: true` from `examples/sticky/index.ts` which was an artifact. - Refactored `identifyActiveStickyNodes` in `src/render-sticky.ts` to use `originalIndex` during population, eliminating the O(N^2) `indexOf(find(...))` lookup. - Extracted next sticky node discovery into a `findNextStickyNode` helper to reduce duplication. --- examples/sticky/index.ts | 1 + examples/sticky/sticky.tsx | 63 ++++++-- src/ink.tsx | 1 + src/render-container.ts | 6 + src/render-node-to-output.ts | 6 + src/render-sticky.ts | 180 ++++++++++++++-------- src/renderer.ts | 4 + src/worker/compositor.ts | 10 +- src/worker/render-worker.ts | 28 +++- src/worker/scroll-optimizer.ts | 3 +- test/repro-issue.test.tsx | 37 ++++- test/repro-nested-scroll-repaint.test.tsx | 99 ++++++++++++ test/sticky-backbuffer-overlap.test.tsx | 40 ++++- 13 files changed, 380 insertions(+), 98 deletions(-) create mode 100644 test/repro-nested-scroll-repaint.test.tsx diff --git a/examples/sticky/index.ts b/examples/sticky/index.ts index 76f3c8871..e45b08477 100644 --- a/examples/sticky/index.ts +++ b/examples/sticky/index.ts @@ -85,6 +85,7 @@ export const instance = render( rows, }), { + debugRainbow: true, renderProcess: true, terminalBuffer: true, alternateBuffer: false, diff --git a/examples/sticky/sticky.tsx b/examples/sticky/sticky.tsx index 4d280e533..69910a27e 100644 --- a/examples/sticky/sticky.tsx +++ b/examples/sticky/sticky.tsx @@ -197,6 +197,10 @@ function ScrollableContent({ } }, [recordFilename, startRecording]); + const [innerStates, setInnerStates] = useState< + Record + >({}); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const boxWidth = columns; const contentWidth = showBorder @@ -205,17 +209,22 @@ function ScrollableContent({ const staticContent = useMemo(() => { const elements = []; - for (let i = 0; i < listItems.length; i += 20) { + for (let i = 0; i < listItems.length; i += 4) { const headerIndex = i; - const headerId = headerIndex / 20; + const headerId = headerIndex / 4; const headerText = `Sticky Header ${headerId}`; const stickyHeaderText = `Sticky Header ${headerId} (sticky top)`; const stickyFooterText = `Sticky Footer ${headerId} (sticky bottom)`; - const itemsInGroup = listItems.slice(headerIndex, headerIndex + 10); - const nextItems = listItems.slice(headerIndex + 10, headerIndex + 20); - + const itemsInGroup = listItems.slice(headerIndex, headerIndex + 2); + const nextItems = listItems.slice(headerIndex + 2, headerIndex + 4); if (headerId % 3 === 0) { + const innerScrollTop = + innerStates[headerId]?.scrollTop ?? + (headerId === 0 || headerId === 3 ? 0 : 40); + const useStaticForInner = + useStatic && (innerStates[headerId]?.isStatic ?? true); + const innerBox = ( {() => innerBox} @@ -395,12 +404,45 @@ function ScrollableContent({ ); return content; - }, [contentWidth, useStatic, listItems]); + }, [contentWidth, useStatic, listItems, innerStates]); useInput((input, key) => { + const handleInnerScroll = (id: number, delta: number) => { + setInnerStates(previous => { + const current = previous[id] || {scrollTop: 0, isStatic: true}; + return { + ...previous, + [id]: { + isStatic: false, + scrollTop: Math.max(0, current.scrollTop + delta), + }, + }; + }); + }; + + if (input === '1') { + handleInnerScroll(0, -1); + return; + } + + if (input === '2') { + handleInnerScroll(0, 1); + return; + } + + if (input === '3') { + handleInnerScroll(3, -1); + return; + } + + if (input === '4') { + handleInnerScroll(3, 1); + return; + } + if (input === ' ') { setListItems(previous => { - const newItems = Array.from({length: 20}).map((_, i) => ({ + const newItems = Array.from({length: 4}).map((_, i) => ({ id: Date.now() + previous.length + i, text: `Line ${previous.length + i} - ${'Lorem ipsum dolor sit amet, consectetur adipiscing elit. '.repeat( ((previous.length + i) * 5) % 6, @@ -530,6 +572,7 @@ function ScrollableContent({ Press up/down arrow or w/s to scroll vertically (w/s for 30 lines, Shift for 10). + Press 1/2 to scroll first inner view, 3/4 for second. Press 'space' to add a block, 'c' to clear list. Press 'b' to toggle scrollbar ({showScrollbar ? 'on' : 'off'}), diff --git a/src/ink.tsx b/src/ink.tsx index 1f439ab16..1f51e5b74 100644 --- a/src/ink.tsx +++ b/src/ink.tsx @@ -402,6 +402,7 @@ export default class Ink { selectionStyle: this.options.selectionStyle, skipScrollbars: Boolean(this.terminalBuffer), terminalBuffer: Boolean(this.terminalBuffer), + stickyHeadersInBackbuffer: this.optionsState.stickyHeadersInBackbuffer, }); if (this.terminalBuffer && root) { diff --git a/src/render-container.ts b/src/render-container.ts index c1d049336..7d25662dc 100644 --- a/src/render-container.ts +++ b/src/render-container.ts @@ -38,6 +38,7 @@ export function handleContainerNode( skipStaticElements: boolean; isStickyRender: boolean; skipStickyHeaders: boolean; + stickyHeadersInBackbuffer?: boolean; selectionMap?: Map; selectionStyle?: (line: StyledLine, index: number) => void; absoluteOffsetX: number; @@ -54,6 +55,7 @@ export function handleContainerNode( skipStaticElements, isStickyRender, skipStickyHeaders, + stickyHeadersInBackbuffer, selectionMap, selectionStyle, absoluteOffsetX, @@ -106,6 +108,7 @@ export function handleContainerNode( node, scrollTop, viewportBottom, + stickyHeadersInBackbuffer, ), ); } @@ -203,6 +206,7 @@ export function handleContainerNode( skipStaticElements, isStickyRender, skipStickyHeaders: false, + stickyHeadersInBackbuffer, selectionMap, selectionStyle, trackSelection, @@ -217,6 +221,7 @@ export function handleContainerNode( selectionMap, selectionStyle, trackSelection, + stickyHeadersInBackbuffer, }); output.endChildRegion(); @@ -247,6 +252,7 @@ export function handleContainerNode( skipStaticElements, isStickyRender, skipStickyHeaders, + stickyHeadersInBackbuffer, selectionMap, selectionStyle, trackSelection, diff --git a/src/render-node-to-output.ts b/src/render-node-to-output.ts index 06103b3fc..77b43ec3d 100644 --- a/src/render-node-to-output.ts +++ b/src/render-node-to-output.ts @@ -78,6 +78,7 @@ const getOrRenderCachedRegion = ( skipStaticElements: boolean; isStickyRender: boolean; skipStickyHeaders: boolean; + stickyHeadersInBackbuffer?: boolean; }, ) => { const { @@ -87,6 +88,7 @@ const getOrRenderCachedRegion = ( skipStaticElements, isStickyRender, skipStickyHeaders, + stickyHeadersInBackbuffer, } = options; const cachedRegion = getCachedRegion(node); @@ -114,6 +116,7 @@ const getOrRenderCachedRegion = ( skipStaticElements, isStickyRender, skipStickyHeaders, + stickyHeadersInBackbuffer, absoluteOffsetX: 0, absoluteOffsetY: 0, }); @@ -264,6 +267,7 @@ function renderNodeToOutput( skipStaticElements: boolean; isStickyRender?: boolean; skipStickyHeaders?: boolean; + stickyHeadersInBackbuffer?: boolean; selectionMap?: Map; selectionStyle?: (line: StyledLine, index: number) => void; trackSelection?: boolean; @@ -373,6 +377,7 @@ function renderNodeToOutput( skipStaticElements, isStickyRender, skipStickyHeaders, + stickyHeadersInBackbuffer: options.stickyHeadersInBackbuffer, }); output.addRegionTree(cachedRegion, x, y); @@ -400,6 +405,7 @@ function renderNodeToOutput( skipStaticElements, isStickyRender, skipStickyHeaders, + stickyHeadersInBackbuffer: options.stickyHeadersInBackbuffer, selectionMap, selectionStyle, absoluteOffsetX: absX, diff --git a/src/render-sticky.ts b/src/render-sticky.ts index 1d64cac21..68ba03ea7 100644 --- a/src/render-sticky.ts +++ b/src/render-sticky.ts @@ -231,11 +231,20 @@ export function identifyActiveStickyNodes( node: DOMElement, scrollTop: number, viewportBottom: number, + stickyHeadersInBackbuffer?: boolean, ) { - let activeTopStickyNodeIndex = -1; - let activeTopStickyNode: StickyNodeInfo | undefined; - let activeBottomStickyNodeIndex = -1; - let activeBottomStickyNode: StickyNodeInfo | undefined; + const activeStickyNodes: Array<{ + stickyNode?: DOMElement; + type: 'top' | 'bottom'; + nextStickyNode?: DOMElement; + nextStickyNodeInfo?: StickyNodeInfo; + cached?: StickyHeader; + anchor?: DOMElement; + originalIndex: number; + }> = []; + + let lastActiveTopIndex = -1; + let lastActiveBottomIndex = -1; for (const [index, stickyNodeInfo] of stickyNodes.entries()) { const {type: stickyType} = stickyNodeInfo; @@ -251,74 +260,107 @@ export function identifyActiveStickyNodes( if ( stickyType === 'top' && - stickyNodeTop < scrollTop && - parentRelativeTop + parentHeight > scrollTop + (stickyHeadersInBackbuffer || + (stickyNodeTop < scrollTop && + parentRelativeTop + parentHeight > scrollTop)) ) { - activeTopStickyNode = stickyNodeInfo; - activeTopStickyNodeIndex = index; + lastActiveTopIndex = index; + if (stickyHeadersInBackbuffer) { + // In backbuffer mode, we want to track ALL of them + activeStickyNodes.push({ + stickyNode: stickyNodeInfo.node, + type: 'top', + cached: stickyNodeInfo.cached, + anchor: stickyNodeInfo.anchor, + originalIndex: index, + }); + } } if ( stickyType === 'bottom' && - Math.floor(stickyNodeBottom) > Math.floor(viewportBottom) && - parentRelativeTop < viewportBottom + (stickyHeadersInBackbuffer || + (Math.floor(stickyNodeBottom) > Math.floor(viewportBottom) && + parentRelativeTop < viewportBottom)) ) { - activeBottomStickyNode = stickyNodeInfo; - activeBottomStickyNodeIndex = index; + lastActiveBottomIndex = index; + if (stickyHeadersInBackbuffer) { + activeStickyNodes.push({ + stickyNode: stickyNodeInfo.node, + type: 'bottom', + cached: stickyNodeInfo.cached, + anchor: stickyNodeInfo.anchor, + originalIndex: index, + }); + } } } - const activeStickyNodes: Array<{ - stickyNode?: DOMElement; - type: 'top' | 'bottom'; - nextStickyNode?: DOMElement; - nextStickyNodeInfo?: StickyNodeInfo; - cached?: StickyHeader; - anchor?: DOMElement; - }> = []; - - if (activeTopStickyNode) { - let nextStickyNode: DOMElement | undefined; - let nextStickyNodeInfo: StickyNodeInfo | undefined; - for (let i = activeTopStickyNodeIndex + 1; i < stickyNodes.length; i++) { + const findNextStickyNode = ( + startIndex: number, + direction: 'forward' | 'backward', + ) => { + const step = direction === 'forward' ? 1 : -1; + const endCondition = + direction === 'forward' + ? (i: number) => i < stickyNodes.length + : (i: number) => i >= 0; + const targetType = direction === 'forward' ? 'top' : 'bottom'; + + for (let i = startIndex + step; endCondition(i); i += step) { const info = stickyNodes[i]!; - if (info.type !== 'bottom') { - nextStickyNode = info.node; - nextStickyNodeInfo = info; - break; + // Top sticky headers block other top sticky headers. + // Bottom sticky headers block other bottom sticky headers. + if (info.type === targetType) { + return info; } } - activeStickyNodes.push({ - stickyNode: activeTopStickyNode.node, - type: 'top', - nextStickyNode, - nextStickyNodeInfo, - cached: activeTopStickyNode.cached, - anchor: activeTopStickyNode.anchor, - }); - } + return undefined; + }; - if (activeBottomStickyNode) { - let nextStickyNode: DOMElement | undefined; - let nextStickyNodeInfo: StickyNodeInfo | undefined; - for (let i = activeBottomStickyNodeIndex - 1; i >= 0; i--) { - const info = stickyNodes[i]!; - if (info.type === 'bottom') { - nextStickyNode = info.node; - nextStickyNodeInfo = info; - break; + if (stickyHeadersInBackbuffer) { + // When stickyHeadersInBackbuffer is ON, we need to populate nextStickyNodeInfo for ALL of them + for (const active of activeStickyNodes) { + const info = findNextStickyNode( + active.originalIndex, + active.type === 'top' ? 'forward' : 'backward', + ); + if (info) { + active.nextStickyNode = info.node; + active.nextStickyNodeInfo = info; } } + } else { + if (lastActiveTopIndex !== -1) { + const activeTopStickyNode = stickyNodes[lastActiveTopIndex]!; + const info = findNextStickyNode(lastActiveTopIndex, 'forward'); + + activeStickyNodes.push({ + stickyNode: activeTopStickyNode.node, + type: 'top', + nextStickyNode: info?.node, + nextStickyNodeInfo: info, + cached: activeTopStickyNode.cached, + anchor: activeTopStickyNode.anchor, + originalIndex: lastActiveTopIndex, + }); + } - activeStickyNodes.push({ - stickyNode: activeBottomStickyNode.node, - type: 'bottom', - nextStickyNode, - nextStickyNodeInfo, - cached: activeBottomStickyNode.cached, - anchor: activeBottomStickyNode.anchor, - }); + if (lastActiveBottomIndex !== -1) { + const activeBottomStickyNode = stickyNodes[lastActiveBottomIndex]!; + const info = findNextStickyNode(lastActiveBottomIndex, 'backward'); + + activeStickyNodes.push({ + stickyNode: activeBottomStickyNode.node, + type: 'bottom', + nextStickyNode: info?.node, + nextStickyNodeInfo: info, + cached: activeBottomStickyNode.cached, + anchor: activeBottomStickyNode.anchor, + originalIndex: lastActiveBottomIndex, + }); + } } return activeStickyNodes; @@ -343,16 +385,17 @@ export function renderActiveStickyNodes( selectionMap?: Map; selectionStyle?: (line: StyledLine, index: number) => void; trackSelection?: boolean; + stickyHeadersInBackbuffer?: boolean; }, ) { const { - x: _x, y, newTransformers, skipStaticElements, selectionMap, selectionStyle, trackSelection, + stickyHeadersInBackbuffer, } = options; const {yogaNode} = node; if (!yogaNode) return; @@ -399,7 +442,7 @@ export function renderActiveStickyNodes( const naturalStickyY = y - currentScrollTop + stickyNodeTop; const stuckStickyY = y + currentBorderTop; - if (nextStickyNodeInfo) { + if (nextStickyNodeInfo && !stickyHeadersInBackbuffer) { let nextNodeTop: number | undefined; if (nextStickyNodeInfo.cached && nextStickyNodeInfo.anchor) { @@ -421,20 +464,24 @@ export function renderActiveStickyNodes( } } - finalStickyY = Math.min( - Math.max(stuckStickyY, naturalStickyY), - maxStickyTop, - ); + finalStickyY = stickyHeadersInBackbuffer + ? Math.min(stuckStickyY, maxStickyTop) + : Math.min(Math.max(stuckStickyY, naturalStickyY), maxStickyTop); maxStuckY = maxStickyTop - (y + currentBorderTop); } else { let minStickyTop = y - currentScrollTop + parentRelativeTop + parentBorderTop; + + if (stickyHeadersInBackbuffer) { + minStickyTop = Number.MIN_SAFE_INTEGER; + } + const naturalStickyY = y - currentScrollTop + stickyNodeTop; const stuckStickyY = y + currentBorderTop + currentClientHeight - stickyNodeHeight; - if (nextStickyNodeInfo) { + if (nextStickyNodeInfo && !stickyHeadersInBackbuffer) { let nextNodeHeight: number | undefined; let nextNodeTop: number | undefined; @@ -460,11 +507,9 @@ export function renderActiveStickyNodes( } } - finalStickyY = Math.max( - Math.min(stuckStickyY, naturalStickyY), - minStickyTop, - ); - + finalStickyY = stickyHeadersInBackbuffer + ? Math.max(stuckStickyY, minStickyTop) + : Math.max(Math.min(stuckStickyY, naturalStickyY), minStickyTop); minStuckY = minStickyTop - (y + currentBorderTop); } @@ -507,6 +552,7 @@ export function renderActiveStickyNodes( maxStuckY, minStuckY, }; + output.addStickyHeader(headerObj); } } diff --git a/src/renderer.ts b/src/renderer.ts index 7c8c3dd67..24faadb95 100644 --- a/src/renderer.ts +++ b/src/renderer.ts @@ -247,6 +247,7 @@ const renderer = ( skipScrollbars?: boolean; trackSelection?: boolean; terminalBuffer?: boolean; + stickyHeadersInBackbuffer?: boolean; }, ): Result => { const { @@ -256,6 +257,7 @@ const renderer = ( skipScrollbars, trackSelection, terminalBuffer, + stickyHeadersInBackbuffer, } = options; if (node.yogaNode) { @@ -299,6 +301,7 @@ const renderer = ( selectionStyle, selectionMap, trackSelection, + stickyHeadersInBackbuffer, }); let staticOutput; @@ -319,6 +322,7 @@ const renderer = ( ? calculateSelectionMap(node.staticNode, selection) : undefined, trackSelection, + stickyHeadersInBackbuffer, }); } diff --git a/src/worker/compositor.ts b/src/worker/compositor.ts index 1bfb77e5c..7a1d0b927 100644 --- a/src/worker/compositor.ts +++ b/src/worker/compositor.ts @@ -144,14 +144,20 @@ export class Compositor { ) { if (header.type === 'top') { if (headerY < 0 && absY + region.height > 0) { - headerY = 0; + const maxStuckYAbs = + header.maxStuckY === undefined ? 0 : absY + header.maxStuckY; + headerY = Math.max(headerY, Math.min(0, maxStuckYAbs)); } } else if (header.type === 'bottom') { const stuckPos = this.options.canvasHeight - (header.stuckLines ?? header.lines).length; if (headerY > stuckPos && absY < stuckPos + headerH) { - headerY = stuckPos; + const minStuckYAbs = + header.minStuckY === undefined + ? stuckPos + : absY + header.minStuckY; + headerY = Math.min(headerY, Math.max(stuckPos, minStuckYAbs)); } } } diff --git a/src/worker/render-worker.ts b/src/worker/render-worker.ts index 7840114fa..edebcdf2d 100644 --- a/src/worker/render-worker.ts +++ b/src/worker/render-worker.ts @@ -690,12 +690,22 @@ export class TerminalBufferWorker { skipScrollbars: false, }); - for (const region of this.sceneManager.regions.values()) { + const processRegionForScroll = ( + node: RegionNode, + offsetX: number, + offsetY: number, + ) => { + const region = this.sceneManager.getRegion(node.id); + if (!region) return; + + const absX = Math.round(region.x + offsetX); + const absY = Math.round(region.y + offsetY); + const operations = this.scrollOptimizer.calculateScrollOperations( region, this.rows, this.columns, - cameraY, + absY, (scrollStart, count, start, end, scrollToBackbuffer) => { const originalScrollTop = region.scrollTop; region.scrollTop = scrollStart; @@ -757,7 +767,17 @@ export class TerminalBufferWorker { this.scrollOptimizer.updateMaxPushed(op.regionId, op.newMaxPushed); } } - } + + for (const child of node.children) { + processRegionForScroll( + child, + absX - (region.scrollLeft ?? 0), + absY - (region.scrollTop ?? 0), + ); + } + }; + + processRegionForScroll(this.sceneManager.root!, 0, -cameraY); } // 2. Compose Frame @@ -966,7 +986,7 @@ export class TerminalBufferWorker { ? (region.scrollHeight ?? 0) : (region.height ?? 0)); - if (absY >= canvas.height) return; + if (absY >= canvas.height && !this.stickyHeadersInBackbuffer) return; if (absY + height < 0 && !this.stickyHeadersInBackbuffer) return; let myClip = { diff --git a/src/worker/scroll-optimizer.ts b/src/worker/scroll-optimizer.ts index bef3a77f5..a16becce7 100644 --- a/src/worker/scroll-optimizer.ts +++ b/src/worker/scroll-optimizer.ts @@ -33,7 +33,7 @@ export class ScrollOptimizer { region: Region, rows: number, columns: number, - cameraY: number, + absY: number, getLinesForScroll: ( scrollStart: number, count: number, @@ -75,7 +75,6 @@ export class ScrollOptimizer { return []; } - const absY = Math.round(region.y - cameraY); const start = Math.max(0, absY); const regionHeight = Math.round(region.height); const end = Math.min(rows, absY + regionHeight); diff --git a/test/repro-issue.test.tsx b/test/repro-issue.test.tsx index a4b972dc6..317c5f3b0 100644 --- a/test/repro-issue.test.tsx +++ b/test/repro-issue.test.tsx @@ -2,6 +2,7 @@ import {PassThrough} from 'node:stream'; import test from 'ava'; import React from 'react'; import xtermHeadless from '@xterm/headless'; +import instances from '../src/instances.js'; import {render} from '../src/index.js'; import ScrollableContent from '../examples/sticky/sticky.js'; import {waitFor} from './helpers/wait-for.js'; @@ -132,13 +133,28 @@ test('repro issue: sticky headers and spurious renders', async t => { setTimeout(resolve, 1500); }); - const contentAfterHon = env.getFullContent(); + const instance = instances.get(env.stdout as unknown as NodeJS.WriteStream); + const termBuffer = ( + instance as unknown as { + terminalBuffer: {lines: Array<{getText: () => string}>}; + } + )?.terminalBuffer; + + let contentAfterHon = ''; + if (termBuffer?.lines && termBuffer.lines.length > 0) { + contentAfterHon = termBuffer.lines + .map(l => l.getText().trimEnd()) + .join('\n'); + } else { + contentAfterHon = env.getFullContent(); + } + t.log('Content after pressing H (on):\n' + contentAfterHon); // Assertion 1: Sticky footer should be visible when stuck to the terminal bottom t.true( - contentAfterHon.replaceAll(/\s+/g, '').includes('StickyFooter4'), - 'Sticky Footer 4 should be visible (stuck to bottom) when stickyHeadersInBackbuffer is on', + contentAfterHon.replaceAll(/\s+/g, '').includes('StickyFooter0'), + 'Sticky Footer 0 should be visible (stuck to bottom) when stickyHeadersInBackbuffer is on', ); // 4. Toggle sticky headers OFF @@ -148,14 +164,21 @@ test('repro issue: sticky headers and spurious renders', async t => { setTimeout(resolve, 1500); }); - const contentAfterHoff = env.getFullContent(); + let contentAfterHoff = ''; + if (termBuffer?.lines && termBuffer.lines.length > 0) { + contentAfterHoff = termBuffer.lines + .map(l => l.getText().trimEnd()) + .join('\n'); + } else { + contentAfterHoff = env.getFullContent(); + } + t.log('Content after pressing H (off):\n' + contentAfterHoff); // Assertion 2: Sticky header should NOT be visible when toggled off t.false( - contentAfterHoff.includes('Header 4 (sticky top)'), - 'Header 4 (sticky top) should not be visible when stickyHeadersInBackbuffer is off', + contentAfterHoff.includes('Header 0 (sticky top)'), + 'Header 0 (sticky top) should not be visible when stickyHeadersInBackbuffer is off', ); - env.unmount(); }); diff --git a/test/repro-nested-scroll-repaint.test.tsx b/test/repro-nested-scroll-repaint.test.tsx new file mode 100644 index 000000000..04c447e64 --- /dev/null +++ b/test/repro-nested-scroll-repaint.test.tsx @@ -0,0 +1,99 @@ +import test from 'ava'; +import {TerminalBufferWorker} from '../src/worker/render-worker.js'; +import {Serializer} from '../src/serialization.js'; +import {createStyledLine} from './helpers/replay-lib.js'; + +const serializer = new Serializer(); + +test('scrolling inner container does not repaint lines below it', async t => { + const columns = 40; + const rows = 20; + let output = ''; + const stdout = { + write(chunk: string) { + output += chunk; + return true; + }, + on() {}, + rows, + columns, + } as unknown as NodeJS.WriteStream; + + const worker = new TerminalBufferWorker(columns, rows, {stdout}); + + const innerLines = Array.from({length: 20}).map((_, i) => + createStyledLine(`Inner Line ${i}`), + ); + const innerLinesSerialized = serializer.serialize(innerLines); + + const outerLines = Array.from({length: 20}).map((_, i) => + createStyledLine(`Outer Line ${i}`), + ); + const outerLinesSerialized = serializer.serialize(outerLines); + + const renderFrame = async (innerScrollTop: number) => { + worker.update( + { + id: 'root', + children: [ + { + id: 'outer', + children: [{id: 'inner', children: []}], + }, + ], + }, + [ + { + id: 'root', + x: 0, + y: 0, + width: columns, + height: rows, + }, + { + id: 'outer', + x: 0, + y: 0, + width: columns, + height: rows, + scrollTop: 0, + scrollHeight: 40, + isScrollable: true, + overflowToBackbuffer: true, + lines: { + updates: [{start: 5, end: 25, data: outerLinesSerialized}], + totalLength: 25, + }, + }, + { + id: 'inner', + x: 0, + y: 0, + width: columns, + height: 5, + scrollTop: innerScrollTop, + scrollHeight: 20, + isScrollable: true, + lines: { + updates: [{start: 0, end: 20, data: innerLinesSerialized}], + totalLength: 20, + }, + }, + ], + ); + + output = ''; + worker.resetLinesUpdated(); + await worker.render(); + return worker.getLinesUpdated(); + }; + + // Initial render + await renderFrame(0); + t.is(worker.getLinesUpdated(), rows, 'Initial render should update all rows'); + + // Scroll inner container by 1 line + const updates = await renderFrame(1); + + t.true(updates <= 6, `Expected <= 6 lines to be updated, but got ${updates}`); +}); diff --git a/test/sticky-backbuffer-overlap.test.tsx b/test/sticky-backbuffer-overlap.test.tsx index 6cd23783b..771daa9e7 100644 --- a/test/sticky-backbuffer-overlap.test.tsx +++ b/test/sticky-backbuffer-overlap.test.tsx @@ -2,6 +2,7 @@ import {PassThrough} from 'node:stream'; import test from 'ava'; import React from 'react'; import xtermHeadless from '@xterm/headless'; +import instances from '../src/instances.js'; import {render, Box, Text} from '../src/index.js'; import {waitFor} from './helpers/wait-for.js'; @@ -12,11 +13,13 @@ test('sticky header should not overlap with bottom border when pushed out', asyn const columns = 20; const term = new XtermTerminal({cols: columns, rows, allowProposedApi: true}); let writeCount = 0; + const chunks: string[] = []; const stdout = { columns, rows, write(chunk: string) { term.write(chunk); + chunks.push(chunk); writeCount++; return true; }, @@ -40,7 +43,13 @@ test('sticky header should not overlap with bottom border when pushed out', asyn scrollTop={0} width={columns} > - + + {' '} STICKY @@ -83,7 +92,13 @@ test('sticky header should not overlap with bottom border when pushed out', asyn scrollTop={4} width={columns} > - + + {' '} STICKY @@ -97,10 +112,24 @@ test('sticky header should not overlap with bottom border when pushed out', asyn await waitFor(() => writeCount > prevWriteCount); // Ensure worker has processed everything, not just the first chunk - await new Promise(resolve => { - setTimeout(resolve, 20); + await new Promise(resolve => { + setTimeout(() => { + resolve(); + }, 50); }); - const firstLine = getLine(0); + + const instance = instances.get(stdout as unknown as NodeJS.WriteStream); + const termBuffer = ( + instance as unknown as { + terminalBuffer: {lines: Array<{getText: () => string}>}; + } + )?.terminalBuffer; + let firstLine = ''; + firstLine = + termBuffer?.lines && termBuffer.lines.length > 0 + ? termBuffer.lines[0].getText().trimEnd() + : getLine(0); + t.log('Terminal row 0: "' + firstLine + '"'); // Expect ONLY bottom border characters. Round border bottom is ╰──────────╯ @@ -110,6 +139,5 @@ test('sticky header should not overlap with bottom border when pushed out', asyn firstLine.includes('STICKY'), 'Should NOT see sticky header text on top of bottom border', ); - unmount(); }); From e5b53296f8a69e43fcb8e3b9a6586c0612517b14 Mon Sep 17 00:00:00 2001 From: jacob314 Date: Thu, 23 Apr 2026 12:20:08 -0700 Subject: [PATCH 2/6] Review fixes. --- src/render-container.ts | 2 + src/render-sticky.ts | 96 ++++++++++--------------- test/repro-issue.test.tsx | 73 ++++++++++++------- test/sticky-backbuffer-overlap.test.tsx | 2 - 4 files changed, 88 insertions(+), 85 deletions(-) diff --git a/src/render-container.ts b/src/render-container.ts index 7d25662dc..dfe944c03 100644 --- a/src/render-container.ts +++ b/src/render-container.ts @@ -24,6 +24,7 @@ import { identifyActiveStickyNodes, renderActiveStickyNodes, type StickyNodeInfo, + type ResolvedStickyHeaderInfo, } from './render-sticky.js'; export function handleContainerNode( @@ -75,6 +76,7 @@ export function handleContainerNode( nextStickyNodeInfo?: StickyNodeInfo; cached?: StickyHeader; anchor?: DOMElement; + resolvedInfo: ResolvedStickyHeaderInfo; }> = []; let verticallyScrollable = false; diff --git a/src/render-sticky.ts b/src/render-sticky.ts index 68ba03ea7..8be3a851c 100644 --- a/src/render-sticky.ts +++ b/src/render-sticky.ts @@ -19,6 +19,7 @@ export type StickyNodeInfo = { type: 'top' | 'bottom'; cached?: StickyHeader; anchor?: DOMElement; + resolvedInfo?: ResolvedStickyHeaderInfo; }; export type ResolvedStickyHeaderInfo = { @@ -130,8 +131,10 @@ export function resolveStickyHeaderInfo( }; } -export function getStickyDescendants(node: DOMElement): StickyNodeInfo[] { - const stickyDescendants: StickyNodeInfo[] = []; +export function getStickyDescendants( + node: DOMElement, + stickyDescendants: StickyNodeInfo[] = [], +): StickyNodeInfo[] { for (const child of node.childNodes) { if (child.nodeName === '#text') { continue; @@ -165,8 +168,12 @@ export function getStickyDescendants(node: DOMElement): StickyNodeInfo[] { const overflowY = domChild.style.overflowY ?? overflow; const isScrollable = overflowX === 'scroll' || overflowY === 'scroll'; - if (!isScrollable && domChild.childNodes) { - stickyDescendants.push(...getStickyDescendants(domChild)); + if ( + !isScrollable && + domChild.childNodes && + domChild.childNodes.length > 0 + ) { + getStickyDescendants(domChild, stickyDescendants); } } } @@ -233,6 +240,10 @@ export function identifyActiveStickyNodes( viewportBottom: number, stickyHeadersInBackbuffer?: boolean, ) { + for (const info of stickyNodes) { + info.resolvedInfo ||= resolveStickyHeaderInfo(info, node); + } + const activeStickyNodes: Array<{ stickyNode?: DOMElement; type: 'top' | 'bottom'; @@ -241,6 +252,7 @@ export function identifyActiveStickyNodes( cached?: StickyHeader; anchor?: DOMElement; originalIndex: number; + resolvedInfo: ResolvedStickyHeaderInfo; }> = []; let lastActiveTopIndex = -1; @@ -254,7 +266,7 @@ export function identifyActiveStickyNodes( naturalStickyNodeHeight, parentRelativeTop, parentHeight, - } = resolveStickyHeaderInfo(stickyNodeInfo, node); + } = stickyNodeInfo.resolvedInfo!; const stickyNodeBottom = stickyNodeTop + naturalStickyNodeHeight; @@ -273,6 +285,7 @@ export function identifyActiveStickyNodes( cached: stickyNodeInfo.cached, anchor: stickyNodeInfo.anchor, originalIndex: index, + resolvedInfo: stickyNodeInfo.resolvedInfo!, }); } } @@ -291,6 +304,7 @@ export function identifyActiveStickyNodes( cached: stickyNodeInfo.cached, anchor: stickyNodeInfo.anchor, originalIndex: index, + resolvedInfo: stickyNodeInfo.resolvedInfo!, }); } } @@ -344,6 +358,7 @@ export function identifyActiveStickyNodes( cached: activeTopStickyNode.cached, anchor: activeTopStickyNode.anchor, originalIndex: lastActiveTopIndex, + resolvedInfo: activeTopStickyNode.resolvedInfo!, }); } @@ -359,6 +374,7 @@ export function identifyActiveStickyNodes( cached: activeBottomStickyNode.cached, anchor: activeBottomStickyNode.anchor, originalIndex: lastActiveBottomIndex, + resolvedInfo: activeBottomStickyNode.resolvedInfo!, }); } } @@ -374,6 +390,7 @@ export function renderActiveStickyNodes( nextStickyNodeInfo?: StickyNodeInfo; cached?: StickyHeader; anchor?: DOMElement; + resolvedInfo: ResolvedStickyHeaderInfo; }>, node: DOMElement, output: Output, @@ -405,17 +422,10 @@ export function renderActiveStickyNodes( type, nextStickyNodeInfo, cached, - anchor, + resolvedInfo, } of activeStickyNodes) { const currentBorderTop = yogaNode.getComputedBorder(Yoga.EDGE_TOP); - const stickyNodeInfo: StickyNodeInfo = { - node: stickyNode, - type, - cached, - anchor, - }; - const { stickyNodeTop, stickyNodeHeight, @@ -424,9 +434,8 @@ export function renderActiveStickyNodes( parentBorderTop, parentBorderBottom, relativeX, - relativeY: _relativeY, nodeId: stickyNodeId, - } = resolveStickyHeaderInfo(stickyNodeInfo, node); + } = resolvedInfo; const currentScrollTop = getScrollTop(node); const currentClientHeight = node.internal_scrollState?.clientHeight ?? 0; @@ -442,25 +451,13 @@ export function renderActiveStickyNodes( const naturalStickyY = y - currentScrollTop + stickyNodeTop; const stuckStickyY = y + currentBorderTop; - if (nextStickyNodeInfo && !stickyHeadersInBackbuffer) { - let nextNodeTop: number | undefined; + if (nextStickyNodeInfo?.resolvedInfo) { + const nextNodeTopInViewport = + y - currentScrollTop + nextStickyNodeInfo.resolvedInfo.stickyNodeTop; - if (nextStickyNodeInfo.cached && nextStickyNodeInfo.anchor) { - const staticRenderPosTop = - getRelativeTop(nextStickyNodeInfo.anchor, node) ?? 0; - nextNodeTop = - staticRenderPosTop + nextStickyNodeInfo.cached.relativeY!; - } else if (nextStickyNodeInfo.node?.yogaNode) { - nextNodeTop = getRelativeTop(nextStickyNodeInfo.node, node) ?? 0; - } - - if (nextNodeTop !== undefined) { - const nextNodeTopInViewport = y - currentScrollTop + nextNodeTop; - - const nextNodePushTop = nextNodeTopInViewport - stickyNodeHeight; - if (nextNodePushTop < maxStickyTop) { - maxStickyTop = nextNodePushTop; - } + const nextNodePushTop = nextNodeTopInViewport - stickyNodeHeight; + if (nextNodePushTop < maxStickyTop) { + maxStickyTop = nextNodePushTop; } } @@ -473,37 +470,18 @@ export function renderActiveStickyNodes( let minStickyTop = y - currentScrollTop + parentRelativeTop + parentBorderTop; - if (stickyHeadersInBackbuffer) { - minStickyTop = Number.MIN_SAFE_INTEGER; - } - const naturalStickyY = y - currentScrollTop + stickyNodeTop; const stuckStickyY = y + currentBorderTop + currentClientHeight - stickyNodeHeight; - if (nextStickyNodeInfo && !stickyHeadersInBackbuffer) { - let nextNodeHeight: number | undefined; - let nextNodeTop: number | undefined; - - if (nextStickyNodeInfo.cached && nextStickyNodeInfo.anchor) { - nextNodeHeight = nextStickyNodeInfo.cached.height; - const staticRenderPosTop = - getRelativeTop(nextStickyNodeInfo.anchor, node) ?? 0; - nextNodeTop = - staticRenderPosTop + nextStickyNodeInfo.cached.relativeY!; - } else if (nextStickyNodeInfo.node?.yogaNode) { - nextNodeHeight = Math.round( - nextStickyNodeInfo.node.yogaNode.getComputedHeight(), - ); - nextNodeTop = getRelativeTop(nextStickyNodeInfo.node, node) ?? 0; - } + if (nextStickyNodeInfo?.resolvedInfo) { + const nextNodeHeight = nextStickyNodeInfo.resolvedInfo.stickyNodeHeight; + const nextNodeTop = nextStickyNodeInfo.resolvedInfo.stickyNodeTop; - if (nextNodeTop !== undefined && nextNodeHeight !== undefined) { - const nextNodeBottomInViewport = - y - currentScrollTop + nextNodeTop + nextNodeHeight; - if (nextNodeBottomInViewport > minStickyTop) { - minStickyTop = nextNodeBottomInViewport; - } + const nextNodeBottomInViewport = + y - currentScrollTop + nextNodeTop + nextNodeHeight; + if (nextNodeBottomInViewport > minStickyTop) { + minStickyTop = nextNodeBottomInViewport; } } diff --git a/test/repro-issue.test.tsx b/test/repro-issue.test.tsx index 317c5f3b0..00fed323d 100644 --- a/test/repro-issue.test.tsx +++ b/test/repro-issue.test.tsx @@ -46,15 +46,20 @@ const createTestEnv = ( (stdin as any).ref = () => stdin; (stdin as any).unref = () => stdin; - const {unmount} = render(, { - stdout, - stdin, - patchConsole: false, - terminalBuffer: true, - renderProcess: false, // Run in-process for easier debugging - debugRainbow: true, - ...options, - }); + const {unmount} = render( + , + { + stdout, + stdin, + patchConsole: false, + terminalBuffer: true, + renderProcess: false, // Run in-process for easier debugging + debugRainbow: true, + ...options, + }, + ); const press = async (key: string) => { const currentCount = writeCount; @@ -102,6 +107,7 @@ const createTestEnv = ( return { term, stdin, + stdout, unmount, press, getLine, @@ -110,7 +116,11 @@ const createTestEnv = ( }; test('repro issue: sticky headers and spurious renders', async t => { - const env = createTestEnv(20, 80); + // Use a small viewport (height 5) so that a group (height ~8) can span the entire viewport. + const env = createTestEnv(5, 80, {initialItems: 2}); + + // Collapse footer so the scrollable area has space + await env.press('f'); // 1. Press space 5 times to add messages for (let i = 0; i < 5; i++) { @@ -119,8 +129,8 @@ test('repro issue: sticky headers and spurious renders', async t => { } // 2. Scroll up to a position where Header 4 (starts at ~160 in actual lines) is stuck. - // We'll scroll up 50 lines from the bottom (193 - 50 = 143). - for (let i = 0; i < 50; i++) { + // We'll scroll up 48 lines from the bottom. + for (let i = 0; i < 48; i++) { // eslint-disable-next-line no-await-in-loop await env.press('up'); } @@ -134,14 +144,20 @@ test('repro issue: sticky headers and spurious renders', async t => { }); const instance = instances.get(env.stdout as unknown as NodeJS.WriteStream); - const termBuffer = ( - instance as unknown as { - terminalBuffer: {lines: Array<{getText: () => string}>}; - } - )?.terminalBuffer; - + const termBuffer = (instance as any)?.terminalBuffer as + | { + workerInstance?: { + screen: Array<{styledChars: {getText: () => string}}>; + }; + lines?: Array<{getText: () => string}>; + } + | undefined; let contentAfterHon = ''; - if (termBuffer?.lines && termBuffer.lines.length > 0) { + if (termBuffer?.workerInstance) { + contentAfterHon = termBuffer.workerInstance.screen + .map(l => l.styledChars.getText().trimEnd()) + .join('\n'); + } else if (termBuffer?.lines && termBuffer.lines.length > 0) { contentAfterHon = termBuffer.lines .map(l => l.getText().trimEnd()) .join('\n'); @@ -153,8 +169,13 @@ test('repro issue: sticky headers and spurious renders', async t => { // Assertion 1: Sticky footer should be visible when stuck to the terminal bottom t.true( - contentAfterHon.replaceAll(/\s+/g, '').includes('StickyFooter0'), - 'Sticky Footer 0 should be visible (stuck to bottom) when stickyHeadersInBackbuffer is on', + contentAfterHon.replaceAll(/\s+/g, '').includes('StickyFooter11'), + 'Sticky Footer 11 should be visible (stuck to bottom)', + ); + + t.true( + contentAfterHon.includes('Sticky Header 11 (sticky top)'), + 'Sticky Header 11 should be visible (stuck to top) when stickyHeadersInBackbuffer is on', ); // 4. Toggle sticky headers OFF @@ -165,7 +186,11 @@ test('repro issue: sticky headers and spurious renders', async t => { }); let contentAfterHoff = ''; - if (termBuffer?.lines && termBuffer.lines.length > 0) { + if (termBuffer?.workerInstance) { + contentAfterHoff = termBuffer.workerInstance.screen + .map(l => l.styledChars.getText().trimEnd()) + .join('\n'); + } else if (termBuffer?.lines && termBuffer.lines.length > 0) { contentAfterHoff = termBuffer.lines .map(l => l.getText().trimEnd()) .join('\n'); @@ -177,8 +202,8 @@ test('repro issue: sticky headers and spurious renders', async t => { // Assertion 2: Sticky header should NOT be visible when toggled off t.false( - contentAfterHoff.includes('Header 0 (sticky top)'), - 'Header 0 (sticky top) should not be visible when stickyHeadersInBackbuffer is off', + contentAfterHoff.includes('Sticky Header 11 (sticky top)'), + 'Sticky Header 11 should not be visible when stickyHeadersInBackbuffer is off', ); env.unmount(); }); diff --git a/test/sticky-backbuffer-overlap.test.tsx b/test/sticky-backbuffer-overlap.test.tsx index 771daa9e7..51625cab0 100644 --- a/test/sticky-backbuffer-overlap.test.tsx +++ b/test/sticky-backbuffer-overlap.test.tsx @@ -49,7 +49,6 @@ test('sticky header should not overlap with bottom border when pushed out', asyn width={columns} flexShrink={0} > - {' '} STICKY @@ -98,7 +97,6 @@ test('sticky header should not overlap with bottom border when pushed out', asyn width={columns} flexShrink={0} > - {' '} STICKY From 87432632a600ea3ad45487759aadab8dd6f80ea8 Mon Sep 17 00:00:00 2001 From: jacob314 Date: Thu, 23 Apr 2026 14:07:22 -0700 Subject: [PATCH 3/6] test: refactor duplicated terminal buffer logic and remove fixed timeouts --- test/helpers/terminal-buffer.ts | 26 ++++++++++ test/repro-issue.test.tsx | 63 +++++++++---------------- test/sticky-backbuffer-overlap.test.tsx | 30 +++++------- 3 files changed, 60 insertions(+), 59 deletions(-) create mode 100644 test/helpers/terminal-buffer.ts diff --git a/test/helpers/terminal-buffer.ts b/test/helpers/terminal-buffer.ts new file mode 100644 index 000000000..3a4a85618 --- /dev/null +++ b/test/helpers/terminal-buffer.ts @@ -0,0 +1,26 @@ +import type NodeJS from 'node:process'; +import instances from '../../src/instances.js'; + +export function getTerminalBufferContent(stdout: NodeJS.WriteStream): string | undefined { + const instance = instances.get(stdout); + const termBuffer = (instance as unknown as { + terminalBuffer?: { + workerInstance?: { + screen: Array<{styledChars: {getText: () => string}}>; + }; + lines?: Array<{getText: () => string}>; + }; + })?.terminalBuffer; + + if (termBuffer?.workerInstance) { + return termBuffer.workerInstance.screen + .map(l => l.styledChars.getText().trimEnd()) + .join('\n'); + } else if (termBuffer?.lines && termBuffer.lines.length > 0) { + return termBuffer.lines + .map(l => l.getText().trimEnd()) + .join('\n'); + } + + return undefined; +} diff --git a/test/repro-issue.test.tsx b/test/repro-issue.test.tsx index 00fed323d..31e91170f 100644 --- a/test/repro-issue.test.tsx +++ b/test/repro-issue.test.tsx @@ -2,7 +2,7 @@ import {PassThrough} from 'node:stream'; import test from 'ava'; import React from 'react'; import xtermHeadless from '@xterm/headless'; -import instances from '../src/instances.js'; +import {getTerminalBufferContent} from './helpers/terminal-buffer.js'; import {render} from '../src/index.js'; import ScrollableContent from '../examples/sticky/sticky.js'; import {waitFor} from './helpers/wait-for.js'; @@ -138,33 +138,19 @@ test('repro issue: sticky headers and spurious renders', async t => { // 3. Toggle sticky headers ON await env.press('h'); - // Wait for the backbuffer delay to expire (1000ms by default in terminal-writer) - await new Promise(resolve => { - setTimeout(resolve, 1500); - }); - - const instance = instances.get(env.stdout as unknown as NodeJS.WriteStream); - const termBuffer = (instance as any)?.terminalBuffer as - | { - workerInstance?: { - screen: Array<{styledChars: {getText: () => string}}>; - }; - lines?: Array<{getText: () => string}>; - } - | undefined; - let contentAfterHon = ''; - if (termBuffer?.workerInstance) { - contentAfterHon = termBuffer.workerInstance.screen - .map(l => l.styledChars.getText().trimEnd()) - .join('\n'); - } else if (termBuffer?.lines && termBuffer.lines.length > 0) { - contentAfterHon = termBuffer.lines - .map(l => l.getText().trimEnd()) - .join('\n'); - } else { - contentAfterHon = env.getFullContent(); + // Wait for the render update to propagate + try { + await waitFor(() => { + const content = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + const cleanContent = content.replaceAll(/\s+/g, ''); + return cleanContent.includes('StickyFooter11') && content.includes('Sticky Header 11 (sticky top)'); + }); + } catch { + // Ignore timeout so the assertions below can provide descriptive failure messages } + const contentAfterHon = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + t.log('Content after pressing H (on):\n' + contentAfterHon); // Assertion 1: Sticky footer should be visible when stuck to the terminal bottom @@ -181,23 +167,18 @@ test('repro issue: sticky headers and spurious renders', async t => { // 4. Toggle sticky headers OFF await env.press('h'); - await new Promise(resolve => { - setTimeout(resolve, 1500); - }); - - let contentAfterHoff = ''; - if (termBuffer?.workerInstance) { - contentAfterHoff = termBuffer.workerInstance.screen - .map(l => l.styledChars.getText().trimEnd()) - .join('\n'); - } else if (termBuffer?.lines && termBuffer.lines.length > 0) { - contentAfterHoff = termBuffer.lines - .map(l => l.getText().trimEnd()) - .join('\n'); - } else { - contentAfterHoff = env.getFullContent(); + // Wait for the render update to propagate + try { + await waitFor(() => { + const content = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + return !content.includes('Sticky Header 11 (sticky top)'); + }); + } catch { + // Ignore timeout so the assertions below can provide descriptive failure messages } + const contentAfterHoff = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + t.log('Content after pressing H (off):\n' + contentAfterHoff); // Assertion 2: Sticky header should NOT be visible when toggled off diff --git a/test/sticky-backbuffer-overlap.test.tsx b/test/sticky-backbuffer-overlap.test.tsx index 51625cab0..baca1ddf7 100644 --- a/test/sticky-backbuffer-overlap.test.tsx +++ b/test/sticky-backbuffer-overlap.test.tsx @@ -2,7 +2,7 @@ import {PassThrough} from 'node:stream'; import test from 'ava'; import React from 'react'; import xtermHeadless from '@xterm/headless'; -import instances from '../src/instances.js'; +import {getTerminalBufferContent} from './helpers/terminal-buffer.js'; import {render, Box, Text} from '../src/index.js'; import {waitFor} from './helpers/wait-for.js'; @@ -109,24 +109,18 @@ test('sticky header should not overlap with bottom border when pushed out', asyn ); await waitFor(() => writeCount > prevWriteCount); - // Ensure worker has processed everything, not just the first chunk - await new Promise(resolve => { - setTimeout(() => { - resolve(); - }, 50); - }); + try { + await waitFor(() => { + const content = getTerminalBufferContent(stdout as unknown as NodeJS.WriteStream); + const firstLine = content ? content.split('\n')[0] : getLine(0); + return (firstLine?.includes('╰') && !firstLine?.includes('STICKY')) ?? false; + }); + } catch { + // Ignore timeout so the assertions below can provide descriptive failure messages + } - const instance = instances.get(stdout as unknown as NodeJS.WriteStream); - const termBuffer = ( - instance as unknown as { - terminalBuffer: {lines: Array<{getText: () => string}>}; - } - )?.terminalBuffer; - let firstLine = ''; - firstLine = - termBuffer?.lines && termBuffer.lines.length > 0 - ? termBuffer.lines[0].getText().trimEnd() - : getLine(0); + const content = getTerminalBufferContent(stdout as unknown as NodeJS.WriteStream); + const firstLine = content ? content.split('\n')[0]! : getLine(0); t.log('Terminal row 0: "' + firstLine + '"'); From cc38b0d5b8263fc235754d8f57303bff92a7fe3f Mon Sep 17 00:00:00 2001 From: jacob314 Date: Thu, 23 Apr 2026 14:14:31 -0700 Subject: [PATCH 4/6] More fixes --- test/helpers/terminal-buffer.ts | 28 ++++++++++++++----------- test/repro-issue.test.tsx | 21 ++++++++++++++----- test/sticky-backbuffer-overlap.test.tsx | 12 ++++++++--- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/test/helpers/terminal-buffer.ts b/test/helpers/terminal-buffer.ts index 3a4a85618..8053f6684 100644 --- a/test/helpers/terminal-buffer.ts +++ b/test/helpers/terminal-buffer.ts @@ -1,25 +1,29 @@ import type NodeJS from 'node:process'; import instances from '../../src/instances.js'; -export function getTerminalBufferContent(stdout: NodeJS.WriteStream): string | undefined { +export function getTerminalBufferContent( + stdout: NodeJS.WriteStream, +): string | undefined { const instance = instances.get(stdout); - const termBuffer = (instance as unknown as { - terminalBuffer?: { - workerInstance?: { - screen: Array<{styledChars: {getText: () => string}}>; + const termBuffer = ( + instance as unknown as { + terminalBuffer?: { + workerInstance?: { + screen: Array<{styledChars: {getText: () => string}}>; + }; + lines?: Array<{getText: () => string}>; }; - lines?: Array<{getText: () => string}>; - }; - })?.terminalBuffer; + } + )?.terminalBuffer; if (termBuffer?.workerInstance) { return termBuffer.workerInstance.screen .map(l => l.styledChars.getText().trimEnd()) .join('\n'); - } else if (termBuffer?.lines && termBuffer.lines.length > 0) { - return termBuffer.lines - .map(l => l.getText().trimEnd()) - .join('\n'); + } + + if (termBuffer?.lines && termBuffer.lines.length > 0) { + return termBuffer.lines.map(l => l.getText().trimEnd()).join('\n'); } return undefined; diff --git a/test/repro-issue.test.tsx b/test/repro-issue.test.tsx index 31e91170f..09ddb00d7 100644 --- a/test/repro-issue.test.tsx +++ b/test/repro-issue.test.tsx @@ -141,15 +141,22 @@ test('repro issue: sticky headers and spurious renders', async t => { // Wait for the render update to propagate try { await waitFor(() => { - const content = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + const content = + getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || + env.getFullContent(); const cleanContent = content.replaceAll(/\s+/g, ''); - return cleanContent.includes('StickyFooter11') && content.includes('Sticky Header 11 (sticky top)'); + return ( + cleanContent.includes('StickyFooter11') && + content.includes('Sticky Header 11 (sticky top)') + ); }); } catch { // Ignore timeout so the assertions below can provide descriptive failure messages } - const contentAfterHon = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + const contentAfterHon = + getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || + env.getFullContent(); t.log('Content after pressing H (on):\n' + contentAfterHon); @@ -170,14 +177,18 @@ test('repro issue: sticky headers and spurious renders', async t => { // Wait for the render update to propagate try { await waitFor(() => { - const content = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + const content = + getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || + env.getFullContent(); return !content.includes('Sticky Header 11 (sticky top)'); }); } catch { // Ignore timeout so the assertions below can provide descriptive failure messages } - const contentAfterHoff = getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || env.getFullContent(); + const contentAfterHoff = + getTerminalBufferContent(env.stdout as unknown as NodeJS.WriteStream) || + env.getFullContent(); t.log('Content after pressing H (off):\n' + contentAfterHoff); diff --git a/test/sticky-backbuffer-overlap.test.tsx b/test/sticky-backbuffer-overlap.test.tsx index baca1ddf7..81f6378d4 100644 --- a/test/sticky-backbuffer-overlap.test.tsx +++ b/test/sticky-backbuffer-overlap.test.tsx @@ -111,15 +111,21 @@ test('sticky header should not overlap with bottom border when pushed out', asyn await waitFor(() => writeCount > prevWriteCount); try { await waitFor(() => { - const content = getTerminalBufferContent(stdout as unknown as NodeJS.WriteStream); + const content = getTerminalBufferContent( + stdout as unknown as NodeJS.WriteStream, + ); const firstLine = content ? content.split('\n')[0] : getLine(0); - return (firstLine?.includes('╰') && !firstLine?.includes('STICKY')) ?? false; + return ( + (firstLine?.includes('╰') && !firstLine?.includes('STICKY')) ?? false + ); }); } catch { // Ignore timeout so the assertions below can provide descriptive failure messages } - const content = getTerminalBufferContent(stdout as unknown as NodeJS.WriteStream); + const content = getTerminalBufferContent( + stdout as unknown as NodeJS.WriteStream, + ); const firstLine = content ? content.split('\n')[0]! : getLine(0); t.log('Terminal row 0: "' + firstLine + '"'); From bf36693d8131e47f48ef1b07a5f8eafcb7d2de2a Mon Sep 17 00:00:00 2001 From: jacob314 Date: Thu, 23 Apr 2026 20:11:43 -0700 Subject: [PATCH 5/6] test: fix sticky overlap test layout and pty spawn errors --- test/exit.tsx | 2 +- test/hooks.tsx | 2 +- test/render.tsx | 2 +- test/sticky-backbuffer-overlap.test.tsx | 2 ++ 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/exit.tsx b/test/exit.tsx index 7ceb81d3e..fc29b0c84 100644 --- a/test/exit.tsx +++ b/test/exit.tsx @@ -66,7 +66,7 @@ test.serial('don’t exit while raw mode is active', async t => { }; const term = spawn( - 'node', + process.execPath, [ '--loader=ts-node/esm', path.join(__dirname, './fixtures/exit-double-raw-mode.tsx'), diff --git a/test/hooks.tsx b/test/hooks.tsx index 784b84de8..1c91f39fa 100644 --- a/test/hooks.tsx +++ b/test/hooks.tsx @@ -26,7 +26,7 @@ const term = (fixture: string, args: string[] = []) => { }; const ps = spawn( - 'node', + process.execPath, [ '--loader=ts-node/esm', path.join(__dirname, `./fixtures/${fixture}.tsx`), diff --git a/test/render.tsx b/test/render.tsx index 952db2a6d..d299f9855 100644 --- a/test/render.tsx +++ b/test/render.tsx @@ -62,7 +62,7 @@ const term = (fixture: string, args: string[] = []) => { }; const ps = spawn( - 'node', + process.execPath, [ '--loader=ts-node/esm', path.join(__dirname, `./fixtures/${fixture}.tsx`), diff --git a/test/sticky-backbuffer-overlap.test.tsx b/test/sticky-backbuffer-overlap.test.tsx index 81f6378d4..fb1b89fa2 100644 --- a/test/sticky-backbuffer-overlap.test.tsx +++ b/test/sticky-backbuffer-overlap.test.tsx @@ -37,6 +37,7 @@ test('sticky header should not overlap with bottom border when pushed out', asyn Date: Fri, 24 Apr 2026 11:10:19 -0700 Subject: [PATCH 6/6] Code review comment fixes. --- src/worker/render-worker.ts | 263 +++++++++++++++++++----------------- 1 file changed, 137 insertions(+), 126 deletions(-) diff --git a/src/worker/render-worker.ts b/src/worker/render-worker.ts index edebcdf2d..22f0569bb 100644 --- a/src/worker/render-worker.ts +++ b/src/worker/render-worker.ts @@ -690,94 +690,14 @@ export class TerminalBufferWorker { skipScrollbars: false, }); - const processRegionForScroll = ( - node: RegionNode, - offsetX: number, - offsetY: number, - ) => { - const region = this.sceneManager.getRegion(node.id); - if (!region) return; - - const absX = Math.round(region.x + offsetX); - const absY = Math.round(region.y + offsetY); - - const operations = this.scrollOptimizer.calculateScrollOperations( - region, - this.rows, - this.columns, - absY, - (scrollStart, count, start, end, scrollToBackbuffer) => { - const originalScrollTop = region.scrollTop; - region.scrollTop = scrollStart; - try { - const getLines = (skipScrollbars: boolean) => { - const canvas = Canvas.create(this.columns, this.rows + count); - this.composeNode( - this.sceneManager.root!, - canvas, - { - clip: undefined, - offsetY: -cameraY, - }, - {skipStickyHeaders: true, skipScrollbars}, - ); - - const lines = canvas.getLines(); - return start === 0 && end + count === lines.length - ? lines - : lines.slice(start, end + count); - }; - - if (scrollToBackbuffer && count > 0 && start === 0) { - const dirtyLines = getLines(false); - const cleanLines = getLines(true); - // First 'count' lines are clean (for backbuffer), the rest are dirty (for viewport) - return [ - ...cleanLines.slice(0, count), - ...dirtyLines.slice(count), - ]; - } - - return getLines(false); - } finally { - region.scrollTop = originalScrollTop; - } - }, - (r, s, a) => compositor.calculateActualStuckTopHeight(r, s, a), - (r, s, a) => compositor.calculateActualStuckBottomHeight(r, s, a), - this.stickyHeadersInBackbuffer, - ); - - for (const op of operations) { - if (op.scrollToBackbuffer) { - if (debugWorker) { - debugLog( - `[RENDER-WORKER] Region ${op.regionId} scrolling ${op.linesToScroll} lines to backbuffer`, - ); - } - - scrolledToBackbuffer.set( - op.regionId, - (scrolledToBackbuffer.get(op.regionId) ?? 0) + op.linesToScroll, - ); - } - - this.terminalWriter.scrollLines(op); - if (op.newMaxPushed !== undefined) { - this.scrollOptimizer.updateMaxPushed(op.regionId, op.newMaxPushed); - } - } - - for (const child of node.children) { - processRegionForScroll( - child, - absX - (region.scrollLeft ?? 0), - absY - (region.scrollTop ?? 0), - ); - } - }; - - processRegionForScroll(this.sceneManager.root!, 0, -cameraY); + this.processRegionForScroll( + this.sceneManager.root!, + 0, + -cameraY, + cameraY, + scrolledToBackbuffer, + compositor, + ); } // 2. Compose Frame @@ -875,44 +795,8 @@ export class TerminalBufferWorker { this.backbuffer = []; if (!this.isAlternateBufferEnabled && computeBackbuffer) { - const composeToBackbuffer = ( - node: RegionNode, - region: Region, - height: number, - offset: number, - ) => { - const canvas = Canvas.create(this.columns, height); - const originalScrollTop = region.scrollTop; - region.scrollTop = offset; - try { - this.composeNode( - node, - canvas, - { - clip: undefined, - offsetY: -region.y, - offsetX: 0, - overrideHeight: height, - isExpanded: true, - }, - { - skipStickyHeaders: true, - skipScrollbars: true, - }, - ); - } finally { - region.scrollTop = originalScrollTop; - } - - for (const line of canvas.getLines()) { - this.backbuffer.push( - this.terminalWriter.clampLine(line.styledChars, this.columns), - ); - } - }; - const rootBackbufferHeight = cameraY; - composeToBackbuffer( + this.composeToBackbuffer( this.sceneManager.root!, rootRegion, rootBackbufferHeight, @@ -925,7 +809,7 @@ export class TerminalBufferWorker { const regionBackbufferHeight = scrollTop; const node = this.findNodeForRegion(region.id); if (node) { - composeToBackbuffer(node, region, regionBackbufferHeight, 0); + this.composeToBackbuffer(node, region, regionBackbufferHeight, 0); } } } @@ -940,6 +824,42 @@ export class TerminalBufferWorker { this.resized = false; } + private composeToBackbuffer( + node: RegionNode, + region: Region, + height: number, + offset: number, + ) { + const canvas = Canvas.create(this.columns, height); + const originalScrollTop = region.scrollTop; + region.scrollTop = offset; + try { + this.composeNode( + node, + canvas, + { + clip: undefined, + offsetY: -region.y, + offsetX: 0, + overrideHeight: height, + isExpanded: true, + }, + { + skipStickyHeaders: true, + skipScrollbars: true, + }, + ); + } finally { + region.scrollTop = originalScrollTop; + } + + for (const line of canvas.getLines()) { + this.backbuffer.push( + this.terminalWriter.clampLine(line.styledChars, this.columns), + ); + } + } + /** * Recursively composites a region and its children onto the provided canvas. * @@ -1168,6 +1088,97 @@ export class TerminalBufferWorker { return visit(this.sceneManager.root); } + private processRegionForScroll( + node: RegionNode, + offsetX: number, + offsetY: number, + cameraY: number, + scrolledToBackbuffer: Map, + compositor: Compositor, + ) { + const region = this.sceneManager.getRegion(node.id); + if (!region) return; + + const exactAbsX = region.x + offsetX; + const exactAbsY = region.y + offsetY; + const absY = Math.round(exactAbsY); + + const operations = this.scrollOptimizer.calculateScrollOperations( + region, + this.rows, + this.columns, + absY, + (scrollStart, count, start, end, scrollToBackbuffer) => { + const originalScrollTop = region.scrollTop; + region.scrollTop = scrollStart; + try { + const getLines = (skipScrollbars: boolean) => { + const canvas = Canvas.create(this.columns, this.rows + count); + this.composeNode( + this.sceneManager.root!, + canvas, + { + clip: undefined, + offsetY: -cameraY, + }, + {skipStickyHeaders: true, skipScrollbars}, + ); + + const lines = canvas.getLines(); + return start === 0 && end + count === lines.length + ? lines + : lines.slice(start, end + count); + }; + + if (scrollToBackbuffer && count > 0 && start === 0) { + const dirtyLines = getLines(false); + const cleanLines = getLines(true); + // First 'count' lines are clean (for backbuffer), the rest are dirty (for viewport) + return [...cleanLines.slice(0, count), ...dirtyLines.slice(count)]; + } + + return getLines(false); + } finally { + region.scrollTop = originalScrollTop; + } + }, + (r, s, a) => compositor.calculateActualStuckTopHeight(r, s, a), + (r, s, a) => compositor.calculateActualStuckBottomHeight(r, s, a), + this.stickyHeadersInBackbuffer, + ); + + for (const op of operations) { + if (op.scrollToBackbuffer) { + if (debugWorker) { + debugLog( + `[RENDER-WORKER] Region ${op.regionId} scrolling ${op.linesToScroll} lines to backbuffer`, + ); + } + + scrolledToBackbuffer.set( + op.regionId, + (scrolledToBackbuffer.get(op.regionId) ?? 0) + op.linesToScroll, + ); + } + + this.terminalWriter.scrollLines(op); + if (op.newMaxPushed !== undefined) { + this.scrollOptimizer.updateMaxPushed(op.regionId, op.newMaxPushed); + } + } + + for (const child of node.children) { + this.processRegionForScroll( + child, + exactAbsX - (region.scrollLeft ?? 0), + exactAbsY - (region.scrollTop ?? 0), + cameraY, + scrolledToBackbuffer, + compositor, + ); + } + } + private logScene(scrolledToBackbuffer?: Map) { if (!debugWorker) { return;