refactor(color): emit colors as authored, drop channel companions - #271
Conversation
📦 Snapshot releasePublished |
The channel-components companion existed to give `#token.alpha` something to write an alpha into. Opacity moved to relative color syntax in #268, and since then nothing inside Tasty has read a companion — so every color declaration, every `tokens` prop entry and every color `@property` carried a second variable with no consumer. Removing it takes one declaration off each color rule, one `@property` registration off each color token on all four render paths, and a whole pre-pass off `PropertyTypeResolver.scanDeclarations`. ~0.8 kB brotli off `main`, 0.9 off `core`; size limits tightened back to ~0.5 kB headroom. No public contract moves — nothing exported changes and class name hashes are identical. The companions were internal implementation that happened to be visible in the emitted CSS, so hand-authored CSS that reached for one needs the token itself instead: `oklch(from var(--purple-color) l c h)` rather than `oklch(var(--purple-color-oklch))`, which also works on the colors the companion could never decompose. Migration note in docs/configuration.md. `colorSpace` keeps its job of choosing the space a statically known color is emitted in. Opacity is unaffected — it was already always written in `oklch`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the companion removal, plus the tests that were missing under it. `createStyle`'s color branch is one expression. `parseColor` derives `name` from the chain it resolved, so `name` implies `color` and the `var(--name-color)` fallback was unreachable; and a value `strToColorSpace` can convert is a literal, which never resolves to a `var()` chain, so `name` and a conversion cannot co-occur either. Both facts are now pinned by a test rather than assumed. Trying the conversion first also keeps the heavier `parseColor` off the path for plain literals, and drops a spurious `unable to parse color` warning for a token set to a bare keyword like `red`. `parseSameSpaceFunc`/`buildSameSpaceString` merge into `normalizeSameSpaceFunc` now that only one caller is left, and `SPACE_FUNCS`/`CANONICAL_FUNC` go with them — every space is named after its own function plus an optional legacy `a`. `processTokens`' two branches converged once the companion left, so they share one resolver that differs only in the property name and in how `true` reads. Tests, placed where the behavior lives: - `styles.test.ts` — the full `createStyle` color-value table, the mutual- exclusion invariant the collapse rests on, the no-spurious-warning case, and `--current-color` republishing including the `#current` self-reference guard, which had no coverage at all. - `color-space.test.ts` — cross-space conversion and hue units (`deg`/`turn`/ `rad`, negative, >360), which had only ever been exercised through the companion assertions, moved onto `strToColorSpace` where the conversion happens; plus same-space name canonicalization for all three spaces. - `process-tokens.test.ts` — one property per token key, and the `$` cases the shared resolver now covers. Migration note added under Color space in docs/configuration.md, and the changeset reframed: no public contract moves, but the emitted CSS does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bbeb4a7 to
ecdb77d
Compare
The companion removal left the color-space conversion with nothing load-bearing
to do. It existed so an opacity suffix had numeric channels to write an alpha
into; relative color syntax has the browser read the channels instead, so a
token's value no longer needs rewriting into any particular space.
So it isn't. `#brand: '#ff8800'` declares `--brand-color: #ff8800` — same for a
native color function, a bare CSS color name, a `color-mix()`. A `#token`
reference still resolves to its `var()` chain, and a plugin color function like
`okhsl()` is still resolved by the parser.
`configure({ colorSpace })` is deprecated: still accepted, inert, warns in
development, removed in the next major. If you relied on the uniform output,
author the token in the space you want it emitted in.
That drops the whole sRGB round-trip — `strToColorSpace`, `resolveToRgbaValues`,
`normalizeSameSpaceFunc`, `rgbToHsl`, `rgbToOklch`, `hexToRgbaValues`, and the
LRU that memoized it. `color-space.ts` goes from 706 lines to 86, and is now
only about how an alpha is applied. Against `main`: ~2.0 kB brotli off `main` and
`core`, ~2.6 kB off `static`, `zero` and `babel-plugin`.
Fixes a real bug the conversion was masking. `parseColor`'s first-character
dispatch answered `false` for any named color starting with a letter it had a
shape for, so `red`, `hotpink`, `lime`, `orange`, `violet`, `coral` and `teal`
were rejected and warned about, while `blue` and `green` were accepted. A miss
now falls through to the named-color lookup.
Coverage moved with the behavior rather than being deleted with the functions:
cross-space conversion and hue units onto `strToRgb` (still public, still
converts), plugin-color-function resolution onto `resolveFunctionColor` (the
generic path that replaced the okhsl branches), and `colorSpace`'s inertness is
pinned across all three values.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`knip` treats test files as entry points, so a helper reachable only from its own test reads as used. Five were: `srgbToOkhsl`, `toTone`, `hexToRgbValues`, `okhslStringToRgb`, `okhstStringToRgb`. Three come out, along with `hexCharToNum` which went with them. `okhslStringToRgb`/`okhstStringToRgb` are leftovers from v3 turning okhsl/okhst into ordinary plugins — the parser resolves those calls through `resolveFunctionColor` now, which is covered on its own. Two stay, and are documented as to why: `srgbToOkhsl` and `toTone` are the reverse halves of `okhslToSrgb` and `fromTone`, which the plugins do use. The engine never calls them, but the tests round-trip through them to check the forward conversion's accuracy — including the OKHSL green-region cases — and a round trip that checks itself is worth more than the fixtures that would replace it. No bundle change: none of these were reachable from a package entry point, so they were already being tree-shaken. `color-math.ts` loses 234 lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`color-math` keeps the reverse direction of two conversions the engine never
calls, so the forward ones — which the `okhsl()`/`okhst()` plugins do use — can
be round-tripped in tests rather than pinned to fixtures the implementation
itself produced.
Nothing re-exports them from a package entry point, so rolldown already drops
them: the constants survive only in a source map, not in any emitted `.js`. That
was true by accident, though, and `knip` cannot notice if it stops being true —
`knip.json` lists test files as entry points, so anything a test imports reads as
used.
`scripts/check-test-only-code.mjs` makes it explicit. It carries the reverse-only
closure (both exports plus the private helpers only they reach) and fails if any
of them appears in an emitted file. Matching is on `name(` / `name =`, so a
`{@link}` in a preserved doc comment is not a false positive.
It also asserts each registered name is still declared in `src`, so renaming or
deleting one fails the check instead of leaving it silently asserting nothing.
Verified all three ways: passes clean; fails on a forced production re-export,
naming the whole closure that comes in with it; fails on a stale registry entry.
Wired into CI after the knip step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Checked whether the producer/writer scale mismatch fixed in tenphi/glaze#94 applies here. It does not: `createColorFunc` already takes channel factors and scales to percentages on output, which is what that PR changed glaze's writers to do. Verified every other boundary too — `hslStringToRgb`, `oklchStringToRgb`, `strToRgb`, and `okhstToSrgb`'s `fromTone(t * 100)` bridge between the 0-1 lightness scale and the 0-100 tone scale — all convert correctly. One sibling of that bug was here, though. `parsePercentage` reads a unitless channel as the factor it looks like, so `okhsl(280 .8 .52)` and `okhsl(280 80% 52%)` are the same color. Drop the `%` and `80` lands in a 0-1 slot, clamps to full saturation, and renders as white — which looks like a color rather than like a mistake, exactly the silent-plausible-output failure glaze#94 also set out to remove. A unitless channel above 1 cannot be a factor, so it now warns once per function in development through the existing `warnOnceDev`, which `resetConfig()` already clears. `1` stays silent, being a legitimate factor, and the emitted color is unchanged. The warning tests are mutation-checked: replacing `warnOnceDev` with a bare `console.warn` fails three of them, including the production case. That one also uses a channel value no earlier test touches — `createColorFunc`'s LRU lives for the process, so a cached value would return before reaching the check and the assertion would have passed for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tenphi
left a comment
There was a problem hiding this comment.
[P2] Remove the companion promise from public typings
The exported token JSDoc still says color tokens create --name-color-{colorSpace}, although this PR removes that property from every rendering path. The stale contract appears in src/types.ts:113, src/config.ts:414, src/styles/types.ts:742, and src/plugins/types.ts:74, and is emitted into the published declarations. Editor tooltips will therefore direct users toward an undefined CSS property. Please update all four public type comments to say that #name creates only --name-color.
Four token JSDoc blocks still said `#name` creates `--name-color` and
`--name-color-{colorSpace}` — `src/types.ts`, `src/config.ts`,
`src/styles/types.ts` and `src/plugins/types.ts`. They ship in the emitted
`.d.ts`, so editor tooltips were pointing at a property no rendering path emits
any more.
My sweep missed them because it grepped for the resolved suffixes
(`-color-oklch`, `-color-rgb`, …) rather than the templated form, so
`--name-color-{colorSpace}` matched nothing. Re-swept on `colorSpace` and
`color-{` across all of `src`, which also caught a stale "with no companion
variable" aside in `overrideColorAlpha`'s doc.
Verified against the built output: no emitted `.d.ts` mentions the companion,
and the corrected line is present in the published declarations.
Reported by @tenphi in review.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixed in 030cffc — all four updated to say You were right that it reaches published declarations, so I verified against the built output rather than just the source: no emitted Worth recording why it slipped: my sweep grepped for the resolved suffixes ( |
Relative colour syntax takes a concrete origin from Safari 16.4, but
`oklch(from currentcolor …)` needs Safari 18. A token defined as `#current`
emitted the bare keyword, so fading it — `{ '#ink': '#current', fill: '#ink.5' }`
— produced exactly that unsupported form. `#current` now emits
`var(--current-color)`, so the origin is a real colour wherever a `color` style
published one.
Three things keep the swap invisible everywhere else:
- `--current-color` is registered with `initial-value: currentcolor` instead of
`transparent`. A registered `<color>` property keeps the keyword as its
computed value and resolves it against each element's own colour, so an
unpublished `#current` is indistinguishable from the keyword. With
`transparent` it would have rendered invisible — the same failure mode as the
`@property` amplification behind the ui-kit outage.
- `colorStyle` publishes `--current-color` for every colour, not only a named
token. A literal `color: 'red'` has to displace an ancestor's token colour, or
a descendant's `#current` reads the ancestor's.
- `#current.N` deliberately keeps `currentcolor` inside its `color-mix()`. The
mix composes, so a nested fade must read the already-faded colour that reaches
it; the variable would carry the outer fade's own operand and mix it twice.
`#current.4` with `#current.18` under it still lands at `.072`.
A value that already reads the inherited colour is not republished: publishing
`var(--current-color)` into itself is a self-reference, which invalidates the
declaration silently, and republishing a `color-mix()` over the keyword would
resolve it again one level down.
Behaviour verified on Safari 16.5.1, 17.3 and 18.4 before writing this, and the
whole chain is pinned by computed-style tests in a real engine — including the
regression I hit on the way, where an unpublished `#current` went transparent
because `DEFAULT_PROPERTIES` still registered it as such.
Still uncovered, unchanged from before: a token defined as `#current` and faded
where nothing published the variable. That origin is the keyword again, so that
one case needs Safari 18.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check ran on the parsed number, so `150%` — legitimate over-saturation that clamps — parsed to `1.5` and tripped the missing-`%` warning. Worse, the warning is deduped per function, so that false positive burned the slot and silenced the genuine `80` typo that came after it. The raw token decides now, not the parsed value: a channel is misscaled only when it carries no `%` and still exceeds 1. The message names just the offending channels rather than both. Also moved the spy and env cleanup out of the test bodies into `afterEach`. An assertion that threw mid-test used to skip `mockRestore()`, leaking a mocked `console.warn` — call history included — into the following tests and turning one real failure into six. Verified: breaking one assertion now fails exactly one test. Reported by @tenphi in review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
What
Two things existed only to serve the opacity suffix back when it needed numeric channels to write an alpha into:
#nametoken declared a second variable beside--name-colorholding its channels decomposed (--brand-color-oklch: 0.75 0.16 55) plus a matching@propertyrule, andcoloremitted--current-color-{space}beside--current-color.colorSpace, so#brand: '#ff8800'declared--brand-color: oklch(0.75 0.16 55).#268 moved opacity to relative color syntax (
oklch(from var(--brand-color) l c h / .5)), which has the browser read the channels off whatever the value resolves to. Neither is load-bearing any more. Both are removed, and a color now passes through untouched:--brand-color: #ff8800.configure({ colorSpace })is deprecated — still accepted, inert, warns in development, removed in the next major.No public contract moves. Nothing exported changes,
public-api.mdis untouched, and class name hashes are identical (hashing is over the input style object). What changes is the emitted CSS.Migration
In Color space and in the changeset:
Relative color syntax is strictly more capable: the companion could only carry channels for a color the engine could evaluate at build time, while
from <color>works on every<color>— acolor-mix(), alight-dark(), a variable Tasty never defined. If you relied oncolorSpacefor uniform output, author the token in the space you want it emitted in.Size
Measured against
main:maincorestaticzerobabel-pluginLimits tightened to ~0.5 kB headroom rather than left slack. Also: one declaration less per color in every rule, one
@propertyregistration less per color token on all four render paths, a pre-pass removed fromPropertyTypeResolver.scanDeclarations, and the LRU that memoized the sRGB round-trip is gone.color-space.tswent 706 → 86 lines and is now only about how an alpha is applied;color-math.tslost 234.Bugs found along the way
parseColorrejected most named colors. Its first-character dispatch answeredfalsefor any named color starting with a letter it had a shape for, sored,hotpink,lime,orange,violet,coralandtealwere rejected and warned about, whileblueandgreenwere accepted. The conversion had been masking it. A miss now falls through to the named-color lookup.unable to parse colorwarning for a token set to a bare keyword, fixed by the same change.okhsl(280 80 52)— the%forgotten — clamped to full saturation and rendered white. Now warns once per function in development. See below.Refactoring
createStyle's color branch went from five outcomes to one expression. Two invariants made that safe, both now pinned by a test rather than assumed:parseColorderivesnamefrom the chain it resolved (sonameimpliescolor), and a value the converter could handle never resolved to avar()chain.parseSameSpaceFunc/buildSameSpaceStringmerged, then went entirely with the conversion.processTokens' two branches converged into one resolver. Gone:strToColorSpace,resolveToRgbaValues,colorFuncName,getColorSpaceSuffix,getColorSpaceComponents,convertColorChainToComponentChain,colorInitialValueToComponents,getComponentPropertySyntax,getDefaultComponents,FormatPropertyOptions,rgbToHsl,rgbToOklch,hexToRgbaValues, and the unusedisColorongetEffectiveDefinition's result.Test-only code, enforced
color-mathkeeps the reverse direction of two conversions the engine never calls — sRGB→OKHSL and lightness→OKHST tone — so the forward ones, which theokhsl()/okhst()plugins use, can be round-tripped in tests rather than pinned to fixtures the implementation itself produced.Rolldown already drops them (their constants survive only in a source map), but that was true by accident, and
knipcan't notice if it stops being true —knip.jsonlists test files as entry points, so anything a test imports reads as used.scripts/check-test-only-code.mjsmakes it explicit: it carries the reverse-only closure and fails if any of it reaches an emitted file, and asserts each registered name is still declared insrcso it can't silently rot. Validated three ways — passes clean, fails on a forced production re-export (naming the whole closure), fails on a stale entry. Wired into CI.Checked against tenphi/glaze#94
The producer/writer scale mismatch fixed there does not apply here —
createColorFuncalready takes channel factors and scales to percentages on output, which is what that PR changed glaze's writers to do. Verified every other boundary too:hslStringToRgb,oklchStringToRgb,strToRgb, andokhstToSrgb'sfromTone(t * 100)bridge between the 0-1 lightness scale and the 0-100 tone scale all convert correctly.One sibling of it was here: the silent clamp described above. Fixed, with mutation-checked tests.
Tests
2067 passing. Coverage moved with the behavior rather than being deleted alongside the functions:
deg/turn/rad, negative, >360) → ontostrToRgb, still public and still converting.resolveFunctionColor, the generic path that replaced the okhsl branches.colorSpace's inertness pinned across all three values;createStyle's full value table and--current-colorrepublishing (including the#currentself-reference guard, previously untested) pinned directly.Verification
pnpm test2067 passing ·pnpm hygiene+pnpm knip+pnpm check:test-onlyclean ·pnpm build && pnpm sizeunder tightened limits ·public-api.mdunchanged · snapshot diffs reviewed line by line.🤖 Generated with Claude Code