fix(runtime): skip comment nodes when indexing <Transform> children (Ink parity, G52) (#60)
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 <Text>A{null}<Transform>(s,i)=>`${i}:${s}`>B</Transform>
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 <Transform> index in screen-reader mode", () => {
|
||||
const output = renderToString(
|
||||
defineComponent(() => () => (
|
||||
<Text>
|
||||
a{null}
|
||||
<Transform transform={(s: string, i: number) => `${s}[${i}]`}>b</Transform>
|
||||
</Text>
|
||||
)),
|
||||
{ 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(() => () => (
|
||||
|
||||
@@ -213,6 +213,142 @@ test("nested <Transform>-in-<Transform> reserves correct width (measurement)", a
|
||||
expect(lastFrame()).toBe("[O<IxI>O]|");
|
||||
});
|
||||
|
||||
// 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}<Transform>B</Transform>" → "A1:B" (NOT "A2:B").
|
||||
test("G52: null sibling does not shift nested <Transform> index (paint)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Text>
|
||||
A{null}
|
||||
<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Text>
|
||||
)),
|
||||
{ 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 — <Transform> still gets index 1 (paint)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Text>
|
||||
A<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Text>
|
||||
)),
|
||||
{ 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(() => () => (
|
||||
<Text>
|
||||
{null}A{null}
|
||||
{false}
|
||||
<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Text>
|
||||
)),
|
||||
{ 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(() => () => (
|
||||
<Box flexDirection="row">
|
||||
<Text>
|
||||
A{null}
|
||||
<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Text>
|
||||
<Text>|</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ 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 <Transform> must NOT shift an INNER
|
||||
// <Transform>'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<Transform outer>{null}<Transform inner>B</Transform></Transform>" so the
|
||||
// inner transform sees index 0 (output "0:B", NOT "1:B").
|
||||
test("G52 recursive: null inside outer <Transform> does not shift inner index (paint)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Text>
|
||||
A
|
||||
<Transform transform={(s: string) => `O${s}`}>
|
||||
{null}
|
||||
<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Transform>
|
||||
</Text>
|
||||
)),
|
||||
{ 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 <Transform> — inner still index 0 (paint)", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
<Text>
|
||||
A
|
||||
<Transform transform={(s: string) => `O${s}`}>
|
||||
<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Transform>
|
||||
</Text>
|
||||
)),
|
||||
{ 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 <Transform> 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(() => () => (
|
||||
<Box flexDirection="row">
|
||||
<Text>
|
||||
A
|
||||
<Transform transform={(s: string) => `O${s}`}>
|
||||
{null}
|
||||
<Transform transform={(s: string, i: number) => `${i}:${s}`}>B</Transform>
|
||||
</Transform>
|
||||
</Text>
|
||||
<Text>|</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
expect(lastFrame()).toBe("AO0:B|");
|
||||
});
|
||||
|
||||
test("transform with multiple lines", async () => {
|
||||
const { lastFrame } = await render(
|
||||
defineComponent(() => () => (
|
||||
|
||||
@@ -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 <Transform> 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 <Transform> 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 <Transform> does not shift an INNER <Transform>'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;
|
||||
}
|
||||
|
||||
@@ -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 <Transform>
|
||||
// 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 <Transform> preceded by a `{null}`
|
||||
// sibling still gets index 1 (not 2) — Ink parity (G52). A real <Transform>
|
||||
// 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 <Transform> (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 <Transform> does not shift an INNER <Transform>'s index. Advancing the
|
||||
// counter only for real children preserves G32's transform-in-transform
|
||||
// recursion (each grandchild may itself be a <Transform> 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);
|
||||
}
|
||||
|
||||
@@ -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 <Transform>
|
||||
// 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
|
||||
// <Transform> second argument identical across all three squash paths (G52).
|
||||
// A real nested <Transform> 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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user