docs: add ADR proposing a single bundled css entry for eds-tokens 3.0.0 - #5199
docs: add ADR proposing a single bundled css entry for eds-tokens 3.0.0#5199pomfrida wants to merge 2 commits into
Conversation
|
@claude review |
|
Claude finished @pomfrida's task in 1m 45s —— View job Review: ADR-0010 — single bundled CSS entry for eds-tokens 3.0.0
This is a strong, well-argued ADR: the options table is honest about the < 1 kB payload trade-off, the "name reuse is deliberate" reconciliation with ADR-0009's opposite conclusion is the kind of reasoning that makes an ADR worth reading, and the colour-scheme carve-out correctly attributes 2.x's system-dark to Verified against the repo (2026-07-20,
|
| Claim | Status |
|---|---|
9 granular files under src/tokens/css/{colors,color-scheme,semantic,primitives,font,density} |
✅ exactly 9 (color-scheme/{light,dark}, colors/default, density/{comfortable,compact,relaxed}, font/default, primitives/default, semantic/default) |
2.x publishes ./css/variables → variables.min.css |
✅ packages/eds-tokens/package.json:15 |
build-dark-scope added the prefers-color-scheme fallback (#4864) |
✅ _build:css:dark-scope script at package.json:61,75 |
tokens_studio_release.yaml exists (where the concat step will live) |
✅ present |
Findings
1. Broken relative links to ADR-0009 and ADR-0013 (medium)
The ADR links ./0009-temporary-next-subpaths-for-eds-tokens-beta.md (lines 10, 171) and ./0013-attribute-scoped-custom-properties-not-light-dark.md (line 176), but neither file is tracked in documentation/adr/ yet — the current top of the sequence is 0008. These are companion PRs per the description, so the links will 404 on GitHub until those merge. Worth calling out the merge-order dependency explicitly, or at least being aware the rendered ADR has dead links until #5194 (and the 0013 PR) land. Since 0010 references 0013, ideally 0013 merges first or alongside.
2. "the existing ./next/css/* wildcard" is not yet in the package (medium)
Lines 116–121 state the beta validation "costs nothing extra" because "the existing ./next/css/* wildcard maps to src/tokens/css/*". Checked against current main: there is no ./next/* export in packages/eds-tokens/package.json and no next/css reference anywhere in the package manifest or the release workflow. That wildcard is what ADR-0009 introduces — so as written, "existing" is only true after #5194 merges. Recommend rewording to make the dependency explicit, e.g. "the ./next/css/* wildcard introduced by ADR-0009 maps to…", so the validation plan doesn't read as resting on a present-tense fact that isn't true against main today. Fix this →
3. Minor: 2.x actually exposes two CSS specifiers
package.json:15-16 maps both ./css/variables and ./css/variables.css to the same variables.min.css. The ADR describes 2.x as "exactly one CSS entry" (line 17) and the title says "one bundled ./css/variables entry". True at the file level, but there are two specifiers. It would be worth one sentence stating whether 3.0.0 reuses the .css alias too, since consumers who imported @equinor/eds-tokens/css/variables.css are equally part of the "one-import habit" being preserved.
4. Note: payload figures unverified
The 4.6 kB / 3.7 kB min+gzip numbers (lines 31-33, 71) can't be checked from the source tree (the bundle/concat step doesn't exist yet). They're plausible and the sub-1 kB conclusion is the load-bearing claim regardless of exact values — just flagging they're asserted, not reproducible from the current repo.
Nits
- Line 138 heading says "Open question" (singular); the validation plan (line 151) and consequences refer to "the open questions" (plural). Only one is listed — align the wording.
- The density set is currently 3 files (
comfortable,compact,relaxed); the open question at line 140 anticipates it "may change shape", which is consistent — no action, just confirming the artifact matches.
Verdict
Accuracy is high and the reasoning is sound. The only substantive fix is #2 — decoupling the "existing wildcard" phrasing from ADR-0009's not-yet-merged export so the validation plan is self-consistent against main. #1 (link/merge ordering) is worth a heads-up. Everything else is polish. Nothing here blocks the "Proposed" status.
• docs/adr-bundled-css-entry-tokens-3
Summary
ADR-0010 (status Proposed) settles the open consequence from ADR-0009 (#5194): the 3.0.0 CSS export surface mirrors 2.x — one bundled
./css/variablesentry, while the granular generated files stay unpublished internal pipeline artifacts.Key verified facts behind the decision:
Colour-scheme behaviour is governed by ADR-0004 (attribute-driven, set once at root): the bundle is a plain concatenation (deliberately not minified: byte-stable output, reviewable release-PR diffs, no extra toolchain dependency — costs ~0.2 kB gzipped) with no added behaviour and no
prefers-color-schemefallback. 2.x's automatic system-dark was inherited fromlight-dark()(#3447) via the #4864 bug-fix rewrite, not a design decision, and does not carry over — flagged as a migration-guide item.Validation plan: the bundle ships on the beta line as
next/css/variables.cssthrough the existing wildcard export (no publish-workflow change), gets exercised during the /next migration, and the ADR is marked Accepted once at least one beta has exposed the exact 3.0.0 surface.Related: ADR-0009 (#5194), companion ADR-0013 (#5200), epic #4740, #5108
Tracking issues