fix: producer render diverges from preview for sub-composition root styling#1886
Conversation
The render path stripped a sub-composition's authored root element and inlined only its children, so any CSS anchored on that root (its id or classes) matched nothing in the compiled HTML even though it resolved fine in Studio preview. Pass flattenInnerRoot (the same prepareFlattenedInnerRoot used by the preview bundler) into the producer's inlineSubCompositions call so the render-time DOM shape matches preview: the authored root survives as a child of the host.
…ppers Compositions that style their own box via the bare `[data-composition-id="X"]` selector (e.g. `display: flex` to center their children) rely on that rule landing on whatever element actually parents their content. Once flattenInnerRoot preserves the authored root as a wrapper below the host (data-hf-inner-root), the bare root selector was only ever rewritten onto the host, one level too high, so the composition's own children lost their flex or grid layout context entirely. Rewrite a bare root composition-id selector to a compound-OR targeting both the host and its data-hf-inner-root descendant, so the box styling reaches whichever element actually holds the composition's content. Selectors with a descendant part (e.g. `[data-composition-id="X"] .title`) are unaffected, since a plain descendant combinator already matches at any depth.
The previous fix rewrote a bare root [data-composition-id] box selector to match both the host and the flattened wrapper. That double-applies any additive property (padding, margin, a non-zero transform): the wrapper is nested inside the host, so a rule like `padding-top: 200px` shifted content down twice instead of once, visible as overlapping elements in compositions that combine flex alignment with an offset (e.g. a captions overlay using `align-items: flex-start; padding-top: 200px`). Use :has()/:not() to target exactly one match: the wrapper when it exists (the flattened case), or the host when it doesn't (the documented non-flattened fallback for callers that omit flattenInnerRoot).
flattenInnerRoot strips data-composition-id from the flattened wrapper,
assuming the host already carries the composition's identity. That's true
for hosts authored with their own data-composition-id, but not for a host
mounted via data-composition-src with no id of its own (an "anonymous"
host) — a pattern the producer already had a dedicated regression test for
(missing-host-comp-id) and correctly supported before flattenInnerRoot was
wired in, via a different code path that preserved the whole composition
element as-is.
Once nothing in the render DOM carries the composition's own id, its root-
styling CSS (`[data-composition-id="X"] { ... }`) and any script that
self-references it (e.g. `document.querySelector('[data-composition-id="X"]')`)
both silently stop resolving. Restore the id onto the wrapper specifically
when the host has none of its own, matching what the pre-flatten producer
path did and what preview visually expects.
…n-file createRuntimeStartTimeResolver's walk-up-to-host fallback (for an inner root with no data-start of its own) only recognized a host carrying data-composition-src or data-composition-id. Once inlining strips data-composition-src and replaces it with data-composition-file, an anonymous host (no data-composition-id of its own) matched neither check, so the walk-up silently failed and the composition's own timeline got seeked using its start time as the fallback (0) instead of the host's real data-start. This surfaced once the previous commit restored data-composition-id onto the flattened wrapper for anonymous hosts: the composition's self-query started resolving to that wrapper, but resolveStartForElement still couldn't find its actual mount time through the host, so any composition with real entrance/exit animation timing rendered stuck at t=0 (visually: missing or frozen at its hidden initial state) instead of at the correct point in its own timeline.
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 86ead17af5b284679c6d0d3375e56bce6599469b.
Peer scan: no reviews posted yet — this is the first pass.
Companion issue #1847: sub-composition authored root wrapper (its classes/id) survives in Studio preview but is stripped at render, so any CSS anchored on that root — .scene-wrapper .title, #scene-root, [data-composition-id="scene"] { padding } — silently no-ops in the compiled MP4.
Summary — Converges producer and preview on a single flatten path: producer's inlineSubCompositions now takes the same prepareFlattenedInnerRoot the preview bundler and runtime compositionLoader already used, so the authored root survives as a data-hf-inner-root wrapper below the host in both worlds. Three self-consistent follow-ups fall out: (a) bare [data-composition-id="X"] box selectors compile to a :has()/:not() pair that lands on exactly one of host-or-wrapper — nice touch, the commit-3 fix for double-application of additive properties like padding-top: 200px is the kind of correction that only shows up because a real fixture blew up; (b) anonymous hosts get their composition id restored onto the wrapper so root-scoped CSS + document.querySelector('[data-composition-id="X"]') self-references still resolve; (c) startResolver walks up through the new post-inlining data-composition-file marker so anonymous-host wrapper timelines seek at the right start. Deletes the now-obsolete data-hf-authored-id post-hoist producer-only branch. 28-fixture sweep run 3x, byte/pixel-identical goldens.
The convergence-on-shared-helper shape is exactly what I'd hope for in this class of bug — no per-side patch. The commit sequence also reads clean: each commit's message calls out the specific divergence the previous commit exposed, which is honest engineering. Nothing here rises to blocker.
Concerns
-
🟠 Anonymous-host id restoration lives in
inlineSubCompositions.ts:375-384, not in the sharedprepareFlattenedInnerRoot. The runtimecompositionLoader.ts:527callsprepareFlattenedInnerRoot(innerRoot)directly, bypassinginlineSubCompositions, and does NOT restoredata-composition-idon the wrapper for an anonymous host. That means an anonymous host loaded live at runtime (not through the bundler / not through the producer) exhibits the same silent-no-op the PR fixes for render: its own root-styling CSS and self-querySelectorwon't resolve, andstartResolverwon't find adata-composition-idon the wrapper to anchor the walk-up (thoughdata-composition-fileon the host may not be present in the pure-runtime shape either, since inlining is what writes that). Two questions I'd want you to sanity-check before I stamp: (1) does an anonymous host ever reachcompositionLoader.load()— i.e., can adata-composition-srchost with nodata-composition-idbe resolved live at runtime, e.g. in Studio's live-preview mode, not through the bundle? (2) if yes, should the id-restoration move intoprepareFlattenedInnerRootitself and take the composition id as a parameter, so the two call sites don't drift? If the answer to (1) is "no, runtime-live only serves bundled/inlined output," this is dead code and I withdraw the concern. -
🟠
sub-comp-id-selectorfixture description is now stale and misleading.packages/producer/tests/sub-comp-id-selector/meta.jsonstill says "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." — which is precisely the divergence this PR eliminates. If someone hits an unrelated#id-scoping bug in six months and greps for it, this text will misdirect them into thinking the workaround is still load-bearing. Worth updating the description to reflect that #id selectors now round-trip throughdata-hf-authored-idviaprepareFlattenedInnerRoot, so the divergence is closed.
Nits
-
🟡 The comment at
packages/producer/src/services/htmlCompiler.ts:751-754says "matching the preview/runtime shape (compositionLoader'sprepareFlattenedInnerRoot)", but the import at the top is from@hyperframes/core/compiler— the canonical implementation lives inhtmlBundler.ts:407, andcompositionLoader.ts:188has its own near-duplicate. Referencing the bundler as the canonical shape (and mentioning thatcompositionLoadermirrors it) would be more accurate. -
🟡
compoundAuthoredRootis still a live option oninlineSubCompositions(inlineSubCompositions.ts:76,163,260,295,compositionScoping.ts:105) even though the producer's only consumer just swapped toflattenInnerRoot. If no in-repo caller still passescompoundAuthoredRoot: true, worth deleting the option in a follow-up — otherwise it's a Chekhov's gun the next reader has to decode. -
🟡 The commit sequence has a squashable authorial arc — commit 3 fixes commit 2, and both fix the same "root box styling on flattened wrapper" story. If the merge is squash-and-merge (the repo default IIUC), the final message can just describe the
:has()/:not()shape; the intermediate double-apply detour is nice narrative but not future-load-bearing. No AI-authorship trailers to strip.
Questions
-
The PR body notes "9 producer unit-test failures are pre-existing and unrelated (a different in-flight file with no diffs from this PR)." — worth naming which file, so a reviewer coming in cold doesn't have to git-blame to trust the claim. Which in-flight PR / file?
-
Any observability signal that would have caught this earlier? The failure mode is "compiled HTML looks fine, CSS rule matches nothing, render is silently unstyled" — no error, no warning. If there's a preview-regression harness that pixel-diffs preview vs. render (I see
preview-parityis a passing check), does it cover the class-based-wrapper case now? If it doesn't, adding a fixture that uses a bare.wrapper-class .childselector — the exact repro shape from issue #1847 — would guard the exact failure class from re-emerging.
What I didn't verify
- Whether
compositionLoader.load()is ever called with an anonymous host at runtime in practice. The code path exists; the operational question is whether Studio's live-preview mode (or any other consumer) ever hands it one. - The claim that all 6 golden-baseline compositions are frame-identical modulo encoder bitrate. Trusting the PR body + green CI.
- Cross-repo consumers of
@hyperframes/*types — the diff doesn't change any exported type shapes (stillElementin /Elementout onflattenInnerRoot), just wires in an existing export. Low risk here.
— Rames D Jusso
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
Review of HF #1886 @ 86ead17 — fix: producer render diverges from preview for sub-composition root styling
Verdict: 🟢 LGTM
Small, disciplined fix. Root cause is correctly diagnosed: producer's inliner was calling into shared inlineSubCompositions with compoundAuthoredRoot: true (host-tagged authored-id workaround) rather than flattenInnerRoot: prepareFlattenedInnerRoot (preview's actual DOM shape). Swapping to the shared preparer converges producer's render DOM onto the runtime compositionLoader's shape. All three named divergences are addressed in this PR, tests exercise each, and no other consumer of the pre-fix helper is left holding the old contract.
Findings
F1 — Root-cause fix is surgical, not a band-aid ✅
packages/producer/src/services/htmlCompiler.ts swaps compoundAuthoredRoot: true → flattenInnerRoot: prepareFlattenedInnerRoot (the same fn packages/core/src/compiler/htmlBundler.ts:798 already passes and that packages/core/src/runtime/compositionLoader.ts:527 runs at preview time). The 10-line block that manually stamped data-hf-authored-id on hosts is removed, because the shared inliner's flatten path now does that work via the wrapper. Producer + bundler + runtime now all go through one preparer. Confirmed by grep: compoundAuthoredRoot: true now has zero non-test callers repo-wide at head SHA (only unit tests that still exercise the legacy path).
F2 — Bare-root CSS scope rewrite: exactly-one-of semantics ✅
packages/core/src/compiler/compositionScoping.ts:118-129 rewrites a bare [data-composition-id="X"] selector to scope:not(:has([data-hf-inner-root])), scope > [data-hf-inner-root]. The XOR shape is the right choice — an earlier attempt (per commit 4da03228) targeted both host AND wrapper and would have double-applied additive properties (padding, margin, non-zero transform). The unit test at compositionScoping.test.ts:625 verifies the exactly-one match via parseHTML + querySelectorAll against both flattened and non-flattened DOM shapes; that's the right level of proof, not just a string assertion. Non-bare selectors ([data-composition-id="X"] .title) intentionally fall through to the plain scope-prefix path (test at compositionScoping.test.ts:660).
F3 — Anonymous-host id restoration ✅
packages/core/src/compiler/inlineSubCompositions.ts:375-384: when !compId && inferredCompId, the composition's own id is stamped back onto the wrapper (which prepareFlattenedInnerRoot had stripped). Correct scoping — compId is the host-authored id (null for anonymous hosts) and inferredCompId comes from the inner root inside the resolved sub-composition file (line 234). Test at inlineSubCompositions.test.ts:178 uses a local flattenInnerRoot stub matching prepareFlattenedInnerRoot's attribute-stripping contract, and asserts both the id restoration and that the composition's own [data-composition-id="scoped-text"] CSS still resolves.
F4 — startResolver walk-up chain is complete ✅
packages/core/src/runtime/startResolver.ts:172-179 adds data-composition-file to the parent-marker check. This is necessary because inlineSubCompositions.ts strips data-composition-src and replaces it with data-composition-file post-inlining (line ~406 of inlineSubCompositions.ts), so an anonymous host post-inlining had NEITHER data-composition-src nor data-composition-id — the walk-up broke silently. Test at startResolver.test.ts:196 covers the anonymous-host post-inlining shape explicitly. The comment now correctly enumerates all three markers with their lifecycle stage (runtime / bundled / post-inline).
F5 — Dispatch chain audit: no orphan consumers 🟢
Grepped every inlineSubCompositions importer at head SHA:
packages/producer/src/services/htmlCompiler.ts— now passesflattenInnerRoot.packages/core/src/compiler/htmlBundler.ts:798— already passedflattenInnerRoot.packages/studio-server/src/helpers/subComposition.ts,packages/cli/src/commands/validate.ts,packages/parsers/src/subCompositionValidity.ts— reference the module but don't call it; no orphan.
The compoundAuthoredRoot option remains declared in inlineSubCompositions.ts:76 and continues to power the fall-through branch in compositionScoping.ts:139, but it's dead in production — only exercised by two unit tests. Not blocking (harmless as a documented legacy shape), but a follow-up to delete would remove a decoy code path. Filing this as a soft nit only.
F6 — Scope discipline ✅
+192/-19 across 8 files, all directly tied to the three named divergences. No unrelated refactors ride along. Layer boundaries respected — no upward imports from producer into anything it shouldn't touch; the new prepareFlattenedInnerRoot import from @hyperframes/core/compiler is already in the exported surface at packages/core/src/compiler/index.ts:33.
F7 — Regression proof is credible ✅
28-fixture Docker sweep × 3 runs, 4 byte-identical + 2 frame-identical vs main, plus 966/966 core unit tests including the three new regression cases. This is the right level of proof for a producer-vs-preview convergence — pixel deltas would surface any drift the unit tests miss.
Soft nit (non-blocking)
Preview parity for anonymous-host script self-references. The runtime compositionLoader.ts:527 calls prepareFlattenedInnerRoot(innerRoot) directly on anonymous hosts and does NOT restore data-composition-id on the wrapper the way the fixed producer/bundler path does. That means document.querySelector('[data-composition-id="X"]') inside a composition script would still resolve nothing in preview for an anonymous host — while it now resolves in render. That's the mirror asymmetry: render used to be broken and preview worked; now render works but a specific corner of preview (anonymous host + self-referencing script) may still be broken. If that's already true on main and outside the scope of this PR, ignore — the PR description's "preview works fine" claim covers root-styling (which the CSS scoping rewrite handles via data-hf-inner-root, not the id), and the anonymous-host script self-ref case may just be documented-unsupported. Worth a follow-up issue if it isn't.
Prior reviewer state at post time
No other reviews on the PR (Rames not yet posted at 86ead17 / freshness-checked immediately pre-post).
Review by Via
- Fix a comment in htmlCompiler.ts that misattributed the canonical prepareFlattenedInnerRoot to compositionLoader.ts; the canonical implementation lives in htmlBundler.ts, which compositionLoader.ts mirrors with its own copy for the live-loaded case. - Update sub-comp-id-selector's stale fixture description: it used to document the #ID divergence this PR closes and recommend a workaround that no longer applies. - Add a regression test proving compositionLoader.ts's anonymous-host path does not share the bug this PR fixes: an anonymous host's authoredCompositionId is null, so mountCompositionContent's innerRoot lookup never runs and prepareFlattenedInnerRoot is never reached for it. It falls through to a raw document.importNode() instead, which never stripped data-composition-id in the first place. - Add producer and end-to-end regression coverage for the literal repro from issue #1847 (a class, not just an id, on the authored root, styled via a descendant selector) — this shape wasn't covered by any existing fixture or unit test.
Verified deterministic across 3 runs in Docker on linux/amd64.
|
Thanks both, addressed in a5b87f1 (on top of 86ead17). @james-russo-rames-d-jusso Concern 1 / @vanceingalls soft nit: anonymous-host id restoration in
|
…pstream heygen-com#1886 The upstream fix (flattenInnerRoot wired into the producer inliner so its DOM shape matches the preview bundler) supersedes our selector-side compensation (:is() root-class rewrite + host class merge). Keeping both risks double-applying root styles — exactly what heygen-com#1886's :has()/:not() pair exists to avoid. Core compiler suite (120 tests, including upstream's heygen-com#1886 regressions) and producer htmlCompiler suite (78 tests) pass on the merged tree.
3 participants
Fixes #1847
Root cause
The producer's render path stripped a sub-composition's authored root element and inlined only its children, so any CSS anchored on that root (its id or classes) matched nothing in the compiled HTML even though it resolved fine in Studio preview.
Fixing that surfaced two more divergences between render and preview, both caught by the existing regression suite once the flatten path was actually exercised end to end:
[data-composition-id="X"]selector lost that styling once the authored root became a wrapper one level below the host, since the styling only ever reached the host.data-composition-id(an "anonymous" host) lost its own composition id entirely once the authored root was flattened, breaking both its own root-styling CSS and any script that self-references it (document.querySelector('[data-composition-id="X"]')), and by extension the runtime's own timeline-seek resolution for that composition.Changes
packages/producer/src/services/htmlCompiler.ts— passflattenInnerRoot(the same function the preview bundler already uses) into the producer's sub-composition inliner, so the render-time DOM shape matches preview: the authored root survives as a child of the host instead of being discarded.packages/core/src/compiler/compositionScoping.ts— a bare root[data-composition-id="X"]selector now compiles to a:has()/:not()pair that matches exactly one of the host or the flattened wrapper, whichever one actually holds the composition's content. Targeting both (an earlier attempt) double-applies additive properties likepadding, since the wrapper is nested inside the host.packages/core/src/compiler/inlineSubCompositions.ts— restore the composition's own id onto the flattened wrapper when the host has none of its own, matching what the render path did before flattening was introduced and what preview already does.packages/core/src/runtime/startResolver.ts— the walk-up-to-host fallback for resolving a composition's start time only recognized a host carryingdata-composition-srcordata-composition-id. Once inlining stripsdata-composition-srcand replaces it withdata-composition-file, an anonymous host matched neither, so any composition mounted through one had its own timeline seeked from the wrong start time.Verification
data-composition-srcfixture sweep (28 fixtures) run 3 times in Docker onlinux/amd64(matching CI's render environment; local macOS headless rendering was flaky/non-deterministic and not used for verification) — all pass with 0 failed frames on every run.main, the remaining 2 are frame-identical (verified by direct pixel comparison, not just the harness's PSNR check) with only encoder bitrate differing.packages/coreunit tests: 966/966 passing, including new regression tests for the root-box-styling and anonymous-host-id-restoration behavior added in this PR.packages/producerunit tests: 548/557 passing; the 9 failures are pre-existing and unrelated to this change (a different in-flight file with no diffs from this PR).