Fix: drop stale <Static> output from fullStaticOutput on identity change - #950
Merged
sindresorhus merged 3 commits intoMay 13, 2026
Conversation
`Ink#fullStaticOutput` is the running concatenation of everything every `<Static>` instance has ever emitted during the lifetime of an Ink instance. It's never cleared. When `shouldClearTerminalForFrame` later fires (output overflow, leaving fullscreen, unmount), Ink writes `clearTerminal + this.fullStaticOutput + output` to redraw the screen, which means: any item that was emitted by a `<Static>` that has since been unmounted (or replaced via key) re-materializes above the current viewport. For long-lived TUIs that drive Static replay by bumping a remount `key` (e.g. Gemini CLI, qwen-code, anything with a /clear / model- switch / Ctrl+O / auth-refresh path), the user-visible symptom is "cleared chat history reappears after a long response scrolls the viewport". The terminal scrollback contract is "what Ink wrote, you see"; the bug is that Ink keeps writing items that no longer exist in the React tree. Fix: track `rootNode.previousStaticNode` across commits. When the reconciler detects that `rootNode.staticNode` has changed identity (first mount, last unmount, or remount via key), fire a new `onStaticChange` hook on the root node BEFORE `onImmediateRender`. Ink installs an `onStaticChange` handler that resets `this.fullStaticOutput = ''`. The new instance (if any) re-emits its items into a fresh accumulator on the immediate render that follows. Scope: - The classic Tap-style "one Static mounted for the lifetime of the app, items keep getting appended" use case is untouched: staticNode identity never changes, onStaticChange never fires. - The vadimdemedes#904 unmount path is strengthened: in addition to clearing `rootNode.staticNode` (so the renderer stops walking a freed Yoga tree), Ink now also drops the now-orphan output from `fullStaticOutput`, so a later overflow-driven clearTerminal rewrite does not resurrect the unmounted Static's items. Adds a regression test: mounts a `<Static>` with items HISTORY-A / HISTORY-B in debug mode (every stdout.write is `fullStaticOutput + dynamicOutput`), unmounts it, asserts that the subsequent write does NOT include HISTORY-A or HISTORY-B. The companion remount-via-key case is intentionally NOT tested here because it additionally depends on the `staticNode`-pointer-clobbered- on-remount bug being fixed separately (see vadimdemedes#948). Each fix stands on its own; this PR is independent of vadimdemedes#948.
chiga0
commented
May 12, 2026
chiga0
left a comment
Contributor
Author
There was a problem hiding this comment.
Review note submitted from local audit.
Generated By GPT-5 model
Run prettier on test/components.tsx to fix CI lint failure flagged in PR vadimdemedes#950 review.
Collaborator
|
Can you fix the merge conflict? |
…-not-reset-on-static-unmount # Conflicts: # test/components.tsx
Contributor
Author
It's all clean now. Please check again. By the way, cloud you please release package v7.0.3 after merged the pull request, thank you very much about that. Our package want to upgrade ink from v6.3 to v7, but find the issue. Thank you again. |
Merged
6 tasks
acoliver
added a commit
to vybestack/llxprt-code
that referenced
this pull request
Aug 26, 2026
…the fix (#3345) The first P0 pass recommended NO-GO on migration and "patch the fork, then reassess". Two of its inputs were wrong or missing, and correcting them changes the recommendation to a two-track plan: fix the leak now, and commit to upstream as the destination. What changed: - I never inspected fork 7.1.0. Having done so: the fork's latest is 7.1.0 from 2026-06-24 with no npm publish or repo push since, and it does NOT contain upstream vadimdemedes/ink#950. Upgrading the fork does not fix the leak. Its answer is <StaticRender>, a fork-only API that does zero the accumulator but writes 6.7x its workload even when chunked per item, and whose own docs say incremental invalidation for growing lists is unsolved. Compiling against fork 7.1.0 costs only 3 errors in one test file, so it is cheap to adopt and still the wrong destination. - The leak does not affect the default configuration. ui.useAlternateBuffer defaults to true and schema defaults really are materialised into settings.merged, so the default session takes AlternateBufferLayout, which mounts no <Static> anywhere in its tree. The exposed population is users who explicitly opted out, excluding screen-reader and CI. For them growth is superlinear: bounded history re-emitted in full on each of six refreshStatic() remounts into an accumulator that never resets. The fix is upstream's own change, backported: 11 added lines across two files. Measured on the pinned fork it resets the accumulator to 0 on remount, exactly matching upstream 7.1.1. The same anchors applied unchanged to fork 7.1.0. It survives a real bundle:cli build, and the published bin prefers the bundle, so it reaches users. Terminal behavior after the reset is not yet tested and is recorded as a prerequisite. The selection gate moves from NO-GO to conditional. A spike walking upstream's public DOMElement tree resolved a cell to a UTF-16 offset and painted inverse video through the public Transform, compiling clean with no deep imports, which retracts the earlier claim that a tree port needs private internals. The remaining cost is parity and roughly 137 direct Text import sites. Adds evidence E23 through E32 and the patch itself to the reproduction appendix. Migration sequence is A, C, B, D, and A, C, and B can each land while still building on the patched fork. Refs #3345
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #949.
Summary
`Ink#fullStaticOutput` is the running concatenation of everything every `` instance has ever emitted during the lifetime of an Ink renderer. It is initialised to `''` in the constructor and only ever grows — no code path shrinks or resets it. When `shouldClearTerminalForFrame` later fires (overflow, leaving fullscreen, fullscreen unmount), Ink writes `clearTerminal + fullStaticOutput + output`, which means any item emitted by a `` that has since been unmounted/replaced re-materialises above the current viewport even though it no longer exists in the React tree.
Real-world symptom: "cleared history reappears after a long response scrolls the viewport" in TUIs that drive a Static replay via a remount key (Gemini CLI, qwen-code, …). End-to-end repro thread: QwenLM/qwen-code#4083.
Fix
Track `rootNode.previousStaticNode` across commits. When `resetAfterCommit` detects that `rootNode.staticNode` has changed identity (first mount, last unmount, or remount via key), fire a new `onStaticChange` hook on the root node before `onImmediateRender`. Ink installs an `onStaticChange` handler that resets `this.fullStaticOutput = ''`. The new instance (if any) re-emits its items into a fresh accumulator on the immediate render that follows.
```diff
// resetAfterCommit, src/reconciler.ts
```
```diff
// constructor wiring, src/ink.tsx
this.rootNode.onImmediateRender = this.onRender;
...
```
Scope (no behaviour change for the classic case)
Test
Adds `fullStaticOutput is reset when unmounts so stale items are not replayed` to `test/components.tsx`. Mounts a `<Static items={['HISTORY-A', 'HISTORY-B']} />` in debug mode (every `stdout.write` is `fullStaticOutput + dynamicOutput`), unmounts it, asserts that the subsequent write does NOT include `HISTORY-A` or `HISTORY-B`. Without the fix the assertion fails; with the fix it passes.
The companion remount-via-key case is intentionally NOT tested here because it additionally depends on the `staticNode`-pointer-clobbered-on-remount bug fixed by #948. Each fix stands alone; this PR is independent of #948 and can be merged in either order. The two fixes compose to give the full "remount produces only new items, no resurrected old items" behaviour.
Verified:
Why a hook on the root node and not inline in the reconciler
The reconciler owns the React → DOM mapping; `fullStaticOutput` is a private Ink-instance writer-side accumulator. A hook keeps the layering intact: the reconciler reports an identity change, Ink decides what to do with it. The same pattern is already used for `onRender`, `onImmediateRender`, and `onComputeLayout`.