From e4f181f888889d62a5e71f7850975b3bd43a370d Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Mon, 15 Jun 2026 01:25:23 +0800 Subject: [PATCH] fix(runtime): measureElement coerces a non-finite (pre-layout) dimension to 0, not NaN (#191) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit yoga's getComputedWidth()/getComputedHeight() return NaN for a node not yet through a layout pass, and `?? 0` does NOT catch NaN (NaN ?? 0 === NaN), so a pre-layout / mis-timed measureElement() read returned { width: NaN, height: NaN } — poisoning user layout math (terminalWidth - measured.width → NaN → a NaN width prop). Coerce non-finite computed dims to 0 (Number.isFinite(v) ? v : 0). 0 is a safe sentinel ("not yet computed"), not the box's true size — the correct usage is to read AFTER layout (the JSDoc already steers callers to defer via nextTick). It's chosen because it is Ink's clear intent (`?? 0`) and matches the DOM precedent (getBoundingClientRect on display:none / img.naturalWidth pre-load return 0, not NaN). Deliberate, low-risk robustness divergence from Ink v7.0.4's NaN-leaking `?? 0`; recorded in ink-divergences.md. Co-authored-by: Claude Opus 4.8 (1M context) --- .agents/docs/ink-divergences.md | 18 +++++++++++ .../composables/use-box-metrics.test.tsx | 32 ++++++++++++++++--- .../runtime/src/composables/useBoxMetrics.ts | 21 ++++++++++-- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/.agents/docs/ink-divergences.md b/.agents/docs/ink-divergences.md index 0e6ab4e..321b8d2 100644 --- a/.agents/docs/ink-divergences.md +++ b/.agents/docs/ink-divergences.md @@ -624,6 +624,24 @@ different runtime behavior, ownership rule, or out-of-contract handling. width and height. The default remains border-box-like, matching Ink's current public sizing model. +### `measureElement` coerces a non-finite (pre-layout) dimension to `0`, not `NaN` + +- **Ink:** `measureElement` returns `{ width: node.yogaNode.getComputedWidth() ?? 0, height: ... ?? 0 }`. + Before the first layout pass yoga's `getComputedWidth()`/`getComputedHeight()` return **`NaN`**, and + `?? 0` does **not** catch `NaN` (`NaN ?? 0 === NaN`), so a pre-layout / mis-timed read returns + `{ width: NaN, height: NaN }`. +- **vue-tui:** coerces a non-finite computed dimension to `0` (`Number.isFinite(v) ? v : 0`), so the same + pre-layout read returns a finite `{ width: 0, height: 0 }`. (The detached-ref case already returns + `{0,0}`; this extends the same safe fallback to the attached-but-not-yet-laid-out case.) +- **Why:** `0` is a **safe sentinel** meaning "not yet computed", not the box's true size — the genuinely + correct usage is to read _after_ layout (the JSDoc already steers callers to defer via `nextTick`). It is + chosen because it is Ink's clear intent (`?? 0`) and it matches the **DOM precedent**: + `getBoundingClientRect()` on a `display:none` element and `img.naturalWidth` before load both return `0`, + not `NaN`. Leaking `NaN` instead poisons user layout math (`terminalWidth - measured.width` → `NaN` → a + `NaN` width prop), so a mis-timed read degrades gracefully rather than corrupting the layout. A low-risk + robustness divergence from Ink's `NaN`-leaking `?? 0`. KEEP. [VOUCHED @hyf0] Tests: "returns finite + { width: 0, height: 0 } for a node not yet laid out (no NaN leak)" in `use-box-metrics.test.tsx`. + ### Out-of-type style values are forwarded, not defensively coerced - **Ink:** several flex/align setters coerce an invalid runtime value to a default: diff --git a/packages/runtime-tests/integration/composables/use-box-metrics.test.tsx b/packages/runtime-tests/integration/composables/use-box-metrics.test.tsx index 6b3a738..59984a7 100644 --- a/packages/runtime-tests/integration/composables/use-box-metrics.test.tsx +++ b/packages/runtime-tests/integration/composables/use-box-metrics.test.tsx @@ -9,6 +9,9 @@ import { useWindowSize, createApp, } from "@vue-tui/runtime"; +// Internal host primitives — build a node + attach yoga WITHOUT a layout pass so +// the pre-layout read below is deterministic (no render/commit timing involved). +import { createBox, attachYoga } from "@vue-tui/runtime/internal"; import { makeFakeStdin, makeFakeWritable } from "../lifecycle/test-streams.ts"; describe("useBoxMetrics", () => { @@ -204,6 +207,24 @@ describe("measureElement", () => { expect(measureElement({ yoga: fakeYoga })).toEqual({ width: 42, height: 17 }); }); + // A node whose yoga node exists but has NOT been through a layout pass: + // yoga.getComputedWidth()/Height() return NaN, and the old `?? 0` did NOT + // catch NaN (NaN ?? 0 === NaN). The pre-layout / mis-timed read must degrade + // to a finite {0,0} (a safe sentinel) instead of leaking NaN into user layout + // math (e.g. terminalWidth - measured.width → NaN → a NaN width prop). + test("returns finite { width: 0, height: 0 } for a node not yet laid out (no NaN leak)", () => { + const box = createBox(); + attachYoga(box); // attaches a yoga node but does NOT calculateLayout + // Guard the premise: pre-layout yoga genuinely reports NaN (not 0), so this + // test exercises the real coercion, not a no-op. + expect(Number.isNaN(box.yoga.getComputedWidth())).toBe(true); + + const result = measureElement(box); + expect(Number.isFinite(result.width)).toBe(true); + expect(Number.isFinite(result.height)).toBe(true); + expect(result).toEqual({ width: 0, height: 0 }); + }); + test("resolves through $el for component instances", () => { const fakeYoga = { getComputedWidth: () => 30, @@ -354,10 +375,13 @@ describe("measureElement", () => { // The deferred (documented-correct) read returns the real terminal width. expect(deferredWidth.value).toBe(80); // The bare read is the uncomputed pre-layout value. On this first-render path - // yoga's getComputedWidth() is NaN (and measureElement's `?? 0` does not - // coalesce NaN), so pin exactly that — a plain `!== 80` would also pass if the - // watcher never ran or returned 0, which would not prove the bare path is stale. - expect(Number.isNaN(bareWidth.value)).toBe(true); + // yoga's getComputedWidth() is NaN; measureElement coerces non-finite dims to + // the safe sentinel 0, so the bare read is a FINITE 0 — not NaN and not the + // real 80. Pin all three facts: it ran (not the -1 seed), it is finite (the + // NaN-leak fix), and it is stale (0 ≠ 80, proving the bare path is pre-layout). + expect(bareWidth.value).not.toBe(-1); + expect(Number.isFinite(bareWidth.value)).toBe(true); + expect(bareWidth.value).toBe(0); }); test("measureElement works when render is throttled", async () => { diff --git a/packages/runtime/src/composables/useBoxMetrics.ts b/packages/runtime/src/composables/useBoxMetrics.ts index 5939dd1..65e2c77 100644 --- a/packages/runtime/src/composables/useBoxMetrics.ts +++ b/packages/runtime/src/composables/useBoxMetrics.ts @@ -117,7 +117,12 @@ function findRootNode(node: TuiNode | null): TuiRoot | null { /** * Imperative function that reads yoga computed dimensions from a TUI node. * - * Returns `{ width: 0, height: 0 }` when the ref is not attached to an element. + * Returns `{ width: 0, height: 0 }` when the ref is not attached to an element, + * and also as a safe fallback when the node's computed dimension is non-finite — + * i.e. it has a yoga node but has not been through a layout pass yet (yoga + * reports `NaN` pre-layout). `0` here is a sentinel meaning "not yet computed", + * NOT the box's true size; the correct pattern is to read *after* layout (see the + * timing note below). * * Timing matters: layout is computed inside the commit scheduler's post-flush * callback, which can run *after* your own `watchPostEffect`/render-time code in @@ -145,9 +150,19 @@ function findRootNode(node: TuiNode | null): TuiRoot | null { export function measureElement(node: unknown): { width: number; height: number } { const tuiNode = resolveYogaNode(node); if (!tuiNode) return { width: 0, height: 0 }; + // yoga returns NaN for a node not yet through a layout pass, and `?? 0` does + // NOT catch NaN (NaN ?? 0 === NaN). Coerce non-finite dims to 0 so a pre-layout + // / mis-timed read degrades to {0,0} instead of leaking NaN into user layout + // math. 0 is a SAFE SENTINEL, not the true size (which is "not yet computed"); + // the correct usage is to read AFTER layout (see the timing note in the JSDoc). + // Matches the DOM precedent (getBoundingClientRect on display:none / + // img.naturalWidth pre-load → 0, not NaN). Deliberate divergence from Ink + // v7.0.4's NaN-leaking `?? 0`. + const width = tuiNode.yoga.getComputedWidth(); + const height = tuiNode.yoga.getComputedHeight(); return { - width: tuiNode.yoga.getComputedWidth() ?? 0, - height: tuiNode.yoga.getComputedHeight() ?? 0, + width: Number.isFinite(width) ? width : 0, + height: Number.isFinite(height) ? height : 0, }; }