Skip to content

fix(scoping): rewrite prefix/substring composition-id selectors in place#2262

Open
Thomaswebstich wants to merge 1 commit into
heygen-com:mainfrom
Thomaswebstich:fix/scoper-prefix-selectors
Open

fix(scoping): rewrite prefix/substring composition-id selectors in place#2262
Thomaswebstich wants to merge 1 commit into
heygen-com:mainfrom
Thomaswebstich:fix/scoper-prefix-selectors

Conversation

@Thomaswebstich

Copy link
Copy Markdown

Problem

A sub-composition whose <style> scopes rules by a prefix/substring composition-id selector — [data-composition-id^="x"] (also *=, $=) — has those rules silently dropped when mounted in a multi-composition document.

scopeSelector() (in compositionScoping.ts) rewrites the exact [data-composition-id="x"] root selector in place to the instance scope, but any prefix/substring form falls through to the prepend branch and becomes a two-level selector:

[data-composition-id="x__hf2"] [data-composition-id^="x"] .thing

That now requires a nested composition-id element — but compositionLoader strips the inner root's data-composition-id at mount, so nothing matches and every class-scoped rule in that sub-comp stops applying (class-styled elements render unstyled). Single-mount renders keep the inner structure, so they're unaffected — the drop only shows when the loader renames/flattens (e.g. duplicate instances in one document).

This bites any authoring pipeline that rewrites ="x" to ^="x" so a sub-comp's own root rules survive the duplicate-instance rename (xx__hf2).

Fix

Widen the composition-id match to accept the substring operators (^=, *=, $=) of the authored id, so the authored-root selector is rewritten in place to the instance scope in every form — exactly like the exact = case — instead of being prepended.

One-line change to the selector regex, plus a regression test covering ^=/*=/$=.

Test

packages/core/src/compiler/compositionScoping.test.ts — new case asserts the prefix/substring forms are consumed in place (and never left intact behind a prepended scope). Full scoping suite passes.

🤖 Generated with Claude Code

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — fix(scoping): rewrite prefix/substring composition-id selectors in place

SSOT check: clean. The fix is exactly one character class addition to the canonical composition-id regex in scopeSelector. The [\\^\\*\\$]?= pattern correctly matches ^=, *=, $= in addition to the existing =, all in one place.

Root cause is correctly identified: prefix-match selectors ([data-composition-id^="x"]) fell through to the prepend branch, creating ${scope} [data-composition-id^="x"] … — a two-level selector that needs a nested composition-id element. The loader strips the inner root's data-composition-id at mount, so nothing matches. The fix rewrites them in place, same as exact match.

The in-place replacement to exact match is correct: [data-composition-id^="scene"][data-composition-id="scene__hf2"]. Each instance gets its own scoped CSS copy, so exact match on the instance ID is the right semantics — the prefix match was an authoring-time workaround that the scoper now handles.

Regex backward compatibility: the ? makes the operator optional, so exact = still matches. ~= (word match) is correctly excluded — composition IDs aren't space-separated. Good.

Test: covers ^=, *=, and includes negative assertions (not.toContain) verifying the prefix/substring forms are consumed in place and never left behind a prepended scope. This is the regression guard.

Minor observation (non-blocking): normalizeCompositionRootSelector (line 142) also has a composition-id regex that only matches exact =. If a sub-comp uses [data-composition-id^="scene"][data-start="0"], the timing attributes won't be stripped — they'll persist in the scoped output as extra specificity. Functionally correct (the timing attributes exist on the element), just inconsistent with how exact = handles it. Same applies to the runtime scoping pattern at line 211. Both are pre-existing and not made worse by this PR — just noting for a potential follow-up.

@miguel-heygen

Copy link
Copy Markdown
Collaborator

@Thomaswebstich after addressing the review, can you rebase main pls? I'll stamp afterwards and merge it! Thanks for contributing ❤️

scopeSelector() only rewrote the EXACT `[data-composition-id="x"]` root
selector in place; prefix/substring forms (`^=`, `*=`, `$=`) of the authored
composition id fell through to the prepend branch, becoming a two-level
`${scope} [data-composition-id^="x"] …` selector. That needs a NESTED
composition-id element, but the loader strips the inner root's composition-id
at mount, so the rule matches nothing and the sub-comp's class-scoped styles
silently drop (class-styled elements render unstyled).

This surfaces when a sub-comp ships prefix-match root selectors — e.g. an
authoring pipeline rewrites `="x"` to `^="x"` so rules survive the
duplicate-instance rename (x -> x__hf2). In a multi-scene document each such
sub-comp then loses its styling on mount, while single-mount renders (which
keep the inner structure) are unaffected.

Match the substring operators alongside `=` so the authored-root selector is
rewritten in place to the instance scope in every form. Adds a regression test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Thomaswebstich
Thomaswebstich force-pushed the fix/scoper-prefix-selectors branch from 3f182f8 to b4a91e9 Compare July 14, 2026 00:08
@Thomaswebstich

Copy link
Copy Markdown
Author

Rebased onto main — conflict resolved. The overlap was purely adjacent: #2089's html/body/:root → composition-box remap landed right above the composition-id regex I changed. They compose cleanly — a descendant prefix/substring selector ([data-composition-id^="x"] .y) is rewritten in place to the instance scope as before, and a bare prefix root selector ([data-composition-id^="x"]) now flows into the new isRootBoxSelectorcompositionBoxSelector path just like exact = does. Updated the regression test to assert that bare-root case against the new box-remap output. All 36 scoping tests pass; lint/format clean.

@miga-heygen thanks for the thorough review 🙏 — noted the follow-up on normalizeCompositionRootSelector / the runtime pattern (line ~211) only stripping timing attrs for exact =. Left it out here to keep this PR to the single-purpose scoper change as you flagged (pre-existing, not worsened); happy to do it as a separate follow-up.

@Thomaswebstich ready for your stamp + merge whenever.

🤖 Addressed by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants