diff --git a/packages/core/src/compiler/compositionScoping.test.ts b/packages/core/src/compiler/compositionScoping.test.ts index 5d806ccdc..df3ce1d59 100644 --- a/packages/core/src/compiler/compositionScoping.test.ts +++ b/packages/core/src/compiler/compositionScoping.test.ts @@ -609,6 +609,71 @@ window.__afterTimeline = window.__timelines.scene; expect(scoped).not.toMatch(/#intro\b/); }); + it("rewrites a bare root [data-composition-id] box selector to target exactly one of host or wrapper", () => { + // A composition styling its own box (e.g. `display:flex` to center its + // children, or `padding` to offset it) via the bare composition-id + // selector. After flattenInnerRoot preserves the authored root as a + // wrapper below the host, that wrapper (marked data-hf-inner-root) is + // what actually parents the real children, so the box styling must land + // there instead of the host. It must land on exactly one of the two: + // targeting both would apply an additive property like `padding` twice, + // since the wrapper is nested inside the host. + const scoped = scopeCssToComposition( + '[data-composition-id="captions"] { display: flex; justify-content: center; }', + "captions", + ); + + expect(scoped).toContain( + '[data-composition-id="captions"]:not(:has([data-hf-inner-root])), ' + + '[data-composition-id="captions"] > [data-hf-inner-root]', + ); + }); + + it("matches exactly the wrapper (not the host too) when both exist in the flattened DOM shape", () => { + // Regression test: an earlier version of this fix targeted both the host + // and the wrapper (a plain OR), which doubles any additive property + // (e.g. padding-top) since the wrapper is nested inside the host. + const scoped = scopeCssToComposition( + '[data-composition-id="captions"] { padding-top: 200px; }', + "captions", + ); + const ruleMatch = scoped.match(/([^{]+)\{/); + const selectorText = ruleMatch?.[1]?.trim(); + if (!selectorText) throw new Error("expected a CSS rule to be produced"); + + const { document } = parseHTML( + '
' + + '
' + + "
", + ); + const matches = [...document.querySelectorAll(selectorText)]; + expect(matches.map((el) => el.id)).toEqual(["wrapper"]); + }); + + it("matches the host when no wrapper is present (non-flattened fallback)", () => { + const scoped = scopeCssToComposition( + '[data-composition-id="captions"] { padding-top: 200px; }', + "captions", + ); + const ruleMatch = scoped.match(/([^{]+)\{/); + const selectorText = ruleMatch?.[1]?.trim(); + if (!selectorText) throw new Error("expected a CSS rule to be produced"); + + const { document } = parseHTML('
'); + const matches = [...document.querySelectorAll(selectorText)]; + expect(matches.map((el) => el.id)).toEqual(["host"]); + }); + + it("leaves root-plus-descendant [data-composition-id] selectors as a plain scope prefix", () => { + const scoped = scopeCssToComposition( + '[data-composition-id="captions"] .title { color: red; }', + "captions", + ); + + expect(scoped).toContain('[data-composition-id="captions"] .title'); + expect(scoped).not.toContain("data-hf-inner-root"); + }); + it('does not rewrite [id="intro"] attribute selectors', () => { // The function only targets #intro hash selectors, not [id="intro"] attribute selectors const result = scopeCssToComposition( diff --git a/packages/core/src/compiler/compositionScoping.ts b/packages/core/src/compiler/compositionScoping.ts index 8ec81fc2b..be49f149a 100644 --- a/packages/core/src/compiler/compositionScoping.ts +++ b/packages/core/src/compiler/compositionScoping.ts @@ -1,6 +1,7 @@ import postcss, { type AtRule, type Node, type Rule } from "postcss"; const AUTHORED_ROOT_ID_ATTR = "data-hf-authored-id"; +const INNER_ROOT_ATTR = "data-hf-inner-root"; function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); @@ -117,6 +118,18 @@ function scopeSelector( "g", ); if (compositionIdPattern.test(trimmed)) { + const isRootBoxSelector = trimmed.replace(compositionIdPattern, "").trim() === ""; + if (isRootBoxSelector) { + // A bare root selector styles the composition's own box (flex/grid/ + // position/padding). When flattenInnerRoot preserves the authored root + // as a wrapper below `scope` (see prepareFlattenedInnerRoot), that + // wrapper is the element real children are laid out in, not `scope` + // itself, so the box styling must land there instead. It must land on + // exactly one of the two: applying it to both compounds any additive + // property (padding, margin, non-zero transform) since the wrapper + // sits nested inside the host and would inherit the effect twice. + return `${scope}:not(:has([${INNER_ROOT_ATTR}])), ${scope} > [${INNER_ROOT_ATTR}]`; + } return selectorWithoutRootTiming.replace(compositionIdPattern, scope); } const leading = selectorWithoutRootTiming.match(/^\s*/)?.[0] ?? ""; diff --git a/packages/core/src/compiler/inlineSubCompositions.test.ts b/packages/core/src/compiler/inlineSubCompositions.test.ts index bf52cffc4..c20a3c750 100644 --- a/packages/core/src/compiler/inlineSubCompositions.test.ts +++ b/packages/core/src/compiler/inlineSubCompositions.test.ts @@ -175,6 +175,52 @@ describe("inlineSubCompositions – #ID selector scoping divergence", () => { expect(scopedCss).toContain('[data-hf-authored-id="intro"]'); }); + it("with flattenInnerRoot: restores data-composition-id on the wrapper for an anonymous host", () => { + // Regression test: a host mounted via data-composition-src with no + // data-composition-id of its own (an "anonymous" host). The composition + // styles its own root box via the bare composition-id selector and a + // script self-references it too — both need something in the render DOM + // to actually carry that id once flattenInnerRoot strips it from the + // wrapper by default. + const { document } = parseHTML(` + +
+
+
+`); + const host = document.querySelector('[data-composition-src="scoped-text.html"]')!; + + const scopedTextHtml = ``; + + function flattenInnerRoot(innerRoot: Element): Element { + const clone = innerRoot.cloneNode(true) as Element; + clone.removeAttribute("data-composition-id"); + clone.removeAttribute("data-start"); + clone.removeAttribute("data-duration"); + clone.setAttribute("data-hf-inner-root", "true"); + return clone; + } + + const result = inlineSubCompositions(document, [host], { + resolveHtml: () => scopedTextHtml, + parseHtml: (html) => parseHTML(html).document, + flattenInnerRoot, + }); + + const wrapper = host.querySelector("[data-hf-inner-root]"); + expect(wrapper?.getAttribute("data-composition-id")).toBe("scoped-text"); + + const scopedCss = result.styles.join("\n"); + expect(scopedCss).toContain("display: flex"); + }); + it("extracts elements from sub-composition with original rel and crossorigin", () => { const subCompWithLinks = ` diff --git a/packages/core/src/compiler/inlineSubCompositions.ts b/packages/core/src/compiler/inlineSubCompositions.ts index bf74cc6a2..2d7f79da5 100644 --- a/packages/core/src/compiler/inlineSubCompositions.ts +++ b/packages/core/src/compiler/inlineSubCompositions.ts @@ -372,6 +372,16 @@ export function inlineSubCompositions( for (const child of [...innerRoot.querySelectorAll("style, script")]) child.remove(); if (flattenInnerRoot) { const prepared = flattenInnerRoot(innerRoot); + if (!compId && inferredCompId) { + // Anonymous host: flattenInnerRoot strips data-composition-id, + // assuming the host already carries the composition's identity. + // When the host has none, nothing in the render DOM matches the + // composition's own root-styling CSS or self-referencing scripts + // (e.g. document.querySelector('[data-composition-id="X"]')). + // Restore it on the wrapper so both keep resolving, same as + // before flattening preserved it via outerHTML. + prepared.setAttribute("data-composition-id", inferredCompId); + } hostEl.innerHTML = prepared.outerHTML || ""; } else { hostEl.innerHTML = compId ? innerRoot.innerHTML || "" : innerRoot.outerHTML || ""; diff --git a/packages/core/src/runtime/compositionLoader.test.ts b/packages/core/src/runtime/compositionLoader.test.ts index ba67cc438..180eb5076 100644 --- a/packages/core/src/runtime/compositionLoader.test.ts +++ b/packages/core/src/runtime/compositionLoader.test.ts @@ -972,6 +972,43 @@ describe("loadExternalCompositions", () => { expect(byCompAfterSecondMount?.["card-last"]).toBeUndefined(); }); }); + + it("preserves data-composition-id unflattened for a host with no id of its own (anonymous host)", async () => { + // Regression test documenting why this file's own prepareFlattenedInnerRoot + // (line ~527) does NOT need the same anonymous-host id-restoration that + // producer/bundler compilation needed: an anonymous host's authoredCompositionId + // is null, so mountCompositionContent's innerRoot lookup never runs, and it + // falls through to a raw document.importNode() of the whole template content + // instead of prepareFlattenedInnerRoot. The composition's own + // data-composition-id is never stripped in the first place, so its root-styling + // CSS and self-referencing querySelector('[data-composition-id="X"]') calls + // already resolve. See PR review discussion on #1886 for the audit trail. + const host = document.createElement("div"); + host.setAttribute("data-composition-src", "https://example.com/scoped-text.html"); + document.body.appendChild(host); + + const compositionHtml = ` + + `; + + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(compositionHtml, { status: 200 })); + + await loadExternalCompositions({ ...defaultParams }); + + // Not flattened: no data-hf-inner-root wrapper was created. + expect(host.querySelector("[data-hf-inner-root]")).toBeNull(); + // The composition's own root element, with its own id intact, is a + // direct descendant of the (still anonymous) host. + const mountedRoot = host.querySelector('[data-composition-id="scoped-text"]'); + expect(mountedRoot).not.toBeNull(); + expect(mountedRoot?.querySelector(".label")?.textContent).toBe( + "Scoped Text Should Stay Styled", + ); + }); }); describe("loadInlineTemplateCompositions", () => { diff --git a/packages/core/src/runtime/startResolver.test.ts b/packages/core/src/runtime/startResolver.test.ts index 272fb78bc..cd15d434f 100644 --- a/packages/core/src/runtime/startResolver.test.ts +++ b/packages/core/src/runtime/startResolver.test.ts @@ -178,6 +178,39 @@ describe("createRuntimeStartTimeResolver", () => { expect(resolver.resolveStartForElement(video)).toBe(54); }); + it("walks up to the host's data-start when the inner root has none (host has its own data-composition-id)", () => { + const host = document.createElement("div"); + host.setAttribute("data-composition-id", "montage"); + host.setAttribute("data-start", "10"); + document.body.appendChild(host); + + const innerRoot = document.createElement("div"); + innerRoot.setAttribute("data-composition-id", "scene-10"); + host.appendChild(innerRoot); + + const resolver = createRuntimeStartTimeResolver({}); + expect(resolver.resolveStartForElement(innerRoot)).toBe(10); + }); + + it("walks up to the host's data-start via data-composition-file (anonymous host, post-inlining)", () => { + // A host mounted via data-composition-src with no data-composition-id of + // its own. After inlining, data-composition-src is stripped and replaced + // with data-composition-file, and the composition's own id is restored + // onto the wrapper (which has no data-start of its own). + const host = document.createElement("div"); + host.setAttribute("data-composition-file", "compositions/reveal1.html"); + host.setAttribute("data-start", "4.619"); + document.body.appendChild(host); + + const wrapper = document.createElement("div"); + wrapper.setAttribute("data-composition-id", "reveal1"); + wrapper.setAttribute("data-hf-inner-root", "true"); + host.appendChild(wrapper); + + const resolver = createRuntimeStartTimeResolver({}); + expect(resolver.resolveStartForElement(wrapper)).toBe(4.619); + }); + it("keeps nested references in the host composition timeline", () => { const host = document.createElement("div"); host.id = "slide-5"; diff --git a/packages/core/src/runtime/startResolver.ts b/packages/core/src/runtime/startResolver.ts index a343bae69..a797717f2 100644 --- a/packages/core/src/runtime/startResolver.ts +++ b/packages/core/src/runtime/startResolver.ts @@ -161,15 +161,20 @@ export function createRuntimeStartTimeResolver(params: { // If this element is a loaded composition inner root (has data-composition-id // but no data-start), walk up to the host parent which carries the actual // timing. This happens when the host uses a different data-composition-id - // than the loaded file — e.g. host="montage" but file has "scene-10". - // Check both data-composition-src (runtime) and data-composition-id (bundled, - // where data-composition-src is stripped after inlining). + // than the loaded file — e.g. host="montage" but file has "scene-10", or + // when the host itself has no data-composition-id at all (an "anonymous" + // host) and the composition's own id was restored onto the inlined wrapper. + // Check data-composition-src (runtime, not yet inlined), data-composition-id + // (bundled/compiled host with its own id), and data-composition-file (the + // marker every inlined host gets, compiled or bundled, once + // data-composition-src is stripped — covers the anonymous-host case). if (element.hasAttribute("data-composition-id")) { const parent = element.parentElement; if ( parent && (parent.hasAttribute("data-composition-src") || - parent.hasAttribute("data-composition-id")) + parent.hasAttribute("data-composition-id") || + parent.hasAttribute("data-composition-file")) ) { const parentStart = resolveStartForElementInternal(parent, fallback); startCache.set(element, parentStart); diff --git a/packages/producer/src/services/htmlCompiler.test.ts b/packages/producer/src/services/htmlCompiler.test.ts index 089930a03..c2c368a82 100644 --- a/packages/producer/src/services/htmlCompiler.test.ts +++ b/packages/producer/src/services/htmlCompiler.test.ts @@ -920,6 +920,68 @@ describe("template-wrapped sub-composition media offsets", () => { expect(compiled.html).toContain("__hfNormalizeSelector"); }); + it("resolves a class selector on the authored root wrapper itself (issue #1847 repro)", async () => { + // The original bug report: a sub-composition root authored as + // `
` styled via + // `.scene-wrapper .title { color: red }`. Class-based descendant + // selectors anchored on the authored root's own class only resolve if + // the root survives as a real element in the render DOM, not just via + // id-selector rewriting to [data-hf-authored-id]. + const projectDir = mkdtempSync(join(tmpdir(), "hf-class-wrapper-")); + const compositionsDir = join(projectDir, "compositions"); + mkdirSync(compositionsDir, { recursive: true }); + writeFileSync( + join(projectDir, "index.html"), + ` + + + +
+
+
+ + +`, + ); + writeFileSync( + join(compositionsDir, "scene.html"), + ``, + ); + + const compiled = await compileForRender(projectDir, join(projectDir, "index.html"), projectDir); + const { document } = parseHTML(compiled.html); + const host = document.querySelector("#scene-host"); + + const wrapper = host?.querySelector(".scene-wrapper"); + expect(wrapper).not.toBeNull(); + expect(wrapper?.getAttribute("data-hf-authored-id")).toBe("scene-root"); + expect(wrapper?.querySelector(".title")?.textContent).toBe("ISSUE 1847 REPRO"); + // The authored class selector round-trips unmodified: no id rewriting + // is needed for a class selector, only the wrapper element surviving. + expect(compiled.html).toContain(".scene-wrapper .title"); + }); + it("preserves the inferred composition boundary when the host has no composition id", async () => { const projectDir = mkdtempSync(join(tmpdir(), "hf-anonymous-host-")); const compositionsDir = join(projectDir, "compositions"); @@ -954,7 +1016,12 @@ describe("template-wrapped sub-composition media offsets", () => { const host = document.querySelector("#scene-host"); expect(host?.getAttribute("data-composition-id")).toBeNull(); - expect(host?.querySelector('[data-composition-id="scene"] .title')?.textContent).toBe("Scene"); + // The host has no data-composition-id of its own, but the composition's + // own id is restored onto the flattened wrapper, so root-scoped + // selectors and self-referencing scripts still resolve. + const wrapper = host?.querySelector("[data-hf-inner-root]"); + expect(wrapper?.getAttribute("data-composition-id")).toBe("scene"); + expect(wrapper?.querySelector(".title")?.textContent).toBe("Scene"); expect(compiled.html).toContain('var __hfCompId = "scene";'); }); }); diff --git a/packages/producer/src/services/htmlCompiler.ts b/packages/producer/src/services/htmlCompiler.ts index 76fee27e4..b0dd79508 100644 --- a/packages/producer/src/services/htmlCompiler.ts +++ b/packages/producer/src/services/htmlCompiler.ts @@ -23,7 +23,10 @@ import { type ResolvedDuration, type UnresolvedElement, } from "@hyperframes/core"; -import { inlineSubCompositions as inlineSubCompositionsShared } from "@hyperframes/core/compiler"; +import { + inlineSubCompositions as inlineSubCompositionsShared, + prepareFlattenedInnerRoot, +} from "@hyperframes/core/compiler"; import { checkSubCompositionUsability, type ParsableDocumentLike, @@ -748,7 +751,14 @@ function inlineSubCompositions( }, parseHtml: (htmlStr: string) => parseHTML(htmlStr).document as unknown as Document, scriptErrorLabel: "[Compiler] Composition script failed", - compoundAuthoredRoot: true, + // Preserve the authored root wrapper as a child of the host, matching + // the preview bundler's shape (htmlBundler.ts's prepareFlattenedInnerRoot, + // which the runtime compositionLoader mirrors with its own copy for the + // live-loaded case). Without this, the wrapper element (and its + // class/id) is discarded and any CSS anchored on it — + // `.wrapper-class .title`, `#wrapper-id` — is dead at render time even + // though it works in preview. + flattenInnerRoot: prepareFlattenedInnerRoot as (innerRoot: Element) => Element, onMissingComposition: (srcPath: string, reason?: string) => { // In the render path this is normally unreachable — compileForRender // calls assertSubCompositionsUsable() before any of this runs, so a @@ -761,18 +771,6 @@ function inlineSubCompositions( }, ); - // Set data-hf-authored-id on host elements so the scoped script proxy - // can rewrite #id selectors (e.g. #us-map → [data-hf-authored-id="us-map"]). - // Unlike flattenInnerRoot (which changes DOM structure and breaks baselines), - // this preserves the existing innerHTML-based inlining while enabling the - // authored-id selector contract. - for (const hostEl of hosts) { - const compId = hostEl.getAttribute("data-composition-id"); - if (compId && !hostEl.getAttribute("data-hf-authored-id")) { - hostEl.setAttribute("data-hf-authored-id", compId); - } - } - // Producer-specific: set explicit pixel dimensions on host elements so // children using width/height: 100% resolve correctly. The runtime does // this automatically but compiled HTML needs it inline. diff --git a/packages/producer/tests/sub-comp-class-selector/meta.json b/packages/producer/tests/sub-comp-class-selector/meta.json new file mode 100644 index 000000000..61eb7ac8e --- /dev/null +++ b/packages/producer/tests/sub-comp-class-selector/meta.json @@ -0,0 +1,12 @@ +{ + "name": "Sub-composition authored-root class selector scoping", + "description": "Regression test for issue #1847 / PR #1886 (the exact reported repro): a sub-composition's authored root carries its own class (not just an id), styled via a descendant selector anchored on that class (`.scene-wrapper .title`). This diverged between preview and render because the producer discarded the authored root element entirely, so no element in the render DOM ever carried the class. The producer now preserves the authored root as a data-hf-inner-root wrapper (matching preview), so the class-based selector resolves identically in both.", + "tags": ["sub-composition", "regression", "selector"], + "minPsnr": 20, + "maxFrameFailures": 10, + "minAudioCorrelation": 0.0, + "maxAudioLagWindows": 120, + "renderConfig": { + "fps": 24 + } +} diff --git a/packages/producer/tests/sub-comp-class-selector/output/compiled.html b/packages/producer/tests/sub-comp-class-selector/output/compiled.html new file mode 100644 index 000000000..00d8970b2 --- /dev/null +++ b/packages/producer/tests/sub-comp-class-selector/output/compiled.html @@ -0,0 +1,380 @@ + + + + + + + + +
+
+
ISSUE 1847 REPRO
+ + + + + +
+
+ + + diff --git a/packages/producer/tests/sub-comp-class-selector/output/output.mp4 b/packages/producer/tests/sub-comp-class-selector/output/output.mp4 new file mode 100644 index 000000000..28024ef79 --- /dev/null +++ b/packages/producer/tests/sub-comp-class-selector/output/output.mp4 @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:22c76cc409f4567792d5f80b9c379c8905ab34cbb90bc11fe398ddf47a4fc4d7 +size 33420 diff --git a/packages/producer/tests/sub-comp-class-selector/src/compositions/scene.html b/packages/producer/tests/sub-comp-class-selector/src/compositions/scene.html new file mode 100644 index 000000000..d3cf5a1be --- /dev/null +++ b/packages/producer/tests/sub-comp-class-selector/src/compositions/scene.html @@ -0,0 +1,36 @@ + diff --git a/packages/producer/tests/sub-comp-class-selector/src/index.html b/packages/producer/tests/sub-comp-class-selector/src/index.html new file mode 100644 index 000000000..ef7f5a9ba --- /dev/null +++ b/packages/producer/tests/sub-comp-class-selector/src/index.html @@ -0,0 +1,44 @@ + + + + + + + + +
+
+
+ + + diff --git a/packages/producer/tests/sub-comp-id-selector/meta.json b/packages/producer/tests/sub-comp-id-selector/meta.json index 6fd6cd86e..33f8b9eaa 100644 --- a/packages/producer/tests/sub-comp-id-selector/meta.json +++ b/packages/producer/tests/sub-comp-id-selector/meta.json @@ -1,6 +1,6 @@ { "name": "Sub-composition #ID selector scoping", - "description": "Documents that sub-compositions using #ID selectors may render differently between preview and render due to the producer stripping the inner root element. Workaround: use [data-composition-id] selectors instead of #ID.", + "description": "Regression test for #1886: a sub-composition's authored root #ID selectors used to render differently between preview and render because the producer stripped the inner root element. The producer now preserves the authored root as a data-hf-inner-root wrapper (matching preview), and #ID selectors are rewritten to a [data-hf-authored-id] attribute on that wrapper, so #ID scoping round-trips correctly in both preview and render.", "tags": ["sub-composition", "regression", "selector"], "minPsnr": 20, "maxFrameFailures": 10,