Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type {
export {
COMPOSITION_VARIABLE_TYPES,
parseCompositionVariables,
isCompositionVariable,
isScalarVariableValue,
} from "@hyperframes/parsers/composition";

Expand Down
6 changes: 5 additions & 1 deletion packages/parsers/src/composition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ export {
resolveAliasDisplayName,
} from "./fontAliases.js";
export { decodeUrlPathVariants } from "./utils/urlPath.js";
export { parseCompositionVariables, isScalarVariableValue } from "./compositionVariables.js";
export {
parseCompositionVariables,
isCompositionVariable,
isScalarVariableValue,
} from "./compositionVariables.js";
8 changes: 7 additions & 1 deletion packages/parsers/src/compositionVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@ function isVariableType(t: unknown): t is CompositionVariableType {
return typeof t === "string" && t in DEFAULT_TYPEOF;
}

function isCompositionVariable(v: unknown): v is CompositionVariable {
/**
* True when the value is a structurally valid variable declaration: id, label,
* a known type, a default matching that type, and options[] for enums. The
* same predicate parseCompositionVariables filters with — exported so writers
* (SDK declaration ops, Studio forms) can validate before persisting.
*/
export function isCompositionVariable(v: unknown): v is CompositionVariable {
if (!isRecord(v)) return false;
if (typeof v.id !== "string" || typeof v.label !== "string") return false;
if (!isVariableType(v.type)) return false;
Expand Down
9 changes: 6 additions & 3 deletions packages/sdk/src/adapters/iframe.sync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,10 @@ window.__timelines = { t: tl };</script>
});

it("mirrors declareVariable/removeVariable onto the live document's schema attribute", async () => {
const iframe = mountIframe(BASE_HTML); // no data-composition-variables at all
const comp = await openComposition(BASE_HTML);
// Full document (not a fragment): declareVariable refuses fragment sources.
const fullDoc = `<!DOCTYPE html><html><body>${BASE_HTML}</body></html>`;
const iframe = mountIframe(fullDoc); // no data-composition-variables at all
const comp = await openComposition(fullDoc);
const adapter = createIframePreviewAdapter(iframe);
adapter.attachSync(comp);

Expand All @@ -231,7 +233,8 @@ window.__timelines = { t: tl };</script>
expect(liveDocEl.getAttribute("data-composition-variables")).toContain("accent");

comp.removeVariable("accent");
expect(liveDocEl.getAttribute("data-composition-variables")).not.toContain("accent");
// Removing the last declaration drops the attribute entirely (null).
expect(liveDocEl.getAttribute("data-composition-variables") ?? "").not.toContain("accent");
});

it("mirrors setTiming onto the live element's data-start/data-end attributes", async () => {
Expand Down
65 changes: 29 additions & 36 deletions packages/sdk/src/engine/apply-patches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,19 @@ import { keyToPath, stylePath } from "./patches.js";
import {
writeVariableDefault,
clearVariableDefault,
declareVariableDecl,
removeVariableDecl,
type VariableDecl,
writeVariableDeclaration,
removeVariableDeclarationEntry,
} from "./variableModel.js";

function isRawDeclarationEntry(value: unknown): value is { id: string } & Record<string, unknown> {
return (
typeof value === "object" &&
value !== null &&
!Array.isArray(value) &&
typeof (value as { id?: unknown }).id === "string"
);
}

// ─── Path parser ────────────────────────────────────────────────────────────

interface ParsedPath {
Expand All @@ -38,7 +46,7 @@ interface ParsedPath {
| "hold"
| "element"
| "variable"
| "variable-decl"
| "variableDeclaration"
| "metadata"
| "script"
| "stylesheet";
Expand Down Expand Up @@ -72,12 +80,12 @@ function parsePath(path: string): ParsedPath | null {
const elemM = /^\/elements\/([^/]+)$/.exec(path);
if (elemM) return { type: "element", id: elemM[1] };

const varDeclM = /^\/variableDeclarations\/(.+)$/.exec(path);
if (varDeclM) return { type: "variableDeclaration", id: varDeclM[1] };

const varM = /^\/variables\/(.+)$/.exec(path);
if (varM) return { type: "variable", id: varM[1] };

const varDeclM = /^\/variable-decls\/(.+)$/.exec(path);
if (varDeclM) return { type: "variable-decl", id: varDeclM[1] };

const metaM = /^\/metadata\/(.+)$/.exec(path);
if (metaM) return { type: "metadata", field: metaM[1] };

Expand Down Expand Up @@ -239,6 +247,20 @@ function applyOne(parsed: ParsedDocument, patch: JsonPatchOp, p: ParsedPath): vo
break;
}

case "variableDeclaration": {
if (!p.id) return;
if (patch.op === "remove") {
removeVariableDeclarationEntry(parsed.document, p.id);
} else if (isRawDeclarationEntry(patch.value)) {
// Replay is faithful, not strict: inverse patches capture raw entries
// (loose hand-authored declarations included) and undo must restore
// them verbatim — gating on isCompositionVariable here would make
// undo of a remove/update on a loose entry silently no-op.
writeVariableDeclaration(parsed.document, patch.value);
}
break;
}

case "variable": {
if (!p.id) return;
// B1: update the JSON model (data-composition-variables) so
Expand All @@ -249,35 +271,6 @@ function applyOne(parsed: ParsedDocument, patch: JsonPatchOp, p: ParsedPath): vo
break;
}

case "variable-decl": {
if (!p.id) return;
// Distinct from "variable" above: this replays the WHOLE declaration
// (declareVariable/removeVariable), not just its default value.
if (patch.op === "remove") {
removeVariableDecl(parsed.document, p.id);
} else {
// Undo of removeVariable bundles {__kind: "reinsert", decl, index} to
// reinsert at the original array position; a plain declareVariable
// forward/replace patch carries the bare decl. Disambiguate on the
// __kind tag, not structural "decl"/"index" key presence — VariableDecl
// has an open index signature, so a genuine variable schema could
// legally declare its own "decl"/"index" fields, and "in" narrowing
// alone can't rule that out.
const value = patch.value;
if (
value &&
typeof value === "object" &&
(value as { __kind?: unknown }).__kind === "reinsert"
) {
const wrapped = value as { decl: VariableDecl; index: number };
declareVariableDecl(parsed.document, wrapped.decl, { atIndex: wrapped.index });
} else {
declareVariableDecl(parsed.document, value as VariableDecl);
}
}
break;
}

case "script": {
if (patch.op === "remove") {
setGsapScript(parsed.document, "");
Expand Down
89 changes: 47 additions & 42 deletions packages/sdk/src/engine/mutate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ function fresh() {
return parseMutable(BASE_HTML);
}

// Full document (BASE_HTML wrapped in <html>) with NO declarations — the shape
// declareVariable requires (it refuses fragment sources whose synthetic <html>
// is stripped on serialize).
function freshDoc() {
return parseMutable(`<!DOCTYPE html><html><body>${BASE_HTML}</body></html>`);
}

/** Full HTML fixture with data-composition-variables for B1/B2 tests. */
const VARIABLES_HTML = `<!DOCTYPE html>
<html data-composition-id="c1" data-composition-duration="5" data-composition-variables='${JSON.stringify(
Expand Down Expand Up @@ -930,11 +937,11 @@ function readVarDecl(

describe("declareVariable", () => {
it("creates the data-composition-variables attribute from scratch when absent", () => {
const parsed = fresh(); // BASE_HTML has no data-composition-variables at all
const parsed = freshDoc(); // full doc, no data-composition-variables at all
expect(parsed.document.documentElement?.getAttribute("data-composition-variables")).toBeNull();
applyOp(parsed, {
type: "declareVariable",
decl: { id: "brand-title", type: "string", label: "Title", default: "Hello" },
declaration: { id: "brand-title", type: "string", label: "Title", default: "Hello" },
});
expect(readVarDecl(parsed, "brand-title")).toEqual({
id: "brand-title",
Expand All @@ -948,25 +955,43 @@ describe("declareVariable", () => {
const parsed = freshWithVars();
applyOp(parsed, {
type: "declareVariable",
decl: { id: "brand-tagline", type: "string", label: "Tagline", default: "Ship it" },
declaration: { id: "brand-tagline", type: "string", label: "Tagline", default: "Ship it" },
});
expect(readVarDecl(parsed, "brand-color-primary")).toBeDefined(); // untouched
expect(readVarDecl(parsed, "brand-tagline")?.default).toBe("Ship it");
});

it("replaces the WHOLE existing decl (not just default) when the id already exists", () => {
it("declareVariable no-ops on an existing id; updateVariableDeclaration replaces the whole decl", () => {
const parsed = freshWithVars();
// Canonical semantics: declareVariable creates only — re-declaring an existing
// id is a no-op; updateVariableDeclaration is the path that replaces a decl.
applyOp(parsed, {
type: "declareVariable",
decl: { id: "brand-color-primary", type: "color", label: "Renamed", default: "#00ff00" },
declaration: {
id: "brand-color-primary",
type: "color",
label: "Ignored",
default: "#111111",
},
});
expect(readVarDecl(parsed, "brand-color-primary")?.label).not.toBe("Ignored");
applyOp(parsed, {
type: "updateVariableDeclaration",
id: "brand-color-primary",
declaration: {
id: "brand-color-primary",
type: "color",
label: "Renamed",
default: "#00ff00",
},
});
const decl = readVarDecl(parsed, "brand-color-primary");
expect(decl?.label).toBe("Renamed");
expect(decl?.default).toBe("#00ff00");
});

it("succeeds where setVariableValue would refuse — creating an undeclared variable", () => {
const parsed = fresh();
const parsed = freshDoc();
// setVariableValue on an undeclared id still writes the --{id} CSS compat
// prop unconditionally (for CSS-only compositions with no JSON schema at
// all) — but the JSON model write itself no-ops, per writeVariableDefault's
Expand All @@ -976,7 +1001,7 @@ describe("declareVariable", () => {
expect(readVarDecl(parsed, "never-declared")).toBeUndefined();
applyOp(parsed, {
type: "declareVariable",
decl: { id: "never-declared", type: "string", label: "New", default: "x" },
declaration: { id: "never-declared", type: "string", label: "New", default: "x" },
});
expect(readVarDecl(parsed, "never-declared")?.default).toBe("x");
});
Expand All @@ -987,36 +1012,10 @@ describe("declareVariable", () => {

const created = applyOp(parsed, {
type: "declareVariable",
decl: { id: "brand-new", type: "string", label: "New", default: "x" },
declaration: { id: "brand-new", type: "string", label: "New", default: "x" },
});
applyPatchesToDocument(parsed, created.inverse);
expect(serializeDocument(parsed)).toBe(before);

const edited = applyOp(parsed, {
type: "declareVariable",
decl: { id: "brand-color-primary", type: "color", label: "Edited", default: "#000000" },
});
applyPatchesToDocument(parsed, edited.inverse);
expect(serializeDocument(parsed)).toBe(before);
});

it("a decl whose own extension data includes 'decl'/'index' keys isn't mistaken for a wrapped reinsert", () => {
// VariableDecl has an open index signature, so a real, weird schema can
// legally carry fields named "decl"/"index" as ordinary extension data —
// this must NOT be confused with removeVariable's {__kind: "reinsert",
// decl, index} inverse-patch wrapper, which is disambiguated by __kind,
// not by structural key presence.
const parsed = fresh();
const pathologicalDecl = {
id: "weird-var",
type: "string",
label: "Weird",
default: "x",
decl: "not-a-real-decl",
index: 999,
};
applyOp(parsed, { type: "declareVariable", decl: pathologicalDecl });
expect(readVarDecl(parsed, "weird-var")).toEqual(pathologicalDecl);
});
});

Expand All @@ -1034,13 +1033,16 @@ describe("removeVariable", () => {
expect(result.inverse).toHaveLength(0);
});

it("inverse restores the exact removed declaration", () => {
it("inverse restores the removed declaration", () => {
// Canonical remove re-adds the declaration on undo (array position is not
// preserved), so assert the decl is restored by content rather than exact
// byte-serialize.
const parsed = freshWithVars();
const before = serializeDocument(parsed);
const original = readVarDecl(parsed, "brand-color-primary");
const result = applyOp(parsed, { type: "removeVariable", id: "brand-color-primary" });
expect(readVarDecl(parsed, "brand-color-primary")).toBeUndefined();
applyPatchesToDocument(parsed, result.inverse);
expect(serializeDocument(parsed)).toBe(before);
expect(readVarDecl(parsed, "brand-color-primary")).toEqual(original);
});
});

Expand Down Expand Up @@ -1151,22 +1153,25 @@ describe("validateOp", () => {

it("returns ok:true for declareVariable / removeVariable when a root exists", () => {
expect(
validateOp(fresh(), {
validateOp(freshDoc(), {
type: "declareVariable",
decl: { id: "v1", type: "string", label: "V1", default: "x" },
declaration: { id: "v1", type: "string", label: "V1", default: "x" },
}).ok,
).toBe(true);
expect(validateOp(fresh(), { type: "removeVariable", id: "v1" }).ok).toBe(true);
});

it("returns ok:false / E_NO_ROOT for declareVariable / removeVariable with no root", () => {
it("refuses declareVariable / removeVariable on a rootless fragment", () => {
const parsed = parseMutable(`no elements at all — just text`);
// declareVariable runs its declaration precondition first, so a wrapped
// fragment (no real <html> to carry the schema) surfaces E_FRAGMENT_COMPOSITION.
const r1 = validateOp(parsed, {
type: "declareVariable",
decl: { id: "v1", type: "string", label: "V1", default: "x" },
declaration: { id: "v1", type: "string", label: "V1", default: "x" },
});
expect(r1.ok).toBe(false);
if (!r1.ok) expect(r1.code).toBe("E_NO_ROOT");
if (!r1.ok) expect(r1.code).toBe("E_FRAGMENT_COMPOSITION");
// removeVariable only needs a root; there is none → E_NO_ROOT.
const r2 = validateOp(parsed, { type: "removeVariable", id: "v1" });
expect(r2.ok).toBe(false);
if (!r2.ok) expect(r2.code).toBe("E_NO_ROOT");
Expand Down
Loading
Loading