fix(core): stop composition variables from shadowing authored CSS custom properties (#2553)

A composition variable mirrored as --<slug> for a mounted sub-composition (default or an explicit data-variable-values value) previously overrode any same-named custom property the document already authored elsewhere (e.g. a :root theme token), since the mirroring had no "already defined" guard — unlike the two other emission paths, which already skip re-emitting when the name collides with an authored definition. Extend that guard to the sub-composition mount path in both the compiler (htmlBundler.ts) and the runtime loader (compositionLoader.ts / getVariables.ts), so an authored definition always wins; render-time --variables overrides still always win.
2026-09-03 04:38:33 +00:00 · 2026-07-16 18:20:13 -04:00
parent 065f8c9c8d
commit 406894061e
5 changed files with 119 additions and 14 deletions
@@ -881,6 +881,44 @@ describe("bundleToSingleHtml", () => {
    expect(bundled).toMatch(/card__hf2[\s\S]*Enterprise[\s\S]*light/);
  });

+  it("does not redefine an authored CSS variable for a bundled sub-composition", async () => {
+    const dir = makeTempProject({
+      "index.html": `<!doctype html>
+<html><head>
+  <style>:root { --accent: #4287f5; } .host-badge { color: var(--accent); }</style>
+</head><body>
+  <div
+    data-composition-id="main"
+    data-width="1920"
+    data-height="1080"
+    data-start="0"
+    data-duration="5">
+    <div
+      data-composition-id="card"
+      data-composition-src="compositions/card.html"
+      data-variable-values='{"accent":"blue"}'></div>
+  </div>
+  <script>window.__timelines={};</script>
+</body></html>`,
+      "compositions/card.html": `<!doctype html>
+<html data-composition-variables='[{"id":"accent","type":"string","label":"Accent","default":"red"}]'>
+  <body>
+    <div
+      data-composition-id="card"
+      data-width="1920"
+      data-height="1080"
+      data-start="0"
+      data-duration="5"></div>
+  </body>
+</html>`,
+    });
+
+    const bundled = await bundleToSingleHtml(dir);
+
+    expect(bundled).toContain(":root { --accent: #4287f5; }");
+    expect(bundled).not.toMatch(/\[data-composition-id="card[^"]*"\]\s*\{[^}]*--accent:\s*blue/);
+  });
+
  it("scopes external sub-composition styles and classic scripts", async () => {
    const dir = makeTempProject({
      "index.html": `<!doctype html>
@@ -1154,9 +1154,10 @@ export function emitRootCompositionVariableStyles(
  variablesByComp: Record<string, Record<string, unknown>> = {},
  overrides: Record<string, unknown> = {},
 ): boolean {
-  const layerFor = makeVariableLayer(document, overrides);
+  const authoredDefines = authoredDefinesPredicate(document);
+  const layerFor = makeVariableLayer(authoredDefines, overrides);
  const rules = [
-    ...hostScopedVariableRules(variablesByComp, overrides),
+    ...hostScopedVariableRules(variablesByComp, overrides, authoredDefines),
    ...rootDeclaredVariableRules(document, layerFor),
    ...declarerVariableRules(document, layerFor),
  ];
@@ -1173,18 +1174,23 @@ type VariableLayer = (
  hostValues: Record<string, unknown>,
 ) => Record<string, unknown>;

+function authoredDefinesPredicate(document: Document): (id: string) => boolean {
+  const authoredCss = [...document.querySelectorAll("style:not([data-hf-composition-variables])")]
+    .map((s) => s.textContent || "")
+    .join("\n");
+  return (id) => new RegExp(`${cssVariableName(id)}\\s*:`).test(authoredCss);
+}
+
 /**
 * Layering for one declarer: authored stylesheet definitions win over
 * declared defaults (the runtime's define-if-absent, applied statically) —
 * a var already defined in any authored <style> block is not emitted. Host
 * values and --variables overrides are explicit intent, never filtered.
 */
-function makeVariableLayer(document: Document, overrides: Record<string, unknown>): VariableLayer {
-  const authoredCss = [...document.querySelectorAll("style:not([data-hf-composition-variables])")]
-    .map((s) => s.textContent || "")
-    .join("\n");
-  const authoredDefines = (id: string): boolean =>
-    new RegExp(`${cssVariableName(id)}\\s*:`).test(authoredCss);
+function makeVariableLayer(
+  authoredDefines: (id: string) => boolean,
+  overrides: Record<string, unknown>,
+): VariableLayer {
  return (declared, hostValues) => {
    const out: Record<string, unknown> = {};
    for (const [id, value] of Object.entries(declared)) {
@@ -1200,14 +1206,24 @@ function makeVariableLayer(document: Document, overrides: Record<string, unknown
  };
 }

-/** Host-scoped rules: per-instance values inherited by the host's subtree. */
+/**
+ * Host-scoped rules: per-instance values inherited by the host's subtree.
+ * A composition variable, whether a declared default or an explicit
+ * data-variable-values value, never redefines a custom property authored by
+ * another part of the document. Render-time --variables overrides remain
+ * explicit user intent and always win.
+ */
 function hostScopedVariableRules(
  variablesByComp: Record<string, Record<string, unknown>>,
  overrides: Record<string, unknown>,
+  authoredDefines: (id: string) => boolean,
 ): string[] {
  const rules: string[] = [];
  for (const [compId, vars] of Object.entries(variablesByComp)) {
-    const withOverrides = { ...vars };
+    const withOverrides: Record<string, unknown> = {};
+    for (const [id, value] of Object.entries(vars)) {
+      if (!authoredDefines(id)) withOverrides[id] = value;
+    }
    for (const [id, value] of Object.entries(overrides)) {
      if (id in vars) withOverrides[id] = value;
    }
@@ -951,6 +951,34 @@ describe("loadExternalCompositions", () => {
      });
    });

+    it("does not redefine a CSS variable already authored on the host", async () => {
+      const style = document.createElement("style");
+      style.textContent = ":root { --accent: #4287f5; }";
+      document.head.appendChild(style);
+
+      const host = document.createElement("div");
+      host.setAttribute("data-composition-src", "https://example.com/card.html");
+      host.setAttribute("data-composition-id", "card-authored-token");
+      host.setAttribute("data-variable-values", '{"accent":"blue"}');
+      document.body.appendChild(host);
+
+      const compositionHtml = `
+        <html data-composition-variables='[
+          {"id":"accent","type":"string","label":"Accent","default":"red"}
+        ]'>
+          <body><div data-composition-id="card-authored-token"><p>x</p></div></body>
+        </html>
+      `;
+      vi.spyOn(globalThis, "fetch").mockResolvedValue(
+        new Response(compositionHtml, { status: 200 }),
+      );
+
+      await loadExternalCompositions({ ...defaultParams });
+
+      expect(host.style.getPropertyValue("--accent")).toBe("");
+      expect(window.getComputedStyle(host).getPropertyValue("--accent")).toBe("#4287f5");
+    });
+
    it("uses declared defaults when host has no data-variable-values", async () => {
      const host = document.createElement("div");
      host.setAttribute("data-composition-src", "https://example.com/card.html");
@@ -3,6 +3,7 @@ import { markFlattenedInnerRoot } from "./flattenedRoot";
 import {
  applyCssVariables,
  clearAppliedCssVariables,
+  filterVariablesIfAbsent,
  parseHostVariableValues,
  readDeclaredDefaults,
  readRenderOverrides,
@@ -764,9 +765,11 @@ export async function loadExternalCompositions(
 * as CSS custom properties on the host so imported var(--slug, literal)
 * fills inside the sub-comp resolve per instance (cascade beats the document
 * root). Inline templates carry declared defaults on the content root;
- * external loads pass them explicitly. Render-time overrides (--variables)
- * always win. Stale custom properties from a previous mount are cleared
- * before (re)applying.
+ * external loads pass them explicitly. A composition variable, whether a
+ * declared default or an explicit data-variable-values value, never
+ * redefines a custom property already defined on the host. Render-time
+ * overrides (--variables) remain explicit user intent and always win. Stale
+ * custom properties from a previous mount are cleared before (re)applying.
 */
 function stashInstanceVariables(
  params: { host: Element; declaredVariableDefaults?: Record<string, unknown> },
@@ -784,7 +787,10 @@ function stashInstanceVariables(
  if (Object.keys(merged).length > 0) {
    if (!window.__hfVariablesByComp) window.__hfVariablesByComp = {};
    window.__hfVariablesByComp[runtimeScopeCompositionId] = merged;
-    applyCssVariables(params.host, { ...merged, ...readRenderOverrides() });
+    applyCssVariables(params.host, {
+      ...filterVariablesIfAbsent(params.host, merged, window),
+      ...readRenderOverrides(),
+    });
  } else if (window.__hfVariablesByComp) {
    delete window.__hfVariablesByComp[runtimeScopeCompositionId];
  }
@@ -99,6 +99,23 @@ export function applyCssVariables(target: Element, variables: Record<string, unk
  if (applied.length > 0) target.setAttribute(APPLIED_VARS_ATTR, applied.join(" "));
 }

+/** Keep only variables whose CSS custom property is not already defined. */
+export function filterVariablesIfAbsent(
+  target: Element,
+  variables: Record<string, unknown>,
+  view: Window | null,
+): Record<string, unknown> {
+  const filtered: Record<string, unknown> = {};
+  for (const [id, value] of Object.entries(variables)) {
+    const name = cssVariableName(id);
+    const existing =
+      (hasInlineStyle(target) ? target.style.getPropertyValue(name) : "") ||
+      (view ? view.getComputedStyle(target).getPropertyValue(name) : "");
+    if (existing.trim() === "") filtered[id] = value;
+  }
+  return filtered;
+}
+
 /** Remove custom properties a previous applyCssVariables call defined. */
 export function clearAppliedCssVariables(target: Element): void {
  if (!hasInlineStyle(target)) return;