mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
feat(core): warn when a variable value is not a declared option
A composition can set a variable to a value outside that variable declared enum options. The value silently falls back, the composition renders something the author did not ask for, and no signal says a choice was ignored. Warns on both paths that resolve a variable, because they are separate. The runtime guard covers a top-level composition. The compile guard covers a sub-composition given instance values: those are baked into the variables table at compile time and the scoped getVariables shim only reads that table, so the runtime guard never runs there. That sub-comp case is the one that motivated this, and it was the silent one. Both call the same helper, so the message and the per-process dedupe set are shared and an author sees one warning either way. A warning rather than an error, for symmetry: the same defect must not carry two severities depending on which mount path an author happened to use. Escalation already has a home in lint, which a project can make blocking. The check is split across four small helpers rather than one function: parsing the declaration, naming the composition, reducing the option set to comparable scalars, and deciding whether a value actually fell back. As one function it audited at 21 cyclomatic and 25 cognitive. Tests were mutation-checked. Five distinct breakages each killed a test, including one that coerces the unknown value to the default instead of warning, which would turn a diagnostic into a silent rewrite.
This commit is contained in:
Miguel Ángel
@@ -3,8 +3,9 @@ import { mkdtempSync, writeFileSync, mkdirSync, symlinkSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { parseHTML } from "linkedom";
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { afterEach, beforeEach, describe, it, expect, vi } from "vitest";
|
||||
import { bundleToSingleHtml } from "./htmlBundler";
|
||||
import { resetUnknownEnumWarnings } from "../runtime/getVariables";
|
||||
import { getHyperframeRuntimeScript } from "../generated/runtime-inline";
|
||||
|
||||
function makeTempProject(files: Record<string, string>): string {
|
||||
@@ -1388,3 +1389,124 @@ describe("bundleToSingleHtml", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A sub-composition given a value outside a declared enum's `options` falls
|
||||
* back silently. The runtime guard in getVariables.ts cannot see it: the
|
||||
* bundler bakes the per-instance values into `window.__hfVariablesByComp` at
|
||||
* compile time and the sub-comp's scoped `getVariables` shim only reads that
|
||||
* table. Compile time is therefore the only place the defect is observable on
|
||||
* this path, so the same warning is emitted here.
|
||||
*/
|
||||
describe("bundleToSingleHtml unknown enum values", () => {
|
||||
let warnings: string[];
|
||||
|
||||
beforeEach(() => {
|
||||
resetUnknownEnumWarnings();
|
||||
warnings = [];
|
||||
vi.spyOn(console, "warn").mockImplementation((...args: unknown[]) => {
|
||||
warnings.push(args.map(String).join(" "));
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
resetUnknownEnumWarnings();
|
||||
});
|
||||
|
||||
const enumWarnings = () => warnings.filter((w) => w.includes("runtime_unknown_enum_value"));
|
||||
|
||||
const ACCENT_ENUM =
|
||||
'[{"id":"accent","type":"enum","label":"Accent","default":"green","options":["green","blue","violet"]}]';
|
||||
|
||||
function makeSubCompProject(variableValues: string, declaration = ACCENT_ENUM): string {
|
||||
return makeTempProject({
|
||||
"index.html": `<!doctype html>
|
||||
<html><head></head><body>
|
||||
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
|
||||
<div
|
||||
data-composition-id="card"
|
||||
data-composition-src="compositions/card.html"
|
||||
data-variable-values='${variableValues}'></div>
|
||||
</div>
|
||||
<script>window.__timelines={};</script>
|
||||
</body></html>`,
|
||||
"compositions/card.html": `<!doctype html>
|
||||
<html data-composition-variables='${declaration}'>
|
||||
<body>
|
||||
<div data-composition-id="card" data-width="1920" data-height="1080"></div>
|
||||
</body>
|
||||
</html>`,
|
||||
});
|
||||
}
|
||||
|
||||
it("warns when a sub-composition instance value is not a declared option", async () => {
|
||||
await bundleToSingleHtml(makeSubCompProject('{"accent":"orange"}'));
|
||||
|
||||
expect(enumWarnings()).toEqual([
|
||||
'[hyperframes] runtime_unknown_enum_value: card variable "accent" got "orange", ' +
|
||||
"which is not a declared option (green, blue, violet). " +
|
||||
'Rendering "green" instead.',
|
||||
]);
|
||||
});
|
||||
|
||||
it("is silent when the instance value is a declared option", async () => {
|
||||
await bundleToSingleHtml(makeSubCompProject('{"accent":"violet"}'));
|
||||
|
||||
expect(enumWarnings()).toEqual([]);
|
||||
});
|
||||
|
||||
it("never inspects a variable declared without options", async () => {
|
||||
const declaration = '[{"id":"accent","type":"string","label":"Accent","default":"green"}]';
|
||||
await bundleToSingleHtml(makeSubCompProject('{"accent":"orange"}', declaration));
|
||||
|
||||
expect(enumWarnings()).toEqual([]);
|
||||
});
|
||||
|
||||
it("is silent for a declared enum absent from the instance values", async () => {
|
||||
await bundleToSingleHtml(makeSubCompProject('{"unrelated":"whatever"}'));
|
||||
|
||||
expect(enumWarnings()).toEqual([]);
|
||||
});
|
||||
|
||||
it("passes the unknown value through to the bundle unrewritten", async () => {
|
||||
const bundled = await bundleToSingleHtml(makeSubCompProject('{"accent":"orange"}'));
|
||||
|
||||
expect(bundled).toContain("window.__hfVariablesByComp = Object.assign({}, ");
|
||||
expect(bundled).toContain('{ "card": { "accent": "orange" } }');
|
||||
expect(bundled).toMatch(/\[data-composition-id="card"\]\s*\{[^}]*--accent:\s*orange/);
|
||||
expect(bundled).not.toContain("--accent: green");
|
||||
});
|
||||
|
||||
it("warns once for the same composition, variable and value across bundles", async () => {
|
||||
const dir = makeSubCompProject('{"accent":"orange"}');
|
||||
await bundleToSingleHtml(dir);
|
||||
await bundleToSingleHtml(dir);
|
||||
|
||||
expect(enumWarnings()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("warns for a <template>-mounted composition too", async () => {
|
||||
const dir = makeTempProject({
|
||||
"index.html": `<!doctype html>
|
||||
<html><head></head><body>
|
||||
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
|
||||
<div data-composition-id="card" data-variable-values='{"accent":"orange"}'></div>
|
||||
</div>
|
||||
<template id="card-template">
|
||||
<div data-composition-id="card" data-width="1920" data-height="1080"
|
||||
data-composition-variables='${ACCENT_ENUM}'></div>
|
||||
</template>
|
||||
<script>window.__timelines={};</script>
|
||||
</body></html>`,
|
||||
});
|
||||
|
||||
await bundleToSingleHtml(dir);
|
||||
|
||||
expect(enumWarnings()).toEqual([
|
||||
'[hyperframes] runtime_unknown_enum_value: card variable "accent" got "orange", ' +
|
||||
"which is not a declared option (green, blue, violet). " +
|
||||
'Rendering "green" instead.',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { markFlattenedInnerRoot } from "../runtime/flattenedRoot";
|
||||
export { FLATTENED_INNER_ROOT_STRIP_ATTRS } from "../runtime/flattenedRoot";
|
||||
import { parseHostVariableValues } from "../runtime/getVariables";
|
||||
import { parseHostVariableValues, warnUnknownEnumValues } from "../runtime/getVariables";
|
||||
import { cssVariableName } from "../tokenSlug";
|
||||
import { readFileSync, existsSync } from "fs";
|
||||
import { resolve, relative, dirname, isAbsolute, sep } from "path";
|
||||
@@ -980,6 +980,13 @@ export async function bundleToSingleHtml(
|
||||
if (runtimeCompId && Object.keys(mergedVariables).length > 0) {
|
||||
compVariablesByComp[runtimeCompId] = mergedVariables;
|
||||
}
|
||||
// Same defect on the <template> mount as on the data-composition-src
|
||||
// mount (see inlineSubCompositions): the merged instance values are
|
||||
// baked in here, so only compile time can see a value that falls back.
|
||||
if (runtimeCompId) {
|
||||
warnUnknownEnumValues(innerDoc.documentElement, mergedVariables, runtimeCompId);
|
||||
warnUnknownEnumValues(innerRoot, mergedVariables, runtimeCompId);
|
||||
}
|
||||
pushSubCompVariableStyles(
|
||||
innerDoc,
|
||||
innerRoot,
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
} from "./compositionScoping";
|
||||
import { checkSubCompositionUsability } from "@hyperframes/parsers/sub-composition-validity";
|
||||
import { enumerateNestedCompositionHosts, planCompositionAssembly } from "./compositionAssembly";
|
||||
import { warnUnknownEnumValues } from "../runtime/getVariables";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Public interface
|
||||
@@ -272,6 +273,14 @@ export function inlineSubCompositions(
|
||||
if (Object.keys(mergedVariables).length > 0) {
|
||||
variablesByComp[runtimeCompId] = mergedVariables;
|
||||
}
|
||||
// Compile time is the only place this defect is visible on the sub-comp
|
||||
// path: the instance value is baked into `__hfVariablesByComp` right
|
||||
// here, and the scoped `getVariables` shim only reads that table, so the
|
||||
// runtime's identical guard never runs. Same helper, so the message and
|
||||
// the per-process dedupe set are shared with the runtime path and the
|
||||
// author sees one warning either way.
|
||||
warnUnknownEnumValues(compDoc.documentElement, mergedVariables, runtimeCompId);
|
||||
warnUnknownEnumValues(innerRoot, mergedVariables, runtimeCompId);
|
||||
}
|
||||
|
||||
// `<head>` <link>/<script src> are hoisted into the ROOT document, so they
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
/**
|
||||
* @vitest-environment jsdom
|
||||
*/
|
||||
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||
import { getVariables, readDeclaredDefaults } from "./getVariables";
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { getVariables, readDeclaredDefaults, resetUnknownEnumWarnings } from "./getVariables";
|
||||
|
||||
const VARIABLES_ATTR = "data-composition-variables";
|
||||
|
||||
@@ -204,6 +204,140 @@ describe("readDeclaredDefaults", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("out-of-set enum values (observability only — never changes the returned map)", () => {
|
||||
// One enum plus a number and a string, so "only enums are inspected" is
|
||||
// pinned by the same declaration the warning reads.
|
||||
const DECLARED = JSON.stringify([
|
||||
{
|
||||
id: "accent",
|
||||
type: "enum",
|
||||
label: "Accent",
|
||||
default: "green",
|
||||
options: [
|
||||
{ value: "green", label: "Green" },
|
||||
{ value: "blue", label: "Blue" },
|
||||
{ value: "violet", label: "Violet" },
|
||||
],
|
||||
},
|
||||
{ id: "swap_at", type: "number", label: "Swap at", default: 0.5 },
|
||||
{ id: "title", type: "string", label: "Title", default: "Hello" },
|
||||
]);
|
||||
const DEFAULTS = { accent: "green", swap_at: 0.5, title: "Hello" };
|
||||
|
||||
let warnings: string[];
|
||||
|
||||
beforeEach(() => {
|
||||
resetUnknownEnumWarnings();
|
||||
document.body.innerHTML = "";
|
||||
setDeclared(DECLARED);
|
||||
document.documentElement.setAttribute("data-composition-id", "morph-swap");
|
||||
setOverrides(undefined);
|
||||
warnings = [];
|
||||
vi.spyOn(console, "warn").mockImplementation((...args: unknown[]) => {
|
||||
warnings.push(args.map(String).join(" "));
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
document.documentElement.removeAttribute("data-composition-id");
|
||||
setDeclared(null);
|
||||
setOverrides(undefined);
|
||||
resetUnknownEnumWarnings();
|
||||
});
|
||||
|
||||
it("a declared option warns nothing", () => {
|
||||
setOverrides({ accent: "violet" });
|
||||
expect(getVariables()).toEqual({ ...DEFAULTS, accent: "violet" });
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it("an absent value warns nothing — absent is the normal case", () => {
|
||||
expect(getVariables()).toEqual(DEFAULTS);
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it("an enum declared without a default and never set warns nothing", () => {
|
||||
setDeclared(
|
||||
JSON.stringify([
|
||||
{ id: "accent", type: "enum", label: "Accent", options: [{ value: "green" }] },
|
||||
]),
|
||||
);
|
||||
expect(getVariables()).toEqual({});
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it("an out-of-set value warns once, naming composition, variable, value and fallback", () => {
|
||||
setOverrides({ accent: "orange" });
|
||||
expect(getVariables()).toEqual({ ...DEFAULTS, accent: "orange" });
|
||||
expect(warnings).toHaveLength(1);
|
||||
const message = warnings[0] ?? "";
|
||||
expect(message).toContain("runtime_unknown_enum_value");
|
||||
expect(message).toContain("morph-swap");
|
||||
expect(message).toContain('"accent"');
|
||||
expect(message).toContain('got "orange"');
|
||||
expect(message).toContain("green, blue, violet");
|
||||
expect(message).toContain('Rendering "green" instead');
|
||||
});
|
||||
|
||||
it("falls back to the root composition id when the declarer carries none", () => {
|
||||
// The real top-level shape: <html> declares, the root <div> has the id.
|
||||
document.documentElement.removeAttribute("data-composition-id");
|
||||
document.body.innerHTML = '<div data-composition-id="hero-scene"></div>';
|
||||
setOverrides({ accent: "orange" });
|
||||
getVariables();
|
||||
expect(warnings).toHaveLength(1);
|
||||
expect(warnings[0]).toContain("hero-scene");
|
||||
});
|
||||
|
||||
it("the same bad value twice warns once; a different bad value warns again", () => {
|
||||
setOverrides({ accent: "orange" });
|
||||
getVariables();
|
||||
getVariables();
|
||||
expect(warnings).toHaveLength(1);
|
||||
|
||||
setOverrides({ accent: "puce" });
|
||||
getVariables();
|
||||
expect(warnings).toHaveLength(2);
|
||||
expect(warnings[1]).toContain('got "puce"');
|
||||
});
|
||||
|
||||
it("non-enum variables are never inspected (any number or string is legal)", () => {
|
||||
setOverrides({ swap_at: 9.9, title: "anything at all" });
|
||||
expect(getVariables()).toEqual({ ...DEFAULTS, swap_at: 9.9, title: "anything at all" });
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not warn when the declared default is itself out of set — nothing fell back", () => {
|
||||
setDeclared(
|
||||
JSON.stringify([
|
||||
{
|
||||
id: "accent",
|
||||
type: "enum",
|
||||
label: "Accent",
|
||||
default: "orange",
|
||||
options: [{ value: "green" }, { value: "blue" }],
|
||||
},
|
||||
]),
|
||||
);
|
||||
expect(getVariables()).toEqual({ accent: "orange" });
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns byte-identical values whether the value is in set or not", () => {
|
||||
setOverrides({ accent: "violet" });
|
||||
const good = getVariables();
|
||||
setOverrides({ accent: "orange" });
|
||||
const bad = getVariables();
|
||||
expect(warnings).toHaveLength(1);
|
||||
// Same keys, same non-enum values, and the bad value passed through
|
||||
// untouched — the composition's own guard still owns the coercion.
|
||||
expect(Object.keys(bad)).toEqual(Object.keys(good));
|
||||
expect(bad).toEqual({ ...good, accent: "orange" });
|
||||
expect(bad.accent).toBe("orange");
|
||||
});
|
||||
});
|
||||
|
||||
describe("css variable injection (figma brand-token chain)", () => {
|
||||
afterEach(() => {
|
||||
document.documentElement.removeAttribute(VARIABLES_ATTR);
|
||||
|
||||
@@ -42,7 +42,119 @@ export function getVariables<
|
||||
}
|
||||
const overrides = readRenderOverrides();
|
||||
|
||||
return { ...declaredDefaults, ...overrides } as Partial<T>;
|
||||
const merged = { ...declaredDefaults, ...overrides };
|
||||
for (const el of declarers) warnUnknownEnumValues(el, merged);
|
||||
return merged as Partial<T>;
|
||||
}
|
||||
|
||||
/**
|
||||
* An enum variable given a value outside its declared `options` is coerced to
|
||||
* the composition's default by the composition's own guard. That fallback is
|
||||
* deliberate (a bad value must never break a frame) but it was silent, so a
|
||||
* meaningless value could sit in a project indefinitely and look correct only
|
||||
* by coincidence. Warn; do not change what renders.
|
||||
*
|
||||
* Reads the option set straight off `data-composition-variables`, so it covers
|
||||
* every declared enum, not just the `accentColors` guard shape, and needs no
|
||||
* per-composition code. Deduped per composition+variable+value so a remount
|
||||
* (studio re-init, seek) cannot spam the console.
|
||||
*
|
||||
* Silent for a valid value, for an absent one (an absent variable resolves to
|
||||
* its own declared default), and for a value that already equals that default
|
||||
* (nothing fell back).
|
||||
*/
|
||||
const warnedUnknownEnumValues = new Set<string>();
|
||||
|
||||
/** The declared entries, or `null` when the attribute is absent or unusable. */
|
||||
function declaredEnumEntries(declarer: Element): Record<string, unknown>[] | null {
|
||||
const raw = declarer.getAttribute("data-composition-variables");
|
||||
if (!raw) return null;
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(raw);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
if (!Array.isArray(parsed)) return null;
|
||||
return parsed.filter(
|
||||
(entry): entry is Record<string, unknown> => !!entry && typeof entry === "object",
|
||||
);
|
||||
}
|
||||
|
||||
/** What to call the composition in the message. */
|
||||
function enumWarningScopeName(declarer: Element, compositionId?: string): string {
|
||||
return (
|
||||
compositionId?.trim() ||
|
||||
declarer.getAttribute("data-composition-id")?.trim() ||
|
||||
// The canonical top-level shape declares the variables on <html>, which
|
||||
// carries no id — the composition id sits on the root element below it.
|
||||
// Without this the message reads "composition variable ...", naming
|
||||
// nothing in a project that has more than one.
|
||||
declarer.querySelector("[data-composition-id]")?.getAttribute("data-composition-id")?.trim() ||
|
||||
"composition"
|
||||
);
|
||||
}
|
||||
|
||||
/** The option set as comparable scalars, dropping shapes we cannot compare. */
|
||||
function allowedOptionValues(options: unknown[]): (string | number)[] {
|
||||
return options
|
||||
.map((option) =>
|
||||
option && typeof option === "object" ? (option as Record<string, unknown>).value : option,
|
||||
)
|
||||
.filter((v): v is string | number => typeof v === "string" || typeof v === "number");
|
||||
}
|
||||
|
||||
/**
|
||||
* True when `value` fell back: it is set, it differs from the declared
|
||||
* default, and it is not in the option set.
|
||||
*
|
||||
* Stringified compare: `--variables` / `data-variable-values` can deliver a
|
||||
* declared numeric option as a string, and that is not the defect here.
|
||||
*/
|
||||
function isUnknownEnumValue(
|
||||
entry: Record<string, unknown>,
|
||||
value: unknown,
|
||||
allowed: (string | number)[],
|
||||
): boolean {
|
||||
if (value === undefined || value === null) return false;
|
||||
// Nothing was overridden: the resolved value IS the declared default, so no
|
||||
// fallback happened and there is nothing to report. A default outside its
|
||||
// own option set is a declaration defect (the linter's job) and warning here
|
||||
// would print the self-contradictory "got X ... rendering X".
|
||||
if ("default" in entry && String(value) === String(entry.default)) return false;
|
||||
if (allowed.length === 0) return false;
|
||||
return !allowed.some((v) => String(v) === String(value));
|
||||
}
|
||||
|
||||
export function warnUnknownEnumValues(
|
||||
declarer: Element | null | undefined,
|
||||
resolved: Record<string, unknown>,
|
||||
compositionId?: string,
|
||||
): void {
|
||||
if (!declarer) return;
|
||||
const entries = declaredEnumEntries(declarer);
|
||||
if (!entries) return;
|
||||
const move = enumWarningScopeName(declarer, compositionId);
|
||||
|
||||
for (const entry of entries) {
|
||||
if (typeof entry.id !== "string" || !Array.isArray(entry.options)) continue;
|
||||
const value = resolved[entry.id];
|
||||
const allowed = allowedOptionValues(entry.options);
|
||||
if (!isUnknownEnumValue(entry, value, allowed)) continue;
|
||||
|
||||
const key = `${move}|${entry.id}|${String(value)}`;
|
||||
if (warnedUnknownEnumValues.has(key)) continue;
|
||||
warnedUnknownEnumValues.add(key);
|
||||
const fallback = "default" in entry ? JSON.stringify(entry.default) : "the composition default";
|
||||
console.warn(
|
||||
`[hyperframes] runtime_unknown_enum_value: ${move} variable "${entry.id}" got ${JSON.stringify(value)}, which is not a declared option (${allowed.join(", ")}). Rendering ${fallback} instead.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/** Test-only: clear the per-page dedupe set. */
|
||||
export function resetUnknownEnumWarnings(): void {
|
||||
warnedUnknownEnumValues.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user