From 4dcdfad7a6e2b43e9d2ea263893cba10830cc88a Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Fri, 9 Oct 2026 03:45:49 +0530 Subject: [PATCH 1/3] fix(core): resolve a relative data-start inside the clip's own composition --- .../core/src/runtime/startResolver.test.ts | 29 +++++++++++++++++++ packages/core/src/runtime/startResolver.ts | 15 ++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/core/src/runtime/startResolver.test.ts b/packages/core/src/runtime/startResolver.test.ts index 486b1359233..0880ffe99e2 100644 --- a/packages/core/src/runtime/startResolver.test.ts +++ b/packages/core/src/runtime/startResolver.test.ts @@ -275,6 +275,35 @@ describe("createRuntimeStartTimeResolver", () => { expect(resolver.resolveStartForElement(video)).toBe(26); }); + it("resolves a reference inside the referencing clip's own composition", () => { + const mount = (hostId: string, start: string) => { + const host = document.createElement("div"); + host.id = hostId; + host.setAttribute("data-composition-id", "card"); + host.setAttribute("data-start", start); + host.setAttribute("data-duration", "2"); + document.body.appendChild(host); + + const first = document.createElement("div"); + first.id = "first"; + first.setAttribute("data-start", "0"); + first.setAttribute("data-duration", "1"); + host.appendChild(first); + + const second = document.createElement("div"); + second.setAttribute("data-start", "first"); + second.setAttribute("data-duration", "1"); + host.appendChild(second); + return second; + }; + const secondInA = mount("card-a", "0"); + const secondInB = mount("card-b", "2"); + + const resolver = createRuntimeStartTimeResolver({}); + expect(resolver.resolveStartForElement(secondInA)).toBe(1); + expect(resolver.resolveStartForElement(secondInB)).toBe(3); + }); + it("returns fallback when reference target not found", () => { const el = document.createElement("div"); el.setAttribute("data-start", "nonexistent"); diff --git a/packages/core/src/runtime/startResolver.ts b/packages/core/src/runtime/startResolver.ts index 28c4b3d19b4..e6ecf1f906b 100644 --- a/packages/core/src/runtime/startResolver.ts +++ b/packages/core/src/runtime/startResolver.ts @@ -46,7 +46,18 @@ export function createRuntimeStartTimeResolver(params: { const durationCache = new WeakMap(); const visiting = new Set(); - const findReferenceTarget = (refId: string): Element | null => { + const compositionScopeOf = (element: Element): Element | null => + (element.hasAttribute("data-composition-id") ? element.parentElement : element)?.closest( + "[data-composition-id]", + ) ?? null; + + const findReferenceTarget = (refId: string, from: Element): Element | null => { + const scope = compositionScopeOf(from); + if (scope) { + for (const candidate of scope.querySelectorAll("[id]")) { + if (candidate.id === refId && compositionScopeOf(candidate) === scope) return candidate; + } + } const byId = doc.getElementById(refId); if (byId) return byId; return ( @@ -135,7 +146,7 @@ export function createRuntimeStartTimeResolver(params: { // Negative when a host's in-point is past its start; the host's slot cuts that part. return resolveHostOffsetForElement(element, fallback) + Math.max(0, expression.value); } - const target = findReferenceTarget(expression.refId); + const target = findReferenceTarget(expression.refId, element); if (!target) return fallback; const targetStart = resolveStartForElementInternal(target, 0); const targetDuration = resolveDurationForElement(target) ?? 0; From 6744ea9ebb30aa8fc226c6033de916f760ac6ca5 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:27:27 +0530 Subject: [PATCH 2/3] fix(engine): render-side data-start references resolve in the clip's own composition The engine's reference resolver still looked an id up document-wide, so with two different compositions sharing an id the producer placed media from the first one while the runtime used the clip's own. Scope it the same way as the runtime's startResolver, with the same document-wide fallback, and correct the SDK comments that called the runtime lookup global. --- .../src/services/videoFrameExtractor.test.ts | 15 ++++++++++ packages/parsers/src/referenceResolver.ts | 30 +++++++++++++++++-- packages/sdk/src/session.timings.test.ts | 4 +-- packages/sdk/src/session.ts | 6 ++-- 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/services/videoFrameExtractor.test.ts b/packages/engine/src/services/videoFrameExtractor.test.ts index 442e77035ab..4ac7a391c2b 100644 --- a/packages/engine/src/services/videoFrameExtractor.test.ts +++ b/packages/engine/src/services/videoFrameExtractor.test.ts @@ -872,6 +872,21 @@ describe("parseVideoElements", () => { expect(videos.find((v) => v.id === "main")?.start).toBe(9); }); + it("resolves a reference inside its own composition when another composition reuses the id", () => { + const videos = parseVideoElements( + '
' + + '
' + + '
' + + "
" + + '
' + + '
' + + '' + + "
" + + "
", + ); + expect(videos.find((v) => v.id === "clip")?.start).toBe(3); + }); + it("falls back to 0 (never NaN) for an unknown reference target", () => { const videos = parseVideoElements( '', diff --git a/packages/parsers/src/referenceResolver.ts b/packages/parsers/src/referenceResolver.ts index 6679ed0bc53..494b33cf5f7 100644 --- a/packages/parsers/src/referenceResolver.ts +++ b/packages/parsers/src/referenceResolver.ts @@ -5,15 +5,39 @@ import { parseNumeric, parseStartExpression } from "./compositionContract.js"; /** Minimal structural DOM shape the reference resolver needs. */ export interface RefResolverEl { + readonly id: string; + readonly parentElement: RefResolverEl | null; getAttribute(name: string): string | null; + hasAttribute(name: string): boolean; + closest(selector: string): RefResolverEl | null; + querySelectorAll(selector: string): Iterable; } export interface RefResolverDoc { getElementById(id: string): RefResolverEl | null; querySelector(selector: string): RefResolverEl | null; } -// By `id`, then `data-composition-id`. The reference grammar allows only `[A-Za-z0-9_.:-]`, so no CSS.escape. -function findReferenceTargetEl(doc: RefResolverDoc, refId: string): RefResolverEl | null { +function compositionScopeOf(el: RefResolverEl): RefResolverEl | null { + return ( + (el.hasAttribute("data-composition-id") ? el.parentElement : el)?.closest( + "[data-composition-id]", + ) ?? null + ); +} + +// By `id` in the clip's own composition, then document-wide by `id`, then `data-composition-id`. +// The reference grammar allows only `[A-Za-z0-9_.:-]`, so no CSS.escape. +function findReferenceTargetEl( + doc: RefResolverDoc, + refId: string, + from: RefResolverEl, +): RefResolverEl | null { + const scope = compositionScopeOf(from); + if (scope) { + for (const candidate of scope.querySelectorAll("[id]")) { + if (candidate.id === refId && compositionScopeOf(candidate) === scope) return candidate; + } + } return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`); } @@ -40,7 +64,7 @@ export function resolveReferencedStart( startCache.set(el, value); return value; } - const target = findReferenceTargetEl(doc, expression.refId); + const target = findReferenceTargetEl(doc, expression.refId, el); if (!target) { startCache.set(el, 0); return 0; diff --git a/packages/sdk/src/session.timings.test.ts b/packages/sdk/src/session.timings.test.ts index c6ce94fa3ce..5677e8da96b 100644 --- a/packages/sdk/src/session.timings.test.ts +++ b/packages/sdk/src/session.timings.test.ts @@ -224,8 +224,8 @@ describe("getElementTimings — relative data-start references", () => { it("resolves a colliding bare id to the TOP-LEVEL match, not a same-scope sibling", async () => { // Bare ids have no scope syntax — resolveScoped's bare-id rule prefers the - // canonical top-level match when one exists, same as the runtime's own (also - // global, not scope-aware) resolver. Both the outer document AND the sub-comp + // canonical top-level match when one exists. The runtime's `id` lookup prefers + // the clip's own composition instead. Both the outer document AND the sub-comp // author an element with the SAME bare id "hf-intro" — a genuine collision. const html = ` diff --git a/packages/sdk/src/session.ts b/packages/sdk/src/session.ts index 86d096126d1..780e0e2a93e 100644 --- a/packages/sdk/src/session.ts +++ b/packages/sdk/src/session.ts @@ -381,9 +381,9 @@ class CompositionImpl implements Composition { this._gsapLabelCache = scripts.length ? { scripts: [...scripts], labels: allLabels } : null; } - // refId remains a bare global id, matching runtime resolution. The shared - // contract owns parsing, canonical duration preference, clamping, and - // legacy data-end compatibility; this adapter only supplies document lookup. + // refId resolves as a bare hf-id (top level first), unlike the runtime's + // composition-scoped `id` lookup. The shared contract owns parsing, canonical + // duration preference, clamping, and legacy data-end; this adapter only supplies lookup. const timingCache = new Map(); const visiting = new Set(); const resolveTiming = (el: Element): ClipTiming => { From 4640c1fa3419d0ffd497a57400846b82162ff5c1 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Thu, 22 Oct 2026 13:13:58 +0530 Subject: [PATCH 3/3] fix(parsers): keep getAttribute-only reference adapters working The scoped lookup needs parentElement, closest and querySelectorAll, but RefResolverEl is exported from @hyperframes/parsers and @hyperframes/engine. Make them optional and fall back to the document-wide lookup when an adapter lacks them. --- packages/parsers/src/referenceResolver.test.ts | 17 +++++++++++++++++ packages/parsers/src/referenceResolver.ts | 16 ++++++++-------- 2 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 packages/parsers/src/referenceResolver.test.ts diff --git a/packages/parsers/src/referenceResolver.test.ts b/packages/parsers/src/referenceResolver.test.ts new file mode 100644 index 00000000000..d53cb5812f4 --- /dev/null +++ b/packages/parsers/src/referenceResolver.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; +import { resolveReferencedStart, type RefResolverEl } from "./referenceResolver.js"; + +describe("resolveReferencedStart", () => { + it("still resolves through an adapter that only implements getAttribute", () => { + const element = (attrs: Record): RefResolverEl => ({ + getAttribute: (name) => attrs[name] ?? null, + }); + const intro = element({ id: "intro", "data-start": "1", "data-duration": "3" }); + const clip = element({ id: "clip", "data-start": "intro + 1" }); + const doc = { + getElementById: (id: string) => (id === "intro" ? intro : null), + querySelector: () => null, + }; + expect(resolveReferencedStart(doc, clip, new Map(), new Set())).toBe(5); + }); +}); diff --git a/packages/parsers/src/referenceResolver.ts b/packages/parsers/src/referenceResolver.ts index 494b33cf5f7..ec449f170ae 100644 --- a/packages/parsers/src/referenceResolver.ts +++ b/packages/parsers/src/referenceResolver.ts @@ -5,12 +5,10 @@ import { parseNumeric, parseStartExpression } from "./compositionContract.js"; /** Minimal structural DOM shape the reference resolver needs. */ export interface RefResolverEl { - readonly id: string; - readonly parentElement: RefResolverEl | null; getAttribute(name: string): string | null; - hasAttribute(name: string): boolean; - closest(selector: string): RefResolverEl | null; - querySelectorAll(selector: string): Iterable; + readonly parentElement?: RefResolverEl | null; + closest?(selector: string): RefResolverEl | null; + querySelectorAll?(selector: string): Iterable; } export interface RefResolverDoc { getElementById(id: string): RefResolverEl | null; @@ -19,7 +17,7 @@ export interface RefResolverDoc { function compositionScopeOf(el: RefResolverEl): RefResolverEl | null { return ( - (el.hasAttribute("data-composition-id") ? el.parentElement : el)?.closest( + (el.getAttribute("data-composition-id") !== null ? el.parentElement : el)?.closest?.( "[data-composition-id]", ) ?? null ); @@ -34,8 +32,10 @@ function findReferenceTargetEl( ): RefResolverEl | null { const scope = compositionScopeOf(from); if (scope) { - for (const candidate of scope.querySelectorAll("[id]")) { - if (candidate.id === refId && compositionScopeOf(candidate) === scope) return candidate; + for (const candidate of scope.querySelectorAll?.("[id]") ?? []) { + if (candidate.getAttribute("id") === refId && compositionScopeOf(candidate) === scope) { + return candidate; + } } } return doc.getElementById(refId) ?? doc.querySelector(`[data-composition-id="${refId}"]`);