fix(runtime): unknown borderStyle string throws (Ink parity) (#124)
An unknown borderStyle name (reachable only via a TS-bypass — the prop type is the cli-boxes keyof union) silently degraded to no border (and wrongly reserved a 1-cell inset). Ink throws a TypeError on it. Align to that "throw on unknown" contract by validating in the Box component's render, so the throw is caught by vue-tui's error boundary (onErrorCaptured -> ErrorOverview), like any other render error — rather than in paint, where a throw would unwind through Vue's post-flush commit and wedge the scheduler. The check resolves cliBoxes[borderStyle] and throws unless it's a genuine BoxStyle (an object with a string `top` glyph), so unknown names, the cli-boxes CJS-interop `default` self-key, and inherited prototype names (toString, constructor) all throw; valid names, false/undefined (no border), and a custom BoxStyle object are unaffected. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1385,6 +1385,141 @@ test("G16: per-edge borderDimColor=false overrides general borderDimColor", asyn
|
||||
expect(bottomLine).toContain("[2m");
|
||||
});
|
||||
|
||||
// audit 2.3 — an UNKNOWN borderStyle string THROWS (Ink parity), rather than
|
||||
// silently degrading to no border (the old vue-tui behavior, which also wrongly
|
||||
// reserved a 1-cell inset). Ink's render-border.ts has no existence check: it
|
||||
// reads box.topLeft/box.top off `cliBoxes[name]` === undefined and crashes with a
|
||||
// TypeError. We align by throwing a clear, descriptive Error — but do it in the
|
||||
// Box component's RENDER (Box.ts), so the throw is caught by vue-tui's existing
|
||||
// error boundary (onErrorCaptured → ErrorOverview → exit), exactly like any other
|
||||
// component render error. paint.ts stays a silent `if (!chars) return` fallback
|
||||
// (a raw throw in the post-flush commit would wedge Vue's scheduler).
|
||||
//
|
||||
// OBSERVED behavior: the error boundary routes the thrown Error through exit(),
|
||||
// which REJECTS waitUntilExit(); @vue-tui/testing's render() surfaces that
|
||||
// rejection (its earlyError detector rethrows before returning a result), so
|
||||
// render() itself REJECTS with the message. The ErrorOverview frame is rendered
|
||||
// internally but never reaches the caller — render() never resolves a result.
|
||||
// (TS-bypass via `as any`: the public prop type is the cli-boxes keyof union, so
|
||||
// an unknown name is reachable only by escaping the type system.)
|
||||
test("UNKNOWN borderStyle throws (Ink parity)", async ({ expect }) => {
|
||||
await expect(
|
||||
render(
|
||||
defineComponent(() => () => (
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
<Box borderStyle={"definitely-not-a-real-style" as any} alignSelf="flex-start">
|
||||
<Text>Hi</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
),
|
||||
).rejects.toThrow(/Unknown borderStyle/);
|
||||
});
|
||||
|
||||
// A bare `in cliBoxes` membership check has two false-accept holes that these
|
||||
// guard against:
|
||||
// 1. cli-boxes' CJS-interop `default` self-key — `"default" in cliBoxes` is true,
|
||||
// but cliBoxes.default is the WHOLE boxes object, not a BoxStyle (no string
|
||||
// `.top`). Paint would then read `.top`/`.topLeft` off it → garbage/crash.
|
||||
// 2. `in` walks the prototype chain, so Object.prototype members like
|
||||
// "toString"/"constructor"/"hasOwnProperty" report as "in cliBoxes" while
|
||||
// resolving to a function/undefined — never a real BoxStyle.
|
||||
// A shape check (resolved value is an object with a string `top`) rejects all of
|
||||
// these. Each name must THROW exactly like any other unknown style.
|
||||
for (const badName of ["default", "toString", "constructor", "hasOwnProperty"]) {
|
||||
test(`borderStyle ${JSON.stringify(badName)} (in-cliBoxes false-accept) throws`, async ({
|
||||
expect,
|
||||
}) => {
|
||||
await expect(
|
||||
render(
|
||||
defineComponent(() => () => (
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
<Box borderStyle={badName as any} alignSelf="flex-start">
|
||||
<Text>Hi</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
),
|
||||
).rejects.toThrow(/Unknown borderStyle/);
|
||||
});
|
||||
}
|
||||
|
||||
// Guard the negative cases that MUST NOT throw — only a non-empty unknown STRING
|
||||
// does. The Box-render check skips any falsy borderStyle (false/undefined/"" =
|
||||
// "no border"), every valid cli-boxes preset name, and a custom BoxStyle OBJECT
|
||||
// (Ink types borderStyle as `keyof Boxes | BoxStyle`). We assert each renders
|
||||
// normally (no rejection from render()) and that a valid style/object actually
|
||||
// draws border glyphs while a falsy one draws none. (`false` is reachable only
|
||||
// via a TS-bypass — Ink's type has no `false`.)
|
||||
test("non-throwing borderStyle (false/undefined/valid/object) renders normally", async ({
|
||||
expect,
|
||||
}) => {
|
||||
const falseResult = await render(
|
||||
defineComponent(() => () => (
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
<Box borderStyle={false as any} alignSelf="flex-start">
|
||||
<Text>Hi</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
const falseFrame = stripAnsi(falseResult.lastFrame()!);
|
||||
expect(falseFrame).toContain("Hi");
|
||||
// No box-drawing glyphs of any border style appear.
|
||||
expect(falseFrame).not.toMatch(/[╭╮╰╯─│┌┐└┘╔╗╚╝═║↘↗↖↙]/);
|
||||
|
||||
const undefinedResult = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box borderStyle={undefined} alignSelf="flex-start">
|
||||
<Text>Hi</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
const undefinedFrame = stripAnsi(undefinedResult.lastFrame()!);
|
||||
expect(undefinedFrame).toContain("Hi");
|
||||
expect(undefinedFrame).not.toMatch(/[╭╮╰╯─│┌┐└┘╔╗╚╝═║↘↗↖↙]/);
|
||||
|
||||
// A valid preset name renders its border (here `round` → ╭╮╰╯), no throw.
|
||||
const validResult = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box borderStyle="round" alignSelf="flex-start">
|
||||
<Text>Hi</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
const validFrame = stripAnsi(validResult.lastFrame()!);
|
||||
expect(validFrame).toContain("Hi");
|
||||
expect(validFrame).toMatch(/[╭╮╰╯]/);
|
||||
|
||||
// A custom BoxStyle OBJECT is valid (Ink parity G13) and must NOT throw — the
|
||||
// check only fires for an unknown STRING. The object's own glyphs render.
|
||||
const customStyle = {
|
||||
topLeft: "A",
|
||||
top: "B",
|
||||
topRight: "C",
|
||||
left: "D",
|
||||
right: "E",
|
||||
bottomLeft: "F",
|
||||
bottom: "G",
|
||||
bottomRight: "H",
|
||||
};
|
||||
const objectResult = await render(
|
||||
defineComponent(() => () => (
|
||||
<Box borderStyle={customStyle} alignSelf="flex-start">
|
||||
<Text>Hi</Text>
|
||||
</Box>
|
||||
)),
|
||||
{ columns: 100 },
|
||||
);
|
||||
const objectFrame = stripAnsi(objectResult.lastFrame()!);
|
||||
expect(objectFrame).toContain("Hi");
|
||||
// Corner glyphs from the custom object are drawn.
|
||||
expect(objectFrame).toContain("A");
|
||||
expect(objectFrame).toContain("H");
|
||||
});
|
||||
|
||||
// borderDimColor should not dim styled child Text touching left edge
|
||||
test("borderDimColor does not dim styled child Text touching left edge", async ({ expect }) => {
|
||||
const { lastFrame } = await render(
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { defineComponent, h, inject, type ExtractPublicPropTypes, type PropType } from "vue";
|
||||
import type cliBoxes from "cli-boxes";
|
||||
import cliBoxes from "cli-boxes";
|
||||
import { AppContextKey } from "../context.ts";
|
||||
import type { WithChildren } from "./with-children.ts";
|
||||
|
||||
@@ -170,6 +170,44 @@ const BoxImpl = defineComponent({
|
||||
return null;
|
||||
}
|
||||
|
||||
// Validate borderStyle during RENDER so an unknown name is caught by
|
||||
// vue-tui's error boundary (onErrorCaptured → ErrorOverview), exactly like
|
||||
// any other component render error. Ink crashes on an unknown borderStyle
|
||||
// with a raw TypeError during paint (render-border.ts reads box.topLeft off
|
||||
// cliBoxes[name] === undefined); we align to that "throw on unknown" contract
|
||||
// but do it here rather than in paint, where a throw would unwind through
|
||||
// Vue's post-flush commit and wedge its scheduler. Only a NON-EMPTY unknown
|
||||
// STRING throws: a falsy value (false/undefined/"" = no border) and a custom
|
||||
// BoxStyle OBJECT are both valid and pass through. (audit 2.3)
|
||||
// `borderStyle.length > 0` (not `!== ""`): once `typeof === "string"` narrows
|
||||
// the prop to the BorderStyle keyof union, an `!== ""` literal comparison has
|
||||
// "no overlap" per TS (the union has no `""` member). The empty string is only
|
||||
// reachable via a TS-bypass; `.length > 0` excludes it without tripping that.
|
||||
//
|
||||
// We validate the RESOLVED box has a real BoxStyle SHAPE rather than testing
|
||||
// `borderStyle in cliBoxes`, because `in` has two false-accept holes that let
|
||||
// a non-box value reach paint (which then reads `.top`/`.topLeft` glyph
|
||||
// strings off it):
|
||||
// 1. cli-boxes' default export carries a CJS-interop `default` self-key, so
|
||||
// `"default" in cliBoxes` is true — but cliBoxes.default is the WHOLE
|
||||
// boxes object, not a BoxStyle (it has no string `top`).
|
||||
// 2. `in` walks the prototype chain, so Object.prototype members
|
||||
// ("toString", "constructor", "hasOwnProperty", …) report as "in
|
||||
// cliBoxes" while resolving to a function/undefined — never a BoxStyle.
|
||||
// Resolving `cliBoxes[name]` and requiring an object with a string `top`
|
||||
// rejects unknown names (undefined), "default" (whole object, no string
|
||||
// `top`), and inherited props, while accepting every real preset.
|
||||
const borderStyle = props.borderStyle;
|
||||
if (typeof borderStyle === "string" && borderStyle.length > 0) {
|
||||
// Cast via `unknown` to add a string index signature: cliBoxes is typed as
|
||||
// the `Boxes` keyof object (no index signature), so a direct cast is a TS2352
|
||||
// "insufficient overlap" error. Same cast paint.ts uses to look up by name.
|
||||
const resolved = (cliBoxes as unknown as Record<string, BoxStyle | undefined>)[borderStyle];
|
||||
if (typeof resolved !== "object" || resolved === null || typeof resolved.top !== "string") {
|
||||
throw new Error(`Unknown borderStyle: ${JSON.stringify(borderStyle)}`);
|
||||
}
|
||||
}
|
||||
|
||||
const ariaLabel = props.ariaLabel;
|
||||
const label = ariaLabel ? h("text", null, ariaLabel) : undefined;
|
||||
|
||||
|
||||
@@ -472,6 +472,12 @@ function drawBorder(
|
||||
typeof style === "string"
|
||||
? (cliBoxes as unknown as Record<string, BoxStyle | undefined>)[style]
|
||||
: style;
|
||||
// Defensive internal fallback: an unknown borderStyle name has no entry in
|
||||
// cliBoxes, so silently draw no border rather than throw. This is unreachable
|
||||
// via the public API — the Box component validates an unknown non-empty
|
||||
// borderStyle string during render and throws there (caught by vue-tui's error
|
||||
// boundary), so paint never sees an invalid name. A raw throw HERE would unwind
|
||||
// through Vue's post-flush commit and wedge its internal flush state. (audit 2.3)
|
||||
if (!chars) return;
|
||||
// No blanket min-size guard here — each edge is drawn independently when it is
|
||||
// visible and its run length is ≥ 1. This matches Ink's render-border.ts which
|
||||
|
||||
Reference in New Issue
Block a user