fix(runtime): validate setElementText context before clearing children (#179)
setElementText(el, text) removed ALL existing children first, then inserted a single text-leaf. When `el` is a non-text container (tui-box / tui-static / root) and `text` is non-empty, the inserted leaf trips insert()'s text-context guard and throws AFTER the removal loop has already run — leaving the node half-cleared (original children gone, nothing inserted). Validate the target context BEFORE the destructive remove so a rejected insert never leaves the node half-cleared. Extract the text-leaf rejection check into a shared rejectsTextLeaf() helper used by BOTH setElementText()'s new pre-check and insert()'s existing guard, so the condition and error message cannot drift. Empty-string clears, text on a tui-text / inside-text context, and non-container no-ops all keep their existing behavior. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -162,6 +162,22 @@ function dirtyTextMeasureOwner(parent: TuiNode): void {
|
||||
else if (owner?.type === "tui-text") markTextDirty(owner);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a text-leaf carrying `value` would be REJECTED by the text-context
|
||||
* guard if inserted into `parent`. A bare string must live inside a <Text>
|
||||
* context; an EMPTY text-leaf is exempt (Vue uses empty text-leaves as fragment
|
||||
* anchors / its common clear path). Shared by `insert()` (its existing guard)
|
||||
* and `setElementText()` (its pre-remove validation) so the two cannot drift —
|
||||
* the condition must stay identical in both call sites.
|
||||
*/
|
||||
function rejectsTextLeaf(parent: TuiContainer, value: string): boolean {
|
||||
return (
|
||||
value !== "" &&
|
||||
(parent.type === "tui-box" || parent.type === "root" || parent.type === "tui-static") &&
|
||||
!isInsideTextOrTransformContext(parent)
|
||||
);
|
||||
}
|
||||
|
||||
export function buildNodeOps(options: TtyRendererOptions): RendererOptions<TuiNode, TuiNode> {
|
||||
const { onCommit } = options;
|
||||
|
||||
@@ -228,6 +244,15 @@ export function buildNodeOps(options: TtyRendererOptions): RendererOptions<TuiNo
|
||||
|
||||
function setElementText(el: TuiNode, text: string): void {
|
||||
if (!isContainer(el)) return;
|
||||
// Validate the target context BEFORE the destructive remove below: the
|
||||
// text-leaf we're about to insert would otherwise hit insert()'s
|
||||
// text-context guard and throw AFTER the children are already gone, leaving
|
||||
// the node half-cleared (children removed, nothing inserted). Throw the same
|
||||
// error up-front instead — shares rejectsTextLeaf() with insert() so the
|
||||
// condition + message cannot drift.
|
||||
if (rejectsTextLeaf(el, text)) {
|
||||
throw new Error(`Text string "${text}" must be rendered inside <Text> component`);
|
||||
}
|
||||
// Remove existing children first (copy since remove mutates the array).
|
||||
for (const child of Array.from(el.children)) remove(child);
|
||||
insert(createTextLeaf(text), el, null);
|
||||
@@ -253,14 +278,10 @@ export function buildNodeOps(options: TtyRendererOptions): RendererOptions<TuiNo
|
||||
throw new Error("<Box> can’t be nested inside <Text> component");
|
||||
}
|
||||
|
||||
// Text-leaf nodes must live inside a <Text> context.
|
||||
// Skip empty text-leaves — Vue uses them as fragment anchors.
|
||||
if (
|
||||
child.type === "text-leaf" &&
|
||||
child.value !== "" &&
|
||||
(parentC.type === "tui-box" || parentC.type === "root" || parentC.type === "tui-static") &&
|
||||
!isInsideTextOrTransformContext(parentC)
|
||||
) {
|
||||
// Text-leaf nodes must live inside a <Text> context. The rejection condition
|
||||
// (incl. the empty-text-leaf fragment-anchor exemption) lives in
|
||||
// rejectsTextLeaf() so setElementText()'s pre-remove pre-check stays in sync.
|
||||
if (child.type === "text-leaf" && rejectsTextLeaf(parentC, child.value)) {
|
||||
throw new Error(`Text string "${child.value}" must be rendered inside <Text> component`);
|
||||
}
|
||||
|
||||
|
||||
@@ -44,3 +44,46 @@ test("isContainer rejects text-leaf and accepts box", () => {
|
||||
expect(isContainer(createBox())).toBe(true);
|
||||
expect(isContainer(createTextLeaf("x"))).toBe(false);
|
||||
});
|
||||
|
||||
test("setElementText rejecting a non-text container leaves existing children intact", () => {
|
||||
// Regression: setElementText used to remove ALL children FIRST, then try to
|
||||
// insert the text-leaf — which throws the text-context guard for a non-text
|
||||
// container, leaving the box half-cleared (children gone, nothing inserted).
|
||||
// The context must be validated BEFORE the destructive remove.
|
||||
const ops = buildNodeOps({ onCommit: () => {} });
|
||||
const box = ops.createElement("tui-box") as ReturnType<typeof createBox>;
|
||||
const child = ops.createElement("tui-text");
|
||||
ops.insert(child, box, null);
|
||||
expect(box.children.length).toBe(1);
|
||||
|
||||
expect(() => ops.setElementText(box, "hello")).toThrow(/must be rendered inside <Text>/);
|
||||
// Key assertion: the rejected insert must not have removed the box's children.
|
||||
expect(box.children.length).toBe(1);
|
||||
expect(box.children[0]).toBe(child);
|
||||
});
|
||||
|
||||
test("setElementText with empty string clears a non-text container's children", () => {
|
||||
// Empty text-leaf is exempted from the text-context guard (Vue's clear path),
|
||||
// so this must keep working: children removed, no throw.
|
||||
const ops = buildNodeOps({ onCommit: () => {} });
|
||||
const box = ops.createElement("tui-box") as ReturnType<typeof createBox>;
|
||||
ops.insert(ops.createElement("tui-text"), box, null);
|
||||
ops.insert(ops.createElement("tui-text"), box, null);
|
||||
expect(box.children.length).toBe(2);
|
||||
|
||||
expect(() => ops.setElementText(box, "")).not.toThrow();
|
||||
expect(box.children.length).toBe(1);
|
||||
expect((box.children[0] as ReturnType<typeof createTextLeaf>).value).toBe("");
|
||||
});
|
||||
|
||||
test("setElementText on a tui-text replaces its children with the text", () => {
|
||||
// A text node IS a text context, so text content is valid and must replace
|
||||
// the existing children.
|
||||
const ops = buildNodeOps({ onCommit: () => {} });
|
||||
const text = ops.createElement("tui-text") as ReturnType<typeof createBox>;
|
||||
ops.insert(ops.createText("old"), text, null);
|
||||
|
||||
expect(() => ops.setElementText(text, "hi")).not.toThrow();
|
||||
expect(text.children.length).toBe(1);
|
||||
expect((text.children[0] as ReturnType<typeof createTextLeaf>).value).toBe("hi");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user