fix(runtime): make exit() first-call-wins (Ink parity, G33) (#55)
The first exit() call now captures its value/error and initiates teardown synchronously; subsequent exit() calls are complete no-ops, so waitUntilExit resolves/rejects with the FIRST value rather than the last. This mirrors Ink's handleAppExit guard (isUnmounted || isUnmounting → early return). Previously each exit() queued a microtask that overwrote pendingExitResult/ pendingExitError before resolveExit ran, making it last-wins. An exitInitiated flag set at the top of exit() now guards the value capture and re-resolve, while the deferred microtask teardown (needed because exit() is called from inside the Vue update cycle) is preserved. Reverses the sweep-1 refutation: exit() was NOT already first-wins guarded. Also guard unmount-in-progress (isUnmounting parity) + value→error test. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Yunfei He
@@ -29,6 +29,7 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor
|
||||
- **2026-05-30 — G02 (useAnimation throttle):** to actually fix the gap by default (not just when a caller passes maxFps), `maxFps` now defaults to **30** (Ink: `options.maxFps ?? 30`) and a single `renderThrottleMs = ceil(1000/maxFps)` feeds BOTH the commit scheduler and `createAnimationScheduler`, mirroring Ink's one-value architecture. Behavior change: the default commit-throttle shifts 32ms→34ms (no test depended on 32ms; explicit `maxFps` already drove commit cadence on main). Test rewrite: the pre-existing `"delta accounts for throttled ticks"` test was weak/self-contradictory (asserted only `delta>0`, passed trivially against the bug) — replaced with `"delta accumulates across coalesced ticks…"` (maxFps:5, 200ms window) plus a default-path test using a count-based assertion (≈31 rendered ticks unfixed vs <15 fixed) so it deterministically discriminates. Both verified red on unfixed.
|
||||
- **2026-05-30 — G05+G15 (border geometry):** rewrote `paint.ts` drawBorder to draw each edge independently (mirroring Ink render-border.ts): removed the blanket `w<2||h<2` early-return (→ `w<1||h<1`) and changed the vertical-side loop to start at `offsetY = top?1:0` with run length `max(0, h - visibleTop - visibleBottom)`. Per conflict policy, 4 existing snapshot tests that encoded the buggy hide-top/bottom output were updated to Ink-correct values (rails on the content row, not shifted/missing) — codex traced each against Ink render-border.ts and confirmed they match (not just code-blessed). Normal (top+bottom) boxes are byte-identical to before.
|
||||
- **2026-05-30 — G07 → CANDIDATE (needs human decision, not auto-fixed):** Ink genuinely does NOT exit on kitty-protocol Ctrl+C (`use-input.ts:245-247` only returns; the `\x03` exit path never fires under kitty). vue-tui DOES exit on kitty Ctrl+C (useInput.ts:100-105 calls app.exit()). But vue-tui's behavior is arguably BETTER UX (Ctrl+C exits under all protocols) and Ink's non-exit looks like a kitty-era oversight, not a deliberate design. Rather than auto-remove a working Ctrl+C-exits behavior to match an Ink limitation, appended it to [[ink-parity]]'s 'Candidate intentional divergences' section for the maintainer to decide (keep & allowlist, or match Ink). No code change.
|
||||
- **2026-05-30 — G33 (exit() first-wins) — REVERSES sweep-1 refutation + test rewrite:** sweep-1 wrongly refuted exit()-second-wins ('already guarded'); sweep-4 + a red test confirmed vue-tui was LAST-wins. Added an `exitInitiated || teardownStarted` guard (Ink's `isUnmounted || isUnmounting` first-wins). Per conflict policy, 3 existing exit tests that ASSERTED the last-wins bug (resolving 'second', with comments noting Ink does first-wins) were rewritten to assert first-wins ('first') — they had documented the divergence; now aligned to Ink.
|
||||
- **2026-05-30 — G23 (SR <Transform>) — spec corrected via empirical Ink check:** the sweep-2 finding claimed Ink applies the Transform's fn to its squashed SR children. Building Ink from source and tracing squash-text-nodes.ts showed Ink only applies `internal_transform` of CHILD nodes, never the top-level node handed to squashTextNodes — so a `<Transform>` directly under a `<Box>` outputs its children CONCATENATED with no transform. Only the `\n`→`""` join was a real bug. Fixed to match Ink (concat, no top-level transform); applying it would have DIVERGED. (Per align-with-Ink; codex independently confirmed.)
|
||||
- **2026-05-29 — G06 REFUTED (false positive from the audit):** the audit claimed `<Transform>`'s fn gets a hardcoded index `0` "instead of the childNode index". Re-verification against Ink `output.ts:230-239` shows Ink's index is the **line index** (transformers apply per output line: `transformer(line, index)`), not a child index — the audit misread it. vue-tui **already** applies per-line line indices for multi-line (block) transforms via the yoga-carrier path: the existing tests `transform with multiple lines` → `[0: hello world]\n[1: goodbye world]` and transform-yoga `[0: hello]\n[1: world]` pass on unmodified code. `paint.ts:314`'s `transform(innerText, 0)` is only the inline `<Transform>`-inside-`<Text>` path, whose content is a single logical line where `0` matches Ink (all inline tests assert `[0: …]`). No observable gap; not fixed.
|
||||
|
||||
@@ -69,8 +70,8 @@ Non-obvious calls made while fixing gaps, recorded for review in the final repor
|
||||
| G29 | render-lifecycle-reconciler | useCursor()/setCursorPosition never applied during normal render commits (only after console writes) | P3 | todo | — | — |
|
||||
| G30 | app-exit-instances-animation-sr | SR: nested <Transform> inside a box-level <Transform> drops the INNER transform fn (refines G23) | P3 | todo | — | — |
|
||||
| G31 | app-exit-instances-animation-sr | useAnimation `interval` option is not reactive; Ink re-subscribes+resets when interval changes (cf. G08 id-reactivity) | P3 | todo | — | — |
|
||||
| G32 | text-wrap-transform | <Transform> nested directly inside another <Transform> (in <Text>) is silently DROPPED — paint+measure lose all content | P1 | todo | — | — |
|
||||
| G33 | app-exit-instances-animation-sr | exit() resolves last-call-wins; Ink is first-call-wins (REVERSES the sweep-1 exit()-second-wins refutation) | P1 | todo | — | — |
|
||||
| G32 | text-wrap-transform | <Transform> nested directly inside another <Transform> (in <Text>) is silently DROPPED — paint+measure lose all content | P1 | merged | `fix/parity-transform-nesting` | #54 |
|
||||
| G33 | app-exit-instances-animation-sr | exit() resolves last-call-wins; Ink is first-call-wins (REVERSES the sweep-1 exit()-second-wins refutation) | P1 | pr-open | `fix/parity-exit-first-wins` | #55 |
|
||||
| G34 | box-layout-border | Box border glyphs + bg fill are subject to ancestor <Transform> transformers; Ink renders chrome with empty transformer list | P3 | todo | — | — |
|
||||
| G35 | static-newline-spacer | <Static> container's own borderStyle/backgroundColor (non-yoga visual style) not painted | P3 | todo | — | — |
|
||||
| G36 | focus | useFocus autoFocus prop not reactive (captured once); Ink re-registers on autoFocus change | P3 | todo | — | — |
|
||||
|
||||
Reference in New Issue
Block a user