Skip to content

Commit 8c210c3

Browse files
vanceingallsclaude
andcommitted
fix(sdk): cross-realm origin sentinel, dual width/height channel, contract docs
Round-2 review (Rames/Miguel) on the engine layer: - ORIGIN_APPLY_PATCHES: unique symbol → namespaced string ('@hyperframes/sdk:applyPatches'). Symbols are realm-local — they don't survive postMessage/structured-clone, which T3 embedded hosts may forward patch events across. Namespaced string keeps collision risk negligible. - setCompositionMetadata width/height: runtime treats data-width/data-height as a forced override of inline style (init.ts applyCompositionSizing). Style is always written; the data-* attr is updated when already present so the edit isn't clobbered on load. Absent attrs stay absent — inverses stay exact. Mirrored in the patch applier; 3 new tests. - JsonPatchOp documented as the emit-only RFC 6902 subset (add/remove/replace); applier header notes move/copy/test are ignored. - SdkDocument.html documented as a build-time snapshot (serialize() is the live state). - patches.ts path-grammar comment fixed: timing/{start|end|trackIndex}. NOT changed (with reasons, see PR reply): moveElement left/top matches Studio's own inline-style commit convention (sourcePatcher); package version follows the repo-wide single-version policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 30f7c4f commit 8c210c3

5 files changed

Lines changed: 83 additions & 9 deletions

File tree

packages/sdk/src/engine/apply-patches.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
*
44
* Not a general-purpose JSON Patch implementation. Translates the well-defined path
55
* grammar back into DOM mutations. Used by applyPatches() for host undo (T3 mode).
6+
*
7+
* Supports only the emit subset (add/remove/replace) — move/copy/test ops and
8+
* unknown paths are silently ignored, matching the JsonPatchOp contract.
69
*/
710

811
import type { JsonPatchOp } from "../types.js";
@@ -164,12 +167,26 @@ function applyOne(parsed: ParsedDocument, patch: JsonPatchOp, p: ParsedPath): vo
164167
case "metadata": {
165168
const root = findRoot(parsed.document);
166169
if (!root || !p.field) return;
170+
// Mirror mutate.ts: style always written; the data-* forced-override
171+
// attribute is updated only when the composition already carries it.
167172
if (p.field === "width") {
168-
if (patch.op === "remove") setElementStyles(root, { width: null });
169-
else setElementStyles(root, { width: `${patch.value}px` });
173+
if (patch.op === "remove") {
174+
setElementStyles(root, { width: null });
175+
root.removeAttribute("data-width");
176+
} else {
177+
setElementStyles(root, { width: `${patch.value}px` });
178+
if (root.hasAttribute("data-width")) root.setAttribute("data-width", String(patch.value));
179+
}
170180
} else if (p.field === "height") {
171-
if (patch.op === "remove") setElementStyles(root, { height: null });
172-
else setElementStyles(root, { height: `${patch.value}px` });
181+
if (patch.op === "remove") {
182+
setElementStyles(root, { height: null });
183+
root.removeAttribute("data-height");
184+
} else {
185+
setElementStyles(root, { height: `${patch.value}px` });
186+
if (root.hasAttribute("data-height")) {
187+
root.setAttribute("data-height", String(patch.value));
188+
}
189+
}
173190
} else if (p.field === "duration") {
174191
if (patch.op === "remove") root.removeAttribute("data-duration");
175192
else root.setAttribute("data-duration", String(patch.value));

packages/sdk/src/engine/mutate.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,3 +388,38 @@ describe("Phase 3b ops", () => {
388388
).toBe(false);
389389
});
390390
});
391+
392+
// ─── setCompositionMetadata — data-width/data-height forced override ─────────
393+
394+
describe("setCompositionMetadata data-* channel", () => {
395+
const ATTR_HTML = `
396+
<div data-hf-id="hf-stage" data-hf-root data-width="1280" data-height="720" style="width: 1280px; height: 720px">
397+
<h1 data-hf-id="hf-title">Hi</h1>
398+
</div>
399+
`.trim();
400+
401+
it("updates data-width/data-height when the composition carries them", () => {
402+
const parsed = parseMutable(ATTR_HTML);
403+
applyOp(parsed, { type: "setCompositionMetadata", width: 1920, height: 1080 });
404+
const root = parsed.document.querySelector("[data-hf-root]");
405+
expect(root?.getAttribute("data-width")).toBe("1920");
406+
expect(root?.getAttribute("data-height")).toBe("1080");
407+
expect(root?.getAttribute("style")).toContain("width: 1920px");
408+
});
409+
410+
it("inverse restores both channels", () => {
411+
const parsed = parseMutable(ATTR_HTML);
412+
const before = serializeDocument(parsed);
413+
const { inverse } = applyOp(parsed, { type: "setCompositionMetadata", width: 1920 });
414+
applyPatchesToDocument(parsed, inverse);
415+
expect(serializeDocument(parsed)).toBe(before);
416+
});
417+
418+
it("does not mint data-* attributes on compositions without them", () => {
419+
const parsed = fresh();
420+
applyOp(parsed, { type: "setCompositionMetadata", width: 1920 });
421+
const root = parsed.document.querySelector("[data-hf-root]");
422+
expect(root?.hasAttribute("data-width")).toBe(false);
423+
expect(root?.getAttribute("style")).toContain("width: 1920px");
424+
});
425+
});

