From 4cb164793d8d0737d7699915af49f63d5bae0828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 7 Aug 2026 21:30:26 +0000 Subject: [PATCH 1/2] refactor(core): route both assembly paths through one owner, and settle four divergences Rendering a composition and mounting one now derive root discovery, scope identity, asset sources and order, hoisted links, variable carriers and nested-host enumeration from the same module. Each keeps its own I/O, which is where they genuinely differ. The compiler's local depth cap and root lookup and the runtime's three pre-filtered head parameters are gone; the runtime hands over the head node and lets the module decide what comes out of it. Four divergences are settled here. Each changes behaviour, so each is stated with what actually differs rather than with a description of the edit. Inline head scripts. The compiler looped head scripts with a src branch and no else, so an inline one was silently discarded on render while the runtime ran it. The compiler was losing code, not holding a convention. Head and content scripts now run through one loop, head first, order preserved. A non-templated sub-composition with an inline head script went from zero collected scripts to one, wrapped, with its body intact. Link hoisting. Conditional on render, unconditional on mount, so a templated sub-composition's webfont link was dropped in video and kept in preview. Hoisting is the superset and matches what the author declared, so hoisting wins. A templated composition with a stylesheet link in its head went from no external links to that link. The parity fixture that previously recorded this shape as a known exclusion now gates it instead. Anonymous hosts. With a host naming no id, the compiler fell back to the first declared composition and scoped to it; the mount left the content unflattened and injected its stylesheet into the host head unscoped, so a composition's CSS leaked into whatever mounted it. The compiler's answer wins. The mount now flattens like every other mount and restores the declared id afterwards. The injected rule went from a bare class selector to one scoped to the composition. This flips an assertion that documented the old behaviour as intentional. Its premise no longer holds. What that test actually cared about, the root and its content being present under the host, still holds and is still asserted; the claim that nothing was flattened is now false and the test asserts the scoping instead. Scope ids. The compiler splits the CSS scope id from the script composition id; they differ only when a host names an id the content does not declare, and there the scripts follow the declared id so their self-referencing queries resolve. The runtime used one for both. The split wins: a host naming captions-comp over content declaring captions now emits scripts bound to captions while its CSS still scopes to captions-comp. Left alone deliberately: the variable-carrier divergence and its TODO, recursion on the mount path, and the three copies each of the flattened-root helper and the id assignment. The first two are behaviour changes with their own units. The third looks mergeable and is not cheaply, and it touches the instancing contract the pixel harness guards. Verified: core 1694 passing, producer 573 passing, the parity contract now nine tests with fixtures gating the two divergences it can observe. Lint clean, typecheck:runtime and the runtime preview guards clean, package cycles unchanged. A trap worth recording: the parity test's compiler arms import core's built dist while the mount arm imports source, so core must be rebuilt before that lane means anything after a compiler change. Skipping it produces a phantom divergence. --- .../compiler/inlineSubCompositions.test.ts | 55 +++++++ .../src/compiler/inlineSubCompositions.ts | 132 +++++++--------- .../src/runtime/compositionLoader.test.ts | 88 +++++++++-- .../core/src/runtime/compositionLoader.ts | 143 +++++++++--------- .../src/services/htmlCompiler.parity.test.ts | 53 ++++++- 5 files changed, 298 insertions(+), 173 deletions(-) diff --git a/packages/core/src/compiler/inlineSubCompositions.test.ts b/packages/core/src/compiler/inlineSubCompositions.test.ts index 26b004babb..a05e993fab 100644 --- a/packages/core/src/compiler/inlineSubCompositions.test.ts +++ b/packages/core/src/compiler/inlineSubCompositions.test.ts @@ -286,6 +286,61 @@ describe("inlineSubCompositions – #ID selector scoping divergence", () => { }); }); + it("collects an inline script instead of discarding it", () => { + // The loop had a `src` branch and no else, so an inline + // script was silently dropped on render while the mount path executed it. + const subCompWithHeadScript = ` + + + +
Hi
+`; + + const document = makeHostDocument("intro"); + const host = document.querySelector('[data-composition-src="intro.html"]')!; + + const result = inlineSubCompositions(document, [host], { + resolveHtml: () => subCompWithHeadScript, + parseHtml: (html) => parseHTML(html).document, + }); + + expect(result.scripts.join("\n")).toContain("window.__headScriptRan = true;"); + expect(result.scriptItems).toContainEqual({ + kind: "inline", + content: expect.stringContaining("window.__headScriptRan = true;"), + }); + }); + + it("hoists a from a TEMPLATED sub-composition's head", () => { + // Hoisting used to be gated on the composition being non-templated, so a + // templated composition's webfont link was kept in preview (the mount path + // hoists unconditionally) and dropped from the render. + const templatedSubCompWithLink = ` + + + + +`; + + const document = makeHostDocument("intro"); + const host = document.querySelector('[data-composition-src="intro.html"]')!; + + const result = inlineSubCompositions(document, [host], { + resolveHtml: () => templatedSubCompWithLink, + parseHtml: (html) => parseHTML(html).document, + }); + + expect(result.externalLinks).toEqual([ + { + href: "https://fonts.googleapis.com/css2?family=Montserrat", + rel: "stylesheet", + crossorigin: undefined, + }, + ]); + }); + it("deduplicates link hrefs across multiple sub-compositions", () => { const subComp = ` diff --git a/packages/core/src/compiler/inlineSubCompositions.ts b/packages/core/src/compiler/inlineSubCompositions.ts index 350f7d9fa8..b7b68e8bb5 100644 --- a/packages/core/src/compiler/inlineSubCompositions.ts +++ b/packages/core/src/compiler/inlineSubCompositions.ts @@ -15,13 +15,13 @@ import { rewriteInlineStyleAssetUrls, type AssetExists, } from "./rewriteSubCompPaths"; -import { queryByAttr } from "../utils/cssSelector"; import { scopeCssToComposition, wrapInlineScriptWithErrorBoundary, wrapScopedCompositionScript, } from "./compositionScoping"; import { checkSubCompositionUsability } from "@hyperframes/parsers/sub-composition-validity"; +import { enumerateNestedCompositionHosts, planCompositionAssembly } from "./compositionAssembly"; // --------------------------------------------------------------------------- // Public interface @@ -141,8 +141,6 @@ function defaultBuildScopeSelector(compId: string): string { return `[data-composition-id="${escaped}"]`; } -const MAX_SUB_COMPOSITION_DEPTH = 20; - // --------------------------------------------------------------------------- // Core implementation // --------------------------------------------------------------------------- @@ -244,21 +242,21 @@ export function inlineSubCompositions( continue; } - // Keep structural flattening tied to an exact mount-id match. A template - // may intentionally use a different local id (for example, a - // `captions-comp` host mounting a `captions` template); flattening that - // fallback root changes the compiled DOM and can invalidate selectors and - // regression goldens. Discover it separately so script timeline - // registration can still map the authored id onto the runtime mount id. - const innerRoot = compId - ? queryByAttr(contentDoc, "data-composition-id", compId) - : contentDoc.querySelector("[data-composition-id]"); - const authoredCompositionRoot = innerRoot ?? contentDoc.querySelector("[data-composition-id]"); - const inferredCompId = - authoredCompositionRoot?.getAttribute("data-composition-id")?.trim() || ""; - const authoredRootId = innerRoot?.getAttribute("id")?.trim() || null; - const scopeCompId = compId || inferredCompId; - const scriptCompositionId = inferredCompId || scopeCompId; + // Which node is the composition root, which id its CSS scopes to, which + // id its scripts scope to, where its assets come from and in what order — + // every one of those is decided by the shared assembly module, so the mount + // path in runtime/compositionLoader.ts decides them the same way. + const plan = planCompositionAssembly({ + contentNode: contentDoc, + head: compDoc.head, + documentElement: compDoc.documentElement, + hasTemplate: Boolean(contentRoot), + compositionId: compId, + }); + const innerRoot = plan.innerRoot; + const authoredRootId = plan.authoredRootId; + const scopeCompId = plan.authoredCompositionId || ""; + const scriptCompositionId = plan.scriptCompositionId || ""; const runtimeScope = runtimeCompId ? buildScopeSelector(runtimeCompId) : ""; // Variable merging (bundler feature). Read declared defaults from the @@ -266,11 +264,11 @@ export function inlineSubCompositions( // (template/fragment sub-comps store their schema on the root div, not a // synthetic ), then let per-instance host values override. if (readVariableDefaults && parseHostVariables && runtimeCompId) { - const mergedVariables = { - ...readVariableDefaults(compDoc.documentElement), - ...(innerRoot ? readVariableDefaults(innerRoot) : {}), - ...parseHostVariables(hostEl), - }; + const mergedVariables: Record = {}; + for (const carrier of plan.variableDefaultCarriers) { + Object.assign(mergedVariables, readVariableDefaults(carrier)); + } + Object.assign(mergedVariables, parseHostVariables(hostEl)); if (Object.keys(mergedVariables).length > 0) { variablesByComp[runtimeCompId] = mergedVariables; } @@ -295,47 +293,35 @@ export function inlineSubCompositions( : css; }; - // When a sub-composition is a full HTML document (no