Skip to content

Fix: <Static> remount via key change drops new items#948

Merged
sindresorhus merged 3 commits into
vadimdemedes:masterfrom
chiga0:fix/static-remount-loses-items-on-key-change
May 12, 2026
Merged

Fix: <Static> remount via key change drops new items#948
sindresorhus merged 3 commits into
vadimdemedes:masterfrom
chiga0:fix/static-remount-loses-items-on-key-change

Conversation

@chiga0

@chiga0 chiga0 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #947.

Summary

`reconciler.removeChildFromContainer` / `removeChild` unconditionally cleared `rootNode.staticNode` whenever the removed node was marked `internal_static`. React processes insertions before deletions inside a single commit, so when a `` is remounted via a `key` prop change:

  1. `createInstance` fires for the new internal_static node and registers `rootNode.staticNode = newNode` (also marking `isStaticDirty = true`).
  2. `appendChildToContainer` attaches the new node.
  3. `removeChildFromContainer` fires for the old internal_static node, sees `removeNode.internal_static === true`, and zeroes out `currentRootNode.staticNode` — clobbering the pointer the new node just registered.

After that, `renderer.ts` sees `node.staticNode === undefined`, emits no static output, and the new ``'s items are silently dropped from stdout.

Fix

Clear `staticNode` only when the node being removed is the one the root currently points to. Identical guard added to both `removeChildFromContainer` (root-level remount) and `removeChild` (nested remount).

```diff

  • if (removeNode.internal_static && currentRootNode) {
  • if (
  • removeNode.internal_static &&
  • currentRootNode &&
  • currentRootNode.staticNode === removeNode
  • ) {
    currentRootNode.staticNode = undefined;
    }
    ```

The existing #904 / #905 path (pure unmount → `staticNode` should be cleared) is unaffected: in that case `currentRootNode.staticNode` still equals `removeNode` at removal time, so the guard passes through.

Test

Adds `remounting via key change emits the new items` to `test/components.tsx`. Asserts that after a key-driven remount of `` with a disjoint items array, the new items reach stdout.

Verified:

  • New test fails on `master` (`1510851`) — confirms the regression
  • New test passes with this patch
  • Existing Static tests all pass — including the rootNode.staticNode dangling reference causes OOM after <Static> unmount #904 `stops accumulating after Static unmounts` regression (the pure-unmount path is preserved)
  • Full `ava` suite shows no new failures vs. `master` baseline (two pre-existing flaky failures on `cursor` and `wrap-ansi doesn't trim leading whitespace` are unrelated and reproduce on `master` without this patch)

Real-world impact

Both Gemini CLI and qwen-code use a `` pattern to force a full replay of the chat history after `/clear`, model switch, Ctrl+O, etc. After upgrading to ink 7, the entire history pane stops rendering immediately after the first such trigger. End-to-end repro on qwen-code: QwenLM/qwen-code#4083 (currently has to revert ink 7 → 6 as a stop-gap).

`reconciler.removeChildFromContainer` / `removeChild` unconditionally
cleared `rootNode.staticNode` whenever the removed node was marked
`internal_static`. React processes insertions before deletions inside a
single commit, so when a `<Static>` is remounted via a key prop change:

1. `createInstance` fires for the new internal_static node and registers
   `rootNode.staticNode = newNode` (also marking `isStaticDirty = true`).
2. `appendChildToContainer` attaches the new node.
3. `removeChildFromContainer` fires for the old internal_static node,
   sees `removeNode.internal_static === true`, and zeroes out
   `currentRootNode.staticNode` — clobbering the pointer the new node
   just registered.

After that, `renderer.ts` sees `node.staticNode === undefined`, emits
no static output, and the new `<Static>`'s items are silently dropped
from stdout. The dynamic frame still renders, so the failure looks like
"items vanished" rather than a crash.

Fix: clear `staticNode` only when the node being removed is the one
the root currently points to. Identical guard added to both
`removeChildFromContainer` (root-level remount) and `removeChild`
(nested remount).

Real-world impact this surfaces in:
- Gemini CLI / qwen-code patterns where a `historyRemountKey` is bumped
  to force a `<Static>` replay (after `/clear`, model switch, Ctrl+O,
  auth refresh, renderMode toggle). Post-upgrade to ink 7, the entire
  history pane stops rendering until the next dependency upgrade
  reverts to ink 6.
- Any app that switches between two `<Static>` instances by key for
  log filtering / session isolation.

Adds a regression test that mounts <Static key=1>, rerenders to
<Static key=2> with disjoint items, and asserts that the new items
reach stdout. Test fails without the reconciler change.

Closes the gap left by vadimdemedes#905 (which cleared the OOM-on-unmount path
but did not consider the insertion-before-deletion ordering of a
key-driven remount).
Comment thread src/reconciler.ts Outdated
// new Static instance.
if (
removeNode.internal_static &&
currentRootNode &&

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This guard currently fails the repo lint (npm run lint) with @typescript-eslint/prefer-optional-chain. Please collapse it to removeNode.internal_static && currentRootNode?.staticNode === removeNode before merging. generated by GPT-5 model

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 12f47b2 — collapsed both guards to currentRootNode?.staticNode === removeNode. npm run lint is now clean (only pre-existing warnings on master remain) and the new remounting <Static> via key change emits the new items test still passes.

Comment thread src/reconciler.ts Outdated
秦奇 and others added 2 commits May 12, 2026 18:49
Address PR review: `npm run lint` was failing with
@typescript-eslint/prefer-optional-chain on both guards.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…er path)

The original regression test wraps <Static> in a <Box>, so the old
node's removal dispatches to `removeChild`. The companion path —
`removeChildFromContainer`, fired when Static is a direct child of
the root container — was not exercised. Add a test that renders
Static at the top level (no wrapping Box) so both removal paths
are covered.

Both new tests verified to fail against the pre-fix reconciler and
pass with the fix applied.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@sindresorhus sindresorhus merged commit be9f44c into vadimdemedes:master May 12, 2026
2 checks passed
@chiga0 chiga0 deleted the fix/static-remount-loses-items-on-key-change branch May 13, 2026 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

<Static> remount via key change drops new items (post-7.0 regression)

2 participants