mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 12:54:29 +00:00
fix(core): stop dropping a mounted composition styles, and gate the divergence (#3094)
## Why A composition mounted as a sub-composition lost its entire stylesheet and scripts whenever they were authored as siblings of the composition root inside its `<template>`. That shape is legal and common, so three catalog components — `oversized-cursor`, `device-frame-stage`, `touch-indicator` — rendered **completely unstyled** in the live preview. `oversized-cursor` drew its pointer at 1280px against an authored `7cqw` (~134px at 1920), because `width: 7cqw` was never declared at all. Confirmed in the mounted document, where only the host's own `<style>` was present. The rendered video was correct the entire time. This was a preview-versus-render divergence, and it survived a fully green test suite. ## How **The fix.** `mountCompositionContent` collected assets from the composition root element, so sibling nodes were invisible to it. It now collects from the source node — a superset of the root, and the single point every mount path routes through (external fetch, inline template, nested). It also strips the mounted *clone* rather than the source: the previous code removed extracted nodes from the node it was handed, which on the inline-template path is a live `<template>` still in the document, so a remount would have found it emptied. **Why nothing caught it.** Every CLI gate — `check`, `lint`, `validate` — reaches the compiler path through `bundleToSingleHtml`, and the compiler always collected from the whole template. Nothing in the CLI exercises the mount path, which is reachable only through the player and Studio. The repo's own parity test assembled a fixture two ways and deep-equalled a contract across them, but **both arms were static-compiler paths** — which is exactly why the runtime could drift unnoticed. **The gate.** A third arm mounts the same fixture through `loadExternalCompositions` and extracts the same contract. Three fixtures run through all three arms, one authoring its assets as root siblings — the shape that broke. `authoredStyleSignatures` was already in the contract and is exactly the signal that was missing, so no contract field was added. **The owner.** Both paths answer the same questions — which nodes are a composition's assets, in what order its scripts run, how its CSS is scoped, which head elements hoist, how nested hosts are discovered, which element carries variable defaults. They now have one module to answer them from. It holds decisions only, never I/O: the two paths differ at their boundary in ways that are essential (Node + linkedom + synchronous + strings; browser + fetch + live DOM + script *execution*), and the runtime ships as a bundle to a CDN, so anything it can reach is weight and risk. Hence zero imports, a structural input type rather than `Document`, and a test asserting the import surface stays empty. Routing both paths through that module is deliberately **not** in this PR — it changes behaviour in four places (below) and belongs where each can be judged and reverted on its own. ## Test plan - [x] Unit tests added/updated - [x] Manual testing performed - [ ] Documentation updated (if applicable) Every claim here was verified in both directions rather than assumed. The fix's regression test fails on pre-fix code and passes after — run both ways. The parity arm was proven able to fail: with the fix reverted, the sibling fixture fails and names the composition's own scoped selector against an empty list, while the other two fixtures stay green, so the arm is targeted rather than blanket-red. Restored, 7/7 pass. `bun run lint` exits 0. Core: 1690 tests passing, plus `typecheck:runtime` and `lint:runtime-preview-guards` clean. Producer: 571 tests passing. The shared module's own defect was reproduced by mutation — collecting from the composition root instead of the whole template fails three of its 17 tests, including the sibling case. ## Found while doing this, not fixed here Deriving the shared decisions surfaced **four more live divergences**, none of them the reported bug, each a behaviour change to decide deliberately: - The compiler silently drops inline `<head>` scripts — it handles the `src` case and has no `else` — while the runtime executes them. - `<link>` hoisting is conditional on render and unconditional on mount, so a templated sub-composition's webfont link is dropped in video and kept in preview. This one reproduces under the new parity arm and is explicitly excluded from its contract, with the reason recorded in the file. - For a host naming no id, the compiler falls back to the first declared composition and scopes to it; the runtime mounts the content whole, unflattened and unscoped. - The compiler keeps two scope ids, CSS and scripts, so a script's self-referencing query resolves when a host names an id the content does not declare; the runtime keeps one. Separately: the mount path **does not recurse at all**, so a sub-composition containing its own `data-composition-src` is silently dropped in live preview. The compiler has a dedicated recursive-discovery suite; the runtime has no nesting, circularity or depth coverage. Each is recorded with its evidence in the commit messages here, and sequenced so the behaviour-changing ones land separately, after this gate exists to catch a mistake in them. ## Not covered This does not heal the published docs by itself. Previews load `@hyperframes/player` unpinned, but the player bakes a version-pinned core runtime URL at build time, and core and player publish in lockstep — so the live catalog only recovers after both ship. There is no hotfix path short of a release.
This commit is contained in:
Miguel Ángel
@@ -90,6 +90,43 @@ describe("loadExternalCompositions", () => {
|
||||
expect(injectedStyles.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("mounts <style>/<script> authored as siblings of the composition root", async () => {
|
||||
// The canonical sub-composition shape puts <style> and <script> directly
|
||||
// inside <template>, NEXT TO the root div rather than inside it. Collecting
|
||||
// only from the root dropped the composition's whole stylesheet, so rules
|
||||
// keyed on the root (`#root { container-type: size }`) never landed and every
|
||||
// container-query unit in the composition resolved against the wrong basis.
|
||||
const host = document.createElement("div");
|
||||
host.setAttribute("data-composition-src", "https://example.com/scene.html");
|
||||
host.setAttribute("data-composition-id", "scene");
|
||||
document.body.appendChild(host);
|
||||
|
||||
const compositionHtml =
|
||||
`
|
||||
<html><body>
|
||||
<template>
|
||||
<style>#root { container-type: size; }</style>
|
||||
<div id="root" data-composition-id="scene"><p>Scene</p></div>
|
||||
<script>window.__sceneRan = true;</scr` +
|
||||
`ipt>
|
||||
</template>
|
||||
</body></html>
|
||||
`;
|
||||
vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(compositionHtml, { status: 200 }));
|
||||
|
||||
const injectedStyles: HTMLStyleElement[] = [];
|
||||
const injectedScripts: HTMLScriptElement[] = [];
|
||||
await loadExternalCompositions({ ...defaultParams, injectedStyles, injectedScripts });
|
||||
|
||||
expect(injectedStyles.map((style) => style.textContent).join("")).toContain(
|
||||
"container-type: size",
|
||||
);
|
||||
expect(injectedScripts.map((script) => script.textContent).join("")).toContain("__sceneRan");
|
||||
// The extracted nodes are stripped from the mounted copy, not duplicated.
|
||||
expect(host.querySelectorAll("style, script")).toHaveLength(0);
|
||||
expect(host.querySelector("p")?.textContent).toBe("Scene");
|
||||
});
|
||||
|
||||
it("preserves head stylesheets when an external composition uses a template", async () => {
|
||||
const host = document.createElement("div");
|
||||
host.setAttribute("data-composition-src", "https://example.com/compositions/scene.html");
|
||||
|
||||
@@ -181,6 +181,20 @@ function resetCompositionHost(host: Element) {
|
||||
host.textContent = "";
|
||||
}
|
||||
|
||||
/**
|
||||
* A composition's `<style>`/`<script>` are extracted and re-injected into the
|
||||
* host document (scoped), so strip them from the copy that gets mounted —
|
||||
* otherwise the mount re-declares the same CSS unscoped and re-runs the script.
|
||||
*
|
||||
* Strips the CLONE, never the source: `sourceNode` is a live `<template>` on the
|
||||
* inline-template path, and mutating it would leave a remount with no styles.
|
||||
*/
|
||||
function stripExtractedCompositionAssets(node: ParentNode): void {
|
||||
for (const el of Array.from(node.querySelectorAll("style, script"))) {
|
||||
el.remove();
|
||||
}
|
||||
}
|
||||
|
||||
function prepareFlattenedInnerRoot(innerRoot: HTMLElement): HTMLElement {
|
||||
const prepared = document.importNode(innerRoot, true) as HTMLElement;
|
||||
markFlattenedInnerRoot(prepared);
|
||||
@@ -450,68 +464,37 @@ async function mountCompositionContent(params: {
|
||||
// element styles like backgrounds and positioning that the composition needs),
|
||||
// then the content styles.
|
||||
if (params.headStyles) injectScopedStyles(params.headStyles);
|
||||
injectScopedStyles(Array.from(contentNode.querySelectorAll<HTMLStyleElement>("style")));
|
||||
// Collect from `sourceNode`, not the composition root: the canonical authored
|
||||
// shape puts <style>/<script> as SIBLINGS of the root inside <template>, so
|
||||
// scanning only the root dropped a composition's entire stylesheet (and with
|
||||
// it `#root { container-type: size }`, leaving every cq* unit unanchored).
|
||||
// `sourceNode` is a superset of the root, so nothing is collected twice.
|
||||
injectScopedStyles(Array.from(params.sourceNode.querySelectorAll<HTMLStyleElement>("style")));
|
||||
|
||||
// Collect head scripts first (e.g. GSAP CDN loaded in <head> of non-template sub-comps),
|
||||
// then content scripts. Head scripts must execute before content scripts.
|
||||
const headScriptPayloads: PendingScript[] = [];
|
||||
if (params.headScripts) {
|
||||
for (const script of params.headScripts) {
|
||||
const scriptType = script.getAttribute("type")?.trim() ?? "";
|
||||
const scriptSrc = script.getAttribute("src")?.trim() ?? "";
|
||||
if (scriptSrc) {
|
||||
const resolvedSrc = resolveScriptSourceUrl(scriptSrc, params.compositionUrl);
|
||||
if (params.compositionUrl && isSameDocumentUrl(resolvedSrc, params.compositionUrl)) {
|
||||
continue;
|
||||
}
|
||||
headScriptPayloads.push({ kind: "external", src: resolvedSrc, type: scriptType });
|
||||
} else {
|
||||
const scriptText = script.textContent?.trim() ?? "";
|
||||
if (scriptText) {
|
||||
headScriptPayloads.push({
|
||||
kind: "inline",
|
||||
content: scriptText,
|
||||
type: scriptType,
|
||||
scopeCompositionId: authoredScopeCompositionId,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const scripts = Array.from(contentNode.querySelectorAll<HTMLScriptElement>("script"));
|
||||
const scriptPayloads: PendingScript[] = [...headScriptPayloads];
|
||||
for (const script of scripts) {
|
||||
const scriptType = script.getAttribute("type")?.trim() ?? "";
|
||||
const scriptSrc = script.getAttribute("src")?.trim() ?? "";
|
||||
if (scriptSrc) {
|
||||
const resolvedSrc = resolveScriptSourceUrl(scriptSrc, params.compositionUrl);
|
||||
const toPendingScript = (script: HTMLScriptElement): PendingScript | null => {
|
||||
const type = script.getAttribute("type")?.trim() ?? "";
|
||||
const src = script.getAttribute("src")?.trim() ?? "";
|
||||
if (src) {
|
||||
const resolvedSrc = resolveScriptSourceUrl(src, params.compositionUrl);
|
||||
// A sub-comp that <script src>s itself would re-enter the mount; skip it.
|
||||
if (params.compositionUrl && isSameDocumentUrl(resolvedSrc, params.compositionUrl)) {
|
||||
script.parentNode?.removeChild(script);
|
||||
continue;
|
||||
}
|
||||
scriptPayloads.push({
|
||||
kind: "external",
|
||||
src: resolvedSrc,
|
||||
type: scriptType,
|
||||
});
|
||||
} else {
|
||||
const scriptText = script.textContent?.trim() ?? "";
|
||||
if (scriptText) {
|
||||
scriptPayloads.push({
|
||||
kind: "inline",
|
||||
content: scriptText,
|
||||
type: scriptType,
|
||||
scopeCompositionId: authoredScopeCompositionId,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
return { kind: "external", src: resolvedSrc, type };
|
||||
}
|
||||
script.parentNode?.removeChild(script);
|
||||
}
|
||||
const remainingStyles = Array.from(contentNode.querySelectorAll<HTMLStyleElement>("style"));
|
||||
for (const style of remainingStyles) {
|
||||
style.parentNode?.removeChild(style);
|
||||
}
|
||||
const content = script.textContent?.trim() ?? "";
|
||||
if (!content) return null;
|
||||
return { kind: "inline", content, type, scopeCompositionId: authoredScopeCompositionId };
|
||||
};
|
||||
|
||||
// <head> scripts first (e.g. a GSAP CDN tag in a non-template sub-comp): they
|
||||
// must execute before the content scripts that call into them.
|
||||
const scriptPayloads = [
|
||||
...(params.headScripts ?? []),
|
||||
...Array.from(params.sourceNode.querySelectorAll<HTMLScriptElement>("script")),
|
||||
]
|
||||
.map(toPendingScript)
|
||||
.filter((payload): payload is PendingScript => payload !== null);
|
||||
|
||||
if (innerRoot) {
|
||||
const widthRaw = innerRoot.getAttribute("data-width");
|
||||
@@ -525,9 +508,13 @@ async function mountCompositionContent(params: {
|
||||
if (innerRoot.hasAttribute("data-timeline-locked")) {
|
||||
params.host.setAttribute("data-timeline-locked", "");
|
||||
}
|
||||
params.host.appendChild(prepareFlattenedInnerRoot(innerRoot));
|
||||
const flattenedRoot = prepareFlattenedInnerRoot(innerRoot);
|
||||
stripExtractedCompositionAssets(flattenedRoot);
|
||||
params.host.appendChild(flattenedRoot);
|
||||
} else if (params.hasTemplate) {
|
||||
params.host.appendChild(document.importNode(contentNode, true));
|
||||
const mountedContent = document.importNode(contentNode, true);
|
||||
stripExtractedCompositionAssets(mountedContent);
|
||||
params.host.appendChild(mountedContent);
|
||||
} else {
|
||||
params.host.innerHTML = params.fallbackBodyInnerHtml;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user