diff --git a/packages/cli/src/commands/render.test.ts b/packages/cli/src/commands/render.test.ts index ce610addd..08ce30c4f 100644 --- a/packages/cli/src/commands/render.test.ts +++ b/packages/cli/src/commands/render.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const producerState = vi.hoisted(() => ({ createdJobs: [] as Array>, @@ -133,21 +133,31 @@ describe("renderLocal browser GPU config", () => { }); describe("parseVariablesArg", () => { - it("returns undefined when neither flag is set", async () => { - const { parseVariablesArg } = await import("./render.js"); + let parseVariablesArg: typeof import("./render.js").parseVariablesArg; + + beforeAll(async () => { + ({ parseVariablesArg } = await import("./render.js")); + }); + + function expectErr( + result: import("./render.js").VariablesParseResult, + ): T { + if (result.ok) throw new Error(`expected error, got ${JSON.stringify(result.value)}`); + return result.error as T; + } + + it("returns undefined when neither flag is set", () => { expect(parseVariablesArg(undefined, undefined)).toEqual({ ok: true, value: undefined }); }); - it("parses inline JSON object", async () => { - const { parseVariablesArg } = await import("./render.js"); + it("parses inline JSON object", () => { expect(parseVariablesArg('{"title":"Hello","n":3}', undefined)).toEqual({ ok: true, value: { title: "Hello", n: 3 }, }); }); - it("parses file JSON via injected reader", async () => { - const { parseVariablesArg } = await import("./render.js"); + it("parses file JSON via injected reader", () => { const fakeReader = (path: string) => { if (path === "vars.json") return '{"theme":"dark"}'; throw new Error("unexpected path"); @@ -158,43 +168,40 @@ describe("parseVariablesArg", () => { }); }); - it("rejects when both flags are set", async () => { - const { parseVariablesArg } = await import("./render.js"); - const result = parseVariablesArg('{"a":1}', "vars.json"); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.title).toMatch(/Conflicting/); + it("rejects when both flags are set", () => { + const err = expectErr(parseVariablesArg('{"a":1}', "vars.json")); + expect(err).toEqual({ kind: "conflict" }); }); - it("rejects unparseable JSON with a source-aware title", async () => { - const { parseVariablesArg } = await import("./render.js"); - const inlineFail = parseVariablesArg("{not json", undefined); - expect(inlineFail.ok).toBe(false); - if (!inlineFail.ok) expect(inlineFail.title).toBe("Invalid JSON in --variables"); - - const fileFail = parseVariablesArg(undefined, "x", () => "{not json"); - expect(fileFail.ok).toBe(false); - if (!fileFail.ok) expect(fileFail.title).toBe("Invalid JSON in --variables-file"); - }); - - it("rejects non-object payloads (array, string, null)", async () => { - const { parseVariablesArg } = await import("./render.js"); - for (const payload of ["[1,2]", '"hello"', "null", "42"]) { - const result = parseVariablesArg(payload, undefined); - expect(result.ok).toBe(false); - if (!result.ok) expect(result.title).toBe("Invalid variables payload"); - } - }); - - it("surfaces filesystem errors from --variables-file", async () => { - const { parseVariablesArg } = await import("./render.js"); - const result = parseVariablesArg(undefined, "missing.json", () => { - throw new Error("ENOENT: no such file"); + it("rejects unparseable JSON with a source-aware kind", () => { + expect(expectErr(parseVariablesArg("{not json", undefined))).toMatchObject({ + kind: "parse-error", + source: "inline", }); - expect(result.ok).toBe(false); - if (!result.ok) { - expect(result.title).toBe("Could not read --variables-file"); - expect(result.message).toMatch(/missing\.json/); - expect(result.message).toMatch(/ENOENT/); + expect(expectErr(parseVariablesArg(undefined, "x", () => "{not json"))).toMatchObject({ + kind: "parse-error", + source: "file", + }); + }); + + it("rejects non-object payloads (array, string, null, number)", () => { + for (const payload of ["[1,2]", '"hello"', "null", "42"]) { + expect(expectErr(parseVariablesArg(payload, undefined))).toEqual({ kind: "shape-error" }); } }); + + it("surfaces filesystem errors from --variables-file", () => { + const err = expectErr<{ + kind: "read-error"; + path: string; + cause: string; + }>( + parseVariablesArg(undefined, "missing.json", () => { + throw new Error("ENOENT: no such file"); + }), + ); + expect(err.kind).toBe("read-error"); + expect(err.path).toBe("missing.json"); + expect(err.cause).toMatch(/ENOENT/); + }); }); diff --git a/packages/cli/src/commands/render.ts b/packages/cli/src/commands/render.ts index d9bab5d92..46ac96260 100644 --- a/packages/cli/src/commands/render.ts +++ b/packages/cli/src/commands/render.ts @@ -398,14 +398,21 @@ interface RenderOptions { variables?: Record; } +export type VariablesParseError = + | { kind: "conflict" } + | { kind: "read-error"; path: string; cause: string } + | { kind: "parse-error"; source: "inline" | "file"; cause: string } + | { kind: "shape-error" }; + export type VariablesParseResult = | { ok: true; value: Record | undefined } - | { ok: false; title: string; message: string }; + | { ok: false; error: VariablesParseError }; /** * Pure parser for `--variables` / `--variables-file` flag pair. Splits out * from `resolveVariablesArg` so validation paths are unit-testable without - * triggering `process.exit`. + * triggering `process.exit`. Reports failures via a structured `kind` + * discriminant so the side-effecting wrapper owns all UI strings. */ export function parseVariablesArg( inline: string | undefined, @@ -413,11 +420,7 @@ export function parseVariablesArg( readFile: (path: string) => string = (p) => readFileSync(resolve(p), "utf8"), ): VariablesParseResult { if (inline != null && filePath != null) { - return { - ok: false, - title: "Conflicting variables flags", - message: "Use either --variables or --variables-file, not both.", - }; + return { ok: false, error: { kind: "conflict" } }; } let raw: string | undefined; let source: "inline" | "file" | undefined; @@ -429,11 +432,13 @@ export function parseVariablesArg( raw = readFile(filePath); source = "file"; } catch (error: unknown) { - const message = error instanceof Error ? error.message : String(error); return { ok: false, - title: "Could not read --variables-file", - message: `${filePath}: ${message}`, + error: { + kind: "read-error", + path: filePath, + cause: error instanceof Error ? error.message : String(error), + }, }; } } @@ -443,23 +448,49 @@ export function parseVariablesArg( try { parsed = JSON.parse(raw); } catch (error: unknown) { - const message = error instanceof Error ? error.message : String(error); return { ok: false, - title: source === "file" ? "Invalid JSON in --variables-file" : "Invalid JSON in --variables", - message, + error: { + kind: "parse-error", + source: source ?? "inline", + cause: error instanceof Error ? error.message : String(error), + }, }; } if (parsed == null || typeof parsed !== "object" || Array.isArray(parsed)) { - return { - ok: false, - title: "Invalid variables payload", - message: 'Variables must be a JSON object (e.g. {"title":"Hello"}).', - }; + return { ok: false, error: { kind: "shape-error" } }; } return { ok: true, value: parsed as Record }; } +function variablesErrorMessage(error: VariablesParseError): { title: string; message: string } { + switch (error.kind) { + case "conflict": + return { + title: "Conflicting variables flags", + message: "Use either --variables or --variables-file, not both.", + }; + case "read-error": + return { + title: "Could not read --variables-file", + message: `${error.path}: ${error.cause}`, + }; + case "parse-error": + return { + title: + error.source === "file" + ? "Invalid JSON in --variables-file" + : "Invalid JSON in --variables", + message: error.cause, + }; + case "shape-error": + return { + title: "Invalid variables payload", + message: 'Variables must be a JSON object (e.g. {"title":"Hello"}).', + }; + } +} + /** * Resolve `--variables` / `--variables-file` into a plain object, or * `undefined` when neither flag is set. Exits the process with a friendly @@ -471,7 +502,8 @@ export function resolveVariablesArg( ): Record | undefined { const result = parseVariablesArg(inline, filePath); if (!result.ok) { - errorBox(result.title, result.message); + const { title, message } = variablesErrorMessage(result.error); + errorBox(title, message); process.exit(1); } return result.value; diff --git a/packages/core/src/runtime/getVariables.ts b/packages/core/src/runtime/getVariables.ts index 50496ccf4..5bae2f9d0 100644 --- a/packages/core/src/runtime/getVariables.ts +++ b/packages/core/src/runtime/getVariables.ts @@ -38,14 +38,10 @@ function readDeclaredDefaults(root: Element | null): Record { const out: Record = {}; for (const entry of parsed) { - if ( - entry && - typeof entry === "object" && - typeof (entry as { id?: unknown }).id === "string" && - "default" in entry - ) { - out[(entry as { id: string }).id] = (entry as { default: unknown }).default; - } + if (!entry || typeof entry !== "object") continue; + const e = entry as Record; + if (typeof e.id !== "string" || !("default" in e)) continue; + out[e.id] = e.default; } return out; } diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 0027378e2..9e4cff429 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -162,9 +162,9 @@ export async function createCaptureSession( if (options.variables && Object.keys(options.variables).length > 0) { const variablesJson = JSON.stringify(options.variables); await page.evaluateOnNewDocument((json: string) => { + type WindowWithVariables = Window & { __hfVariables?: Record }; try { - (window as unknown as { __hfVariables?: Record }).__hfVariables = - JSON.parse(json); + (window as WindowWithVariables).__hfVariables = JSON.parse(json); } catch { // The CLI validated the JSON before this point — a parse failure here // means the page swapped JSON.parse, which is the page's problem.