fix(runtime): validate invalid foreground color props
This commit is contained in:
@@ -576,15 +576,16 @@ different runtime behavior, ownership rule, or out-of-contract handling.
|
|||||||
|
|
||||||
### Invalid input is validated at the component layer, not the paint layer
|
### Invalid input is validated at the component layer, not the paint layer
|
||||||
|
|
||||||
- **Principle:** vue-tui validates invalid render input (a chalk-**modifier**
|
- **Principle:** vue-tui validates the covered invalid render inputs — a
|
||||||
`backgroundColor` like `"bold"`, an unknown `borderStyle`) at the
|
chalk-**modifier** `backgroundColor` like `"bold"`, a foreground color key that exists
|
||||||
|
on chalk but is not callable like `"level"`, and an unknown `borderStyle` — at the
|
||||||
**component-render layer** (`box.ts` / `text.ts`), not down at the **paint layer**. A bad
|
**component-render layer** (`box.ts` / `text.ts`), not down at the **paint layer**. A bad
|
||||||
value therefore throws where the **error boundary** catches it -> `ErrorOverview` -> a
|
value therefore throws where the **error boundary** catches it -> `ErrorOverview` -> a
|
||||||
clean `reject` of `waitUntilExit()`, exactly like any other component error. The app
|
clean `reject` of `waitUntilExit()`, exactly like any other component error. The app
|
||||||
reports the error instead of crashing.
|
reports the error instead of crashing.
|
||||||
- **Ink:** validates the same inputs **lazily at paint** (`colorize` / `render-border`, run
|
- **Ink:** validates the same covered inputs **lazily at paint** (`colorize` /
|
||||||
from the reconciler's commit hook): **outside** React's ErrorBoundary, so a bad value is
|
`render-border`, run from the reconciler's commit hook): **outside** React's
|
||||||
an uncaught crash, not a recoverable error.
|
ErrorBoundary, so a bad value is an uncaught crash, not a recoverable error.
|
||||||
- **Why:** the key constraint is where paint runs. vue-tui's paint runs in a Vue
|
- **Why:** the key constraint is where paint runs. vue-tui's paint runs in a Vue
|
||||||
**post-flush callback** (`queuePostFlushCb`, decoupled from render), so a throw there
|
**post-flush callback** (`queuePostFlushCb`, decoupled from render), so a throw there
|
||||||
escapes `onErrorCaptured` and wedges the scheduler. Unlike a component error, it cannot
|
escapes `onErrorCaptured` and wedges the scheduler. Unlike a component error, it cannot
|
||||||
@@ -601,12 +602,13 @@ different runtime behavior, ownership rule, or out-of-contract handling.
|
|||||||
that bypasses component validation), so it stands on that prior record, not an in-audit
|
that bypasses component validation), so it stands on that prior record, not an in-audit
|
||||||
reproduction.
|
reproduction.
|
||||||
- **Cost:** the component-layer check is eager (no paint-time layout/squash info), so it
|
- **Cost:** the component-layer check is eager (no paint-time layout/squash info), so it
|
||||||
over-throws in a few degenerate, invalid-input-only cases Ink never reaches. Realistic
|
over-throws in a few degenerate, invalid-input-only cases Ink never reaches. For the
|
||||||
inputs match Ink; both error on bad input. Only the channel (recoverable reject vs crash)
|
covered public inputs in normal reachable cases, both libraries error; only the channel
|
||||||
differs. Maintainer decision (2026-06-07): KEEP. vue-tui makes the more reliable
|
(recoverable reject vs crash) differs. Maintainer decision (2026-06-07): KEEP. vue-tui
|
||||||
library choice here: reject the same invalid input with a recoverable, prop-specific
|
makes the more reliable library choice here: reject the same invalid input with a
|
||||||
error instead of preserving Ink's lower-level paint crash and chalk implementation
|
recoverable, prop-specific error instead of preserving Ink's lower-level paint crash and
|
||||||
message. Tests: `background-color.test.tsx`, plus the `borderStyle` validation tests.
|
chalk implementation message. Tests: `background-color.test.tsx`, plus the `borderStyle`
|
||||||
|
validation tests.
|
||||||
|
|
||||||
## Non-Behavioral Notes
|
## Non-Behavioral Notes
|
||||||
|
|
||||||
|
|||||||
@@ -875,6 +875,46 @@ test('foreground color="bold" (a chalk modifier) bolds, does NOT throw', async (
|
|||||||
expect(lastFrame()).toContain("\x1b[1m");
|
expect(lastFrame()).toContain("\x1b[1m");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Foreground colors have their own invalid named-color edge: some chalk keys
|
||||||
|
// exist but are not callable color/modifier methods. Ink calls chalk[color] in
|
||||||
|
// that case and crashes; vue-tui validates during render so the app rejects
|
||||||
|
// through ErrorOverview instead of letting paint wedge the scheduler.
|
||||||
|
test('foreground color="level" throws (Ink parity, recoverable channel)', async ({ expect }) => {
|
||||||
|
await expect(
|
||||||
|
render(
|
||||||
|
defineComponent(() => () => (
|
||||||
|
<Box alignSelf="flex-start">
|
||||||
|
<Text color="level">Hi</Text>
|
||||||
|
</Box>
|
||||||
|
)),
|
||||||
|
{ columns: 100 },
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/color/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
for (const borderColorProp of [
|
||||||
|
"borderColor",
|
||||||
|
"borderTopColor",
|
||||||
|
"borderBottomColor",
|
||||||
|
"borderLeftColor",
|
||||||
|
"borderRightColor",
|
||||||
|
] as const) {
|
||||||
|
test(`<Box ${borderColorProp}="level"> throws (Ink parity, recoverable channel)`, async ({
|
||||||
|
expect,
|
||||||
|
}) => {
|
||||||
|
await expect(
|
||||||
|
render(
|
||||||
|
defineComponent(() => () => (
|
||||||
|
<Box borderStyle="single" {...{ [borderColorProp]: "level" }} alignSelf="flex-start">
|
||||||
|
<Text>Hi</Text>
|
||||||
|
</Box>
|
||||||
|
)),
|
||||||
|
{ columns: 100 },
|
||||||
|
),
|
||||||
|
).rejects.toThrow(/color/i);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// --- A12 gating: vue must throw WHERE Ink colorizes, and NOT elsewhere ---
|
// --- A12 gating: vue must throw WHERE Ink colorizes, and NOT elsewhere ---
|
||||||
//
|
//
|
||||||
// Ink throws on a chalk-modifier-name bg LAZILY — only when it actually
|
// Ink throws on a chalk-modifier-name bg LAZILY — only when it actually
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { defineComponent, h, inject, type ExtractPublicPropTypes, type PropType } from "vue";
|
import { defineComponent, h, inject, type ExtractPublicPropTypes, type PropType } from "vue";
|
||||||
import cliBoxes from "cli-boxes";
|
import cliBoxes from "cli-boxes";
|
||||||
import { AppContextKey } from "../context.ts";
|
import { AppContextKey } from "../context.ts";
|
||||||
import { assertValidBackgroundColor } from "../paint/text-style.ts";
|
import { assertValidBackgroundColor, assertValidForegroundColor } from "../paint/text-style.ts";
|
||||||
import type { WithChildren } from "./with-children.ts";
|
import type { WithChildren } from "./with-children.ts";
|
||||||
|
|
||||||
type Spacing = number;
|
type Spacing = number;
|
||||||
@@ -216,29 +216,46 @@ const BoxImpl = defineComponent({
|
|||||||
// width=0 with no left/right border) produces an empty string Ink skips.
|
// width=0 with no left/right border) produces an empty string Ink skips.
|
||||||
// Matching that needs layout, unavailable at render. Niche + invalid-input-only.
|
// Matching that needs layout, unavailable at render. Niche + invalid-input-only.
|
||||||
if (props.borderStyle) {
|
if (props.borderStyle) {
|
||||||
const stringBg = (value: unknown) => (typeof value === "string" ? value : undefined);
|
const stringStyle = (value: unknown) => (typeof value === "string" ? value : undefined);
|
||||||
const generalBg = stringBg(props.borderBackgroundColor);
|
const generalBg = stringStyle(props.borderBackgroundColor);
|
||||||
|
const generalFg = stringStyle(props.borderColor);
|
||||||
if (props.borderTop !== false) {
|
if (props.borderTop !== false) {
|
||||||
|
assertValidForegroundColor(
|
||||||
|
stringStyle(props.borderTopColor) ?? generalFg,
|
||||||
|
"borderTopColor",
|
||||||
|
);
|
||||||
assertValidBackgroundColor(
|
assertValidBackgroundColor(
|
||||||
stringBg(props.borderTopBackgroundColor) ?? generalBg,
|
stringStyle(props.borderTopBackgroundColor) ?? generalBg,
|
||||||
"borderTopBackgroundColor",
|
"borderTopBackgroundColor",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (props.borderBottom !== false) {
|
if (props.borderBottom !== false) {
|
||||||
|
assertValidForegroundColor(
|
||||||
|
stringStyle(props.borderBottomColor) ?? generalFg,
|
||||||
|
"borderBottomColor",
|
||||||
|
);
|
||||||
assertValidBackgroundColor(
|
assertValidBackgroundColor(
|
||||||
stringBg(props.borderBottomBackgroundColor) ?? generalBg,
|
stringStyle(props.borderBottomBackgroundColor) ?? generalBg,
|
||||||
"borderBottomBackgroundColor",
|
"borderBottomBackgroundColor",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (props.borderLeft !== false) {
|
if (props.borderLeft !== false) {
|
||||||
|
assertValidForegroundColor(
|
||||||
|
stringStyle(props.borderLeftColor) ?? generalFg,
|
||||||
|
"borderLeftColor",
|
||||||
|
);
|
||||||
assertValidBackgroundColor(
|
assertValidBackgroundColor(
|
||||||
stringBg(props.borderLeftBackgroundColor) ?? generalBg,
|
stringStyle(props.borderLeftBackgroundColor) ?? generalBg,
|
||||||
"borderLeftBackgroundColor",
|
"borderLeftBackgroundColor",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (props.borderRight !== false) {
|
if (props.borderRight !== false) {
|
||||||
|
assertValidForegroundColor(
|
||||||
|
stringStyle(props.borderRightColor) ?? generalFg,
|
||||||
|
"borderRightColor",
|
||||||
|
);
|
||||||
assertValidBackgroundColor(
|
assertValidBackgroundColor(
|
||||||
stringBg(props.borderRightBackgroundColor) ?? generalBg,
|
stringStyle(props.borderRightBackgroundColor) ?? generalBg,
|
||||||
"borderRightBackgroundColor",
|
"borderRightBackgroundColor",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ import {
|
|||||||
type VNode,
|
type VNode,
|
||||||
} from "vue";
|
} from "vue";
|
||||||
import { AppContextKey } from "../context.ts";
|
import { AppContextKey } from "../context.ts";
|
||||||
import { assertValidBackgroundColor } from "../paint/text-style.ts";
|
import { assertValidBackgroundColor, assertValidForegroundColor } from "../paint/text-style.ts";
|
||||||
import type { WithChildren } from "./with-children.ts";
|
import type { WithChildren } from "./with-children.ts";
|
||||||
|
|
||||||
type WrapMode =
|
type WrapMode =
|
||||||
@@ -73,6 +73,7 @@ const TextImpl = defineComponent({
|
|||||||
// a group of only empty/inert children) gets past it, so we additionally
|
// a group of only empty/inert children) gets past it, so we additionally
|
||||||
// skip validation when the content would render empty. (A12)
|
// skip validation when the content would render empty. (A12)
|
||||||
if (wouldRenderNonEmptyText(children)) {
|
if (wouldRenderNonEmptyText(children)) {
|
||||||
|
assertValidForegroundColor(props.color);
|
||||||
assertValidBackgroundColor(props.backgroundColor);
|
assertValidBackgroundColor(props.backgroundColor);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,12 @@
|
|||||||
import chalk from "chalk";
|
import chalk from "chalk";
|
||||||
import { expect, test } from "vite-plus/test";
|
import { expect, test } from "vite-plus/test";
|
||||||
import { applyChalk, assertValidBackgroundColor, isInvalidBackgroundColor } from "./text-style.ts";
|
import {
|
||||||
|
applyChalk,
|
||||||
|
assertValidBackgroundColor,
|
||||||
|
assertValidForegroundColor,
|
||||||
|
isInvalidBackgroundColor,
|
||||||
|
isInvalidForegroundColor,
|
||||||
|
} from "./text-style.ts";
|
||||||
|
|
||||||
test("named color applies chalk method", () => {
|
test("named color applies chalk method", () => {
|
||||||
const prev = chalk.level;
|
const prev = chalk.level;
|
||||||
@@ -208,6 +214,19 @@ test("assertValidBackgroundColor throws only for a modifier name, with the label
|
|||||||
expect(() => assertValidBackgroundColor(undefined)).not.toThrow();
|
expect(() => assertValidBackgroundColor(undefined)).not.toThrow();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("assertValidForegroundColor throws only for chalk keys that are not methods", () => {
|
||||||
|
expect(isInvalidForegroundColor("level")).toBe(true);
|
||||||
|
expect(() => assertValidForegroundColor("level")).toThrow(/color/i);
|
||||||
|
expect(() => assertValidForegroundColor("level", "borderTopColor")).toThrow(/borderTopColor/);
|
||||||
|
|
||||||
|
// Valid foreground forms and unknown strings keep Ink's bare-text fallback.
|
||||||
|
expect(isInvalidForegroundColor("bold")).toBe(false);
|
||||||
|
expect(isInvalidForegroundColor("red")).toBe(false);
|
||||||
|
expect(isInvalidForegroundColor("#abcdef")).toBe(false);
|
||||||
|
expect(isInvalidForegroundColor("not-a-real-color")).toBe(false);
|
||||||
|
expect(isInvalidForegroundColor(undefined)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
// Foreground is UNAFFECTED: `color="bold"` resolves `chalk.bold` (a real fn) and
|
// Foreground is UNAFFECTED: `color="bold"` resolves `chalk.bold` (a real fn) and
|
||||||
// applies the modifier — Ink does NOT throw on a foreground modifier name, and
|
// applies the modifier — Ink does NOT throw on a foreground modifier name, and
|
||||||
// neither does vue-tui. (Only the bg path has the missing-`bg*`-method problem.)
|
// neither does vue-tui. (Only the bg path has the missing-`bg*`-method problem.)
|
||||||
|
|||||||
@@ -68,6 +68,19 @@ export function isInvalidBackgroundColor(color: unknown): boolean {
|
|||||||
return typeof bgMethod !== "function";
|
return typeof bgMethod !== "function";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Detect a foreground color value that Ink's `colorize` would THROW on.
|
||||||
|
*
|
||||||
|
* Ink's foreground path calls `chalk[color](str)` when `color in chalk`. That
|
||||||
|
* works for real colors and modifiers (`red`, `bold`) but throws for non-method
|
||||||
|
* chalk properties such as `level`.
|
||||||
|
*/
|
||||||
|
export function isInvalidForegroundColor(color: unknown): boolean {
|
||||||
|
if (typeof color !== "string" || color.length === 0) return false;
|
||||||
|
const method = (chalk as unknown as Record<string, unknown>)[color];
|
||||||
|
return color in (chalk as unknown as Record<string, unknown>) && typeof method !== "function";
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Throw (during component render) if `color` is a chalk-modifier-name
|
* Throw (during component render) if `color` is a chalk-modifier-name
|
||||||
* backgroundColor — the exact case Ink's colorize.ts throws on. No-op for every
|
* backgroundColor — the exact case Ink's colorize.ts throws on. No-op for every
|
||||||
@@ -82,6 +95,18 @@ export function assertValidBackgroundColor(color: unknown, label = "backgroundCo
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Throw during component render for foreground color names that Ink's paint path
|
||||||
|
* would throw on. `label` names the offending prop in the message.
|
||||||
|
*/
|
||||||
|
export function assertValidForegroundColor(color: unknown, label = "color"): void {
|
||||||
|
if (isInvalidForegroundColor(color)) {
|
||||||
|
throw new Error(
|
||||||
|
`Invalid ${label}: ${JSON.stringify(color)} (chalk has this key but it is not a color method)`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function applyChalk(text: string, props: TextProps): string {
|
export function applyChalk(text: string, props: TextProps): string {
|
||||||
// Mirror Ink's Text.tsx `transform` (commit 40b3a75): apply each enabled
|
// Mirror Ink's Text.tsx `transform` (commit 40b3a75): apply each enabled
|
||||||
// style as its OWN nested chalk call, in the exact order
|
// style as its OWN nested chalk call, in the exact order
|
||||||
|
|||||||
Reference in New Issue
Block a user