From fef7139e0c1db92b3b2fad4dcc84d2c8cf58f171 Mon Sep 17 00:00:00 2001 From: RtlZeroMemory <58250858+RtlZeroMemory@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:29:59 +0400 Subject: [PATCH 1/5] fix(core): harden runtime lifecycle and layer routing --- CHANGELOG.md | 8 + docs/guide/lifecycle-and-updates.md | 9 +- .../src/app/__tests__/onEventHandlers.test.ts | 61 +++++++ .../src/app/__tests__/onFocusChange.test.ts | 31 ++++ .../__tests__/syncFrameAckFastPath.test.ts | 38 +++++ .../src/app/__tests__/terminalProfile.test.ts | 133 ++++++++++++++++ packages/core/src/app/createApp.ts | 150 +++++++++++++----- .../src/app/createApp/topLevelViewError.ts | 3 +- packages/core/src/app/rawRenderer.ts | 7 +- packages/core/src/app/widgetRenderer.ts | 66 ++++---- packages/core/src/debug/describeThrown.ts | 8 + .../__tests__/keybinding.conflicts.test.ts | 10 +- packages/core/src/perf/__tests__/perf.test.ts | 38 +++++ packages/core/src/perf/perf.ts | 8 +- .../__tests__/duplicateIdFatal.test.ts | 26 ++- .../runtime/__tests__/focus.layers.test.ts | 19 +-- .../runtime/__tests__/localStateStore.test.ts | 39 ++++- .../src/runtime/__tests__/widgetMeta.test.ts | 80 +++++++++- packages/core/src/runtime/commit.ts | 86 ++++++++-- packages/core/src/runtime/localState.ts | 18 ++- packages/core/src/runtime/router/layer.ts | 65 +++++--- packages/core/src/runtime/widgetMeta.ts | 124 +++++++++++---- .../widgets/__tests__/layers.golden.test.ts | 4 +- .../src/widgets/__tests__/modal.focus.test.ts | 15 +- .../widgets/__tests__/overlay.edge.test.ts | 4 +- 25 files changed, 853 insertions(+), 197 deletions(-) create mode 100644 packages/core/src/debug/describeThrown.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a49b09..48aebc1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,15 @@ All notable changes to Rezi are documented in this file. The format is based on Keep a Changelog and the project follows Semantic Versioning. ## [Unreleased] +### Bug Fixes + +- **core/composition + hooks**: Composite widgets now use a layout-transparent default wrapper, animation hooks share a frame driver, transition/orchestration hooks stop relying on stringified config signatures, `useAnimatedValue` transition playback preserves progress across pause/resume, `useStagger` restarts on same-length item replacement, and streaming hook reconnect delays clamp away tight-loop reconnects. +- **core/runtime + perf**: Hardened lifecycle start/stop/fatal edges, sync frame follow-up scheduling, focus/layer callback failure handling, focus container metadata/state publication, and perf ring-buffer rollover stats. + +### Documentation +- **docs/guide**: Synced composition, animation, and hook reference docs with the current hook surface, easing presets, callback semantics, viewport availability, and stable parser examples for streaming hooks. +- **docs/lifecycle**: Corrected `onEvent(...)` examples, fatal payload fields, hot-reload state guarantees, and `run()` behavior when signal registration is unavailable. ## [0.1.0-alpha.57] - 2026-03-06 ### Documentation diff --git a/docs/guide/lifecycle-and-updates.md b/docs/guide/lifecycle-and-updates.md index b73dc642..01bd84b7 100644 --- a/docs/guide/lifecycle-and-updates.md +++ b/docs/guide/lifecycle-and-updates.md @@ -29,6 +29,7 @@ Use `run()` for batteries-included lifecycle wiring in Node/Bun apps: - `run()` wraps `start()` - registers `SIGINT` / `SIGTERM` / `SIGHUP` - on signal: `stop()` then `dispose()` best-effort, then exits with code `0` +- if signal handlers cannot be registered in the current runtime, `run()` still starts the app and resolves once startup completes Use `start()` directly when you need manual signal/process control. @@ -60,8 +61,8 @@ Set one mode before calling `start()`. Switching modes while running is rejected Rezi supports development-time view hot swapping without process restarts. - `app.view(fn)` remains the initial setup API (`Created`/`Stopped` states) -- `app.replaceView(fn)` swaps the active widget view while `Running` -- `app.replaceRoutes(routes)` swaps route definitions for route-managed apps while `Running` +- `app.replaceView(fn)` swaps the active widget view while `Running`, and also updates the configured widget view in `Created`/`Stopped` +- `app.replaceRoutes(routes)` swaps route definitions for route-managed apps while `Running`, and also updates the configured route table in `Created`/`Stopped` When a new view function is applied, Rezi keeps: @@ -170,7 +171,7 @@ app.keys({ ```typescript const unsubscribe = app.onEvent((ev) => { - if (ev.kind === "fatal") console.error(ev.code, ev.message); + if (ev.kind === "fatal") console.error(ev.code, ev.detail); }); // later: unsubscribe() ``` @@ -194,7 +195,7 @@ In addition, these actions also flow through `app.onEvent(...)`, which enables cross-cutting middleware, logging, analytics, and undo stacks. ```typescript -app.on("event", (ev) => { +app.onEvent((ev) => { if (ev.kind === "action" && ev.action === "toggle") { console.log(`Checkbox ${ev.id} toggled to ${ev.checked}`); } diff --git a/packages/core/src/app/__tests__/onEventHandlers.test.ts b/packages/core/src/app/__tests__/onEventHandlers.test.ts index 816aeada..fb81274d 100644 --- a/packages/core/src/app/__tests__/onEventHandlers.test.ts +++ b/packages/core/src/app/__tests__/onEventHandlers.test.ts @@ -1,4 +1,5 @@ import { assert, test } from "@rezi-ui/testkit"; +import { ui } from "../../index.js"; import { createApp } from "../createApp.js"; import { encodeZrevBatchV1, flushMicrotasks, makeBackendBatch } from "./helpers.js"; import { StubBackend } from "./stubBackend.js"; @@ -42,3 +43,63 @@ test("onEvent handler ordering + unsubscribe semantics (#80)", async () => { // It is not called for subsequent events. assert.deepEqual(calls, ["A", "B", "C", "A", "C"]); }); + +test("onEvent handler failure aborts the current batch before later events commit", async () => { + const backend = new StubBackend(); + const app = createApp({ backend, initialState: 0 }); + + const rendered: number[] = []; + app.view((state) => { + rendered.push(state); + return ui.text(`count:${String(state)}`); + }); + + let applyResizeUpdates = false; + app.onEvent((ev) => { + if (ev.kind !== "engine") return; + if (ev.event.kind === "text") throw new Error("boom"); + if (applyResizeUpdates && ev.event.kind === "resize") { + app.update((n) => n + 1); + } + }); + + const fatals: string[] = []; + app.onEvent((ev) => { + if (ev.kind === "fatal") fatals.push(ev.detail); + }); + + await app.start(); + backend.pushBatch( + makeBackendBatch({ + bytes: encodeZrevBatchV1({ + events: [{ kind: "resize", timeMs: 1, cols: 80, rows: 24 }], + }), + }), + ); + await flushMicrotasks(10); + + assert.deepEqual(rendered, [0]); + assert.equal(backend.requestedFrames.length, 1); + + backend.resolveNextFrame(); + await flushMicrotasks(10); + + applyResizeUpdates = true; + backend.pushBatch( + makeBackendBatch({ + bytes: encodeZrevBatchV1({ + events: [ + { kind: "text", timeMs: 2, codepoint: 65 }, + { kind: "resize", timeMs: 3, cols: 81, rows: 24 }, + ], + }), + }), + ); + await flushMicrotasks(20); + + assert.equal(fatals.length, 1); + assert.deepEqual(rendered, [0]); + assert.equal(backend.requestedFrames.length, 1); + assert.equal(backend.stopCalls, 1); + assert.equal(backend.disposeCalls, 1); +}); diff --git a/packages/core/src/app/__tests__/onFocusChange.test.ts b/packages/core/src/app/__tests__/onFocusChange.test.ts index 6197d6a5..f87c2a00 100644 --- a/packages/core/src/app/__tests__/onFocusChange.test.ts +++ b/packages/core/src/app/__tests__/onFocusChange.test.ts @@ -84,3 +84,34 @@ test("onFocusChange unsubscribe stops future callbacks", async () => { await settleNextFrame(backend); }); + +test("onFocusChange handler failure faults immediately before focus-render follow-up", async () => { + const backend = new StubBackend(); + const app = createApp({ + backend, + initialState: 0, + }); + + app.view(() => + ui.column({}, [ui.input({ id: "name", value: "" }), ui.button({ id: "save", label: "Save" })]), + ); + + const fatals: string[] = []; + app.onEvent((ev) => { + if (ev.kind === "fatal") fatals.push(ev.detail); + }); + app.onFocusChange(() => { + throw new Error("focus boom"); + }); + + await app.start(); + await pushEvents(backend, [{ kind: "resize", timeMs: 1, cols: 40, rows: 10 }]); + await settleNextFrame(backend); + + await pushEvents(backend, [{ kind: "key", timeMs: 2, key: ZR_KEY_TAB, action: "down" }]); + + assert.equal(fatals.length, 1); + assert.equal(backend.requestedFrames.length, 1); + assert.equal(backend.stopCalls, 1); + assert.equal(backend.disposeCalls, 1); +}); diff --git a/packages/core/src/app/__tests__/syncFrameAckFastPath.test.ts b/packages/core/src/app/__tests__/syncFrameAckFastPath.test.ts index dcc55255..3de46703 100644 --- a/packages/core/src/app/__tests__/syncFrameAckFastPath.test.ts +++ b/packages/core/src/app/__tests__/syncFrameAckFastPath.test.ts @@ -1,4 +1,5 @@ import { assert, test } from "@rezi-ui/testkit"; +import { defineWidget, ui } from "../../index.js"; import { createApp } from "../createApp.js"; import { encodeZrevBatchV1, flushMicrotasks, makeBackendBatch } from "./helpers.js"; import { StubBackend } from "./stubBackend.js"; @@ -51,3 +52,40 @@ test("sync frame-ack marker allows next render without waiting for frameSettled app.dispose(); }); + +test("sync frame-ack still schedules a follow-up frame for effect-driven invalidation", async () => { + const backend = new SyncFrameAckBackend(); + const app = createApp({ + backend, + initialState: 0, + config: { maxFramesInFlight: 1 }, + }); + + const seenCounts: number[] = []; + const Counter = defineWidget<{ key?: string }>((_props, ctx) => { + const [count, setCount] = ctx.useState(0); + seenCounts.push(count); + ctx.useEffect(() => { + if (count === 0) setCount(1); + }, [count]); + return ui.text(`count:${String(count)}`); + }); + + app.view(() => Counter({ key: "counter" })); + await app.start(); + + backend.pushBatch( + makeBackendBatch({ + bytes: encodeZrevBatchV1({ + events: [{ kind: "resize", timeMs: 1, cols: 80, rows: 24 }], + }), + }), + ); + + await flushMicrotasks(20); + + assert.equal(backend.requestedFrames.length, 2); + assert.deepEqual(seenCounts, [0, 1]); + + app.dispose(); +}); diff --git a/packages/core/src/app/__tests__/terminalProfile.test.ts b/packages/core/src/app/__tests__/terminalProfile.test.ts index 3badd054..a30308c5 100644 --- a/packages/core/src/app/__tests__/terminalProfile.test.ts +++ b/packages/core/src/app/__tests__/terminalProfile.test.ts @@ -1,4 +1,5 @@ import { assert, test } from "@rezi-ui/testkit"; +import { ZrUiError } from "../../abi.js"; import type { BackendEventBatch, RuntimeBackend } from "../../backend.js"; import { DEFAULT_TERMINAL_CAPS, type TerminalCaps } from "../../terminalCaps.js"; import { @@ -8,6 +9,12 @@ import { } from "../../terminalProfile.js"; import { createApp } from "../createApp.js"; +async function flushMicrotasks(count = 5): Promise { + for (let i = 0; i < count; i++) { + await Promise.resolve(); + } +} + function createBackend(overrides: Partial = {}): RuntimeBackend { const pendingPoll = new Promise(() => undefined); return { @@ -22,6 +29,17 @@ function createBackend(overrides: Partial = {}): RuntimeBackend }; } +function deferred(): Readonly<{ + promise: Promise; + resolve: (value: T) => void; +}> { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return Object.freeze({ promise, resolve }); +} + test("createApp exposes backend terminal profile after start", async () => { let getCapsCalls = 0; const expected: TerminalProfile = Object.freeze({ @@ -93,3 +111,118 @@ test("createApp falls back to default profile when caps/profile both fail", asyn await app.stop(); app.dispose(); }); + +test("createApp keeps lifecycle locked while terminal profile load is pending", async () => { + const profileDeferred = deferred(); + let startCalls = 0; + const backend = createBackend({ + start: async () => { + startCalls++; + }, + getTerminalProfile: async () => profileDeferred.promise, + }); + const app = createApp({ backend, initialState: {} }); + app.draw(() => {}); + + const startPromise = app.start(); + await flushMicrotasks(); + + assert.throws( + () => app.start(), + (error: unknown) => + error instanceof Error && error.message === "start: lifecycle operation already in flight", + ); + + profileDeferred.resolve(DEFAULT_TERMINAL_PROFILE); + await startPromise; + + assert.equal(startCalls, 1); + await app.stop(); + app.dispose(); +}); + +test("dispose during pending terminal profile load aborts startup and stops backend", async () => { + const profileDeferred = deferred(); + let stopCalls = 0; + let disposeCalls = 0; + const backend = createBackend({ + stop: async () => { + stopCalls++; + }, + dispose: () => { + disposeCalls++; + }, + getTerminalProfile: async () => profileDeferred.promise, + }); + const app = createApp({ backend, initialState: {} }); + app.draw(() => {}); + + const startPromise = app.start(); + await flushMicrotasks(); + + app.dispose(); + profileDeferred.resolve(DEFAULT_TERMINAL_PROFILE); + await startPromise; + await flushMicrotasks(); + + assert.equal(stopCalls, 1); + assert.equal(disposeCalls, 1); +}); + +test("createApp rejects a second start while terminal profile loading is still pending", async () => { + const profile = deferred(); + let startCalls = 0; + const backend = createBackend({ + start: async () => { + startCalls++; + }, + getTerminalProfile: async () => profile.promise, + }); + const app = createApp({ backend, initialState: {} }); + app.draw(() => {}); + + const startPromise = app.start(); + await Promise.resolve(); + + assert.throws( + () => app.start(), + (error: unknown) => error instanceof ZrUiError && error.code === "ZRUI_INVALID_STATE", + ); + assert.equal(startCalls, 1); + + profile.resolve(DEFAULT_TERMINAL_PROFILE); + await startPromise; + await app.stop(); + app.dispose(); +}); + +test("dispose during pending startup prevents late completion from reviving the app", async () => { + const profile = deferred(); + let disposeCalls = 0; + let stopCalls = 0; + const backend = createBackend({ + stop: async () => { + stopCalls++; + }, + getTerminalProfile: async () => profile.promise, + dispose: () => { + disposeCalls++; + }, + }); + const app = createApp({ backend, initialState: {} }); + app.draw(() => {}); + + const startPromise = app.start(); + await Promise.resolve(); + app.dispose(); + profile.resolve(DEFAULT_TERMINAL_PROFILE); + await startPromise; + + assert.equal(stopCalls, 1); + assert.equal(disposeCalls, 1); + assert.deepEqual(app.getTerminalProfile(), DEFAULT_TERMINAL_PROFILE); + assert.throws( + () => app.update((state) => state), + (error: unknown) => error instanceof ZrUiError && error.code === "ZRUI_INVALID_STATE", + ); +}); diff --git a/packages/core/src/app/createApp.ts b/packages/core/src/app/createApp.ts index 0eee7ed5..176787c4 100644 --- a/packages/core/src/app/createApp.ts +++ b/packages/core/src/app/createApp.ts @@ -55,6 +55,7 @@ import { normalizeBreakpointThresholds, } from "../layout/responsive.js"; import type { Rect } from "../layout/types.js"; +import { describeThrown } from "../debug/describeThrown.js"; import { PERF_ENABLED, perfMarkEnd, perfMarkStart, perfNow, perfRecord } from "../perf/perf.js"; import type { EventTimeUnwrapState } from "../protocol/types.js"; import { parseEventBatchV1 } from "../protocol/zrev_v1.js"; @@ -369,11 +370,6 @@ type WorkItem = /** Event handler registration with deactivation flag. */ type HandlerSlot = Readonly<{ fn: EventHandler; active: { value: boolean } }>; -function describeThrown(v: unknown): string { - if (v instanceof Error) return `${v.name}: ${v.message}`; - return String(v); -} - /** * Convert a text codepoint to a key code for keybinding matching. * Letters are normalized to uppercase (A-Z = 65-90). @@ -573,8 +569,11 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl let inEventHandlerDepth = 0; let lifecycleBusy: "start" | "stop" | null = null; + let backendStarted = false; + let lifecycleGeneration = 0; let pollToken = 0; let settleActiveRun: (() => void) | null = null; + let renderRequestQueuedForCurrentTurn = false; let userCommitScheduled = false; @@ -595,7 +594,14 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } if (!schedule) return; if (sm.state !== "Running") return; - if (scheduler.isScheduled || scheduler.isExecuting) return; + if (scheduler.isExecuting) { + if (!renderRequestQueuedForCurrentTurn) { + renderRequestQueuedForCurrentTurn = true; + scheduler.enqueue({ kind: "renderRequest" }); + } + return; + } + if (scheduler.isScheduled) return; scheduler.enqueue({ kind: "renderRequest" }); } @@ -676,7 +682,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl drawlistEncodedStringCacheCap: config.drawlistEncodedStringCacheCap, requestRender: requestRenderFromRenderer, requestView: requestViewFromRenderer, - onUserCodeError: (detail) => enqueueFatal("ZRUI_USER_CODE_THROW", detail), + onUserCodeError: (detail) => fatalNowOrEnqueue("ZRUI_USER_CODE_THROW", detail), collectRuntimeBreadcrumbs: runtimeBreadcrumbsEnabled, }); const focusDispatcher = createFocusDispatcher({ @@ -684,7 +690,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl getFocusInfo: () => widgetRenderer.getCurrentFocusInfo(), initialFocusedId: widgetRenderer.getFocusedId(), onHandlerError: (error: unknown) => { - enqueueFatal("ZRUI_USER_CODE_THROW", `onFocusChange handler threw: ${describeThrown(error)}`); + fatalNowOrEnqueue("ZRUI_USER_CODE_THROW", `onFocusChange handler threw: ${describeThrown(error)}`); }, }); @@ -820,14 +826,14 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } catch (e: unknown) { // Late events can race while a stop is already in-flight; avoid double-fatal. if (lifecycleBusy === "stop") return; - enqueueFatal( + fatalNowOrEnqueue( "ZRUI_BACKEND_ERROR", `stop threw after unhandled quit input: ${describeThrown(e)}`, ); return; } void stopPromise.catch((e: unknown) => { - enqueueFatal( + fatalNowOrEnqueue( "ZRUI_BACKEND_ERROR", `stop rejected after unhandled quit input: ${describeThrown(e)}`, ); @@ -856,6 +862,9 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl if (st === "Disposed" || st === "Faulted") { throwCode("ZRUI_INVALID_STATE", `${method}: app is ${st}`); } + if (lifecycleBusy !== null) { + throwCode("ZRUI_INVALID_STATE", `${method}: lifecycle operation already in flight`); + } } function assertNotReentrant(method: string): void { @@ -903,7 +912,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl }); } - function emit(ev: UiEvent): void { + function emit(ev: UiEvent): boolean { const snapshot: EventHandler[] = []; for (const slot of handlers) { if (slot.active.value) snapshot.push(slot.fn); @@ -915,14 +924,14 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl try { fn(ev); } catch (e: unknown) { - // Treat handler exceptions as fatal, but defer out of the handler stack. - enqueueFatal("ZRUI_USER_CODE_THROW", `onEvent handler threw: ${describeThrown(e)}`); - return; + fatalNowOrEnqueue("ZRUI_USER_CODE_THROW", `onEvent handler threw: ${describeThrown(e)}`); + return false; } } } finally { inEventHandlerDepth--; } + return true; } function emitFocusChangeIfNeeded(): boolean { @@ -931,6 +940,9 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl function doFatal(code: ZrUiErrorCode, detail: string): void { if (sm.state !== "Running") return; + lifecycleBusy = null; + lifecycleGeneration++; + backendStarted = false; // 1) emit fatal to handlers (registration order, best-effort) const fatalEv: UiEvent = { kind: "fatal", code, detail }; @@ -979,6 +991,29 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } } + function cleanupStartedBackendAfterAbort(): void { + if (!backendStarted) return; + backendStarted = false; + try { + void backend + .stop() + .catch(() => undefined) + .finally(() => { + try { + backend.dispose(); + } catch { + // ignore + } + }); + } catch { + try { + backend.dispose(); + } catch { + // ignore + } + } + } + function releaseOnce(batch: BackendEventBatch): () => void { let released = false; return () => { @@ -1012,7 +1047,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl try { if (engineTruncated || droppedBatches > 0) { - emit({ kind: "overrun", engineTruncated, droppedBatches }); + if (!emit({ kind: "overrun", engineTruncated, droppedBatches })) return; if (sm.state !== "Running") return; } @@ -1026,7 +1061,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl interactiveBudget = 2; } noteBreadcrumbEvent(ev.kind); - emit({ kind: "engine", event: ev }); + if (!emit({ kind: "engine", event: ev })) return; if (sm.state !== "Running") return; if (ev.kind === "resize") { const prev = viewport; @@ -1113,7 +1148,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl const keyResult = routeKeyEvent(routeInputState, ev, keyCtx); applyRoutedKeybindingState(routeInputState, keyResult.nextState); if (keyResult.handlerError !== undefined) { - enqueueFatal( + fatalNowOrEnqueue( "ZRUI_USER_CODE_THROW", `keybinding handler threw: ${describeThrown(keyResult.handlerError)}`, ); @@ -1156,7 +1191,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl const keyResult = routeKeyEvent(routeInputState, syntheticKeyEvent, keyCtx); applyRoutedKeybindingState(routeInputState, keyResult.nextState); if (keyResult.handlerError !== undefined) { - enqueueFatal( + fatalNowOrEnqueue( "ZRUI_USER_CODE_THROW", `keybinding handler threw: ${describeThrown(keyResult.handlerError)}`, ); @@ -1176,14 +1211,15 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl noteBreadcrumbConsumptionPath("widgetRouting"); routed = widgetRenderer.routeEngineEvent(ev); } catch (e: unknown) { - enqueueFatal("ZRUI_USER_CODE_THROW", `widget routing threw: ${describeThrown(e)}`); + fatalNowOrEnqueue("ZRUI_USER_CODE_THROW", `widget routing threw: ${describeThrown(e)}`); return; } + if (sm.state !== "Running") return; if (!emitFocusChangeIfNeeded()) return; if (routed.needsRender) markDirty(DIRTY_RENDER); if (routed.action) { noteBreadcrumbAction(routed.action); - emit({ kind: "action", ...routed.action }); + if (!emit({ kind: "action", ...routed.action })) return; if (sm.state !== "Running") return; } if ( @@ -1446,6 +1482,10 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl if (plan.commit) consumedDirtyFlags |= DIRTY_VIEW; dirtyTracker.clearConsumedFlags(consumedDirtyFlags, dirtyVersionStart); scheduleThemeTransitionContinuation(); + if (dirtyTracker.getFlags() !== 0 && !renderRequestQueuedForCurrentTurn) { + renderRequestQueuedForCurrentTurn = true; + scheduler.enqueue({ kind: "renderRequest" }); + } } function drainIgnored(items: readonly WorkItem[]): void { @@ -1461,6 +1501,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } function processTurn(items: readonly WorkItem[]): void { + renderRequestQueuedForCurrentTurn = false; const st = sm.state; if (st === "Disposed" || st === "Faulted") { drainIgnored(items); @@ -1536,7 +1577,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl batch = await backend.pollEvents(); } catch (e: unknown) { if (sm.state === "Running" && token === pollToken) { - enqueueFatal("ZRUI_BACKEND_ERROR", `pollEvents rejected: ${describeThrown(e)}`); + fatalNowOrEnqueue("ZRUI_BACKEND_ERROR", `pollEvents rejected: ${describeThrown(e)}`); } return; } @@ -1679,33 +1720,48 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl start(): Promise { assertOperational("start"); assertNotReentrant("start"); - if (lifecycleBusy) - throwCode("ZRUI_INVALID_STATE", "start: lifecycle operation already in flight"); sm.assertOneOf(["Created", "Stopped"], "start: must be Created or Stopped"); if (mode === null) throwCode("ZRUI_NO_RENDER_MODE", "start: no render mode selected"); lifecycleBusy = "start"; + const startGeneration = ++lifecycleGeneration; let p: Promise; try { p = backend.start(); } catch (e: unknown) { - lifecycleBusy = null; + if (lifecycleGeneration === startGeneration) lifecycleBusy = null; throwCode("ZRUI_BACKEND_ERROR", `backend.start threw: ${describeThrown(e)}`); } return p.then( async () => { - lifecycleBusy = null; - topLevelViewError = null; - terminalProfile = await loadTerminalProfile(backend); - widgetRenderer.setTerminalProfile(terminalProfile); - sm.toRunning(); - markDirty(DIRTY_VIEW, false); - pollToken++; - void pollLoop(pollToken); - scheduler.enqueue({ kind: "kick" }); + try { + backendStarted = true; + if (lifecycleGeneration !== startGeneration) { + cleanupStartedBackendAfterAbort(); + return; + } + topLevelViewError = null; + const loadedTerminalProfile = await loadTerminalProfile(backend); + if (lifecycleGeneration !== startGeneration) { + cleanupStartedBackendAfterAbort(); + return; + } + terminalProfile = loadedTerminalProfile; + widgetRenderer.setTerminalProfile(terminalProfile); + sm.toRunning(); + markDirty(DIRTY_VIEW, false); + pollToken++; + void pollLoop(pollToken); + scheduler.enqueue({ kind: "kick" }); + } finally { + if (lifecycleGeneration === startGeneration && lifecycleBusy === "start") { + lifecycleBusy = null; + } + } }, (e: unknown) => { + if (lifecycleGeneration !== startGeneration) return; lifecycleBusy = null; throw new ZrUiError("ZRUI_BACKEND_ERROR", `backend.start rejected: ${describeThrown(e)}`); }, @@ -1715,8 +1771,6 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl run(): Promise { assertOperational("run"); assertNotReentrant("run"); - if (lifecycleBusy) - throwCode("ZRUI_INVALID_STATE", "run: lifecycle operation already in flight"); sm.assertOneOf(["Created", "Stopped"], "run: must be Created or Stopped"); if (mode === null) throwCode("ZRUI_NO_RENDER_MODE", "run: no render mode selected"); @@ -1775,11 +1829,10 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl stop(): Promise { assertOperational("stop"); assertNotReentrant("stop"); - if (lifecycleBusy) - throwCode("ZRUI_INVALID_STATE", "stop: lifecycle operation already in flight"); sm.assertOneOf(["Running"], "stop: must be Running"); lifecycleBusy = "stop"; + const stopGeneration = ++lifecycleGeneration; // Stop polling immediately so in-flight pollEvents rejections from backend.stop() // are treated as part of shutdown (not a fatal backend error). pollToken++; @@ -1790,18 +1843,26 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl try { p = backend.stop(); } catch (e: unknown) { - lifecycleBusy = null; + if (lifecycleGeneration === stopGeneration) lifecycleBusy = null; throwCode("ZRUI_BACKEND_ERROR", `backend.stop threw: ${describeThrown(e)}`); } return p.then( () => { - lifecycleBusy = null; - themeTransition = null; - sm.toStopped(); - settleActiveRun?.(); + try { + if (lifecycleGeneration !== stopGeneration) return; + backendStarted = false; + themeTransition = null; + sm.toStopped(); + settleActiveRun?.(); + } finally { + if (lifecycleGeneration === stopGeneration && lifecycleBusy === "stop") { + lifecycleBusy = null; + } + } }, (e: unknown) => { + if (lifecycleGeneration !== stopGeneration) return; lifecycleBusy = null; throw new ZrUiError("ZRUI_BACKEND_ERROR", `backend.stop rejected: ${describeThrown(e)}`); }, @@ -1815,6 +1876,8 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl const st0 = sm.state; if (st0 === "Disposed") return; + lifecycleGeneration++; + lifecycleBusy = null; pollToken++; themeTransition = null; try { @@ -1823,13 +1886,14 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl // ignore } - if (st0 === "Running") { + if (st0 === "Running" || backendStarted) { try { void backend.stop().catch(() => undefined); } catch { // ignore } } + backendStarted = false; try { backend.dispose(); } catch { diff --git a/packages/core/src/app/createApp/topLevelViewError.ts b/packages/core/src/app/createApp/topLevelViewError.ts index f1734fb6..a0c3ab2b 100644 --- a/packages/core/src/app/createApp/topLevelViewError.ts +++ b/packages/core/src/app/createApp/topLevelViewError.ts @@ -1,4 +1,5 @@ import type { ZrevEvent } from "../../events.js"; +import { describeThrown } from "../../debug/describeThrown.js"; import { ZR_MOD_ALT, ZR_MOD_CTRL, ZR_MOD_META, ZR_MOD_SHIFT } from "../../keybindings/keyCodes.js"; import type { VNode } from "../../widgets/types.js"; import { ui } from "../../widgets/ui.js"; @@ -30,7 +31,7 @@ export function captureTopLevelViewError(value: unknown): TopLevelViewError { ...(typeof value.stack === "string" && value.stack.length > 0 ? { stack: value.stack } : {}), }); } - const detail = String(value); + const detail = describeThrown(value); return Object.freeze({ code: "ZRUI_USER_CODE_THROW", detail, diff --git a/packages/core/src/app/rawRenderer.ts b/packages/core/src/app/rawRenderer.ts index ae39ba66..011ebd8f 100644 --- a/packages/core/src/app/rawRenderer.ts +++ b/packages/core/src/app/rawRenderer.ts @@ -11,6 +11,7 @@ */ import { FRAME_ACCEPTED_ACK_MARKER, type RuntimeBackend } from "../backend.js"; +import { describeThrown } from "../debug/describeThrown.js"; import { type DrawlistBuilder, createDrawlistBuilder } from "../drawlist/index.js"; import { FRAME_AUDIT_ENABLED, drawlistFingerprint, emitFrameAudit } from "../perf/frameAudit.js"; import { perfMarkEnd, perfMarkStart } from "../perf/perf.js"; @@ -34,12 +35,6 @@ export type RawRenderSubmitResult = detail: string; }>; -/** Format thrown value for error message. */ -function describeThrown(v: unknown): string { - if (v instanceof Error) return `${v.name}: ${v.message}`; - return String(v); -} - /** * Renderer for raw draw API mode. * diff --git a/packages/core/src/app/widgetRenderer.ts b/packages/core/src/app/widgetRenderer.ts index d4ff90a7..e2aa4d57 100644 --- a/packages/core/src/app/widgetRenderer.ts +++ b/packages/core/src/app/widgetRenderer.ts @@ -947,6 +947,9 @@ function cloneFocusManagerState(state: FocusManagerState): FocusManagerState { ...(state.pendingFocusedId === undefined ? {} : { pendingFocusedId: state.pendingFocusedId }), zones: new Map(state.zones), trapStack: Object.freeze([...state.trapStack]), + ...(state.trapReturnFocusById === undefined + ? {} + : { trapReturnFocusById: new Map(state.trapReturnFocusById) }), lastFocusedByZone: new Map(state.lastFocusedByZone), }); } @@ -1815,13 +1818,12 @@ export class WidgetRenderer { /** * Determine whether a key event should bypass the keybinding system. * - * Why: When dropdowns or modal overlays are active, widgets must be able to - * consume Escape to close/deny/exit without being preempted by global - * keybindings (e.g., "Escape => menu"). + * Why: Active overlays own keyboard interaction. Global keybindings should + * not preempt dropdown navigation, modal dismissal, or overlay-local + * shortcuts while an overlay is present. */ shouldBypassKeybindings(event: ZrevEvent): boolean { if (event.kind !== "key" || event.action !== "down") return false; - if (event.key !== ZR_KEY_ESCAPE) return false; return this.hasActiveOverlay(); } @@ -2135,7 +2137,7 @@ export class WidgetRenderer { diffViewerById: this.diffViewerById, rectById: this.rectById, scrollOverrides: this.scrollOverrides, - findNearestScrollableAncestor: (targetId) => this.findNearestScrollableAncestor(targetId), + findScrollableAncestors: (targetId) => this.findScrollableAncestors(targetId), }); if (wheelRoute) return wheelRoute; @@ -2384,8 +2386,8 @@ export class WidgetRenderer { if (prev?.onExit) { try { prev.onExit(); - } catch { - // Swallow callback errors to preserve routing determinism. + } catch (error: unknown) { + this.reportUserCodeError(`focusZone onExit threw: ${describeThrown(error)}`); } } } @@ -2395,30 +2397,30 @@ export class WidgetRenderer { if (next?.onEnter) { try { next.onEnter(); - } catch { - // Swallow callback errors to preserve routing determinism. + } catch (error: unknown) { + this.reportUserCodeError(`focusZone onEnter threw: ${describeThrown(error)}`); } } } } - private findNearestScrollableAncestor( + private findScrollableAncestors( targetId: string | null, - ): Readonly<{ nodeId: string; meta: LayoutOverflowMetadata }> | null { - if (targetId === null || !this.committedRoot || !this.layoutTree) return null; + ): readonly Readonly<{ nodeId: string; meta: LayoutOverflowMetadata }>[] { + if (targetId === null || !this.committedRoot || !this.layoutTree) return Object.freeze([]); type ScrollableMatch = Readonly<{ nodeId: string; meta: LayoutOverflowMetadata }>; type Cursor = Readonly<{ runtimeNode: RuntimeInstance; layoutNode: LayoutTree; - nearest: ScrollableMatch | null; + scrollables: readonly ScrollableMatch[]; }>; const stack: Cursor[] = [ { runtimeNode: this.committedRoot, layoutNode: this.layoutTree, - nearest: null, + scrollables: Object.freeze([]), }, ]; @@ -2428,7 +2430,7 @@ export class WidgetRenderer { const runtimeNode = frame.runtimeNode; const layoutNode = frame.layoutNode; - let nearest = frame.nearest; + let scrollables = frame.scrollables; const props = runtimeNode.vnode.props as Readonly<{ id?: unknown; @@ -2441,11 +2443,13 @@ export class WidgetRenderer { const hasScrollableAxis = meta.contentWidth > meta.viewportWidth || meta.contentHeight > meta.viewportHeight; if (hasScrollableAxis) { - nearest = { nodeId, meta }; + scrollables = Object.freeze([...scrollables, { nodeId, meta }]); } } - if (nodeId === targetId) return nearest; + if (nodeId === targetId) { + return Object.freeze([...scrollables].reverse()); + } const childCount = Math.min(runtimeNode.children.length, layoutNode.children.length); for (let i = childCount - 1; i >= 0; i--) { @@ -2455,12 +2459,12 @@ export class WidgetRenderer { stack.push({ runtimeNode: runtimeChild, layoutNode: layoutChild, - nearest, + scrollables, }); } } - return null; + return Object.freeze([]); } private applyScrollOverridesToVNode( @@ -3947,15 +3951,7 @@ export class WidgetRenderer { this._constraintNodesWithAffectedDescendants = this._pooledConstraintNodesWithAffectedDescendants; } - const hasDisplayConstraint = constraintGraph?.hasDisplayConstraints ?? false; - if (hasDisplayConstraint || this._constraintHasStaticHiddenDisplay) { - this.rebuildConstraintHiddenState(this.committedRoot, resolvedValuesForLayout); - } else { - this._pooledHiddenConstraintInstanceIds.clear(); - this._pooledHiddenConstraintWidgetIds.clear(); - this._hiddenConstraintInstanceIds = this._pooledHiddenConstraintInstanceIds; - this._hiddenConstraintWidgetIds = this._pooledHiddenConstraintWidgetIds; - } + this.rebuildConstraintHiddenState(this.committedRoot, resolvedValuesForLayout); if ( constraintGraph !== null && ((resolvedValuesForLayout !== null && resolvedValuesForLayout.size > 0) || @@ -3999,7 +3995,10 @@ export class WidgetRenderer { // constraints can converge one depth level at a time. if (constraintGraph !== null && constraintGraph.nodes.length > 0) { let settlePasses = 0; - const maxSettlePasses = Math.max(3, Math.min(12, constraintGraph.nodes.length + 1)); + // Nested parent/intrinsic chains can converge one dependency level at a time. + // Use the graph size instead of an arbitrary small cap so first-frame layout + // can fully settle for deep but valid trees. + const maxSettlePasses = Math.max(3, constraintGraph.nodes.length + 1); while (settlePasses < maxSettlePasses) { buildLayoutRectIndexes( nextLayoutTree, @@ -4045,14 +4044,7 @@ export class WidgetRenderer { ? CONSTRAINT_RESOLUTION_CACHE_HIT : CONSTRAINT_RESOLUTION_COMPUTED; - if (hasDisplayConstraint || this._constraintHasStaticHiddenDisplay) { - this.rebuildConstraintHiddenState(this.committedRoot, resolvedValuesForLayout); - } else { - this._pooledHiddenConstraintInstanceIds.clear(); - this._pooledHiddenConstraintWidgetIds.clear(); - this._hiddenConstraintInstanceIds = this._pooledHiddenConstraintInstanceIds; - this._hiddenConstraintWidgetIds = this._pooledHiddenConstraintWidgetIds; - } + this.rebuildConstraintHiddenState(this.committedRoot, resolvedValuesForLayout); let settledConstrainedRootVNode = this.committedRoot.vnode; if ( diff --git a/packages/core/src/debug/describeThrown.ts b/packages/core/src/debug/describeThrown.ts new file mode 100644 index 00000000..2920c8f1 --- /dev/null +++ b/packages/core/src/debug/describeThrown.ts @@ -0,0 +1,8 @@ +export function describeThrown(value: unknown): string { + if (value instanceof Error) return `${value.name}: ${value.message}`; + try { + return String(value); + } catch { + return "[unstringifiable thrown value]"; + } +} diff --git a/packages/core/src/keybindings/__tests__/keybinding.conflicts.test.ts b/packages/core/src/keybindings/__tests__/keybinding.conflicts.test.ts index 52b79f00..994f6844 100644 --- a/packages/core/src/keybindings/__tests__/keybinding.conflicts.test.ts +++ b/packages/core/src/keybindings/__tests__/keybinding.conflicts.test.ts @@ -351,7 +351,7 @@ describe("routing semantics", () => { assert.deepEqual(closed, ["modal"]); }); - test("layer escape skips non-closable top layer and closes next layer", () => { + test("layer escape is owned by a non-closable top layer", () => { const closed: string[] = []; const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE, 1), { @@ -367,18 +367,18 @@ describe("routing semantics", () => { }); assert.equal(result.consumed, true); - assert.equal(result.closedLayerId, "base"); - assert.deepEqual(closed, ["base"]); + assert.equal(result.closedLayerId, undefined); + assert.deepEqual(closed, []); }); - test("layer escape is not consumed when closable layer has no callback", () => { + test("layer escape is consumed when the top layer has no close callback", () => { const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE, 1), { layerStack: ["modal"], closeOnEscape: new Map([["modal", true]]), onClose: new Map(), }); - assert.equal(result.consumed, false); + assert.equal(result.consumed, true); }); }); diff --git a/packages/core/src/perf/__tests__/perf.test.ts b/packages/core/src/perf/__tests__/perf.test.ts index 80b91602..fbeb9dcc 100644 --- a/packages/core/src/perf/__tests__/perf.test.ts +++ b/packages/core/src/perf/__tests__/perf.test.ts @@ -47,4 +47,42 @@ describe("perf instrumentation", () => { // biome-ignore lint/complexity/useLiteralKeys: TypeScript requires bracket access for index-signature properties. assert.equal((snapshot.counters["test_counter"] ?? 0) >= 1, true); }); + + test("rolling snapshots recompute max from the retained sample window", () => { + perfReset(); + if (!PERF_ENABLED) { + assert.deepEqual(perfSnapshot().phases, {}); + return; + } + + for (let i = 0; i < 1100; i++) { + perfRecord("commit", 10); + } + for (let i = 0; i < 1100; i++) { + perfRecord("commit", 5); + } + + const commit = perfSnapshot().phases.commit; + assert.ok(commit !== undefined); + assert.equal(commit?.max, 5); + assert.deepEqual(commit?.worst10, Object.freeze(new Array(10).fill(5))); + }); + + test("perfReset clears retained samples and counters", () => { + perfReset(); + perfRecord("layout", 3); + perfCount("before_reset"); + perfReset(); + + const snapshot = perfSnapshot(); + if (!PERF_ENABLED) { + assert.deepEqual(snapshot.phases, {}); + assert.deepEqual(snapshot.counters, {}); + return; + } + + assert.equal(snapshot.phases.layout, undefined); + // biome-ignore lint/complexity/useLiteralKeys: TypeScript requires bracket access for index-signature properties. + assert.equal(snapshot.counters["before_reset"], undefined); + }); }); diff --git a/packages/core/src/perf/perf.ts b/packages/core/src/perf/perf.ts index cfc7d4d9..a96dfcba 100644 --- a/packages/core/src/perf/perf.ts +++ b/packages/core/src/perf/perf.ts @@ -130,7 +130,6 @@ type PhaseRing = { cursor: number; count: number; sum: number; - max: number; }; function createPhaseRing(): PhaseRing { @@ -139,7 +138,6 @@ function createPhaseRing(): PhaseRing { cursor: 0, count: 0, sum: 0, - max: 0, }; } @@ -154,10 +152,6 @@ function recordSample(ring: PhaseRing, dt: number): void { ring.sum += dt; ring.cursor = (ring.cursor + 1) % RING_CAP; ring.count = Math.min(ring.count + 1, RING_CAP); - - if (dt > ring.max) { - ring.max = dt; - } } function computeStats(ring: PhaseRing): PhaseStats | null { @@ -188,7 +182,7 @@ function computeStats(ring: PhaseRing): PhaseStats | null { p50: arr[p50Idx] ?? 0, p95: arr[p95Idx] ?? 0, p99: arr[p99Idx] ?? 0, - max: ring.max, + max: arr[arr.length - 1] ?? 0, worst10, }); } diff --git a/packages/core/src/runtime/__tests__/duplicateIdFatal.test.ts b/packages/core/src/runtime/__tests__/duplicateIdFatal.test.ts index 7f148e63..ab9dc274 100644 --- a/packages/core/src/runtime/__tests__/duplicateIdFatal.test.ts +++ b/packages/core/src/runtime/__tests__/duplicateIdFatal.test.ts @@ -1,5 +1,5 @@ import { assert, test } from "@rezi-ui/testkit"; -import type { VNode } from "../../index.js"; +import { type VNode, ui } from "../../index.js"; import type { FilePickerProps } from "../../widgets/types.js"; import { commitVNodeTree } from "../commit.js"; import { createInstanceIdAllocator } from "../instance.js"; @@ -38,6 +38,14 @@ function link(id?: string): VNode { return { kind: "link", props }; } +function focusZone(id: string): VNode { + return ui.focusZone({ id }, []); +} + +function focusTrap(id: string): VNode { + return ui.focusTrap({ id, active: true }, []); +} + test("duplicate Button ids anywhere in the tree trigger deterministic fatal ZRUI_DUPLICATE_ID (#66)", () => { const allocator = createInstanceIdAllocator(1); @@ -70,6 +78,22 @@ test("duplicate advanced widget ids trigger deterministic fatal ZRUI_DUPLICATE_I ); }); +test("duplicate focus container ids trigger deterministic fatal ZRUI_DUPLICATE_ID", () => { + const allocator = createInstanceIdAllocator(1); + + const tree = column([focusZone("dup"), box([focusTrap("dup")])]); + const res = commitVNodeTree(null, tree, { allocator }); + + assert.equal(res.ok, false); + if (res.ok) return; + + assert.equal(res.fatal.code, "ZRUI_DUPLICATE_ID"); + assert.equal( + res.fatal.detail, + 'Duplicate focus container id "dup". First: , second: . Hint: focusZone, focusTrap, and modal ids must be unique across the tree.', + ); +}); + test("reused interactive id index is cleared at the start of each commit cycle", () => { const allocator = createInstanceIdAllocator(1); const interactiveIdIndex = new Map(); diff --git a/packages/core/src/runtime/__tests__/focus.layers.test.ts b/packages/core/src/runtime/__tests__/focus.layers.test.ts index 741fdea4..8c63c7d8 100644 --- a/packages/core/src/runtime/__tests__/focus.layers.test.ts +++ b/packages/core/src/runtime/__tests__/focus.layers.test.ts @@ -353,7 +353,7 @@ describe("focus layers - ESC and layer stack routing", () => { assert.equal(closedLayer, "modal-b"); }); - test("ESC skips top layer when closeOnEscape is false", () => { + test("ESC is owned by the top layer when closeOnEscape is false", () => { let closedLayer: string | null = null; const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE), { layerStack: ["modal-a", "modal-b"], @@ -372,11 +372,11 @@ describe("focus layers - ESC and layer stack routing", () => { }); assert.equal(result.consumed, true); - assert.equal(result.closedLayerId, "modal-a"); - assert.equal(closedLayer, "modal-a"); + assert.equal(result.closedLayerId, undefined); + assert.equal(closedLayer, null); }); - test("ESC skips closable layers without callbacks", () => { + test("ESC is consumed by the top layer when it has no close callback", () => { let closedLayer: string | null = null; const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE), { layerStack: ["base", "middle", "top"], @@ -396,11 +396,11 @@ describe("focus layers - ESC and layer stack routing", () => { }); assert.equal(result.consumed, true); - assert.equal(result.closedLayerId, "middle"); - assert.equal(closedLayer, "middle"); + assert.equal(result.closedLayerId, undefined); + assert.equal(closedLayer, null); }); - test("ESC is not consumed when all layers are non-closable or missing callbacks", () => { + test("ESC is consumed by the top layer even when lower layers could close", () => { const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE), { layerStack: ["a", "b", "c"], closeOnEscape: new Map([ @@ -411,7 +411,7 @@ describe("focus layers - ESC and layer stack routing", () => { onClose: new Map(), }); - assert.equal(result.consumed, false); + assert.equal(result.consumed, true); assert.equal(result.closedLayerId, undefined); }); @@ -447,7 +447,8 @@ describe("focus layers - ESC and layer stack routing", () => { }); assert.equal(result.consumed, true); - assert.equal(result.closedLayerId, "modal"); + assert.equal(result.closedLayerId, undefined); + assert.ok(result.callbackError instanceof Error); }); test("popLayer removes a middle layer deterministically", () => { diff --git a/packages/core/src/runtime/__tests__/localStateStore.test.ts b/packages/core/src/runtime/__tests__/localStateStore.test.ts index 3b90a38f..53fa49a8 100644 --- a/packages/core/src/runtime/__tests__/localStateStore.test.ts +++ b/packages/core/src/runtime/__tests__/localStateStore.test.ts @@ -1,5 +1,9 @@ import { assert, test } from "@rezi-ui/testkit"; -import { createRuntimeLocalStateStore } from "../localState.js"; +import { + createRuntimeLocalStateStore, + createTreeStateStore, + createVirtualListStateStore, +} from "../localState.js"; test("runtime-local state store set/get/delete keyed by instanceId (#66)", () => { const store = createRuntimeLocalStateStore(); @@ -27,3 +31,36 @@ test("runtime-local state store set/get/delete keyed by instanceId (#66)", () => store.delete(1); assert.equal(store.get(1), undefined); }); + +test("virtual list state store clones measuredHeights inputs", () => { + const store = createVirtualListStateStore(); + const measuredHeights = new Map([[0, 2]]); + + const state = store.set("list", { measuredHeights }); + measuredHeights.set(1, 9); + + assert.notEqual(state.measuredHeights, measuredHeights); + assert.equal(state.measuredHeights?.has(1), false); + assert.equal(store.get("list").measuredHeights?.has(1), false); +}); + +test("tree state store clones loading and expanded set inputs", () => { + const store = createTreeStateStore(); + const loadingKeys = new Set(["loading-a"]); + const expandedSet = new Set(["root"]); + + const state = store.set("tree", { + loadingKeys, + expandedSetRef: Object.freeze(["root"]), + expandedSet, + }); + loadingKeys.add("loading-b"); + expandedSet.add("child"); + + assert.notEqual(state.loadingKeys, loadingKeys); + assert.notEqual(state.expandedSet, expandedSet); + assert.equal(state.loadingKeys.has("loading-b"), false); + assert.equal(state.expandedSet?.has("child"), false); + assert.equal(store.get("tree").loadingKeys.has("loading-b"), false); + assert.equal(store.get("tree").expandedSet?.has("child"), false); +}); diff --git a/packages/core/src/runtime/__tests__/widgetMeta.test.ts b/packages/core/src/runtime/__tests__/widgetMeta.test.ts index 2bffc752..71b9e6a1 100644 --- a/packages/core/src/runtime/__tests__/widgetMeta.test.ts +++ b/packages/core/src/runtime/__tests__/widgetMeta.test.ts @@ -1,6 +1,7 @@ import { assert, test } from "@rezi-ui/testkit"; +import { ZrUiError } from "../../abi.js"; import { type VNode, ui } from "../../index.js"; -import { commitVNodeTree } from "../commit.js"; +import { type RuntimeInstance, commitVNodeTree } from "../commit.js"; import { createInstanceIdAllocator } from "../instance.js"; import { collectAllWidgetMetadata, @@ -10,6 +11,7 @@ import { collectFocusableIds, collectInputMetaById, collectPressableIds, + createWidgetMetadataCollector, } from "../widgetMeta.js"; function commitTree(vnode: VNode) { @@ -21,6 +23,22 @@ function commitTree(vnode: VNode) { return res.value.root; } +function runtimeNode( + instanceId: number, + vnode: VNode, + children: readonly RuntimeInstance[] = [], +): RuntimeInstance { + return { + instanceId, + vnode, + children, + dirty: false, + selfDirty: false, + renderPacketKey: 0, + renderPacket: null, + }; +} + test("widgetMeta: collectFocusableIds + collectEnabledMap (Buttons + Inputs) (#82, #92)", () => { const vnode = ui.column({}, [ ui.button({ id: "a", label: "A" }), @@ -353,6 +371,66 @@ test("widgetMeta: collectAllWidgetMetadata includes accessible focus semantics", assert.equal(ageInfo.announcement, "Age — Required"); }); +test("widgetMeta: reusable collector publishes stable snapshots across collects", () => { + const collector = createWidgetMetadataCollector(); + + const first = collector.collect( + commitTree( + ui.column({}, [ + ui.button({ id: "first-btn", label: "First" }), + ui.input({ id: "first-input", value: "hello" }), + ui.focusZone({ id: "zone-a" }, [ui.button({ id: "zone-btn", label: "Zone" })]), + ui.focusTrap( + { id: "trap-a", active: true }, + [ui.button({ id: "trap-btn", label: "Trap" })], + ), + ]), + ), + ); + + const second = collector.collect( + commitTree( + ui.column({}, [ + ui.button({ id: "second-btn", label: "Second" }), + ui.focusZone({ id: "zone-b" }, [ui.button({ id: "zone-b-btn", label: "Zone B" })]), + ]), + ), + ); + + assert.deepEqual(first.focusableIds, ["first-btn", "first-input", "zone-btn", "trap-btn"]); + assert.deepEqual([...first.enabledById.entries()], [ + ["first-btn", true], + ["first-input", true], + ["zone-btn", true], + ["trap-btn", true], + ]); + assert.equal(first.pressableIds.has("first-btn"), true); + assert.equal(first.pressableIds.has("second-btn"), false); + assert.equal(first.inputById.get("first-input")?.value, "hello"); + assert.deepEqual(first.zones.get("zone-a")?.focusableIds, ["zone-btn"]); + assert.deepEqual(first.traps.get("trap-a")?.focusableIds, ["trap-btn"]); + + assert.deepEqual(second.focusableIds, ["second-btn", "zone-b-btn"]); + assert.equal(second.pressableIds.has("second-btn"), true); + assert.equal(second.zones.has("zone-b"), true); +}); + +test("widgetMeta: duplicate focus container ids throw deterministic ZrUiError", () => { + const zoneVNode = ui.focusZone({ id: "dup" }, []); + const trapVNode = ui.focusTrap({ id: "dup", active: true }, []); + const rootVNode = ui.column({}, [zoneVNode, trapVNode]); + const root = runtimeNode(1, rootVNode, [runtimeNode(2, zoneVNode), runtimeNode(3, trapVNode)]); + + assert.throws( + () => collectAllWidgetMetadata(root), + (error: unknown) => + error instanceof ZrUiError && + error.code === "ZRUI_DUPLICATE_ID" && + error.message === + 'Duplicate focus container id "dup". First: , second: . Hint: focusZone, focusTrap, and modal ids must be unique across the tree.', + ); +}); + /* ========== Performance Tests ========== */ /** diff --git a/packages/core/src/runtime/commit.ts b/packages/core/src/runtime/commit.ts index 80b18ede..40b32b5e 100644 --- a/packages/core/src/runtime/commit.ts +++ b/packages/core/src/runtime/commit.ts @@ -18,7 +18,9 @@ import { resolveEasing } from "../animation/easing.js"; import { normalizeDurationMs } from "../animation/interpolate.js"; +import { describeThrown } from "../debug/describeThrown.js"; import type { ResponsiveViewportSnapshot } from "../layout/responsive.js"; +import { defaultTheme } from "../theme/defaultTheme.js"; import { mergeThemeOverride } from "../theme/interop.js"; import type { Theme } from "../theme/theme.js"; import type { ColorTokens } from "../theme/tokens.js"; @@ -539,9 +541,18 @@ function themedPropsEqual(a: unknown, b: unknown): boolean { return deepEqualUnknown(ao.theme, bo.theme); } +function fragmentPropsEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + const ao = (a ?? {}) as { key?: unknown }; + const bo = (b ?? {}) as typeof ao; + return ao.key === bo.key; +} + function canFastReuseContainerSelf(prev: VNode, next: VNode): boolean { if (prev.kind !== next.kind) return false; switch (prev.kind) { + case "fragment": + return fragmentPropsEqual(prev.props, (next as typeof prev).props); case "box": return boxPropsEqual(prev.props, (next as typeof prev).props); case "row": @@ -571,6 +582,9 @@ function diagWhichPropFails(prev: VNode, next: VNode): string | undefined { }; const ap = (prev.props ?? {}) as ReuseDiagProps; const bp = (next.props ?? {}) as ReuseDiagProps; + if (prev.kind === "fragment" && ap.key !== bp.key) { + return "key"; + } if (prev.kind === "row" || prev.kind === "column") { for (const k of ["pad", "gap", "align", "justify", "items"] as const) { if (ap[k] !== bp[k]) return k; @@ -801,12 +815,52 @@ function ensureInteractiveId( return null; } +type FocusContainerKind = "focusZone" | "focusTrap" | "modal"; + +function isFocusContainerVNode(vnode: VNode): vnode is VNode & { kind: FocusContainerKind } { + return vnode.kind === "focusZone" || vnode.kind === "focusTrap" || vnode.kind === "modal"; +} + +function ensureFocusContainerId( + seen: Map, + instanceId: InstanceId, + vnode: VNode, +): CommitFatal | null { + if (!isFocusContainerVNode(vnode)) return null; + + const id = (vnode as { props: { id?: unknown } }).props.id; + if (typeof id !== "string" || id.length === 0) { + return { + code: "ZRUI_INVALID_PROPS", + detail: `focus container missing required id (kind=${vnode.kind}, instanceId=${String(instanceId)})`, + }; + } + if (id.trim().length === 0) { + return { + code: "ZRUI_INVALID_PROPS", + detail: `focus container id must contain non-whitespace characters (kind=${vnode.kind}, instanceId=${String(instanceId)})`, + }; + } + + const existing = seen.get(id); + if (existing !== undefined) { + return { + code: "ZRUI_DUPLICATE_ID", + detail: `Duplicate focus container id "${id}". First: <${existing}>, second: <${vnode.kind}>. Hint: focusZone, focusTrap, and modal ids must be unique across the tree.`, + }; + } + + seen.set(id, vnode.kind); + return null; +} + function isVNode(v: unknown): v is VNode { return typeof v === "object" && v !== null && "kind" in v; } function commitChildrenForVNode(vnode: VNode): readonly VNode[] { if ( + vnode.kind === "fragment" || vnode.kind === "box" || vnode.kind === "row" || vnode.kind === "column" || @@ -991,22 +1045,23 @@ function resolveCompositeChildTheme(parentTheme: Theme, vnode: VNode): Theme { return parentTheme; } -function readCompositeColorTokens(ctx: CommitCtx): ColorTokens | null { +function readCompositeColorTokens(ctx: CommitCtx): ColorTokens { const composite = ctx.composite; - if (!composite) return null; + if (!composite) return defaultTheme.definition.colors; const theme = currentCompositeTheme(ctx); - if (theme !== null && composite.getColorTokens) { - return composite.getColorTokens(theme); + if (theme !== null) { + return composite.getColorTokens ? composite.getColorTokens(theme) : theme.definition.colors; } - return composite.colorTokens ?? null; + return composite.colorTokens ?? defaultTheme.definition.colors; } type CommitCtx = Readonly<{ allocator: InstanceIdAllocator; localState: RuntimeLocalStateStore | undefined; seenInteractiveIds: Map; + seenFocusContainerIds: Map; prevNodeStack: Array; containerChildOverrides: Map; layoutDepthRef: { value: number }; @@ -1017,9 +1072,9 @@ type CommitCtx = Readonly<{ composite: Readonly<{ registry: CompositeInstanceRegistry; appState: unknown; - colorTokens?: ColorTokens | null; + colorTokens?: ColorTokens; theme?: Theme; - getColorTokens?: (theme: Theme) => ColorTokens | null; + getColorTokens?: (theme: Theme) => ColorTokens; viewport?: ResponsiveViewportSnapshot; onInvalidate: (instanceId: InstanceId) => void; onUseViewport?: () => void; @@ -1105,7 +1160,7 @@ function commitErrorBoundaryFallback( ok: false, fatal: { code: "ZRUI_USER_CODE_THROW", - detail: e instanceof Error ? `${e.name}: ${e.message}` : String(e), + detail: describeThrown(e), }, }; } @@ -1132,6 +1187,7 @@ function formatNodePath(nodePath: readonly string[]): string { function isContainerVNode(vnode: VNode): boolean { return ( + vnode.kind === "fragment" || vnode.kind === "box" || vnode.kind === "row" || vnode.kind === "column" || @@ -1185,6 +1241,7 @@ function rewriteCommittedVNode(next: VNode, committedChildren: readonly VNode[]) } if ( + next.kind === "fragment" || next.kind === "box" || next.kind === "row" || next.kind === "column" || @@ -1597,7 +1654,7 @@ function executeCompositeRender( ok: false, fatal: { code: "ZRUI_USER_CODE_THROW", - detail: e instanceof Error ? `${e.name}: ${e.message}` : String(e), + detail: describeThrown(e), }, }; } @@ -1707,7 +1764,7 @@ function executeCompositeRender( ok: false, fatal: { code: "ZRUI_USER_CODE_THROW", - detail: e instanceof Error ? `${e.name}: ${e.message}` : String(e), + detail: describeThrown(e), }, }; } @@ -1723,7 +1780,7 @@ function executeCompositeRender( ok: false, fatal: { code: "ZRUI_USER_CODE_THROW", - detail: e instanceof Error ? `${e.name}: ${e.message}` : String(e), + detail: describeThrown(e), }, }; } @@ -1914,6 +1971,8 @@ function commitNode( const idFatal = ensureInteractiveId(ctx.seenInteractiveIds, instanceId, vnode); if (idFatal) return { ok: false, fatal: idFatal }; + const focusContainerFatal = ensureFocusContainerId(ctx.seenFocusContainerIds, instanceId, vnode); + if (focusContainerFatal) return { ok: false, fatal: focusContainerFatal }; if (ctx.collectLifecycleInstanceIds) { if (prev) ctx.lists.reused.push(instanceId); @@ -1997,9 +2056,9 @@ export function commitVNodeTree( composite?: Readonly<{ registry: CompositeInstanceRegistry; appState: unknown; - colorTokens?: ColorTokens | null; + colorTokens?: ColorTokens; theme?: Theme; - getColorTokens?: (theme: Theme) => ColorTokens | null; + getColorTokens?: (theme: Theme) => ColorTokens; viewport?: ResponsiveViewportSnapshot; onInvalidate: (instanceId: InstanceId) => void; onUseViewport?: () => void; @@ -2019,6 +2078,7 @@ export function commitVNodeTree( allocator: opts.allocator, localState: opts.localState, seenInteractiveIds: interactiveIdIndex, + seenFocusContainerIds: new Map(), prevNodeStack: [], containerChildOverrides: new Map(), layoutDepthRef: { value: 0 }, diff --git a/packages/core/src/runtime/localState.ts b/packages/core/src/runtime/localState.ts index 1134c101..9d1f20d4 100644 --- a/packages/core/src/runtime/localState.ts +++ b/packages/core/src/runtime/localState.ts @@ -88,6 +88,14 @@ function freezeState(s: RuntimeLocalState): RuntimeLocalState { }); } +function cloneReadonlyMap(value: ReadonlyMap): ReadonlyMap { + return new Map(value); +} + +function cloneReadonlySet(value: ReadonlySet): ReadonlySet { + return new Set(value); +} + /** Create a new local state store instance. */ export function createRuntimeLocalStateStore(): RuntimeLocalStateStore { const table = new Map(); @@ -152,7 +160,9 @@ export function createVirtualListStateStore(): VirtualListStateStore { set: (id, patch) => { const prev = table.get(id) ?? DEFAULT_VLIST_STATE; const measuredHeights = - patch.measuredHeights !== undefined ? patch.measuredHeights : prev.measuredHeights; + patch.measuredHeights !== undefined + ? cloneReadonlyMap(patch.measuredHeights) + : prev.measuredHeights; const measuredWidth = patch.measuredWidth !== undefined ? patch.measuredWidth : prev.measuredWidth; const measuredItemCount = @@ -352,14 +362,16 @@ export function createTreeStateStore(): TreeStateStore { const prev = table.get(id) ?? DEFAULT_TREE_STATE; const next: TreeLocalState = Object.freeze({ focusedKey: patch.focusedKey !== undefined ? patch.focusedKey : prev.focusedKey, - loadingKeys: patch.loadingKeys !== undefined ? patch.loadingKeys : prev.loadingKeys, + loadingKeys: + patch.loadingKeys !== undefined ? cloneReadonlySet(patch.loadingKeys) : prev.loadingKeys, scrollTop: patch.scrollTop !== undefined ? patch.scrollTop : prev.scrollTop, viewportHeight: patch.viewportHeight !== undefined ? patch.viewportHeight : prev.viewportHeight, flatCache: patch.flatCache !== undefined ? patch.flatCache : prev.flatCache, expandedSetRef: patch.expandedSetRef !== undefined ? patch.expandedSetRef : prev.expandedSetRef, - expandedSet: patch.expandedSet !== undefined ? patch.expandedSet : prev.expandedSet, + expandedSet: + patch.expandedSet !== undefined ? cloneReadonlySet(patch.expandedSet) : prev.expandedSet, prefixCache: patch.prefixCache !== undefined ? patch.prefixCache : prev.prefixCache, }); table.set(id, next); diff --git a/packages/core/src/runtime/router/layer.ts b/packages/core/src/runtime/router/layer.ts index 278b2df9..3099934f 100644 --- a/packages/core/src/runtime/router/layer.ts +++ b/packages/core/src/runtime/router/layer.ts @@ -1,3 +1,4 @@ +import { describeThrown } from "../../debug/describeThrown.js"; import type { ZrevEvent } from "../../events.js"; import type { LayerRoutingCtx, LayerRoutingResult } from "./types.js"; @@ -5,8 +6,19 @@ import type { LayerRoutingCtx, LayerRoutingResult } from "./types.js"; /* MUST match packages/core/src/keybindings/keyCodes.ts */ const ZR_KEY_ESCAPE = 1; +const NODE_ENV = + (globalThis as { process?: { env?: { NODE_ENV?: string } } }).process?.env?.NODE_ENV ?? + "development"; +const DEV_MODE = NODE_ENV !== "production"; + +function warnDev(message: string): void { + if (!DEV_MODE) return; + const c = (globalThis as { console?: { warn?: (msg: string) => void } }).console; + c?.warn?.(message); +} + /** - * Route ESC key to close topmost layer. + * Route ESC key to the topmost layer only. * * @param event - The ZREV event * @param ctx - Layer routing context @@ -19,30 +31,33 @@ export function routeLayerEscape(event: ZrevEvent, ctx: LayerRoutingCtx): LayerR const { layerStack, closeOnEscape, onClose } = ctx; - // Find topmost layer that supports close-on-escape - for (let i = layerStack.length - 1; i >= 0; i--) { - const layerId = layerStack[i]; - if (!layerId) continue; - - const canClose = closeOnEscape.get(layerId) ?? true; - if (canClose) { - const closeCallback = onClose.get(layerId); - // A layer without an onClose callback can't actually close, so don't - // consume ESC and allow lower layers/widgets to handle it. - if (!closeCallback) continue; - - try { - closeCallback(); - } catch { - // Swallow errors from close callbacks - } - - return Object.freeze({ - closedLayerId: layerId, - consumed: true, - }); - } + const layerId = layerStack[layerStack.length - 1]; + if (!layerId) { + return Object.freeze({ consumed: false }); + } + + const canClose = closeOnEscape.get(layerId) ?? true; + if (canClose !== true) { + return Object.freeze({ consumed: true }); + } + + const closeCallback = onClose.get(layerId); + if (!closeCallback) { + return Object.freeze({ consumed: true }); + } + + try { + closeCallback(); + } catch (error: unknown) { + warnDev(`[rezi] layer onClose callback threw: ${describeThrown(error)}`); + return Object.freeze({ + consumed: true, + callbackError: error, + }); } - return Object.freeze({ consumed: false }); + return Object.freeze({ + closedLayerId: layerId, + consumed: true, + }); } diff --git a/packages/core/src/runtime/widgetMeta.ts b/packages/core/src/runtime/widgetMeta.ts index 176f74b8..2fd085a5 100644 --- a/packages/core/src/runtime/widgetMeta.ts +++ b/packages/core/src/runtime/widgetMeta.ts @@ -16,6 +16,7 @@ * @see docs/guide/runtime-and-layout.md */ +import { ZrUiError } from "../abi.js"; import { getWidgetProtocol, kindIsFocusable, kindIsPressable } from "../widgets/protocol.js"; import type { FocusZoneNavigation } from "../widgets/types.js"; import type { RuntimeInstance } from "./commit.js"; @@ -57,6 +58,28 @@ function isEnabledInteractive(v: RuntimeInstance["vnode"]): string | null { return id; } +type FocusContainerKind = "focusZone" | "focusTrap" | "modal"; + +function duplicateFocusContainerDetail( + id: string, + firstKind: FocusContainerKind, + secondKind: FocusContainerKind, +): string { + return `Duplicate focus container id "${id}". First: <${firstKind}>, second: <${secondKind}>. Hint: focusZone, focusTrap, and modal ids must be unique across the tree.`; +} + +function recordFocusContainerId( + seen: Map, + id: string, + kind: FocusContainerKind, +): void { + const existing = seen.get(id); + if (existing !== undefined) { + throw new ZrUiError("ZRUI_DUPLICATE_ID", duplicateFocusContainerDetail(id, existing, kind)); + } + seen.set(id, kind); +} + /** * Collect focusable ids from a committed runtime tree. * @@ -127,6 +150,7 @@ export type InputMeta = Readonly<{ instanceId: InstanceId; value: string; disabled: boolean; + readOnly: boolean; multiline: boolean; rows: number; wordWrap: boolean; @@ -398,6 +422,7 @@ export function collectInputMetaById(tree: RuntimeInstance): ReadonlyMap { const m = new Map(); + const seen = new Map(); const stack: Array<{ node: RuntimeInstance; parentZoneId: string | null }> = [ { node: tree, parentZoneId: null }, @@ -531,7 +559,8 @@ export function collectFocusZones(tree: RuntimeInstance): ReadonlyMap 0 ? props.id : null; - if (id !== null && !m.has(id)) { + if (id !== null) { + recordFocusContainerId(seen, id, "focusZone"); const tabIndex = typeof props.tabIndex === "number" ? props.tabIndex : 0; const navigation = props.navigation === "linear" || @@ -606,6 +635,7 @@ export function collectFocusZones(tree: RuntimeInstance): ReadonlyMap { const m = new Map(); + const seen = new Map(); const stack: RuntimeInstance[] = [tree]; while (stack.length > 0) { @@ -621,22 +651,18 @@ export function collectFocusTraps(tree: RuntimeInstance): ReadonlyMap 0 ? props.id : null; - if (id !== null && !m.has(id)) { + if (id !== null) { + recordFocusContainerId(seen, id, node.vnode.kind); const active = node.vnode.kind === "modal" ? true : props.active === true; const returnFocusTo = typeof props.returnFocusTo === "string" ? props.returnFocusTo : null; const initialFocus = typeof props.initialFocus === "string" ? props.initialFocus : null; - // Collect focusable ids from trap children (not traversing into nested zones/traps) + // Collect focusable ids from trap children, including nested zones but + // excluding nested traps/modals which manage their own focus scope. const focusableIds: string[] = []; for (const child of node.children) { - if ( - child.vnode.kind === "focusZone" || - child.vnode.kind === "focusTrap" || - child.vnode.kind === "modal" - ) { - continue; - } - const childFocusables = collectFocusableIdsInSubtree(child); + if (child.vnode.kind === "focusTrap" || child.vnode.kind === "modal") continue; + const childFocusables = collectFocusableIdsInTrapSubtree(child); focusableIds.push(...childFocusables); } @@ -663,6 +689,30 @@ export function collectFocusTraps(tree: RuntimeInstance): ReadonlyMap 0) { + const current = stack.pop(); + if (!current) continue; + + if (current.vnode.kind === "focusTrap" || current.vnode.kind === "modal") { + continue; + } + + const id = isFocusableInteractive(current.vnode) ? isEnabledInteractive(current.vnode) : null; + if (id !== null) out.push(id); + + for (let i = current.children.length - 1; i >= 0; i--) { + const child = current.children[i]; + if (child) stack.push(child); + } + } + + return Object.freeze(out); +} + // --------------------------------------------------------------------------- // Single-Pass Metadata Collector // --------------------------------------------------------------------------- @@ -720,6 +770,7 @@ export class WidgetMetadataCollector { private readonly _inputById = new Map(); // Zone/trap intermediate data (reused) + private readonly _focusContainerKindsById = new Map(); private readonly _zoneDataById = new Map< string, Omit & { @@ -755,6 +806,7 @@ export class WidgetMetadataCollector { this._enabledById.clear(); this._pressableIds.clear(); this._inputById.clear(); + this._focusContainerKindsById.clear(); this._zoneDataById.clear(); this._trapDataById.clear(); this._zoneFocusables.clear(); @@ -805,15 +857,22 @@ export class WidgetMetadataCollector { buildFocusInfo(vnode, focusableId, this._fieldStack), ); } - // Attribute to current container (innermost zone/trap) - if (this._containerStack.length > 0) { - const container = this._containerStack[this._containerStack.length - 1]; - if (container) { - if (container.kind === "zone") { - this._zoneFocusables.get(container.id)?.push(focusableId); - } else { - this._trapFocusables.get(container.id)?.push(focusableId); - } + // Attribute to the nearest zone and nearest enclosing trap. Focusables + // inside zones still participate in an enclosing trap's focus scope. + for (let i = this._containerStack.length - 1; i >= 0; i--) { + const container = this._containerStack[i]; + if (!container) continue; + if (container.kind === "zone") { + this._zoneFocusables.get(container.id)?.push(focusableId); + break; + } + } + for (let i = this._containerStack.length - 1; i >= 0; i--) { + const container = this._containerStack[i]; + if (!container) continue; + if (container.kind === "trap") { + this._trapFocusables.get(container.id)?.push(focusableId); + break; } } } @@ -848,6 +907,7 @@ export class WidgetMetadataCollector { id?: unknown; value?: unknown; disabled?: unknown; + readOnly?: unknown; multiline?: unknown; rows?: unknown; wordWrap?: unknown; @@ -858,6 +918,7 @@ export class WidgetMetadataCollector { const value = typeof props.value === "string" ? props.value : null; if (id !== null && value !== null && !this._inputById.has(id)) { const disabled = props.disabled === true; + const readOnly = props.readOnly === true; const multiline = props.multiline === true; const rowsRaw = typeof props.rows === "number" && Number.isFinite(props.rows) ? props.rows : 3; @@ -873,6 +934,7 @@ export class WidgetMetadataCollector { instanceId: node.instanceId, value, disabled, + readOnly, multiline, rows, wordWrap, @@ -902,7 +964,8 @@ export class WidgetMetadataCollector { }; const id = typeof props.id === "string" && props.id.length > 0 ? props.id : null; - if (id !== null && !this._zoneDataById.has(id)) { + if (id !== null) { + recordFocusContainerId(this._focusContainerKindsById, id, "focusZone"); const tabIndex = typeof props.tabIndex === "number" ? props.tabIndex : 0; const navigation = props.navigation === "linear" || @@ -961,7 +1024,8 @@ export class WidgetMetadataCollector { }; const id = typeof props.id === "string" && props.id.length > 0 ? props.id : null; - if (id !== null && !this._trapDataById.has(id)) { + if (id !== null) { + recordFocusContainerId(this._focusContainerKindsById, id, vnode.kind); const active = vnode.kind === "modal" ? true : props.active === true; const returnFocusTo = typeof props.returnFocusTo === "string" ? props.returnFocusTo : null; @@ -991,7 +1055,7 @@ export class WidgetMetadataCollector { id, Object.freeze({ ...data, - focusableIds: Object.freeze(focusables), + focusableIds: Object.freeze(focusables.slice()), }), ); } @@ -1003,19 +1067,19 @@ export class WidgetMetadataCollector { id, Object.freeze({ ...data, - focusableIds: Object.freeze(focusables), + focusableIds: Object.freeze(focusables.slice()), }), ); } return Object.freeze({ focusableIds: Object.freeze(this._focusableIds.slice()), - focusInfoById: this._focusInfoById, - enabledById: this._enabledById, - pressableIds: this._pressableIds, - inputById: this._inputById, - zones: this._zones, - traps: this._traps, + focusInfoById: new Map(this._focusInfoById), + enabledById: new Map(this._enabledById), + pressableIds: new Set(this._pressableIds), + inputById: new Map(this._inputById), + zones: new Map(this._zones), + traps: new Map(this._traps), hasRoutingWidgets, }); } diff --git a/packages/core/src/widgets/__tests__/layers.golden.test.ts b/packages/core/src/widgets/__tests__/layers.golden.test.ts index a4833fcf..f7694d38 100644 --- a/packages/core/src/widgets/__tests__/layers.golden.test.ts +++ b/packages/core/src/widgets/__tests__/layers.golden.test.ts @@ -813,8 +813,8 @@ describe("Layer ESC Routing", () => { ]), }); - assert.equal(result.closedLayerId, "modal1"); - assert.equal(closedLayer, "modal1"); + assert.equal(result.closedLayerId, undefined); + assert.equal(closedLayer, null); }); test("non-ESC key is not consumed", () => { diff --git a/packages/core/src/widgets/__tests__/modal.focus.test.ts b/packages/core/src/widgets/__tests__/modal.focus.test.ts index 31245143..fa024130 100644 --- a/packages/core/src/widgets/__tests__/modal.focus.test.ts +++ b/packages/core/src/widgets/__tests__/modal.focus.test.ts @@ -131,8 +131,8 @@ describe("modal.focus - layer escape routing", () => { ]), }); - assert.equal(result.closedLayerId, "base"); - assert.deepEqual(closed, ["base"]); + assert.equal(result.closedLayerId, undefined); + assert.deepEqual(closed, []); }); test("skips closable layer without onClose callback", () => { @@ -146,8 +146,8 @@ describe("modal.focus - layer escape routing", () => { onClose: new Map([["base", () => closed.push("base")]]), }); - assert.equal(result.closedLayerId, "base"); - assert.deepEqual(closed, ["base"]); + assert.equal(result.closedLayerId, undefined); + assert.deepEqual(closed, []); }); test("swallows onClose callback errors and still consumes", () => { @@ -165,16 +165,17 @@ describe("modal.focus - layer escape routing", () => { }); assert.equal(result.consumed, true); - assert.equal(result.closedLayerId, "modal"); + assert.equal(result.closedLayerId, undefined); + assert.ok(result.callbackError instanceof Error); }); - test("returns not consumed when no layer can close", () => { + test("returns consumed when the top layer owns escape but cannot close", () => { const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE), { layerStack: ["modal"], closeOnEscape: new Map([["modal", false]]), onClose: new Map([["modal", () => undefined]]), }); - assert.equal(result.consumed, false); + assert.equal(result.consumed, true); }); test("defaults closeOnEscape to true when map entry missing", () => { diff --git a/packages/core/src/widgets/__tests__/overlay.edge.test.ts b/packages/core/src/widgets/__tests__/overlay.edge.test.ts index 43a971d6..0a7ae8bc 100644 --- a/packages/core/src/widgets/__tests__/overlay.edge.test.ts +++ b/packages/core/src/widgets/__tests__/overlay.edge.test.ts @@ -130,7 +130,7 @@ describe("overlay.edge - layout sizing and clamping", () => { }); describe("overlay.edge - cross-overlay escape ordering", () => { - test("closeOnEscape=false lets dropdown consume escape", () => { + test("closeOnEscape=false keeps escape owned by the top layer", () => { const layerResult = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE), { layerStack: ["modal"], closeOnEscape: new Map([["modal", false]]), @@ -143,7 +143,7 @@ describe("overlay.edge - cross-overlay escape ordering", () => { selectedIndex: 0, }); - assert.equal(layerResult.consumed, false); + assert.equal(layerResult.consumed, true); assert.equal(dropdownResult.consumed, true); assert.equal(dropdownResult.shouldClose, true); }); From 4ab56900951553b01df38e52c35292c96c228443 Mon Sep 17 00:00:00 2001 From: RtlZeroMemory <58250858+RtlZeroMemory@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:39:47 +0400 Subject: [PATCH 2/5] fix(core): finish runtime lifecycle hardening --- .../__tests__/commandPaletteRouting.test.ts | 4 +-- packages/core/src/app/widgetRenderer.ts | 9 ++++- packages/core/src/runtime/commit.ts | 33 +++++-------------- packages/core/src/runtime/router/types.ts | 2 ++ .../widgets/__tests__/layers.golden.test.ts | 2 +- .../src/widgets/__tests__/modal.focus.test.ts | 2 +- 6 files changed, 23 insertions(+), 29 deletions(-) diff --git a/packages/core/src/app/__tests__/commandPaletteRouting.test.ts b/packages/core/src/app/__tests__/commandPaletteRouting.test.ts index d58414d3..53b05b56 100644 --- a/packages/core/src/app/__tests__/commandPaletteRouting.test.ts +++ b/packages/core/src/app/__tests__/commandPaletteRouting.test.ts @@ -307,7 +307,7 @@ describe("commandPalette async fetch contracts", () => { }); describe("commandPalette escape contracts in layered focus contexts", () => { - test("modal layer with closeOnEscape=false routes Escape to focused command palette", () => { + test("modal layer with closeOnEscape=false keeps Escape owned by the top layer", () => { const backend = createNoopBackend(); const renderer = new WidgetRenderer({ backend, @@ -348,7 +348,7 @@ describe("commandPalette escape contracts in layered focus contexts", () => { assert.equal(renderer.getFocusedId(), "cp"); renderer.routeEngineEvent(keyEvent(ZR_KEY_ESCAPE)); - assert.deepEqual(events, ["palette-close"]); + assert.deepEqual(events, []); }); test("modal layer with closeOnEscape=true closes layer before palette handler", () => { diff --git a/packages/core/src/app/widgetRenderer.ts b/packages/core/src/app/widgetRenderer.ts index e2aa4d57..f4cd49b0 100644 --- a/packages/core/src/app/widgetRenderer.ts +++ b/packages/core/src/app/widgetRenderer.ts @@ -2137,7 +2137,7 @@ export class WidgetRenderer { diffViewerById: this.diffViewerById, rectById: this.rectById, scrollOverrides: this.scrollOverrides, - findScrollableAncestors: (targetId) => this.findScrollableAncestors(targetId), + findNearestScrollableAncestor: (targetId) => this.findNearestScrollableAncestor(targetId), }); if (wheelRoute) return wheelRoute; @@ -2467,6 +2467,13 @@ export class WidgetRenderer { return Object.freeze([]); } + private findNearestScrollableAncestor( + targetId: string | null, + ): Readonly<{ nodeId: string; meta: LayoutOverflowMetadata }> | null { + const matches = this.findScrollableAncestors(targetId); + return matches.length > 0 ? (matches[0] ?? null) : null; + } + private applyScrollOverridesToVNode( vnode: VNode, overrides: ReadonlyMap> = this diff --git a/packages/core/src/runtime/commit.ts b/packages/core/src/runtime/commit.ts index 40b32b5e..7565f513 100644 --- a/packages/core/src/runtime/commit.ts +++ b/packages/core/src/runtime/commit.ts @@ -541,18 +541,9 @@ function themedPropsEqual(a: unknown, b: unknown): boolean { return deepEqualUnknown(ao.theme, bo.theme); } -function fragmentPropsEqual(a: unknown, b: unknown): boolean { - if (a === b) return true; - const ao = (a ?? {}) as { key?: unknown }; - const bo = (b ?? {}) as typeof ao; - return ao.key === bo.key; -} - function canFastReuseContainerSelf(prev: VNode, next: VNode): boolean { if (prev.kind !== next.kind) return false; switch (prev.kind) { - case "fragment": - return fragmentPropsEqual(prev.props, (next as typeof prev).props); case "box": return boxPropsEqual(prev.props, (next as typeof prev).props); case "row": @@ -582,9 +573,6 @@ function diagWhichPropFails(prev: VNode, next: VNode): string | undefined { }; const ap = (prev.props ?? {}) as ReuseDiagProps; const bp = (next.props ?? {}) as ReuseDiagProps; - if (prev.kind === "fragment" && ap.key !== bp.key) { - return "key"; - } if (prev.kind === "row" || prev.kind === "column") { for (const k of ["pad", "gap", "align", "justify", "items"] as const) { if (ap[k] !== bp[k]) return k; @@ -860,7 +848,6 @@ function isVNode(v: unknown): v is VNode { function commitChildrenForVNode(vnode: VNode): readonly VNode[] { if ( - vnode.kind === "fragment" || vnode.kind === "box" || vnode.kind === "row" || vnode.kind === "column" || @@ -1045,16 +1032,16 @@ function resolveCompositeChildTheme(parentTheme: Theme, vnode: VNode): Theme { return parentTheme; } -function readCompositeColorTokens(ctx: CommitCtx): ColorTokens { +function readCompositeColorTokens(ctx: CommitCtx): ColorTokens | null { const composite = ctx.composite; - if (!composite) return defaultTheme.definition.colors; + if (!composite) return null; const theme = currentCompositeTheme(ctx); - if (theme !== null) { - return composite.getColorTokens ? composite.getColorTokens(theme) : theme.definition.colors; + if (theme !== null && composite.getColorTokens) { + return composite.getColorTokens(theme); } - return composite.colorTokens ?? defaultTheme.definition.colors; + return composite.colorTokens ?? null; } type CommitCtx = Readonly<{ @@ -1072,9 +1059,9 @@ type CommitCtx = Readonly<{ composite: Readonly<{ registry: CompositeInstanceRegistry; appState: unknown; - colorTokens?: ColorTokens; + colorTokens?: ColorTokens | null; theme?: Theme; - getColorTokens?: (theme: Theme) => ColorTokens; + getColorTokens?: (theme: Theme) => ColorTokens | null; viewport?: ResponsiveViewportSnapshot; onInvalidate: (instanceId: InstanceId) => void; onUseViewport?: () => void; @@ -1187,7 +1174,6 @@ function formatNodePath(nodePath: readonly string[]): string { function isContainerVNode(vnode: VNode): boolean { return ( - vnode.kind === "fragment" || vnode.kind === "box" || vnode.kind === "row" || vnode.kind === "column" || @@ -1241,7 +1227,6 @@ function rewriteCommittedVNode(next: VNode, committedChildren: readonly VNode[]) } if ( - next.kind === "fragment" || next.kind === "box" || next.kind === "row" || next.kind === "column" || @@ -2056,9 +2041,9 @@ export function commitVNodeTree( composite?: Readonly<{ registry: CompositeInstanceRegistry; appState: unknown; - colorTokens?: ColorTokens; + colorTokens?: ColorTokens | null; theme?: Theme; - getColorTokens?: (theme: Theme) => ColorTokens; + getColorTokens?: (theme: Theme) => ColorTokens | null; viewport?: ResponsiveViewportSnapshot; onInvalidate: (instanceId: InstanceId) => void; onUseViewport?: () => void; diff --git a/packages/core/src/runtime/router/types.ts b/packages/core/src/runtime/router/types.ts index 077bdfc3..a0766df3 100644 --- a/packages/core/src/runtime/router/types.ts +++ b/packages/core/src/runtime/router/types.ts @@ -127,6 +127,8 @@ export type LayerRoutingResult = Readonly<{ closedLayerId?: string; /** Whether the event was consumed. */ consumed: boolean; + /** Error thrown by the close callback, if any. */ + callbackError?: unknown; }>; /** Dropdown routing context. */ diff --git a/packages/core/src/widgets/__tests__/layers.golden.test.ts b/packages/core/src/widgets/__tests__/layers.golden.test.ts index f7694d38..502da682 100644 --- a/packages/core/src/widgets/__tests__/layers.golden.test.ts +++ b/packages/core/src/widgets/__tests__/layers.golden.test.ts @@ -794,7 +794,7 @@ describe("Layer ESC Routing", () => { assert.equal(closedLayer, "modal2"); }); - test("ESC skips layers with closeOnEscape=false", () => { + test("ESC is owned by the top layer when closeOnEscape=false", () => { let closedLayer: string | null = null; const result = routeLayerEscape(escEvent, { diff --git a/packages/core/src/widgets/__tests__/modal.focus.test.ts b/packages/core/src/widgets/__tests__/modal.focus.test.ts index fa024130..da53b8a8 100644 --- a/packages/core/src/widgets/__tests__/modal.focus.test.ts +++ b/packages/core/src/widgets/__tests__/modal.focus.test.ts @@ -117,7 +117,7 @@ describe("modal.focus - layer escape routing", () => { assert.deepEqual(closed, ["b"]); }); - test("skips layers with closeOnEscape=false", () => { + test("top layer owns escape when closeOnEscape=false", () => { const closed: string[] = []; const result = routeLayerEscape(keyEvent(ZR_KEY_ESCAPE), { layerStack: ["base", "modal"], From eb3a0055bc13c060206167c4e974899aca9c0354 Mon Sep 17 00:00:00 2001 From: RtlZeroMemory <58250858+RtlZeroMemory@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:59:42 +0400 Subject: [PATCH 3/5] fix(core): address runtime review feedback --- .../core/src/app/__tests__/eventPump.test.ts | 38 ++++++ .../widgetRenderer.integration.test.ts | 36 ++++++ packages/core/src/app/createApp.ts | 35 ++++-- packages/core/src/app/widgetRenderer.ts | 18 ++- .../runtime/__tests__/localStateStore.test.ts | 24 ++++ .../src/runtime/__tests__/widgetMeta.test.ts | 38 ++++-- packages/core/src/runtime/commit.ts | 11 +- packages/core/src/runtime/localState.ts | 118 +++++++++++++++++- packages/core/src/runtime/widgetMeta.ts | 1 + .../widgets/__tests__/layers.golden.test.ts | 1 + 10 files changed, 287 insertions(+), 33 deletions(-) diff --git a/packages/core/src/app/__tests__/eventPump.test.ts b/packages/core/src/app/__tests__/eventPump.test.ts index 1a1e49a5..83a382fb 100644 --- a/packages/core/src/app/__tests__/eventPump.test.ts +++ b/packages/core/src/app/__tests__/eventPump.test.ts @@ -56,3 +56,41 @@ test("parse failure is fatal protocol error and still releases batch (#60/#63)", assert.equal(backend.stopCalls, 1); assert.equal(backend.disposeCalls, 1); }); + +test("faulted turn drains remaining batches without double-releasing processed batch", async () => { + const backend = new StubBackend(); + const app = createApp({ backend, initialState: 0 }); + app.draw((g) => g.clear()); + + app.onEvent((ev) => { + if (ev.kind === "engine" && ev.event.kind === "text") { + throw new Error("boom"); + } + }); + + await app.start(); + + let firstReleased = 0; + let secondReleased = 0; + backend.pushBatch( + makeBackendBatch({ + bytes: encodeZrevBatchV1({ + events: [{ kind: "text", timeMs: 1, codepoint: 65 }], + }), + onRelease: () => firstReleased++, + }), + ); + backend.pushBatch( + makeBackendBatch({ + bytes: encodeZrevBatchV1({ + events: [{ kind: "text", timeMs: 2, codepoint: 66 }], + }), + onRelease: () => secondReleased++, + }), + ); + + await flushMicrotasks(20); + + assert.equal(firstReleased, 1); + assert.equal(secondReleased, 1); +}); diff --git a/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts b/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts index 2387f291..e181e2e5 100644 --- a/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts +++ b/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts @@ -825,6 +825,42 @@ describe("WidgetRenderer integration battery", () => { assert.equal(renderer.getFocusedId(), "b"); }); + test("focusZone error reporting swallows onUserCodeError sink failures", () => { + const backend = createNoopBackend(); + const renderer = new WidgetRenderer({ + backend, + requestRender: () => {}, + onUserCodeError: () => { + throw new Error("sink-fail"); + }, + }); + + const vnode = ui.column({}, [ + ui.focusZone( + { + id: "zone-1", + onEnter: () => { + throw new Error("boom"); + }, + }, + [ui.button({ id: "a", label: "A" })], + ), + ui.focusZone({ id: "zone-2" }, [ui.button({ id: "b", label: "B" })]), + ]); + + const res = renderer.submitFrame( + () => vnode, + undefined, + { cols: 40, rows: 10 }, + defaultTheme, + noRenderHooks(), + ); + assert.ok(res.ok); + + assert.doesNotThrow(() => renderer.routeEngineEvent(keyEvent(3 /* TAB */))); + assert.equal(renderer.getFocusedId(), "a"); + }); + test("focusZone callbacks use final state after toast focus reconciliation", () => { const backend = createNoopBackend(); const renderer = new WidgetRenderer({ diff --git a/packages/core/src/app/createApp.ts b/packages/core/src/app/createApp.ts index 176787c4..ef3cc510 100644 --- a/packages/core/src/app/createApp.ts +++ b/packages/core/src/app/createApp.ts @@ -690,7 +690,10 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl getFocusInfo: () => widgetRenderer.getCurrentFocusInfo(), initialFocusedId: widgetRenderer.getFocusedId(), onHandlerError: (error: unknown) => { - fatalNowOrEnqueue("ZRUI_USER_CODE_THROW", `onFocusChange handler threw: ${describeThrown(error)}`); + fatalNowOrEnqueue( + "ZRUI_USER_CODE_THROW", + `onFocusChange handler threw: ${describeThrown(error)}`, + ); }, }); @@ -1014,11 +1017,15 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } } - function releaseOnce(batch: BackendEventBatch): () => void { + function releaseOnce( + batch: BackendEventBatch, + releasedBatches: Set, + ): () => void { let released = false; return () => { if (released) return; released = true; + releasedBatches.add(batch); try { batch.release(); } catch { @@ -1027,8 +1034,11 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl }; } - function processEventBatch(batch: BackendEventBatch): void { - const release = releaseOnce(batch); + function processEventBatch( + batch: BackendEventBatch, + releasedBatches: Set, + ): void { + const release = releaseOnce(batch, releasedBatches); const parseToken = perfMarkStart("event_parse"); const parsed = parseEventBatchV1(batch.bytes, { @@ -1488,9 +1498,10 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } } - function drainIgnored(items: readonly WorkItem[]): void { + function drainIgnored(items: readonly WorkItem[], releasedBatches: Set): void { for (const it of items) { - if (it.kind === "eventBatch") { + if (it.kind === "eventBatch" && !releasedBatches.has(it.batch)) { + releasedBatches.add(it.batch); try { it.batch.release(); } catch { @@ -1502,27 +1513,29 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl function processTurn(items: readonly WorkItem[]): void { renderRequestQueuedForCurrentTurn = false; + const releasedBatches = new Set(); const st = sm.state; if (st === "Disposed" || st === "Faulted") { - drainIgnored(items); + drainIgnored(items, releasedBatches); return; } let sawKick = false; for (const item of items) { if (sm.state === "Faulted" || sm.state === "Disposed") { - drainIgnored(items); + drainIgnored(items, releasedBatches); return; } switch (item.kind) { case "fatal": { doFatal(item.code, item.detail); - drainIgnored(items); + drainIgnored(items, releasedBatches); return; } case "eventBatch": { if (sm.state !== "Running") { + releasedBatches.add(item.batch); try { item.batch.release(); } catch { @@ -1530,9 +1543,9 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl } break; } - processEventBatch(item.batch); + processEventBatch(item.batch, releasedBatches); if (sm.state !== "Running") { - drainIgnored(items); + drainIgnored(items, releasedBatches); return; } commitUpdates(); diff --git a/packages/core/src/app/widgetRenderer.ts b/packages/core/src/app/widgetRenderer.ts index f4cd49b0..73b45e34 100644 --- a/packages/core/src/app/widgetRenderer.ts +++ b/packages/core/src/app/widgetRenderer.ts @@ -1806,6 +1806,20 @@ export class WidgetRenderer { } } + private reportFocusZoneCallbackError(phase: "onEnter" | "onExit", error: unknown): void { + const detail = `focusZone ${phase} threw: ${describeThrown(error)}`; + try { + this.reportUserCodeError(detail); + } catch (sinkError: unknown) { + const c = (globalThis as { console?: { error?: (message: string) => void } }).console; + c?.error?.( + `[rezi][runtime] onUserCodeError sink threw while reporting focusZone ${phase}: ${describeThrown( + sinkError, + )}; original=${detail}`, + ); + } + } + private invokeBlurCallbackSafely(callback: (() => void) | undefined): void { if (typeof callback !== "function") return; try { @@ -2387,7 +2401,7 @@ export class WidgetRenderer { try { prev.onExit(); } catch (error: unknown) { - this.reportUserCodeError(`focusZone onExit threw: ${describeThrown(error)}`); + this.reportFocusZoneCallbackError("onExit", error); } } } @@ -2398,7 +2412,7 @@ export class WidgetRenderer { try { next.onEnter(); } catch (error: unknown) { - this.reportUserCodeError(`focusZone onEnter threw: ${describeThrown(error)}`); + this.reportFocusZoneCallbackError("onEnter", error); } } } diff --git a/packages/core/src/runtime/__tests__/localStateStore.test.ts b/packages/core/src/runtime/__tests__/localStateStore.test.ts index 53fa49a8..86c441bb 100644 --- a/packages/core/src/runtime/__tests__/localStateStore.test.ts +++ b/packages/core/src/runtime/__tests__/localStateStore.test.ts @@ -42,6 +42,14 @@ test("virtual list state store clones measuredHeights inputs", () => { assert.notEqual(state.measuredHeights, measuredHeights); assert.equal(state.measuredHeights?.has(1), false); assert.equal(store.get("list").measuredHeights?.has(1), false); + assert.throws(() => + ( + state.measuredHeights as unknown as { + set: (key: number, value: number) => void; + } + ).set(2, 7), + ); + assert.equal(store.get("list").measuredHeights?.has(2), false); }); test("tree state store clones loading and expanded set inputs", () => { @@ -63,4 +71,20 @@ test("tree state store clones loading and expanded set inputs", () => { assert.equal(state.expandedSet?.has("child"), false); assert.equal(store.get("tree").loadingKeys.has("loading-b"), false); assert.equal(store.get("tree").expandedSet?.has("child"), false); + assert.throws(() => + ( + state.loadingKeys as unknown as { + add: (value: string) => void; + } + ).add("loading-c"), + ); + assert.throws(() => + ( + state.expandedSet as unknown as { + add: (value: string) => void; + } + ).add("grandchild"), + ); + assert.equal(store.get("tree").loadingKeys.has("loading-c"), false); + assert.equal(store.get("tree").expandedSet?.has("grandchild"), false); }); diff --git a/packages/core/src/runtime/__tests__/widgetMeta.test.ts b/packages/core/src/runtime/__tests__/widgetMeta.test.ts index 71b9e6a1..e36622c7 100644 --- a/packages/core/src/runtime/__tests__/widgetMeta.test.ts +++ b/packages/core/src/runtime/__tests__/widgetMeta.test.ts @@ -380,10 +380,9 @@ test("widgetMeta: reusable collector publishes stable snapshots across collects" ui.button({ id: "first-btn", label: "First" }), ui.input({ id: "first-input", value: "hello" }), ui.focusZone({ id: "zone-a" }, [ui.button({ id: "zone-btn", label: "Zone" })]), - ui.focusTrap( - { id: "trap-a", active: true }, - [ui.button({ id: "trap-btn", label: "Trap" })], - ), + ui.focusTrap({ id: "trap-a", active: true }, [ + ui.button({ id: "trap-btn", label: "Trap" }), + ]), ]), ), ); @@ -398,12 +397,15 @@ test("widgetMeta: reusable collector publishes stable snapshots across collects" ); assert.deepEqual(first.focusableIds, ["first-btn", "first-input", "zone-btn", "trap-btn"]); - assert.deepEqual([...first.enabledById.entries()], [ - ["first-btn", true], - ["first-input", true], - ["zone-btn", true], - ["trap-btn", true], - ]); + assert.deepEqual( + [...first.enabledById.entries()], + [ + ["first-btn", true], + ["first-input", true], + ["zone-btn", true], + ["trap-btn", true], + ], + ); assert.equal(first.pressableIds.has("first-btn"), true); assert.equal(first.pressableIds.has("second-btn"), false); assert.equal(first.inputById.get("first-input")?.value, "hello"); @@ -415,6 +417,22 @@ test("widgetMeta: reusable collector publishes stable snapshots across collects" assert.equal(second.zones.has("zone-b"), true); }); +test("widgetMeta: pooled collector does not attribute trap focusables to outer zones", () => { + const committed = commitTree( + ui.focusZone({ id: "outer-zone" }, [ + ui.button({ id: "outer-btn", label: "Outer" }), + ui.focusTrap({ id: "inner-trap", active: true }, [ + ui.button({ id: "inner-btn", label: "Inner" }), + ]), + ]), + ); + + const metadata = collectAllWidgetMetadata(committed); + + assert.deepEqual(metadata.zones.get("outer-zone")?.focusableIds, ["outer-btn"]); + assert.deepEqual(metadata.traps.get("inner-trap")?.focusableIds, ["inner-btn"]); +}); + test("widgetMeta: duplicate focus container ids throw deterministic ZrUiError", () => { const zoneVNode = ui.focusZone({ id: "dup" }, []); const trapVNode = ui.focusTrap({ id: "dup", active: true }, []); diff --git a/packages/core/src/runtime/commit.ts b/packages/core/src/runtime/commit.ts index 7565f513..78ce3294 100644 --- a/packages/core/src/runtime/commit.ts +++ b/packages/core/src/runtime/commit.ts @@ -1680,10 +1680,7 @@ function executeCompositeRender( ok: false, fatal: { code: "ZRUI_USER_CODE_THROW", - detail: - evalRes.threw instanceof Error - ? `${evalRes.threw.name}: ${evalRes.threw.message}` - : String(evalRes.threw), + detail: describeThrown(evalRes.threw), }, }; } @@ -1956,7 +1953,11 @@ function commitNode( const idFatal = ensureInteractiveId(ctx.seenInteractiveIds, instanceId, vnode); if (idFatal) return { ok: false, fatal: idFatal }; - const focusContainerFatal = ensureFocusContainerId(ctx.seenFocusContainerIds, instanceId, vnode); + const focusContainerFatal = ensureFocusContainerId( + ctx.seenFocusContainerIds, + instanceId, + vnode, + ); if (focusContainerFatal) return { ok: false, fatal: focusContainerFatal }; if (ctx.collectLifecycleInstanceIds) { diff --git a/packages/core/src/runtime/localState.ts b/packages/core/src/runtime/localState.ts index 9d1f20d4..a9b3249d 100644 --- a/packages/core/src/runtime/localState.ts +++ b/packages/core/src/runtime/localState.ts @@ -88,12 +88,120 @@ function freezeState(s: RuntimeLocalState): RuntimeLocalState { }); } +class ReadonlyMapSnapshot implements ReadonlyMap { + private readonly map: Map; + + constructor(value: ReadonlyMap) { + this.map = new Map(value); + } + + get size(): number { + return this.map.size; + } + + get(key: K): V | undefined { + return this.map.get(key); + } + + has(key: K): boolean { + return this.map.has(key); + } + + forEach(callbackfn: (value: V, key: K, map: ReadonlyMap) => void, thisArg?: unknown): void { + this.map.forEach((value, key) => callbackfn.call(thisArg, value, key, this)); + } + + entries(): MapIterator<[K, V]> { + return this.map.entries(); + } + + keys(): MapIterator { + return this.map.keys(); + } + + values(): MapIterator { + return this.map.values(); + } + + [Symbol.iterator](): MapIterator<[K, V]> { + return this.map[Symbol.iterator](); + } + + get [Symbol.toStringTag](): string { + return "Map"; + } + + set(): never { + throw new TypeError("Cannot mutate readonly map snapshot"); + } + + delete(): never { + throw new TypeError("Cannot mutate readonly map snapshot"); + } + + clear(): never { + throw new TypeError("Cannot mutate readonly map snapshot"); + } +} + +class ReadonlySetSnapshot implements ReadonlySet { + private readonly set: Set; + + constructor(value: ReadonlySet) { + this.set = new Set(value); + } + + get size(): number { + return this.set.size; + } + + has(value: T): boolean { + return this.set.has(value); + } + + forEach(callbackfn: (value: T, value2: T, set: ReadonlySet) => void, thisArg?: unknown): void { + this.set.forEach((value) => callbackfn.call(thisArg, value, value, this)); + } + + entries(): SetIterator<[T, T]> { + return this.set.entries(); + } + + keys(): SetIterator { + return this.set.keys(); + } + + values(): SetIterator { + return this.set.values(); + } + + [Symbol.iterator](): SetIterator { + return this.set[Symbol.iterator](); + } + + get [Symbol.toStringTag](): string { + return "Set"; + } + + add(): never { + throw new TypeError("Cannot mutate readonly set snapshot"); + } + + delete(): never { + throw new TypeError("Cannot mutate readonly set snapshot"); + } + + clear(): never { + throw new TypeError("Cannot mutate readonly set snapshot"); + } +} + function cloneReadonlyMap(value: ReadonlyMap): ReadonlyMap { - return new Map(value); + return Object.freeze(new ReadonlyMapSnapshot(value)); } function cloneReadonlySet(value: ReadonlySet): ReadonlySet { - return new Set(value); + return Object.freeze(new ReadonlySetSnapshot(value)); } /** Create a new local state store instance. */ @@ -319,7 +427,7 @@ export type TreeFlatCache = Readonly<{ const DEFAULT_TREE_STATE: TreeLocalState = Object.freeze({ focusedKey: null, - loadingKeys: Object.freeze(new Set()), + loadingKeys: cloneReadonlySet(new Set()), scrollTop: 0, viewportHeight: 0, flatCache: null, @@ -387,7 +495,7 @@ export function createTreeStateStore(): TreeStateStore { newLoading.add(nodeKey); const next: TreeLocalState = Object.freeze({ ...prev, - loadingKeys: Object.freeze(newLoading) as ReadonlySet, + loadingKeys: cloneReadonlySet(newLoading), }); table.set(id, next); return next; @@ -401,7 +509,7 @@ export function createTreeStateStore(): TreeStateStore { newLoading.delete(nodeKey); const next: TreeLocalState = Object.freeze({ ...prev, - loadingKeys: Object.freeze(newLoading) as ReadonlySet, + loadingKeys: cloneReadonlySet(newLoading), }); table.set(id, next); return next; diff --git a/packages/core/src/runtime/widgetMeta.ts b/packages/core/src/runtime/widgetMeta.ts index 2fd085a5..23fdd42a 100644 --- a/packages/core/src/runtime/widgetMeta.ts +++ b/packages/core/src/runtime/widgetMeta.ts @@ -862,6 +862,7 @@ export class WidgetMetadataCollector { for (let i = this._containerStack.length - 1; i >= 0; i--) { const container = this._containerStack[i]; if (!container) continue; + if (container.kind === "trap") break; if (container.kind === "zone") { this._zoneFocusables.get(container.id)?.push(focusableId); break; diff --git a/packages/core/src/widgets/__tests__/layers.golden.test.ts b/packages/core/src/widgets/__tests__/layers.golden.test.ts index 502da682..8b9002fc 100644 --- a/packages/core/src/widgets/__tests__/layers.golden.test.ts +++ b/packages/core/src/widgets/__tests__/layers.golden.test.ts @@ -814,6 +814,7 @@ describe("Layer ESC Routing", () => { }); assert.equal(result.closedLayerId, undefined); + assert.equal(result.consumed, true); assert.equal(closedLayer, null); }); From 67d04690442d124ab4f49fa856aea67319f831ff Mon Sep 17 00:00:00 2001 From: RtlZeroMemory <58250858+RtlZeroMemory@users.noreply.github.com> Date: Fri, 6 Mar 2026 14:15:08 +0400 Subject: [PATCH 4/5] fix(core): address final runtime review nits --- .../__tests__/widgetRenderer.integration.test.ts | 6 ++++++ packages/core/src/app/createApp.ts | 3 ++- packages/core/src/app/widgetRenderer.ts | 15 ++++++++++----- .../core/src/runtime/__tests__/widgetMeta.test.ts | 3 ++- packages/core/src/runtime/localState.ts | 8 ++++++-- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts b/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts index e181e2e5..19a20adb 100644 --- a/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts +++ b/packages/core/src/app/__tests__/widgetRenderer.integration.test.ts @@ -827,10 +827,13 @@ describe("WidgetRenderer integration battery", () => { test("focusZone error reporting swallows onUserCodeError sink failures", () => { const backend = createNoopBackend(); + let onEnterCalls = 0; + let onUserCodeErrorCalls = 0; const renderer = new WidgetRenderer({ backend, requestRender: () => {}, onUserCodeError: () => { + onUserCodeErrorCalls++; throw new Error("sink-fail"); }, }); @@ -840,6 +843,7 @@ describe("WidgetRenderer integration battery", () => { { id: "zone-1", onEnter: () => { + onEnterCalls++; throw new Error("boom"); }, }, @@ -859,6 +863,8 @@ describe("WidgetRenderer integration battery", () => { assert.doesNotThrow(() => renderer.routeEngineEvent(keyEvent(3 /* TAB */))); assert.equal(renderer.getFocusedId(), "a"); + assert.equal(onEnterCalls, 1); + assert.equal(onUserCodeErrorCalls, 1); }); test("focusZone callbacks use final state after toast focus reconciliation", () => { diff --git a/packages/core/src/app/createApp.ts b/packages/core/src/app/createApp.ts index ef3cc510..0ba78092 100644 --- a/packages/core/src/app/createApp.ts +++ b/packages/core/src/app/createApp.ts @@ -31,6 +31,7 @@ import { FRAME_ACCEPTED_ACK_MARKER, type RuntimeBackend, } from "../backend.js"; +import { describeThrown } from "../debug/describeThrown.js"; import type { UiEvent, ZrevEvent } from "../events.js"; import type { BindingMap, @@ -55,7 +56,6 @@ import { normalizeBreakpointThresholds, } from "../layout/responsive.js"; import type { Rect } from "../layout/types.js"; -import { describeThrown } from "../debug/describeThrown.js"; import { PERF_ENABLED, perfMarkEnd, perfMarkStart, perfNow, perfRecord } from "../perf/perf.js"; import type { EventTimeUnwrapState } from "../protocol/types.js"; import { parseEventBatchV1 } from "../protocol/zrev_v1.js"; @@ -641,6 +641,7 @@ export function createApp(opts: CreateAppStateOptions | CreateAppRoutesOnl // Theme-aware composite widgets resolve recipe styles during commit, so // transition frames must invalidate view/commit, not only render. markDirty(DIRTY_VIEW, false); + renderRequestQueuedForCurrentTurn = true; scheduler.enqueue({ kind: "renderRequest" }); } diff --git a/packages/core/src/app/widgetRenderer.ts b/packages/core/src/app/widgetRenderer.ts index 73b45e34..1e9372c9 100644 --- a/packages/core/src/app/widgetRenderer.ts +++ b/packages/core/src/app/widgetRenderer.ts @@ -2434,7 +2434,7 @@ export class WidgetRenderer { { runtimeNode: this.committedRoot, layoutNode: this.layoutTree, - scrollables: Object.freeze([]), + scrollables: [], }, ]; @@ -2457,12 +2457,12 @@ export class WidgetRenderer { const hasScrollableAxis = meta.contentWidth > meta.viewportWidth || meta.contentHeight > meta.viewportHeight; if (hasScrollableAxis) { - scrollables = Object.freeze([...scrollables, { nodeId, meta }]); + scrollables = [...scrollables, { nodeId, meta }]; } } if (nodeId === targetId) { - return Object.freeze([...scrollables].reverse()); + return Object.freeze(scrollables.slice().reverse()); } const childCount = Math.min(runtimeNode.children.length, layoutNode.children.length); @@ -4016,10 +4016,15 @@ export class WidgetRenderer { // constraints can converge one depth level at a time. if (constraintGraph !== null && constraintGraph.nodes.length > 0) { let settlePasses = 0; + const MAX_CONSTRAINT_SETTLE_PASSES = 256; // Nested parent/intrinsic chains can converge one dependency level at a time. // Use the graph size instead of an arbitrary small cap so first-frame layout - // can fully settle for deep but valid trees. - const maxSettlePasses = Math.max(3, constraintGraph.nodes.length + 1); + // can fully settle for deep but valid trees, but bound worst-case synchronous + // frame time for pathological graphs and emit an audit signal if we hit the cap. + const maxSettlePasses = Math.min( + MAX_CONSTRAINT_SETTLE_PASSES, + Math.max(3, constraintGraph.nodes.length + 1), + ); while (settlePasses < maxSettlePasses) { buildLayoutRectIndexes( nextLayoutTree, diff --git a/packages/core/src/runtime/__tests__/widgetMeta.test.ts b/packages/core/src/runtime/__tests__/widgetMeta.test.ts index e36622c7..7fc30dc8 100644 --- a/packages/core/src/runtime/__tests__/widgetMeta.test.ts +++ b/packages/core/src/runtime/__tests__/widgetMeta.test.ts @@ -427,7 +427,8 @@ test("widgetMeta: pooled collector does not attribute trap focusables to outer z ]), ); - const metadata = collectAllWidgetMetadata(committed); + const collector = createWidgetMetadataCollector(); + const metadata = collector.collect(committed); assert.deepEqual(metadata.zones.get("outer-zone")?.focusableIds, ["outer-btn"]); assert.deepEqual(metadata.traps.get("inner-trap")?.focusableIds, ["inner-btn"]); diff --git a/packages/core/src/runtime/localState.ts b/packages/core/src/runtime/localState.ts index a9b3249d..f7511198 100644 --- a/packages/core/src/runtime/localState.ts +++ b/packages/core/src/runtime/localState.ts @@ -108,7 +108,9 @@ class ReadonlyMapSnapshot implements ReadonlyMap { } forEach(callbackfn: (value: V, key: K, map: ReadonlyMap) => void, thisArg?: unknown): void { - this.map.forEach((value, key) => callbackfn.call(thisArg, value, key, this)); + for (const [key, value] of this.map) { + callbackfn.call(thisArg, value, key, this); + } } entries(): MapIterator<[K, V]> { @@ -160,7 +162,9 @@ class ReadonlySetSnapshot implements ReadonlySet { } forEach(callbackfn: (value: T, value2: T, set: ReadonlySet) => void, thisArg?: unknown): void { - this.set.forEach((value) => callbackfn.call(thisArg, value, value, this)); + for (const value of this.set) { + callbackfn.call(thisArg, value, value, this); + } } entries(): SetIterator<[T, T]> { From ff0b22ecdac3e3c7902154db01ad13ca403a8e57 Mon Sep 17 00:00:00 2001 From: RtlZeroMemory <58250858+RtlZeroMemory@users.noreply.github.com> Date: Fri, 6 Mar 2026 14:17:58 +0400 Subject: [PATCH 5/5] fix(core): sort lifecycle error imports --- packages/core/src/app/createApp/topLevelViewError.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/app/createApp/topLevelViewError.ts b/packages/core/src/app/createApp/topLevelViewError.ts index a0c3ab2b..79ec61a0 100644 --- a/packages/core/src/app/createApp/topLevelViewError.ts +++ b/packages/core/src/app/createApp/topLevelViewError.ts @@ -1,5 +1,5 @@ -import type { ZrevEvent } from "../../events.js"; import { describeThrown } from "../../debug/describeThrown.js"; +import type { ZrevEvent } from "../../events.js"; import { ZR_MOD_ALT, ZR_MOD_CTRL, ZR_MOD_META, ZR_MOD_SHIFT } from "../../keybindings/keyCodes.js"; import type { VNode } from "../../widgets/types.js"; import { ui } from "../../widgets/ui.js";