fix(sdk): address PR #2100 review feedback on attachSync

- Script-mirror filter changed from an exact "/script/gsap" match to
  path.startsWith("/script/") — the documented contract is "never mirror
  script-tag rewrites," not just today's one known path; startsWith covers
  any future script-kind patch under the same intent.
- _syncDetach is now cleared when the caller invokes the returned detach
  function directly, not only on the next attachSync call — avoids holding
  a stale (already-unsubscribed) reference between an explicit detach() and
  a later attachSync(other).
- The initial applyOverrideSet call is now wrapped in try/catch: a bad
  initial snapshot no longer prevents the ongoing patch subscription from
  attaching, matching the SDK's existing swallow-and-warn precedent for
  silent-failure paths (adapters/iframe.ts's tainted-canvas warning).
- Added a test proving declareVariable/removeVariable (the /variable-decls/
  patches PR #2098 introduces) mirror onto the live document's
  data-composition-variables attribute — the existing suite only covered
  setVariableValue's CSS-custom-property path, not the schema-metadata path.
This commit is contained in:
Vance Ingalls
2026-07-09 11:52:14 -07:00
parent fbb6a33c91
commit c94de6034e
2 changed files with 35 additions and 5 deletions
@@ -219,6 +219,21 @@ window.__timelines = { t: tl };</script>
expect(liveRoot.style.getPropertyValue("--accent")).toBe("#0f0");
});
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);
const adapter = createIframePreviewAdapter(iframe);
adapter.attachSync(comp);
comp.declareVariable({ id: "accent", type: "string", label: "Accent", default: "#fff" });
const liveDocEl = iframe.contentDocument!.documentElement;
expect(liveDocEl.getAttribute("data-composition-variables")).toContain("accent");
comp.removeVariable("accent");
expect(liveDocEl.getAttribute("data-composition-variables")).not.toContain("accent");
});
it("mirrors setTiming onto the live element's data-start/data-end attributes", async () => {
const iframe = mountIframe(BASE_HTML);
const comp = await openComposition(BASE_HTML);
+20 -5
View File
@@ -754,20 +754,35 @@ class IframePreviewAdapter implements PreviewAdapter {
const doc = this.iframe.contentDocument;
if (doc) {
applyOverrideSet({ document: doc, wrapped: false, stamped: "" }, comp.getOverrides());
try {
applyOverrideSet({ document: doc, wrapped: false, stamped: "" }, comp.getOverrides());
} catch (err) {
// Don't let a bad initial snapshot prevent the ongoing subscription
// below from attaching — future patches should still mirror even if
// this composition's current overrides couldn't be applied.
console.warn("[hyperframes] attachSync: initial override sync failed:", err);
}
}
const unsubscribe = comp.on("patch", ({ patches }) => {
const rawUnsubscribe = comp.on("patch", ({ patches }) => {
const liveDoc = this.iframe.contentDocument;
if (!liveDoc) return;
applyPatchesToDocument(
{ document: liveDoc, wrapped: false, stamped: "" },
patches.filter((p) => p.path !== "/script/gsap"),
// "Never mirror script-tag rewrites" is the documented contract, not
// just today's one known path — startsWith so a future script kind
// (e.g. "/script/label") is covered by the same intent, not just an
// exact string this filter happens to know about today.
patches.filter((p) => !p.path.startsWith("/script/")),
);
});
this._syncDetach = unsubscribe;
return unsubscribe;
const detach = (): void => {
rawUnsubscribe();
if (this._syncDetach === detach) this._syncDetach = null;
};
this._syncDetach = detach;
return detach;
}
}