packages/sdk/src/engine/mutate.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,18 @@ function handleSetCompositionMetadata(
304304
const root = findRoot(parsed.document);
305305
if (!root) return result;
306306

307+
// The runtime treats data-width/data-height as a FORCED override of inline
308+
// style when present (core/runtime/init.ts applyCompositionSizing). So:
309+
// style is always written; the data-* attribute is updated only when the
310+
// composition already carries it — otherwise a style-only write would be
311+
// clobbered on load. Absent attributes stay absent (keeps inverses exact).
307312
if (op.width !== undefined) {
308313
const styles = getElementStyles(root);
309-
const oldWidth = styles["width"] ?? null;
314+
const oldAttr = root.getAttribute("data-width");
315+
const oldWidth = oldAttr ?? styles["width"] ?? null;
310316
const newVal = `${op.width}px`;
311317
setElementStyles(root, { width: newVal });
318+
if (oldAttr !== null) root.setAttribute("data-width", String(op.width));
312319
const path = metaPath("width");
313320
const p = scalarChange(path, oldWidth !== null ? parseFloat(oldWidth) : null, op.width);
314321
result.forward.push(p.forward);
@@ -317,9 +324,11 @@ function handleSetCompositionMetadata(
317324

318325
if (op.height !== undefined) {
319326
const styles = getElementStyles(root);
320-
const oldHeight = styles["height"] ?? null;
327+
const oldAttr = root.getAttribute("data-height");
328+
const oldHeight = oldAttr ?? styles["height"] ?? null;
321329
const newVal = `${op.height}px`;
322330
setElementStyles(root, { height: newVal });
331+
if (oldAttr !== null) root.setAttribute("data-height", String(op.height));
323332
const path = metaPath("height");
324333
const p = scalarChange(path, oldHeight !== null ? parseFloat(oldHeight) : null, op.height);
325334
result.forward.push(p.forward);

packages/sdk/src/engine/patches.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* /elements/{hfId}/inlineStyles/{camelCaseProp}
66
* /elements/{hfId}/text
77
* /elements/{hfId}/attributes/{name}
8-
* /elements/{hfId}/timing/{start|duration|trackIndex}
8+
* /elements/{hfId}/timing/{start|end|trackIndex} ← end = computed absolute data-end
99
* /elements/{hfId}/hold/{start|end|fill}
1010
* /elements/{hfId} ← whole subtree (removeElement)
1111
* /variables/{variableId}

packages/sdk/src/types.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ export interface SdkDocument {
2828
readonly width: number | null;
2929
readonly height: number | null;
3030
readonly compositionDuration: number | null;
31-
/** ensureHfIds-stamped HTML used as the source of truth for serialization */
31+
/**
32+
* BUILD-TIME snapshot of the ensureHfIds-stamped HTML. Never updated after
33+
* mutations — use Composition.serialize() for the current document state.
34+
*/
3235
readonly html: string;
3336
}
3437

@@ -105,6 +108,11 @@ export interface GsapTweenSpec {
105108

106109
// ─── Patch layer (F2: RFC 6902 frozen contract) ───────────────────────────────
107110

111+
/**
112+
* Emit-only subset of RFC 6902: the SDK never emits move/copy/test, and
113+
* applyPatches() ignores ops outside this subset. Hosts feeding patches back
114+
* must restrict themselves to add/remove/replace.
115+
*/
108116
export interface JsonPatchOp {
109117
op: "add" | "remove" | "replace";
110118
path: string;
@@ -131,8 +139,13 @@ export interface PatchEvent {
131139
* Reserved origin tag for applyPatches().
132140
* Host listeners MUST skip this origin to prevent undo loops:
133141
* comp.on('patch', ({ origin }) => { if (origin === ORIGIN_APPLY_PATCHES) return; ... })
142+
*
143+
* A namespaced string (not a unique symbol) so the sentinel survives realm
144+
* boundaries — postMessage, structured clone, JSON — which T3 embedded hosts
145+
* may forward patch events across. The namespace prefix keeps collision risk
146+
* with host-chosen origins negligible.
134147
*/
135-
export const ORIGIN_APPLY_PATCHES: unique symbol = Symbol("applyPatches");
148+
export const ORIGIN_APPLY_PATCHES = "@hyperframes/sdk:applyPatches" as const;
136149

137150
/** Default origin when none specified — UI-driven dispatch. */
138151
export const ORIGIN_LOCAL = "local" as const;

0 commit comments

Comments
 (0)