fix: preserve last <Static> line erased after a full-clear frame - #974
Conversation
…imdemedes#973) In the full-clear render path, Ink wrote the live region as `output` (no trailing newline in non-fullscreen mode) but recorded its height via `log.sync(outputToRender)`, where `outputToRender` includes the trailing newline. The recorded line count was therefore one row greater than what was on screen, so the next incremental frame's `eraseLines` reached one row too far and erased the last committed `<Static>` line. Write `outputToRender` in the full-clear path so the physical output matches the height it records (and the value already assigned to `lastOutputToRender`), consistent with the other two render paths.
CI installs dependencies with `npm install` (no committed lockfile), so `prettier: ^3.8.1` now resolves to 3.9.x, which formats these short union types on a single line. The committed code predates that change, so the lint step failed on files unrelated to this PR's fix. Reformat the four affected declarations to match the prettier version CI resolves.
One more thing: I’m not fully convinced this closes #973 as originally reported. It fixes a real nearby bug in the full clear path, but the issue thread’s captured failure points at the |
|
Addressed all three points. The prettier floor is now ^3.9.4 so a 3.8.x install can no longer satisfy the range and fail lint. The fixture now awaits a flushed frame between the phases as suggested. One thing came up while doing it. In the default legacy mode a freshly scheduled update has not committed yet when waitUntilRenderFlush runs, so the awaits alone let all three updates batch into one frame and the rewritten test passed even with the fix reverted. Each phase now yields a macrotask before the flush so React commits first, and the test additionally asserts the frames were distinct so coalescing fails loudly instead of passing silently. I verified the test fails with the fix reverted and passes with it. On the closing question, you are right. I edited the body so it no longer closes #973, since the incremental path from the original capture is untouched here. The reporter offered to capture again with a patched build, which would settle whether this full clear bug was the mechanism in the wild. |
| import React, {useEffect, useState} from 'react'; | ||
| import {Static, Box, Text, render, useApp} from '../../src/index.js'; | ||
|
|
||
| // Reproduces vadimdemedes/ink#973: the last line of a <Static> block taller |
There was a problem hiding this comment.
Use a multi-line code comment for this whole thing. No hard-wrapping.
|
In, |
2 participants
What
Related to #973 (does not close it). This fixes a full clear accounting bug found while investigating that report. The last line of a
<Static>block taller than the viewport was erased (and not repainted) when the live region updated after a frame routed through the full clear path. The incrementalhasStaticOutputpath implicated in the original capture is unchanged, so #973 stays open pending a recapture with a patched build.Root cause
In the full-clear path of
renderInteractiveFrame(src/ink.tsx), Ink wrote the live region asoutput(no trailing newline in non-fullscreen mode) but recorded its height viathis.log.sync(outputToRender), whereoutputToRenderisoutput + '\n'. The recorded line count was one row greater than what was actually on screen, so the next incremental frame'seraseLines(previousLineCount)reached one row too far and erased the last committed<Static>line. The full-clear path was the only one of the three render paths writing bareoutput; the other two already writeoutputToRender.Fix
Write
outputToRenderin the full-clear path so the physical output matches the height it records (and the value already assigned tolastOutputToRender). In fullscreen modeoutputToRender === output, so that path is byte-identical; in non-fullscreen mode the trailing newline rests the cursor exactly where the steady-state path already leaves it.Test
Added a regression test that drives a
<Static>block taller than a 4-row viewport through the frame sequence that triggers the bug. The erased line still exists in the raw output stream, so a small helper reconstructs the visible terminal buffer (replaying only the handful of escapes Ink emits — no new dependency) and asserts the last static line is still on screen. The test fails before this change and passes after. Full suite passes (1030 tests).Note on the
style:andchore:commits: CI installs withnpm install(no committed lockfile), so it resolves prettier 3.9.x, while^3.8.1still accepts an existing 3.8.x install, which formats these files differently and fails XO. Thechore:commit raises the floor to^3.9.4so any install satisfying the range lints the way CI does, and thestyle:commit contains the corresponding reformat.