Skip to content

fix(cli): theme override keys now match the class names components actually render (#4109)#4204

Open
jiunshinn wants to merge 1 commit into
facebook:mainfrom
jiunshinn:fix/4109-theme-textinput-selector
Open

fix(cli): theme override keys now match the class names components actually render (#4109)#4204
jiunshinn wants to merge 1 commit into
facebook:mainfrom
jiunshinn:fix/4109-theme-textinput-selector

Conversation

@jiunshinn

Copy link
Copy Markdown
Contributor

Problem

astryx theme build emits TextInput component-override CSS under .astryx-textinput, but <TextInput> renders the class astryx-text-input (packages/core/src/TextInput/TextInput.tsx L399: themeProps('text-input', ...)). Every themed TextInput override ships as a dead rule and silently does nothing.

Repro (before this PR), building a theme with both spellings:

  ⚠ Unknown component "text-input". Did you mean: text, textinput?   ← the CORRECT key warns…
.astryx-textinput  { border-radius: 16px; }  /* blessed key → dead rule, matches nothing */
.astryx-text-input { border-width: 2px; }    /* "unknown" key → the one that actually works */

Root cause

Theme CSS selectors are derived verbatim from the components key: core's shared generator (packages/core/src/theme/generateThemeRules.ts, componentClassSelector) emits .astryx-<key>, and components render astryx-<token> via themeProps()/stableClassName() (packages/core/src/utils/themeProps.ts, packages/core/src/naming.ts). So the key is the selector contract — and the CLI's KNOWN_COMPONENTS validation table (packages/cli/src/commands/build-theme.mjs) listed 15 multi-word components un-hyphenated while the components render hyphenated tokens. The table was wrong on both sides at once:

  1. The broken spelling (textinput) validated cleanly and emitted dead CSS — silent failure.
  2. The correct spelling (text-input) — which the first-party themes (packages/themes/butter, stone) already use — tripped the "Unknown component" warning, whose "did you mean" then steered authors to the broken spelling.

Audit: every KNOWN_COMPONENTS key vs. the class core renders

Cross-checked all 53 keys against the literals passed to themeProps() / buildClassName() / stableClassName() in packages/core/src (and each component's doc.mjs theming.targets[].className).

Key (before) Rendered class Verdict → key (after)
appshell astryx-app-shell ❌ → app-shell
aspectratio astryx-aspect-ratio ❌ → aspect-ratio
checkboxinput astryx-checkbox-input ❌ → checkbox-input
dateinput astryx-date-input ❌ → date-input
dropdownmenu astryx-dropdown-menu ❌ → dropdown-menu
formlayout astryx-form-layout ❌ → form-layout
mobilenav astryx-mobile-nav ❌ → mobile-nav
moremenu astryx-more-menu ❌ → more-menu
numberinput astryx-number-input ❌ → number-input
radiolist astryx-radio-list ❌ → radio-list
sidenav astryx-side-nav ❌ → side-nav
tablist astryx-tab-list ❌ → tab-list
textinput astryx-text-input ❌ → text-input (the issue)
timeinput astryx-time-input ❌ → time-input
topnav astryx-top-nav ❌ → top-nav
layer (none — Layer is a positioning hook/primitive; nothing renders astryx-layer) ❌ removed
remaining 37 (avatar, badge, banner, button, card, ..., empty-state, navicon, progressbar, statusdot, ...) match ✅ unchanged

navicon, statusdot, and progressbar genuinely render one-word classes — left as-is.

Fix

CLI-only; core's generator already emits exactly what the (correct) key says, so it needs no change.

  • KNOWN_COMPONENTS: renamed the 15 keys to the rendered class token, removed layer, and documented the contract (key ⇔ stableClassName token) on the table. Legacy un-hyphenated keys now warn loudly at build time — ⚠ Unknown component "textinput". Did you mean: text, text-input? — instead of silently shipping dead CSS, which is the "fail loudly" behavior TextInput theme overrides emit dead selector .astryx-textinput (component renders .astryx-text-input) #4109 asks for.
  • Visual-prop lists for the renamed entries: prop validation never ran for these keys before (unknown component short-circuits it), so their prop lists were unvalidated dead data. Once the keys are correct, the stale lists would emit new false warnings — e.g. stone/butter's 'text-input': {'status:error': ...} would warn Unknown prop "status" ... has no variant props, which is false (TextInput renders themeProps('text-input', {size, status})). Synced the 15 renamed entries to the axes each component actually passes to themeProps(), verified identical to each doc.mjs theming.targets[].visualProps: app-shell ['variant'] (was ['position']), aspect-ratio ['shape'], checkbox-input ['size'], date-input/number-input/text-input/time-input ['size','status'], form-layout ['direction'], mobile-nav ['side'], radio-list ['orientation','size'] (was missing size), side-nav/top-nav ['mode'], tab-list ['size'] (was ['type']). Untouched entries keep their existing lists.
  • loadKnownValues dir matching: the component→doc-dir lookup only stripped non-letters from the directory side, so hyphenated keys (text-input) never resolved their doc file and every value was treated as custom in variant-declaration generation. Now strips both sides.

Result on the first-party themes: building stone/butter previously produced false "Unknown component" warnings for text-input, date-input, number-input, time-input; after this PR those build warning-free, and the only remaining warnings are for genuinely-missing keys (field-status, multi-selector, progressbar-fill/-track, top-nav-item/-heading) — which is #4110's territory (see below).

Why there is no visual-regression risk

The old selectors (.astryx-textinput etc.) matched nothing in the DOM — no component has ever rendered those classes, so no consumer theme rule under them ever applied. A consumer theme using the old keys goes from "dead rule, silently ignored" to "dead rule, loud warning"; the emitted CSS for those keys is unchanged. A consumer using the correct hyphenated keys (like the workaround in #4109, or the first-party themes) gets identical CSS output minus the spurious warnings. Nothing that renders today renders differently after this PR.

Test plan

New suite packages/cli/src/commands/build-theme.component-selectors.test.mjs (mirrors the build-theme.variants.test.mjs conventions, core built via ensureCoreBuilt):

  1. Pins TextInput theme overrides emit dead selector .astryx-textinput (component renders .astryx-text-input) #4109 end-to-end: a theme with text-input / tab-list / top-nav / date-input overrides builds warning-free and emits .astryx-text-input (+ the other hyphenated selectors), never the dead un-hyphenated forms.
  2. Pins the loud failure: the legacy textinput key still builds (warn-don't-block, matching existing validator behavior) but warns Unknown component "textinput" with text-input in the suggestions.
  3. Selector-contract invariant: every KNOWN_COMPONENTS key (now exported) must be a class token found by scanning packages/core/src for themeProps(...)/buildClassName(...)/stableClassName(...) literals — derived from source, not a hardcoded list, so the table can't drift from the DOM again (test files excluded from the scan so a stray literal can't mask a mismatch).
  • pnpm exec vitest run packages/cli/src/commands → 33 files, 801 tests passed
  • pnpm exec vitest run packages/cli → 117 files, 2026 tests passed
  • node scripts/check-changesets.mjs → valid (pre-1.0 patch-only)

Fixes #4109

Related

Reviewer questions

  1. layer removal: nothing in core renders an astryx-layer class (Layer is a hook/primitive; only the --astryx-layer-* anchor var exists) and it has no derived-var entries, so components.layer could never do anything. Removed rather than kept as a whitelisted dead key — happy to reinstate if Layer theming is planned.
  2. Warn vs. error: unknown components still warn-and-continue (existing validator behavior). If you'd rather hard-fail the build on unknown keys now that the table is trustworthy, I can tighten it here or in a follow-up.
  3. Some unchanged entries also have stale prop lists (textarea, selector, typeahead, tokenizer all render a status axis the table doesn't know about, producing pre-existing false "Unknown prop" warnings on the first-party themes). Left alone to keep this PR scoped to the renamed entries — can file/fix separately if wanted.

@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 22, 2026 3:17pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 22, 2026
@github-actions github-actions Bot added community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge labels Jul 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Analysis Report

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.7KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

@jiunshinn
jiunshinn marked this pull request as ready for review July 22, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TextInput theme overrides emit dead selector .astryx-textinput (component renders .astryx-text-input)

1 participant