From a5a7cf124b03f5c6ae65e7c2d52d7dd7983b2ae5 Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Sat, 30 May 2026 12:55:48 +0800 Subject: [PATCH] fix(runtime): skip comment nodes when indexing children (Ink parity, G52) (#60) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vue materializes a null/v-if/false render as a COMMENT host node that occupies a positional slot in node.children. React never produces a childNode for such children, so Ink's squash loop (squash-text-nodes.ts:13) never advances `index` past them — empirically A{null}(s,i)=>`${i}:${s}`>B yields "A1:B". vue-tui's three squash loops used the raw positional loop counter, so a preceding comment took a slot and shifted the Transform to "A2:B". The fix maintains a separate transform index that advances only for children React would have produced — i.e. skips comment nodes — applied IDENTICALLY in the paint, measure, and screen-reader paths so all three agree and match Ink. G21 (which switched these loops from a hardcoded 0 to the positional counter) introduced the precondition; its real-sibling positional indexing and G32's transform-in-transform recursion remain intact. Co-authored-by: Claude Opus 4.8 --- .agents/docs/parity-ledger.md | 124 ++++++++-------- .../accessibility/screen-reader.test.tsx | 19 +++ .../integration/components/transform.test.tsx | 136 ++++++++++++++++++ packages/runtime/src/host/text-measure.ts | 35 +++-- packages/runtime/src/paint/paint.ts | 41 ++++-- packages/runtime/src/paint/screen-reader.ts | 21 ++- 6 files changed, 284 insertions(+), 92 deletions(-) diff --git a/.agents/docs/parity-ledger.md b/.agents/docs/parity-ledger.md index 715aaf0..3ac2da0 100644 --- a/.agents/docs/parity-ledger.md +++ b/.agents/docs/parity-ledger.md @@ -6,14 +6,15 @@ ## Sweep history -| sweep | Ink SHA | candidates → confirmed | status | -| -------------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | -| sweep-1 (2026-05-29) | `40b3a75` | 16 verified → 14 confirmed (2 refuted) | recorded | -| sweep-2 (2026-05-30) | `40b3a75` | 8 confirmed (re-audit after 16 fixes merged) → 6 new gaps (G18-G23) + 1 candidate (G24) | recorded | -| sweep-3 (2026-05-30) | `40b3a75` | 8 confirmed (re-audit, 22 fixes merged) → 7 new gaps (G25-G31, all LOW) + 1 refuted | recorded | -| sweep-4 (2026-05-30) | `40b3a75` | confirmation re-audit → 7 new (2 MEDIUM: G32-G33; 5 low: G34-G38) + 2 refuted-reversed | recorded | -| sweep-5 (2026-05-30) | `40b3a75` | final re-audit (G32+G33 merged) → 6 cand → 5 confirmed: 1 MEDIUM (G39 SR default-Box separator) + 4 LOW (G40-G43) | recorded | -| sweep-6 (2026-05-30) | `40b3a75` | re-audit (G39 merged) → 9 cand → 8 confirmed: **3 MEDIUM (G44 Static layout-style dropped, G45 programmatic focus-while-disabled, G46 non-empty SR-frame trailing newline) + 5 LOW (G47-G51)** | recorded | +| sweep | Ink SHA | candidates → confirmed | status | +| -------------------- | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | +| sweep-1 (2026-05-29) | `40b3a75` | 16 verified → 14 confirmed (2 refuted) | recorded | +| sweep-2 (2026-05-30) | `40b3a75` | 8 confirmed (re-audit after 16 fixes merged) → 6 new gaps (G18-G23) + 1 candidate (G24) | recorded | +| sweep-3 (2026-05-30) | `40b3a75` | 8 confirmed (re-audit, 22 fixes merged) → 7 new gaps (G25-G31, all LOW) + 1 refuted | recorded | +| sweep-4 (2026-05-30) | `40b3a75` | confirmation re-audit → 7 new (2 MEDIUM: G32-G33; 5 low: G34-G38) + 2 refuted-reversed | recorded | +| sweep-5 (2026-05-30) | `40b3a75` | final re-audit (G32+G33 merged) → 6 cand → 5 confirmed: 1 MEDIUM (G39 SR default-Box separator) + 4 LOW (G40-G43) | recorded | +| sweep-6 (2026-05-30) | `40b3a75` | re-audit (G39 merged) → 9 cand → 8 confirmed: **3 MEDIUM (G44 Static layout-style dropped, G45 programmatic focus-while-disabled, G46 non-empty SR-frame trailing newline) + 5 LOW (G47-G51)** | recorded | +| sweep-7 (2026-05-30) | `40b3a75` | re-audit (G44/G45/G46 merged) → 6 confirmed: **1 MEDIUM (G52 Transform index shift from comment-node siblings) + 5 LOW (display-removal, dup focus id, debug dedup, cross-realm Error exit, useAnimation reset paused)** | recorded | Refuted (NOT gaps): ~~`exit()` second-wins — vue-tui is already guarded~~ **REVERSED by sweep-4: it IS last-wins vs Ink first-wins → now tracked as G33**; kitty key-release printable-text suppression — Ink behaves the same. @@ -39,59 +40,60 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor `status` ∈ `todo · in-progress · pr-open · merged · blocked · refuted · candidate`. Priority: correctness/behavior first, omissions next. -| id | area | summary | priority | status | branch | PR | -| --- | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ----------------------------------------------------------------------------------------------- | ---------------------------------------------- | ---- | --- | --- | -| G01 | static-newline-spacer | Static keeps every already-written item permanently mounted instead of unmounting it | P1 | merged | `fix/parity-static-unmount` | #34 | -| G02 | app-exit-instances-animation-sr | useAnimation does not coalesce ticks within the render-throttle window — delta does not 'account for throttled renders' | P1 | merged | `fix/parity-useanimation-throttle` | #35 | -| G03 | render-lifecycle-reconciler | Live screen-reader render path is missing; commit() always paints the visual grid | P1 | merged | `fix/parity-sr-render` | #36 | -| G04 | box-layout-border | Border edges incorrectly inherit the Box backgroundColor | P2 | merged | `fix/parity-border-bg` | #30 | -| G05 | box-layout-border | Borders skipped when content area is 1 cell tall or wide (w<2 / h<2 guard) | P2 | merged | `fix/parity-border-1cell` | #37 | -| G06 | text-wrap-transform | Nested / transform fn receives hardcoded index 0 instead of childNode index | P2 | refuted | — | — | -| G07 | input-keypress-kitty-paste | Kitty-protocol Ctrl+C triggers app exit in vue-tui but only suppresses the handler in Ink | P2 | kept | — (see ink-parity.md) | — | -| G08 | focus | useFocus does not react to changes in the id prop | P2 | merged | `fix/parity-usefocus-id` | #31 | -| G09 | stdout-stderr-stdin-size-cursor | External stdout/stderr writes are not wrapped in synchronized-update (BSU/ESU) markers | P2 | merged | `fix/parity-external-bsu` | #39 | -| G10 | stdout-stderr-stdin-size-cursor | setRawMode silently no-ops in unsupported environments instead of throwing a descriptive error | P2 | merged | `fix/parity-setrawmode-throw` | #40 | -| G11 | render-lifecycle-reconciler | Resize handler does not clear+reset on terminal-width decrease | P2 | merged | `fix/parity-resize-clear` | #41 | -| G12 | render-lifecycle-reconciler | Renderer frame width/rows lack terminal-size fallback (only ?? defaults) | P2 | merged | `fix/parity-renderer-size` | #33 | -| G13 | box-layout-border | Custom border style objects (BoxStyle) not supported | P3 | merged | `fix/parity-custom-border` | #42 | -| G14 | app-exit-instances-animation-sr | No per-stdout instance reuse/guard — two concurrent renderers can compete for the same stdout | P3 | merged | `fix/parity-instance-reuse` | #43 | -| G15 | box-layout-border | Vertical border sides not shifted up when borderTop=false (Ink offsetY) — left/right rails mispositioned | P2 | merged | `fix/parity-border-1cell` | #37 | -| G16 | box-layout-border | Per-edge borderDimColor=false cannot override general borderDimColor (`\|\| dimAll` vs Ink's `??`) | P3 | merged | `fix/parity-border-dim` | #44 | -| G17 | render-lifecycle-reconciler | Screen-reader live-path edges: still grid-painted (Ink linearizes, skipStaticElements:false) + empty SR frame gets a trailing newline (Ink writes wrapped output directly) | P3 | merged | `fix/parity-sr-edges` | #45 | -| G18 | render-lifecycle-reconciler | No signal-based teardown — terminal corrupted on SIGINT/SIGTERM/SIGHUP (Ink signal-exit at mount) | P1 | merged | `fix/parity-signal-teardown` | #47 | -| G19 | box-layout-border | Dynamic removal of most yoga style props does not reset to default (stale layout) | P2 | merged | `fix/parity-yoga-reset` | #48 | -| G20 | stdout-stderr-stdin-size-cursor | writeToStdout/writeToStderr lack an isUnmounted/teardown guard (post-teardown writes corrupt terminal) | P2 | merged | `fix/parity-write-after-unmount` | #49 | -| G21 | text-wrap-transform | Nested in gets hardcoded index 0 vs child sibling position (squash path) | P3 | merged | `fix/parity-transform-index` | #50 | -| G22 | app-exit-instances-animation-sr | SR role dedup inherits grandparent role; Ink dedups only vs immediate parent | P3 | merged | `fix/parity-sr-role-dedup` | #51 | -| G23 | app-exit-instances-animation-sr | under SR-joins children with newline; Ink concatenates | P3 | pr-open | `fix/parity-sr-transform-concat` | #52 | -| G24 | render-lifecycle-reconciler | Renders ALL nodes; Ink renders only the most-recent staticNode (multi-Static support — kept ) | P3 | kept | — (see ink-parity.md) | — | -| G25 | text-wrap-transform | truncate wrap keeps multi-line when each line fits but whole string overflows (Ink collapses via cliTruncate on whole string) | P3 | todo | — | — | -| G26 | input-keypress-kitty-paste | useInput suppresses input text on kitty key-RELEASE events; Ink delivers the char on release (REVERSES the sweep-1 refutation) | P3 | todo | — | — | -| G27 | stdout-stderr-stdin-size-cursor | releaseRawMode defers input detach + parser reset; Ink stops input synchronously (clearInputState) | P3 | todo | — | — | -| G28 | stdout-stderr-stdin-size-cursor | useStdin() leaks internal members; Ink narrows the public return to PublicProps {stdin,setRawMode,isRawModeSupported} | P3 | todo | — | — | -| G29 | render-lifecycle-reconciler | useCursor()/setCursorPosition never applied during normal render commits (only after console writes) | P3 | todo | — | — | -| G30 | app-exit-instances-animation-sr | SR: nested inside a box-level drops the INNER transform fn (refines G23) | P3 | todo | — | — | -| G31 | app-exit-instances-animation-sr | useAnimation `interval` option is not reactive; Ink re-subscribes+resets when interval changes (cf. G08 id-reactivity) | P3 | todo | — | — | -| G32 | text-wrap-transform | nested directly inside another (in ) is silently DROPPED — paint+measure lose all content | P1 | merged | `fix/parity-transform-nesting` | #54 | -| G33 | app-exit-instances-animation-sr | exit() resolves last-call-wins; Ink is first-call-wins (REVERSES the sweep-1 exit()-second-wins refutation) | P1 | merged | `fix/parity-exit-first-wins` | #55 | -| G34 | box-layout-border | Box border glyphs + bg fill are subject to ancestor transformers; Ink renders chrome with empty transformer list | P3 | todo | — | — | -| G35 | static-newline-spacer | container's own borderStyle/backgroundColor (non-yoga visual style) not painted | P3 | todo | — | — | -| G36 | focus | useFocus autoFocus prop not reactive (captured once); Ink re-registers on autoFocus change | P3 | todo | — | — | -| G37 | render-lifecycle-reconciler | onRender renderTime metric includes stdout write + static capture, not just paint | P3 | todo | — | — | -| G38 | app-exit-instances-animation-sr | useAnimation/scheduler quantizes interval via Math.round; Ink preserves fractional intervals | P3 | todo | — | — | -| G39 | app-exit-instances-animation-sr | SR linearization: a default `` (no flexDirection prop) joins children with `\n`; Ink defaults flexDirection to `row` so it joins with a space (MEDIUM, found sweep-5) | P1 | merged | `fix/parity-sr-default-box-separator` | #56 | -| G40 | render-lifecycle-reconciler | Output write CLAMPS chars at grid width (paint.ts offsetX>=width break/continue); Ink writes overflow past the grid (unbounded output.ts loop) — sweep-5 LOW | P3 | todo | — | — | -| G41 | render-lifecycle-reconciler | onRender renderTime INCLUDES synchronous stdout-write time; Ink calls onRender before the write — sweep-5 LOW (overlaps G37) | P3 | todo | — | — | -| G42 | render-lifecycle-reconciler | Debug mode does NOT re-emit accumulated `` history each frame — sweep-5 LOW | P3 | todo | — | — | -| G43 | render-lifecycle-reconciler | Non-interactive unmount SKIPS the trailing newline when the final frame is empty — sweep-5 LOW | P3 | todo | — | — | -| G44 | static-newline-spacer | `` layout props (flexDirection/padding/justify/align/gap/margin/width) dropped from isolated paint — only column-default + visual border/color survive (MEDIUM, sweep-6) | P1 | pr-open | `fix/parity-static-layout-style` | #59 | -| G45 | focus | Programmatic focusNext()/focusPrevious() are no-ops while focus is disabled; Ink keeps them live (guard belongs only in Tab listener) (MEDIUM, sweep-6) | P1 | pr-open | `fix/parity-programmatic-focus-while-disabled` | #57 | -| G46 | render-lifecycle-reconciler | Non-empty multi-line screen-reader frames append a spurious trailing `\n` + off-by-one erase that Ink does not (G17 only handled the empty case) (MEDIUM, sweep-6) | P1 | pr-open | `fix/parity-nonempty-sr-frame-newline` | #58 | -| G47 | box-layout-border | Per-edge border toggles (borderTop/Bottom/Left/Right) reserve yoga border space even with no borderStyle set (only on reactive patch) — sweep-6 LOW | P3 | todo | — | — | -| G48 | text-wrap-transform | Measure path omits sanitizeAnsi that Ink applies in squashTextNodes (diverges only for private-param/intermediate-byte CSI) — sweep-6 LOW | P3 | todo | — | — | -| G49 | stdout-stderr-stdin-size-cursor | restoreLastOutput() uses `??` (nullish) instead of Ink's ` | | ` (falsy) fallback, dropping the restored trailing newline before the first frame — sweep-6 LOW | P3 | todo | — | — | -| G50 | app-exit-instances-animation-sr | useApp's `waitUntilRenderFlush()` not reachable from inside the render tree (intentional minimal surface — cf. allowlist) — sweep-6 LOW | P3 | todo | — | — | -| G51 | app-exit-instances-animation-sr | Screen-reader text squash does not sanitize ANSI (transform-injected control sequences leak) — sweep-6 LOW (same family as the squashTextContent note) | P3 | todo | — | — | +| id | area | summary | priority | status | branch | PR | +| --- | ------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ------- | ---------------------------------------------- | --- | +| G01 | static-newline-spacer | Static keeps every already-written item permanently mounted instead of unmounting it | P1 | merged | `fix/parity-static-unmount` | #34 | +| G02 | app-exit-instances-animation-sr | useAnimation does not coalesce ticks within the render-throttle window — delta does not 'account for throttled renders' | P1 | merged | `fix/parity-useanimation-throttle` | #35 | +| G03 | render-lifecycle-reconciler | Live screen-reader render path is missing; commit() always paints the visual grid | P1 | merged | `fix/parity-sr-render` | #36 | +| G04 | box-layout-border | Border edges incorrectly inherit the Box backgroundColor | P2 | merged | `fix/parity-border-bg` | #30 | +| G05 | box-layout-border | Borders skipped when content area is 1 cell tall or wide (w<2 / h<2 guard) | P2 | merged | `fix/parity-border-1cell` | #37 | +| G06 | text-wrap-transform | Nested / transform fn receives hardcoded index 0 instead of childNode index | P2 | refuted | — | — | +| G07 | input-keypress-kitty-paste | Kitty-protocol Ctrl+C triggers app exit in vue-tui but only suppresses the handler in Ink | P2 | kept | — (see ink-parity.md) | — | +| G08 | focus | useFocus does not react to changes in the id prop | P2 | merged | `fix/parity-usefocus-id` | #31 | +| G09 | stdout-stderr-stdin-size-cursor | External stdout/stderr writes are not wrapped in synchronized-update (BSU/ESU) markers | P2 | merged | `fix/parity-external-bsu` | #39 | +| G10 | stdout-stderr-stdin-size-cursor | setRawMode silently no-ops in unsupported environments instead of throwing a descriptive error | P2 | merged | `fix/parity-setrawmode-throw` | #40 | +| G11 | render-lifecycle-reconciler | Resize handler does not clear+reset on terminal-width decrease | P2 | merged | `fix/parity-resize-clear` | #41 | +| G12 | render-lifecycle-reconciler | Renderer frame width/rows lack terminal-size fallback (only ?? defaults) | P2 | merged | `fix/parity-renderer-size` | #33 | +| G13 | box-layout-border | Custom border style objects (BoxStyle) not supported | P3 | merged | `fix/parity-custom-border` | #42 | +| G14 | app-exit-instances-animation-sr | No per-stdout instance reuse/guard — two concurrent renderers can compete for the same stdout | P3 | merged | `fix/parity-instance-reuse` | #43 | +| G15 | box-layout-border | Vertical border sides not shifted up when borderTop=false (Ink offsetY) — left/right rails mispositioned | P2 | merged | `fix/parity-border-1cell` | #37 | +| G16 | box-layout-border | Per-edge borderDimColor=false cannot override general borderDimColor (`\|\| dimAll` vs Ink's `??`) | P3 | merged | `fix/parity-border-dim` | #44 | +| G17 | render-lifecycle-reconciler | Screen-reader live-path edges: still grid-painted (Ink linearizes, skipStaticElements:false) + empty SR frame gets a trailing newline (Ink writes wrapped output directly) | P3 | merged | `fix/parity-sr-edges` | #45 | +| G18 | render-lifecycle-reconciler | No signal-based teardown — terminal corrupted on SIGINT/SIGTERM/SIGHUP (Ink signal-exit at mount) | P1 | merged | `fix/parity-signal-teardown` | #47 | +| G19 | box-layout-border | Dynamic removal of most yoga style props does not reset to default (stale layout) | P2 | merged | `fix/parity-yoga-reset` | #48 | +| G20 | stdout-stderr-stdin-size-cursor | writeToStdout/writeToStderr lack an isUnmounted/teardown guard (post-teardown writes corrupt terminal) | P2 | merged | `fix/parity-write-after-unmount` | #49 | +| G21 | text-wrap-transform | Nested in gets hardcoded index 0 vs child sibling position (squash path) | P3 | merged | `fix/parity-transform-index` | #50 | +| G22 | app-exit-instances-animation-sr | SR role dedup inherits grandparent role; Ink dedups only vs immediate parent | P3 | merged | `fix/parity-sr-role-dedup` | #51 | +| G23 | app-exit-instances-animation-sr | under SR-joins children with newline; Ink concatenates | P3 | pr-open | `fix/parity-sr-transform-concat` | #52 | +| G24 | render-lifecycle-reconciler | Renders ALL nodes; Ink renders only the most-recent staticNode (multi-Static support — kept ) | P3 | kept | — (see ink-parity.md) | — | +| G25 | text-wrap-transform | truncate wrap keeps multi-line when each line fits but whole string overflows (Ink collapses via cliTruncate on whole string) | P3 | todo | — | — | +| G26 | input-keypress-kitty-paste | useInput suppresses input text on kitty key-RELEASE events; Ink delivers the char on release (REVERSES the sweep-1 refutation) | P3 | todo | — | — | +| G27 | stdout-stderr-stdin-size-cursor | releaseRawMode defers input detach + parser reset; Ink stops input synchronously (clearInputState) | P3 | todo | — | — | +| G28 | stdout-stderr-stdin-size-cursor | useStdin() leaks internal members; Ink narrows the public return to PublicProps {stdin,setRawMode,isRawModeSupported} | P3 | todo | — | — | +| G29 | render-lifecycle-reconciler | useCursor()/setCursorPosition never applied during normal render commits (only after console writes) | P3 | todo | — | — | +| G30 | app-exit-instances-animation-sr | SR: nested inside a box-level drops the INNER transform fn (refines G23) | P3 | todo | — | — | +| G31 | app-exit-instances-animation-sr | useAnimation `interval` option is not reactive; Ink re-subscribes+resets when interval changes (cf. G08 id-reactivity) | P3 | todo | — | — | +| G32 | text-wrap-transform | nested directly inside another (in ) is silently DROPPED — paint+measure lose all content | P1 | merged | `fix/parity-transform-nesting` | #54 | +| G33 | app-exit-instances-animation-sr | exit() resolves last-call-wins; Ink is first-call-wins (REVERSES the sweep-1 exit()-second-wins refutation) | P1 | merged | `fix/parity-exit-first-wins` | #55 | +| G34 | box-layout-border | Box border glyphs + bg fill are subject to ancestor transformers; Ink renders chrome with empty transformer list | P3 | todo | — | — | +| G35 | static-newline-spacer | container's own borderStyle/backgroundColor (non-yoga visual style) not painted | P3 | todo | — | — | +| G36 | focus | useFocus autoFocus prop not reactive (captured once); Ink re-registers on autoFocus change | P3 | todo | — | — | +| G37 | render-lifecycle-reconciler | onRender renderTime metric includes stdout write + static capture, not just paint | P3 | todo | — | — | +| G38 | app-exit-instances-animation-sr | useAnimation/scheduler quantizes interval via Math.round; Ink preserves fractional intervals | P3 | todo | — | — | +| G39 | app-exit-instances-animation-sr | SR linearization: a default `` (no flexDirection prop) joins children with `\n`; Ink defaults flexDirection to `row` so it joins with a space (MEDIUM, found sweep-5) | P1 | merged | `fix/parity-sr-default-box-separator` | #56 | +| G40 | render-lifecycle-reconciler | Output write CLAMPS chars at grid width (paint.ts offsetX>=width break/continue); Ink writes overflow past the grid (unbounded output.ts loop) — sweep-5 LOW | P3 | todo | — | — | +| G41 | render-lifecycle-reconciler | onRender renderTime INCLUDES synchronous stdout-write time; Ink calls onRender before the write — sweep-5 LOW (overlaps G37) | P3 | todo | — | — | +| G42 | render-lifecycle-reconciler | Debug mode does NOT re-emit accumulated `` history each frame — sweep-5 LOW | P3 | todo | — | — | +| G43 | render-lifecycle-reconciler | Non-interactive unmount SKIPS the trailing newline when the final frame is empty — sweep-5 LOW | P3 | todo | — | — | +| G44 | static-newline-spacer | `` layout props (flexDirection/padding/justify/align/gap/margin/width) dropped from isolated paint — only column-default + visual border/color survive (MEDIUM, sweep-6) | P1 | pr-open | `fix/parity-static-layout-style` | #59 | +| G45 | focus | Programmatic focusNext()/focusPrevious() are no-ops while focus is disabled; Ink keeps them live (guard belongs only in Tab listener) (MEDIUM, sweep-6) | P1 | pr-open | `fix/parity-programmatic-focus-while-disabled` | #57 | +| G46 | render-lifecycle-reconciler | Non-empty multi-line screen-reader frames append a spurious trailing `\n` + off-by-one erase that Ink does not (G17 only handled the empty case) (MEDIUM, sweep-6) | P1 | pr-open | `fix/parity-nonempty-sr-frame-newline` | #58 | +| G47 | box-layout-border | Per-edge border toggles (borderTop/Bottom/Left/Right) reserve yoga border space even with no borderStyle set (only on reactive patch) — sweep-6 LOW | P3 | todo | — | — | +| G48 | text-wrap-transform | Measure path omits sanitizeAnsi that Ink applies in squashTextNodes (diverges only for private-param/intermediate-byte CSI) — sweep-6 LOW | P3 | todo | — | — | +| G49 | stdout-stderr-stdin-size-cursor | restoreLastOutput() uses nullish-coalescing instead of Ink's falsy-OR fallback, dropping the restored trailing newline before the first frame — sweep-6 LOW | P3 | todo | — | — | +| G50 | app-exit-instances-animation-sr | useApp's `waitUntilRenderFlush()` not reachable from inside the render tree (intentional minimal surface — cf. allowlist) — sweep-6 LOW | P3 | todo | — | — | +| G51 | app-exit-instances-animation-sr | Screen-reader text squash does not sanitize ANSI (transform-injected control sequences leak) — sweep-6 LOW (same family as the squashTextContent note) | P3 | todo | — | — | +| G52 | text-wrap-transform | `` `index` arg shifts when null/false/v-if comment-node siblings precede it inside `` — Vue comment nodes occupy positional slots; Ink (React) skips null children (MEDIUM, sweep-7; G21's positional-index fix introduced the precondition) | P1 | pr-open | `fix/parity-transform-index-comment-skip` | #60 | ## Gap details diff --git a/packages/runtime-tests/integration/accessibility/screen-reader.test.tsx b/packages/runtime-tests/integration/accessibility/screen-reader.test.tsx index 9bf817c..9ea5354 100644 --- a/packages/runtime-tests/integration/accessibility/screen-reader.test.tsx +++ b/packages/runtime-tests/integration/accessibility/screen-reader.test.tsx @@ -438,6 +438,25 @@ describe("Transform accessibility", () => { expect(output).toBe("ab"); }); + // G52: Vue materializes a null/v-if/false render as a COMMENT host node that + // occupies a positional slot. React never produces a childNode for such + // children (Ink squash-text-nodes.ts:13 never advances index past them), so + // the SR squash path must skip comment nodes when indexing the transform — + // staying in lockstep with paint and measurement. + test("G52: null sibling does not shift nested index in screen-reader mode", () => { + const output = renderToString( + defineComponent(() => () => ( + + a{null} + `${s}[${i}]`}>b + + )), + { isScreenReaderEnabled: true }, + ); + // The null produces a comment that must NOT take a slot: a=0, Transform=1. + expect(output).toBe("ab[1]"); + }); + test("renders children normally when screen reader is disabled", () => { const output = renderToString( defineComponent(() => () => ( diff --git a/packages/runtime-tests/integration/components/transform.test.tsx b/packages/runtime-tests/integration/components/transform.test.tsx index 5e5c2ce..8c2d705 100644 --- a/packages/runtime-tests/integration/components/transform.test.tsx +++ b/packages/runtime-tests/integration/components/transform.test.tsx @@ -213,6 +213,142 @@ test("nested -in- reserves correct width (measurement)", a expect(lastFrame()).toBe("[OO]|"); }); +// G52: Vue materializes a `null`/`false`/`v-if` render as a COMMENT host node +// that occupies a positional slot in `node.children`. React never produces a +// childNode for such children, so Ink's squash loop (squash-text-nodes.ts:13) +// never advances `index` past them. The transform index must therefore advance +// only for children React would have rendered — comment nodes must be skipped. +// Reproduces "A{null}B" → "A1:B" (NOT "A2:B"). +test("G52: null sibling does not shift nested index (paint)", async () => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + A{null} + `${i}:${s}`}>B + + )), + { columns: 100 }, + ); + // "A" = index 0, the null produces a comment that must NOT take a slot, so the + // Transform stays at index 1. + expect(lastFrame()).toBe("A1:B"); +}); + +test("G52 control: no null sibling — still gets index 1 (paint)", async () => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + A `${i}:${s}`}>B + + )), + { columns: 100 }, + ); + // Without the null sibling the Transform is the 2nd child → index 1. Pairing + // this with the case above proves the null is what (wrongly) shifts the index. + expect(lastFrame()).toBe("A1:B"); +}); + +test("G52: multiple null siblings don't shift index (paint)", async () => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + {null}A{null} + {false} + `${i}:${s}`}>B + + )), + { columns: 100 }, + ); + // Three comment nodes (one before A, two after) must all be skipped: A=0, + // Transform=1. + expect(lastFrame()).toBe("A1:B"); +}); + +test("G52: null sibling does not shift measured width (measurement)", async () => { + // If measurement counted the comment slot the Transform index would differ + // between paint and measure, desyncing reserved width. A trailing sibling + // marker pins the measured width: "A1:B" is 4 cols, so "|" must land at col 4. + const { lastFrame } = await render( + defineComponent(() => () => ( + + + A{null} + `${i}:${s}`}>B + + | + + )), + { columns: 100 }, + ); + expect(lastFrame()).toBe("A1:B|"); +}); + +// G52 (recursive twin): the comment-skip must also apply to the RECURSIVE +// grandchild loop that recurses transform-in-transform (G32's domain). A +// `{null}`/comment inside an OUTER must NOT shift an INNER +// 's index. Ink iterates the outer transform's real childNodes +// (squash-text-nodes.ts:13); React produces no node for `{null}`, so the inner +// transform stays at index 0. Reproduces +// "A{null}B" so the +// inner transform sees index 0 (output "0:B", NOT "1:B"). +test("G52 recursive: null inside outer does not shift inner index (paint)", async () => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + A + `O${s}`}> + {null} + `${i}:${s}`}>B + + + )), + { columns: 100 }, + ); + // Inside the outer transform: the {null} comment must not take a slot, so the + // inner transform stays at index 0 → "0:B", wrapped by outer → "O0:B". + expect(lastFrame()).toBe("AO0:B"); +}); + +test("G52 recursive control: no null inside outer — inner still index 0 (paint)", async () => { + const { lastFrame } = await render( + defineComponent(() => () => ( + + A + `O${s}`}> + `${i}:${s}`}>B + + + )), + { columns: 100 }, + ); + // Sole child of the outer transform → index 0. Pairs with the case above to + // prove the null is what (wrongly) shifts the recursive index. + expect(lastFrame()).toBe("AO0:B"); +}); + +test("G52 recursive: null inside outer does not shift measured width (measurement)", async () => { + // If measurement counted the comment slot, the inner transform index would + // differ between paint and measure, desyncing reserved width. A trailing + // sibling marker pins the measured width: "AO0:B" is 5 cols, so "|" lands at + // col 5. + const { lastFrame } = await render( + defineComponent(() => () => ( + + + A + `O${s}`}> + {null} + `${i}:${s}`}>B + + + | + + )), + { columns: 100 }, + ); + expect(lastFrame()).toBe("AO0:B|"); +}); + test("transform with multiple lines", async () => { const { lastFrame } = await render( defineComponent(() => () => ( diff --git a/packages/runtime/src/host/text-measure.ts b/packages/runtime/src/host/text-measure.ts index 487cfa6..7602cfc 100644 --- a/packages/runtime/src/host/text-measure.ts +++ b/packages/runtime/src/host/text-measure.ts @@ -7,15 +7,20 @@ import type { TextProps, TuiNode, TuiText, TuiVirtualText } from "./nodes.ts"; export function flattenLeaves(node: TuiText | TuiVirtualText): string { if (!node.children || node.children.length === 0) return ""; let out = ""; - // `index` is the child's POSITIONAL index among ALL siblings — the plain loop - // counter over node.children, matching Ink squash-text-nodes.ts:13,38 (index - // is the loop position over node.childNodes). Must use the SAME index basis as - // paint.ts renderTextWithInlineStyles so measurement and paint agree on what a - // nested receives as its second argument. - node.children.forEach((child, index) => { - out += squashTransformChild(child, index); - // Skip comments inserted by Vue for null/undefined renders - }); + // `transformIndex` advances only for children React would have produced as DOM + // childNodes — matching Ink squash-text-nodes.ts:13 (the loop position over + // node.childNodes). Vue materializes null/v-if/false renders as COMMENT host + // nodes that occupy a positional slot in node.children, but React skips null + // children, so comments must NOT advance the index. This is the measurement + // twin of paint.ts renderTextWithInlineStyles and MUST use the SAME index + // basis so a nested receives the same second argument at measure + // and paint time — keeping reserved width in sync (G52). Real-sibling + // positional indexing (G21) is preserved. + let transformIndex = 0; + for (const child of node.children) { + out += squashTransformChild(child, transformIndex); + if (child.type !== "comment") transformIndex++; + } return out; } @@ -37,9 +42,17 @@ function squashTransformChild(child: TuiNode, index: number): string { } if (child.type === "transform") { let innerText = ""; - child.children.forEach((grandchild, grandIndex) => { + // Recursive twin of the G52 fix in flattenLeaves: a grandchild's positional + // index must skip Vue comment nodes (null/v-if/false renders) so a `{null}` + // inside this OUTER does not shift an INNER 's index — + // and so measure and paint agree on every nested transform's second argument + // (keeping reserved width in sync). Advancing only for real children preserves + // G32's transform-in-transform recursion to any depth. + let grandIndex = 0; + for (const grandchild of child.children) { innerText += squashTransformChild(grandchild, grandIndex); - }); + if (grandchild.type !== "comment") grandIndex++; + } if (innerText.length > 0 && child.transform) innerText = child.transform(innerText, index); return innerText; } diff --git a/packages/runtime/src/paint/paint.ts b/packages/runtime/src/paint/paint.ts index da2203c..ede45bf 100644 --- a/packages/runtime/src/paint/paint.ts +++ b/packages/runtime/src/paint/paint.ts @@ -287,15 +287,23 @@ function renderTextWithInlineStyles(node: TuiText | TuiVirtualText, acc: TextPro const defined = Object.fromEntries(Object.entries(node.props).filter(([, v]) => v !== undefined)); const merged: TextProps = { ...acc, ...defined }; let out = ""; - // `index` is the child's POSITIONAL index among ALL siblings (text-leaves, - // virtual-text, transforms, comments alike) — it is the plain loop counter, - // matching Ink squash-text-nodes.ts:13,38 where `internal_transform(text, - // index)` receives the loop index over `node.childNodes`. A nested - // that is the Nth child therefore gets `index = N`, not a hardcoded 0. - node.children.forEach((child, index) => { - out += squashTransformChild(child, index, merged); - // Skip comments inserted by Vue for null/undefined renders - }); + // `transformIndex` is the child's POSITIONAL index among the siblings React + // would have produced as DOM childNodes — matching Ink squash-text-nodes.ts:13 + // where `internal_transform(text, index)` receives the loop index over + // `node.childNodes`. In React a `null`/`undefined`/`false` child produces NO + // childNode, so it never advances `index`; Vue, by contrast, materializes + // those renders as COMMENT host nodes that DO occupy a positional slot in + // `node.children`. We therefore advance `transformIndex` only for real + // children (skipping comments), so a nested preceded by a `{null}` + // sibling still gets index 1 (not 2) — Ink parity (G52). A real + // among real siblings still gets its correct positional index (G21). + let transformIndex = 0; + for (const child of node.children) { + out += squashTransformChild(child, transformIndex, merged); + // Comments (Vue's null/v-if/false renders) contribute "" and, like React's + // absent childNodes, must NOT advance the transform index. + if (child.type !== "comment") transformIndex++; + } return sanitizeAnsi(out); } @@ -317,11 +325,18 @@ function squashTransformChild(child: TuiNode, index: number, merged: TextProps): } if (child.type === "transform") { let innerText = ""; - child.children.forEach((grandchild, grandIndex) => { - // A grandchild may itself be a (or text/virtual-text/text) — - // recurse with the SAME logic so nesting works to any depth. + // Recursive twin of the G52 fix in renderTextWithInlineStyles: a grandchild's + // positional index must skip Vue comment nodes (null/v-if/false renders), + // which React would not have produced as childNodes, so a `{null}` inside this + // OUTER does not shift an INNER 's index. Advancing the + // counter only for real children preserves G32's transform-in-transform + // recursion (each grandchild may itself be a recursed to any depth) + // while keeping the index basis identical to the top-level loop. + let grandIndex = 0; + for (const grandchild of child.children) { innerText += squashTransformChild(grandchild, grandIndex, merged); - }); + if (grandchild.type !== "comment") grandIndex++; + } if (innerText.length > 0 && child.transform) { innerText = child.transform(innerText, index); } diff --git a/packages/runtime/src/paint/screen-reader.ts b/packages/runtime/src/paint/screen-reader.ts index 8d1d4bd..1ba8b11 100644 --- a/packages/runtime/src/paint/screen-reader.ts +++ b/packages/runtime/src/paint/screen-reader.ts @@ -7,11 +7,16 @@ import type { TuiNode, TuiText, TuiVirtualText, TuiBox } from "../host/nodes.ts" */ function squashTextContent(node: TuiText | TuiVirtualText): string { let text = ""; - // Use forEach so `index` is the child's POSITIONAL index among ALL siblings — - // matching paint.ts renderTextWithInlineStyles and Ink squash-text-nodes.ts:13,38 - // (index is the plain loop counter over node.childNodes). A nested - // must receive its sibling position, not a hardcoded 0. - node.children.forEach((child, index) => { + // `index` advances only for children React would have produced as DOM + // childNodes — matching paint.ts renderTextWithInlineStyles and Ink + // squash-text-nodes.ts:13 (the loop position over node.childNodes). Vue + // materializes null/v-if/false renders as COMMENT host nodes that occupy a + // positional slot in node.children, but React skips null children, so comments + // must NOT advance the index. Staying in lockstep with paint/measure keeps the + // second argument identical across all three squash paths (G52). + // A real nested still receives its sibling position (G21). + let index = 0; + for (const child of node.children) { if (child.type === "text-leaf") { text += child.value; } else if (child.type === "virtual-text") { @@ -31,8 +36,10 @@ function squashTextContent(node: TuiText | TuiVirtualText): string { } text += innerText; } - // Skip comments - }); + // Comments (Vue's null/v-if/false renders) contribute nothing and, like + // React's absent childNodes, must NOT advance the transform index. + if (child.type !== "comment") index++; + } return text; }