fix(core,sdk): namespace composition variables so they stop shadowing theme tokens - #3125
Open
miguel-heygen wants to merge 1 commit into
Open
fix(core,sdk): namespace composition variables so they stop shadowing theme tokens#3125miguel-heygen wants to merge 1 commit into
miguel-heygen wants to merge 1 commit into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
… theme tokens A declared composition variable was written to the composition root as a CSS custom property named after its own id, with no namespace. A variable called accent therefore set --accent inline and shadowed the host theme for that whole subtree. That is worse than a naming clash. The accent enum values are green, blue and violet, which are not colours, they are selectors a composition maps onto theme slots. A representative composition maps blue to var(--accent, #18181b). The runtime then set --accent to blue, so the lookup resolved to the CSS keyword and no host theme could win. green and violet escaped only because they route to --brand and --accent-2, which nothing shadowed, which is why this survived: it was invisible for two of three values. Variables are now written to --hf-var-<slug>. The bare name is still written as a deprecated alias, but only for ids that are not reserved theme tokens, which is what actually fixes the collision. Four writers had the bug, not one: the runtime bindings, the scoped getVariables path, the compiler stylesheet, and the SDK mutate and apply-patches path. Fixing only the runtime left the compiler emitting the bare name into compiled output, where a host theme supplied as an inline style attribute still rendered the keyword. All four now route through one helper, and the helper takes the raw id so callers cannot derive the name themselves. That last point closed a real defect rather than a tidy-up. Two sites derived the property name differently, one verbatim and one slugged, so an id like Accent produced two disjoint property sets: one path reserved it, the other aliased it, and an SDK edit silently never landed. A test pins that every injection path derives one name per id. Docs that taught binding the bare name are corrected, including the capstone, whose ink variable is reserved and would have re-skinned the ground while quietly ignoring the ink. Rendered output is unchanged there, so the published videos stay accurate.
miguel-heygen
force-pushed
the
fix/namespace-composition-variables
branch
from
August 9, 2026 17:45
671f480 to
e6b8e39
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A declared composition variable no longer overwrites a same-named theme token. Variables are written to
--hf-var-<slug>; the bare--<slug>remains as a deprecated alias, but only for ids that are not reserved theme tokens.24 files, +777/-79.
Why
Every declared variable was written to the composition root as a CSS custom property named after its own id, unnamespaced. A variable called
accentset--accentinline and shadowed the host theme for that whole subtree.That is worse than a naming clash. The
accentenum's values aregreen | blue | violet, which are not colours: they are selectors a composition maps onto theme slots. A representative composition does exactly this:The mapping is correct. The runtime then set
--accent: blueon the same element, so the lookup resolved to the CSS keyword and no host theme could win.It was invisible for two of three values.
greenandvioletroute to--brandand--accent-2, which nothing shadowed, so onlybluemisbehaved. That is why it survived.How
One helper in
packages/core/src/runtime/themeTokens.tsowns both the reserved list and the name derivation. It takes the raw id, so no caller can derive a name itself.Four writers had the bug, not one: the runtime bindings, the scoped
getVariablespath, the compiler's emitted stylesheet, and the SDK'smutate/apply-patches. Fixing only the runtime left the compiler writing the bare name into compiled output, where a host theme supplied as an inlinestyleattribute still rendered the keyword. The compiler'sauthoredDefinesguard only scans<style>elements, so it missed that path; the guard was not widened, because namespacing makes the question moot.The unification closed a real defect, not a tidy-up. Two sites derived the property name differently, one verbatim and one slugged. For an id like
Accentthat produced two disjoint property sets: one path treated it as reserved, the other aliased it, so an SDKsetVariableValue("Accent", …)silently never landed. A test now pins that every injection path derives one name per id.The reserved list is 15 role slots, derived from the CSS blocks that actually define
--accentrather than from memory. The reserved check is deliberately case-sensitive, because custom properties are:--Accentgenuinely does not shadow--accent.Test plan
packages/corepackages/sdkbun run lintfallow auditEnd to end, through the real compile path, host theme on an inline
styleattribute (the case that defeated the old guard):--accent: blue;, rendered pixel rgb(0, 0, 255)--hf-var-accent: blue;only, rendered pixel rgb(124, 58, 237), and--accentresolves to the host's#7c3aedMutants isolate each site rather than sharing one test: breaking the compiler kills 3 core tests and 0 SDK; breaking
mutate.tskills 5 SDK and 0 core; breakingapply-patches.tskills exactly the one testmutate.ts's mutant left alive; breaking the runtime kills 6 core runtime tests and 0 compiler.Producer fixtures verified unmoved by hashing rather than
git diff, since those files are permanently dirty for an unrelated LFS reason:sha256over the sorted manifest of all 74compiled.htmlfiles is byte-identical before and after.Docs
Six surfaces taught binding the bare name and are corrected, including
docs/concepts/variables.mdx, which now carries the reserved list and the slug rule as the single owner.The capstone needed judgement. It declares
groundandinkand promises one--variablescall re-skins the journey, butinkis reserved, so it would have re-skinned the ground and quietly ignored the ink. Renaming was rejected: the id appears in the render command and the quoted prompt, so renaming would make the page describe a command that never produced the embedded video. It now bindsvar(--hf-var-ink). Rendered output is unchanged, so both published videos stay accurate.Not covered
The deprecated bare alias has no removal version yet. Every notice says "a future release";
grep "will be removed in a future release"finds all occurrences at once when that is decided.cssVariableNamelowercases without camel-splitting, soaccentColorbecomes--hf-var-accentcolor. That shape predates this change and is left alone; changing it is a separate contract change.The Docker regression harness (real renders, PSNR) was not run. Its compilation stage was exercised directly and no fixture bytes move.