fix: emit the beta semantic token layer under the color-scheme scopes - #5239
Conversation
The Tokens Studio CSS export declared the semantic layer at :root while the scale aliases it references only exist under [data-color-scheme] scopes. Custom properties substitute where they are declared, so the semantic tokens resolved once at the root and never re-resolved inside [data-color-scheme] subtrees, breaking subtree colour-scheme switching (ADR-0004/ADR-0013). The saved EDS-CSS export configuration now sets rootSelector: ':root, [data-color-scheme]' (was the ':root' default), so every non-dimensional layer is also declared on any element carrying the attribute and var() references substitute against the nearest scope. Token content is unchanged - the diff is selector lines only. The <html>-mirror workaround in the Storybook decorator is no longer needed and is removed. Closes #5226
|
@claude review |
|
Claude finished @pomfrida's task in 5m 27s —— View job Review
The diagnosis in the PR body is correct — custom properties substitute where they are declared, so the semantic layer genuinely had to move inside the I verified the mechanical claims locally and they hold:
That said, widening the selector for every layer rather than just the semantic layer has two consequences I think need attention before this lands. 1. Density silently reverts to
|
| Token | [data-color-scheme] descendant, before |
after |
|---|---|---|
--eds-text-disabled |
var(--eds-neutral-7) |
var(--eds-neutral-5) |
--eds-border-disabled |
var(--eds-neutral-7) |
var(--eds-neutral-2) |
--eds-border-focus |
var(--eds-light-blue-7) / var(--eds-dark-blue-7) |
var(--eds-border-focus) |
semantic/default.css:29 is --eds-border-focus: var(--eds-border-focus) — a self-reference. That's a cycle, so the property computes to the guaranteed-invalid value and any consumer's var(--eds-border-focus) falls back or becomes unset. It's already broken at :root today (the semantic block outranks the light/dark block on <html> for the same reason), so this is an upstream token-source bug, not something this PR introduced — but the PR extends its blast radius from "<html> only" to "every [data-color-scheme] subtree", which is where components will actually read it. Looks like a semantic token aliasing a same-named scheme token and collapsing to itself after prefix flattening; worth fixing in Tokens Studio.
Practically this makes "Token content is byte-identical — the diff is selector lines only" and "no contract change for consumers" true of the bytes but not of the resolved values. Nothing in the repo consumes these three yet (they're beta and unexported from package.json), so severity is low today — but it's the kind of thing that should be recorded rather than discovered during migration.
3. Minor points
preview.mjs:59cites ADR-0013, which doesn't exist.documentation/adr/stops at0009-temporary-next-subpaths-for-eds-tokens-beta.md, andADR-0013appears nowhere else in the repo — the only hit is the comment added by this diff (the PR body cites it too). Either point at the ADR that actually covers subtree colour-scheme switching, or drop the reference. Fix this →scripts/generate-css-bundle.mjsheader is now inaccurate. It states "every mode file is:root- or attribute-scoped, so plain concatenation is conflict-free… source order does not affect resolution." With:root, [data-color-scheme]the widened blocks overlap the light/dark blocks on the same elements at equal specificity, so source order now decides the winner (that's exactly what produces finding 2). Worth updating the comment, and worth considering an assertion in the bundle script that no custom property is declared in both the color-scheme layer and a widened layer — it would have caught these three.- Consumer overrides on intermediate ancestors get reset. Any of the 459 tokens a consumer sets on a container is re-declared to the bundle default on nested
[data-color-scheme]elements. Inherent to the approach and probably acceptable, but it's a documented-behaviour item since "set a token on a wrapper" is a common escape hatch. - The wrapper comment says "Both token bundles scope their tokens under
[data-color-scheme]", which is a bit stronger than the PR body's own side note that legacy declares--eds-color-bg-neutral-surfaceonly under:root, [data-color-scheme="light"]. Worth softening so the next reader doesn't rely on it.
Not verified
I could not check name collisions against the built legacy 2.x bundle — packages/eds-tokens/build/ isn't committed and dependencies aren't installed in this run. The mechanism is worth a look though: for any name legacy declares only under :root, [data-color-scheme="light"], the widened beta block now wins inside [data-color-scheme="dark"] subtrees, flipping the deliberate "legacy wins collisions" import order in preview.css. Scanning the committed sources I found no overlap between the beta root-block names and either src/css or Foundation/*.css, so this may well be moot.
Summary: the fix is the right one in principle, and the reproducibility claims check out. I'd narrow the widening to the semantic layer (finding 1 is a concrete regression), and either fix or explicitly record the --eds-border-focus self-reference before components start consuming the beta semantic layer.
• branch fix/tokens-semantic-layer-color-scheme-scope
The wrapper used the legacy --eds-color-bg-neutral-surface, which has no dark declaration in the 2.x bundle, so the story canvas stayed light in dark mode. The beta semantic tokens re-resolve per [data-color-scheme] subtree after the rootSelector change, so the canvas now follows the toolbar toggle. Legacy is left untouched - it is being replaced, not maintained.
Review follow-up on #5239. Widening every non-dimensional layer via the export's global rootSelector regressed the density cascade: the density base (comfortable) declares the same --eds-density-* names as the compact/relaxed variants, so re-declaring it on every [data-color-scheme] element clobbered a [data-density] ancestor's values for the whole subtree. The export format has no per-layer selector, so the saved EDS-CSS configuration is reverted to the default rootSelector (:root) and the semantic layer - the only layer that references the scheme-scoped scale aliases - is widened repo-side by scripts/widen-semantic-scope.mjs, chained before generate-css-bundle.mjs in the generate:css-bundle package script so the release workflow needs no change. The script is idempotent and fails loudly if the export layout changes. Also drops a comment reference to ADR-0013 (not merged yet, PR #5200) and documents the #5221 name-collision caveat in the bundle header.
|
Review findings addressed in cb4119b: 1. Density regression — fixed. The saved EDS-CSS export configuration is reverted to the default 2. Token collisions — documented, deferred to #5221. The three duplicate names ( 3. Minor points — fixed. The ADR-0013 reference is dropped from the decorator comment (the ADR is still unmerged in #5200), the wrapper comment now says the legacy bundle scopes its scheme-dependent tokens rather than all of them, and the bundle header's "source order does not affect resolution" claim is corrected. Consumer-override reset on scoped elements is now limited to the semantic layer, which is the layer where per-subtree re-declaration is the intended behaviour. Not verified / legacy collisions: with the widening narrowed to the semantic layer, the overlap surface against legacy is the semantic names only — the committed-source scan finding no overlap should carry over, and the "legacy wins collisions" import order in preview.css is unaffected for every layer that stays |
The widen step is chained in the generate:css-bundle package script, but running the bundler directly would silently bundle an unwidened semantic layer and regress subtree colour-scheme switching (#5226). The bundler now fails loudly instead.
|
@claude review |
|
Claude finished @pomfrida's task in 6m 4s —— View job Review
The narrowing to the semantic layer is the right call and it's correctly done. I re-verified last round's findings against
One new issue, then smaller ones. 1. Prettier would rewrite the widened selector, and both scripts then hard-fail with a misleading error
Prettier breaks comma-separated CSS selector lists onto one line each. That's observable in this repo: :root,
[data-color-scheme] {
I reproduced the failure by writing the two-line form: Both exit 1, and the message blames the export layout when in fact the file is already correctly widened. Cheap fix: make both matches whitespace-tolerant regexes ( 2. The guard covers only
|
Review follow-up on #5239, round two. - The widened selector is matched with whitespace-tolerant regexes and emitted in Prettier's canonical one-selector-per-line form, shared between the widen script and the bundler via semantic-scope.mjs, so a prettier --write no longer breaks the pipeline with a misleading 'export layout changed' error. - Every semantic/*.css file is widened and asserted, mirroring the bundler's directory glob - a file the export adds later cannot slip through with the narrow :root selector. - src/tokens/* is added to .prettierignore (root and package-local): the directory is machine output and was never Prettier-conformant (the export emits double-quoted attribute selectors and no trailing newline, failing prettier:check on main too), and hand-formatting it would be reverted by the next export run. The package prettier:check now passes.
|
Round-two findings addressed in bbdc0e4: 1. Prettier fragility — fixed, with one correction to the premise. The selector is now matched with whitespace-tolerant regexes and emitted in Prettier's canonical one-selector-per-line form, with the pattern shared between both scripts via 2. Semantic glob gap — fixed. The widen script now widens every 3. Bundle-level regression test + eds-tokens tests not running in CI — agreed, but deferred to its own issue. Wiring Verified after the changes: the chained |
|
@claude review |
|
Claude finished @pomfrida's task in 8m 13s —— View job Review
Round three looks good. Your correction to my premise is right, and I confirmed it rather than taking it on trust: even with the two-line selector, Verified locally (deps installed, all reruns left the tree clean):
One thing I checked because removing an Nothing blocking. Four small items, all optional. 1. The widen script is non-recursive while the bundler asserts recursively — a nested semantic file deadlocks the pipeline
Reproduced by adding The nested file is never widened, and the failure message prescribes a remedy that can't fix it — re-running the widen script is a no-op, so the release pipeline is stuck until someone edits the script. Failing loudly beats shipping narrow, so this is strictly better than before; it's just one word away from being fixable by the message it prints. 2.
|
…elease workflow Review follow-up on #5239, round three. - The widen script now globs semantic/ recursively, matching the bundler's assertion glob - a nested file the export adds later gets widened instead of deadlocking the pipeline with a remedy message that is a no-op. - A selector that matches the tolerant regex but not the canonical form is normalised, so the committed bytes are a function of content only. - The release workflow's bundle step and auto-PR body now mention the semantic-layer widening, so the selector diff in tokens-release PRs is traceable to #5226 instead of looking like an export change.
|
Round-three items addressed in 5e41f85: 1. Recursive mismatch — fixed. The widen script now globs 2. Non-canonical whitespace — normalised. A selector matching the tolerant regex but not the canonical form is rewritten to 3. Workflow description — updated. The release workflow's bundle step is renamed/documented and the auto-PR body now notes that the 4. Deferred follow-ups — now traceable: #5246 (run the eds-tokens vitest suite in CI) and #5247 (regression test asserting the |
…th the widened semantic block The widened semantic layer and the color-scheme scope rules both apply to [data-color-scheme] elements at equal specificity, so source order decides the three duplicate names tracked in #5221. With the previous alphabetical order the semantic block sorted last and won - and its --eds-border-focus is a self-reference, computing to the guaranteed-invalid value, which would have dropped focus outlines for the incoming Chip (#5225) and Button (#5222) migrations inside scoped subtrees (both use var(--eds-border-focus) for their focus ring). The bundler now ranks color-scheme/* after every other layer, so the scheme-specific values win on scoped elements and every resolved value matches the pre-widening state. Verified in the browser: --eds-border-focus resolves to the light/dark blue per subtree, and text-disabled/border-disabled keep their scheme values.
|
One more hardening after a pre-merge worst-case pass (48353e6): the round-two review's finding 2 turned out to matter concretely — the incoming Chip (#5225) and Button (#5222) migrations both use The bundler now concatenates |
1 participant
Closes #5226
Problem
The Tokens Studio CSS export declared the semantic layer (
--eds-background-*,--eds-text-*, …) in a:rootblock, while the scale aliases it references (--eds-accent-N,--eds-neutral-N, …) only exist under[data-color-scheme="light"|"dark"]. CSS custom properties substitute where they are declared, so the semantic tokens resolved once at the root and never re-resolved inside[data-color-scheme]subtrees — subtree colour-scheme switching (a dark panel inside a light app, per ADR-0004) was broken for the semantic layer, and Storybook needed an<html>-mirror workaround (#5224).Fix
The semantic layer — the only layer that references the scheme-scoped scale aliases — is widened to
:root, [data-color-scheme]by a new post-export step,scripts/widen-semantic-scope.mjs, chained beforegenerate-css-bundle.mjsin thegenerate:css-bundlepackage script (so the release workflow needs no change). It is thereby also declared on any element carrying the attribute, andvar()references substitute against the nearest scope. The script is idempotent and fails loudly if the export layout changes.The saved EDS-CSS export configuration keeps the default
rootSelector: ":root". An earlier revision of this PR set the export's globalrootSelectorinstead, but that widened every non-dimensional layer and regressed the density cascade (the density base declares the same--eds-density-*names as the compact/relaxed variants, so re-declaring it on[data-color-scheme]elements clobbered a[data-density]ancestor's values — caught in review). The export format has no per-layer selector, hence the repo-side step.mainis the semantic layer's selector line (source + bundle) onlygenerate:ts-tokensoutput is unchanged (the generator parses declarations, not selectors)<html>-mirroruseEffectin the Storybook decorator is removed; the wrapper div's owndata-color-schemeis now sufficientCollision handling (names tracked in #5221):
border-focus,text-disabledandborder-disabledare declared in both the color-scheme layer and the semantic layer (including a--eds-border-focus: var(--eds-border-focus)self-reference). Both blocks apply to[data-color-scheme]elements at equal specificity, so the bundler concatenates the color-scheme files last — the scheme-specific values win and every resolved value matches the pre-widening state. This matters concretely: the incoming Chip (#5225) and Button (#5222) migrations usevar(--eds-border-focus)for their focus rings, and letting the semantic self-reference win would have dropped focus outlines inside scoped subtrees. The duplicate names themselves remain a token-content bug to fix upstream (#5221).Storybook wrapper canvas
The wrapper's background token — the legacy
--eds-color-bg-neutral-surface— is only declared under:root, [data-color-scheme="light"]in the 2.x bundle (no dark value), so the story canvas stayed light in dark mode. Rather than patching the legacy pipeline (being replaced, not maintained), the wrapper now uses the beta semantic tokens--eds-background-container-canvas-default/--eds-text-primary.Verification
Checked in a running Storybook from this branch (computed values via the browser, no
<html>attribute present):--eds-background-container-canvas-defaultin light wrapperoklch(98% 0 0)<div data-color-scheme="dark">oklch(19% .0014 242.96)--eds-density-spacing-mdat root (comfortable)16px[data-density="compact"]container12pxdata-color-schemechild inside that container12px(ancestor density preserved — review finding 1)Wrapper canvas follows the toolbar toggle: dark gives
background: oklch(0.19 …)/color: oklch(0.91 …), lightoklch(0.98 …)/oklch(0.37 …).