fix(theme): preserve deprecated target compatibility - #6126
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
9bf1e8e to
7d3f245
Compare
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsBanner (@astryxdesign/core) · View in Storybook
CodeBlock (@astryxdesign/core) · View in Storybook
DateInput (@astryxdesign/core) · View in Storybook
DateRangeInput (@astryxdesign/core) · View in Storybook
DateTimeInput (@astryxdesign/core) · View in Storybook
Field (@astryxdesign/core) · View in Storybook
FormLayout (@astryxdesign/core) · View in Storybook
HoverCard (@astryxdesign/core) · View in Storybook
Indicator (@astryxdesign/core) · View in Storybook
MultiSelector (@astryxdesign/core) · View in Storybook
NavIcon (@astryxdesign/core) · View in Storybook
Popover (@astryxdesign/core) · View in Storybook
ProgressBar (@astryxdesign/core) · View in Storybook
Selector (@astryxdesign/core) · View in Storybook
StatusDot (@astryxdesign/core) · View in Storybook
Table (@astryxdesign/core) · View in Storybook
TextArea (@astryxdesign/core) · View in Storybook
TextInput (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: 2 accessibility violation(s) found — 1 critical, 1 serious. DateRangeInput - 1 issue(s)
ProgressBar - 1 issue(s)
Visual RegressionStatus: Skipped — Broad stable scope is deferred to the daily release gate. It covers 382 trusted baseline shots instead of recapturing them for this PR. View the report Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
7d3f245 to
a4d41a7
Compare
a4d41a7 to
b3bd763
Compare
0978503 to
98a4ed1
Compare
b3bd763 to
942fe53
Compare
942fe53 to
fc448aa
Compare
fc448aa to
0221c55
Compare
0221c55 to
2dcd0d2
Compare
3e1033c to
f34f299
Compare
f34f299 to
67392df
Compare
cixzhang
left a comment
There was a problem hiding this comment.
Semantic verdict: approve
Theme builders now get one canonical target vocabulary without losing the generic compatibility mechanisms needed for future staged aliases.
- All 21 retired keys have exact replacements shared by discovery, the codemod, diagnostics, and documentation; collisions preserve both declarations with an actionable TODO.
- Maintained published themes and bundled templates use canonical keys only, and build validation covers root, light/dark, and ordered adaptation rules.
legacyNames,ThemePropsOptions,deprecatedFor, andInputClearButton.iconClassNameremain available. The docsite labels staged aliases but excludes them from copyable examples.
Independent exact-head semantic review found no blocking issue. Local full repository and docsite builds, typechecks, generated-data checks, and focused mapping/runtime tests pass. Exact-head CI remains the merge gate.
[Reviewed by Robohands]
67392df to
1cd0bde
Compare
1cd0bde to
db6b681
Compare
cixzhang
left a comment
There was a problem hiding this comment.
Semantic verdict: approve
Theme builders now get one canonical target vocabulary without losing the generic compatibility mechanisms needed for future staged aliases.
- All 21 retired keys have exact replacements shared by discovery, the codemod, diagnostics, and documentation; collisions preserve both declarations with an actionable TODO.
- Maintained published themes and bundled templates use canonical keys only, and build validation covers root, light/dark, and ordered adaptation rules.
legacyNames,ThemePropsOptions,deprecatedFor, andInputClearButton.iconClassNameremain available. The docsite labels staged aliases but excludes them from copyable examples.
Independent exact-head semantic review found no blocking issue. Local full repository and docsite builds, typechecks, generated-data checks, and focused mapping/runtime tests pass. Exact-head CI remains the merge gate.
[Reviewed by Robohands]
db6b681 to
f07b59d
Compare
cixzhang
left a comment
There was a problem hiding this comment.
Semantic verdict: approve
Theme builders now get one canonical target vocabulary without losing the generic compatibility mechanisms needed for future staged aliases.
- All 21 retired keys have exact replacements shared by discovery, the codemod, diagnostics, and documentation; collisions preserve both declarations with an actionable TODO.
- Maintained published themes and bundled templates use canonical keys only, and build validation covers root, light/dark, and ordered adaptation rules.
legacyNames,ThemePropsOptions,deprecatedFor, andInputClearButton.iconClassNameremain available. The docsite labels staged aliases but excludes them from copyable examples.
Independent exact-head semantic review found no blocking issue. Local full repository and docsite builds, typechecks, generated-data checks, and focused mapping/runtime tests pass. Exact-head CI remains the merge gate.
[Reviewed by Robohands]
f07b59d to
cf69b13
Compare
cixzhang
left a comment
There was a problem hiding this comment.
Semantic verdict: request changes
themeTargets() exposes inconsistent object shapes: parent/child canonicalization creates an own deprecatedFor: undefined property on five current targets, while JSON serialization removes it. Programmatic and JSON consumers therefore receive different projections of the stable schema.
Please assign deprecatedFor only when the child supplies a string, then add a direct-API regression asserting current rows do not own the field while deprecated rows carry their exact replacement. The compatibility-preserving aliases, Popover guidance, canonical first-party examples, changeset, RTL/a11y applicability, and exact-head CI otherwise look good.
[Reviewed by Robohands]
cf69b13 to
050a571
Compare
|
AI review status for this pull request.
|
# Conflicts: # apps/storybook/rtl-audit/verified-not-applicable.json
# Conflicts: # docs/architecture/component-theming-surface.md
There was a problem hiding this comment.
Theme builders using released bare prop or state selectors such as .primary, .sm, and .checked remain broken at this head: themeProps() still omits those classes, and its test requires that omission. Those selectors shipped in 0.5.4, while the current compatibility contract keeps them supported until 0.7.0.
Please restore same-element emission of the released bare value/state classes, including numeric level-* values, and add regression coverage from the 0.5.4 output. The 21 target-name aliases, exact replacement discovery, and build warnings otherwise check out.
|
/approve-spec 8ffba7b |
Why
Theme consumers may still use deprecated component target keys. Removing those aliases would silently break working themes, so this change keeps compatibility while steering new theme authoring to canonical keys.
What
Risk
Non-breaking. Existing deprecated target keys continue to work; canonical keys remain the preferred path for new themes.
Testing
pnpm check:knowledge