fix(core): scope getElementById inside compositions

[miguel-heygen]

Problem

Fixes #646.

When sibling sub-compositions contain the same element IDs, scoped document.getElementById() can return null for every composition after the first. This breaks blocks that reuse IDs like <canvas id="gl-canvas"> and then call document.getElementById("gl-canvas").getContext(...) from their scoped script.

How to reproduce

Create two composition roots with the same ID-bearing child, then execute the scoped wrapper for the second composition:

<div data-composition-id="scene-a"><canvas id="gl-canvas"></canvas></div>
<div data-composition-id="scene-b"><canvas id="gl-canvas"></canvas></div>

window.__selectedComp =
  document.getElementById("gl-canvas")
    ?.closest("[data-composition-id]")
    ?.getAttribute("data-composition-id") || "null";

Before this patch, wrapping that script with wrapScopedCompositionScript(..., "scene-b") produces "null" because the global DOM lookup finds scene-a first and the scoping containment check rejects it.

What this fixes

• Keeps document.getElementById() scoped to the active composition root when duplicate IDs exist across sibling sub-compositions.
• Preserves the fast path when the browser's global getElementById() already returns an element inside the active root.
• Handles IDs that need selector escaping, such as clip:1, instead of relying on a fragile "#" + id selector.
• Adds regression coverage for duplicate WebGL-style IDs and selector-special IDs.

Root cause

wrapScopedCompositionScript() proxied document.getElementById() by calling the real document's global getElementById(id) first and then checking whether that one result lived inside the scoped composition root.

That works only when the first global match is inside the active composition. With duplicate IDs, the browser returns the first match in document order, so the second composition sees the first composition's element, rejects it during containment filtering, and returns null without searching inside its own root.

The fix falls back to the scoped root when the global result is outside the active composition: it checks the root element itself, then queries inside the root using CSS.escape() when available and a quoted [id="..."] fallback for non-browser/test environments.

Verification

Local checks

• bun test packages/core/src/compiler/compositionScoping.test.ts
  • Saw the two new regression tests fail before the fix with Expected: "scene-b" / Received: "null".
  • Passed after the fix: 11 tests.
• bunx oxlint packages/core/src/compiler/compositionScoping.ts packages/core/src/compiler/compositionScoping.test.ts
• bunx oxfmt --check packages/core/src/compiler/compositionScoping.ts packages/core/src/compiler/compositionScoping.test.ts
• bun run --filter @hyperframes/core build:hyperframes-runtime
• bun run --filter @hyperframes/core test after generating the runtime inline module: 47 files, 665 tests passed.
• bun run --filter @hyperframes/core typecheck after generating the runtime inline module.
• bun run --filter @hyperframes/core build.
• Lefthook pre-commit: lint, format, typecheck.

Browser verification

Used agent-browser against a generated proof page that embeds the actual patched wrapScopedCompositionScript() output and duplicates both gl-canvas and clip:1 across scene-a and scene-b.

The browser page reported:

{
  "byIdRoot": "scene-b",
  "specialIdRoot": "scene-b",
  "queryRoot": "scene-b"
}

Saved local proof artifacts:

• qa-artifacts/issue-646/proof.html
• qa-artifacts/issue-646/proof.png
• qa-artifacts/issue-646/proof.webm

ffprobe confirmed the recording is a VP8 WebM at 1440x1000.

Notes

• The proof artifacts are local QA artifacts and are intentionally not committed.
• I checked that issue fix(core): scoped getElementById fails with duplicate element IDs across sub-compositions #646 has no linked closing PR and open PR searches for 646 / getElementById compositionScoping did not find an existing fix.

[jrusso1020]

LGTM — surgical fix to a real bug, preserves the fast path, anticipates the broader bug class.

What I verified

Root-cause analysis matches the source. The bug at compositionScoping.ts:144-150 (pre-fix) does exactly what issue #646 describes: target.getElementById(id) returns the first global DOM match, then __hfContains(found) rejects it for any composition past the first → null. Issue's reproduction is correct and the suggested fix matches the implementation here (with one improvement — see below).

The fast-path preservation is correct. When the global lookup happens to land in the active composition (most common case — unique IDs, or first composition), the implementation skips the scoped query entirely. No perf regression for the happy path.

Special-character ID handling is the catch-beyond-the-bug-report. Issue #646 mentions CSS.escape only as part of the suggested fix. This PR's second test (clip:1) is the genuinely valuable one — it pins behavior for any ID that's a valid HTML id but not a valid bare CSS selector. A naive "#" + id would have shipped with the issue's bug fixed but a new bug introduced. Catching this and writing the regression for it is the right shape.

The non-DOM-environment fallback (__hfEscapeAttr + [id="..."]) is necessary, not over-engineering. I traced this:

• compositionScoping.ts:152 — if (typeof CSS !== "undefined" && CSS && typeof CSS.escape === "function")
• The unit tests use new Function("window", wrapped)(fakeWindow) where fakeWindow only has {document, __selectedComp, __timelines}. No CSS global. Without the fallback, the tests themselves would fail on a CSS is not defined ReferenceError. The fallback isn't speculative — it's required by the test harness setup.

Helper definitions exist and compose correctly:

• __hfEscapeAttr at :106-108 — was already in the file; the new code reuses it
• __hfFindRoot at :121-126 — caches the root in __hfRoot after first lookup
• __hfContains at :127-130 — !root || node === root || root.contains(node). When root is null (composition root not found), returns true for any node — meaning the original behavior in the no-active-root degenerate case was "let the global match through." The new implementation preserves that: the if (found && __hfContains(found)) return found; guard returns the global match in that edge case before the scoped fallback runs. ✓

Tiny observations (non-blocking)

• if (!root) return found || null; at :148 is functionally unreachable in normal operation. Reasoning: __hfFindRoot() caches in __hfRoot; if the cached value is null, then __hfContains(found) returns true (because of the !root || ... short-circuit), so if (found && __hfContains(found)) return found; already returned. Hitting this if (!root) line requires the cache to flip from non-null to null between the two __hfFindRoot() calls in this function, which the cache doesn't allow. Harmless dead code; not worth removing in this PR (could be slightly destabilizing if I'm missing a reentrant case).

• Test gap for "ID on the root element itself." :150 has if (root.id === idValue) return root; — this handles <div data-composition-id="scene-b" id="my-root">…</div> calling document.getElementById("my-root"). Logic looks right but no test pins it. Easy follow-up.

• Empty catch {} blocks at :155 and :159. They swallow querySelector errors silently — if root.querySelector("#" + CSS.escape(...)) ever throws (very unusual but theoretically possible if CSS.escape produces something the engine still can't parse), the user gets null and no signal. Matches the file's existing style; flagging only because debugging "I called getElementById and got null" would be hard if these ever fired. Not blocking.

Praise worth surfacing

• Test fidelity is high. new Function("window", wrapped)(fakeWindow) executes the actual generated wrapper code against a real parsed DOM, rather than mocking the proxy or stubbing getElementById. That's the right shape — and it's why I trust the regression coverage.
• The browser-based proof. Generating qa-artifacts/issue-646/proof.html + capturing PNG + WebM with agent-browser and three independent assertions (byIdRoot, specialIdRoot, queryRoot all returning "scene-b") is more rigorous than the unit tests alone. Confirms the fix works in a real CSS.escape-supporting environment, not just the test harness's fallback path.
• Order of operations in the fast path → scoped path → fallback chain. Each transition has a clear precondition: fast path requires global match in active comp; scoped path requires active root; fallback CSS.escape is only used when the global is available. Reads cleanly without nested branching.

Ship it.

— Review by Rames Jusso (pr-review)