dogfooding fixes: theme build custom-variant type augmentations#3469
Open
josephfarina wants to merge 1 commit into
Open
dogfooding fixes: theme build custom-variant type augmentations#3469josephfarina wants to merge 1 commit into
josephfarina wants to merge 1 commit into
Conversation
Generate the augmentation against the component's real interface (e.g. ButtonVariantMap) instead of a non-existent XDS-prefixed name, so custom variants declared in a theme type-check. Only emit an augmentation when the target interface actually exists in core: props backed by closed literal-union types (Button size, Heading type/level) have no augmentation point, so they are skipped instead of producing dead declarations. The main <name>.d.ts now references the generated <name>.variants.d.ts via a triple-slash directive so importing the theme loads the augmentation. Fixes #3371
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
astryx theme buildsupports declaring custom component variants in a theme (e.g.button['variant:accentOutline']) and generates a.variants.d.tswith a TypeScript module augmentation so the custom value type-checks. In practice the augmentation was dead code, for three reasons. Fixes #3371.The three bugs
1. Wrong interface name. The codegen emitted the augmentation against
XDS${Component}${Prop}Map:XDSButtonVariantMapdoesn't exist anywhere in@astryxdesign/core— the real, augmentable interface isButtonVariantMap(no prefix). So the augmentation created a new, unused interface and never widenedButtonVariant, andvariant="accentOutline"failed withTS2322.2. Props with no augmentation point still got augmented. Some themeable props are backed by closed literal-union types, not an augmentable
*Mapinterface — e.g. Buttonsize(keyof typeof sizeStyles) and Headingtype/level('display-1' | ...,1 | 2 | ...). The codegen assumed every prop had a${Component}${Prop}Mapinterface and emitteddeclare moduleblocks against interfaces that don't exist (ButtonSizeMap,HeadingTypeMap), which are doubly dead.3. The generated file was never referenced. Even a correct augmentation never loaded:
<name>.variants.d.tswas emitted but nothing referenced it, so importing the theme's types didn't pull in the augmentation.The fix
${Component}${Prop}Map(e.g.ButtonVariantMap,BannerStatusMap).@astryxdesign/core/<Component>actually exports a matching interface. This is derived by reading core's shippeddist/<Component>/index.d.ts(falling back tosrc/<Component>/index.tsin the monorepo), so props with no augmentation point are skipped instead of producing dead declarations. It also self-corrects if a component's interface name ever diverges from the convention.<name>.d.tsnow emits a/// <reference path="./<name>.variants.d.ts" />directive so importing the theme also loads the augmentation.Verification
Ran the real CLI against a theme declaring
button['variant:accentOutline'],button['size:jumbo'], andheading['type:hero']:.variants.d.tstargetsinterface ButtonVariantMapunderdeclare module '@astryxdesign/core/Button'— noXDS-prefixed interface.sizeor Headingtype(noButtonSizeMap/HeadingTypeMap, nodeclare module '@astryxdesign/core/Heading').<name>.d.tscontains the triple-slash reference to the variants file.<Button variant="accentOutline">and the built-in<Button variant="primary">both type-check while the module augmentation merges with core's original interface.Tests
New
build-theme.variants.test.mjs(4 cases, run against the real CLI binary like the existingbuild-theme.*.test.mjs):ButtonVariantMap;size, Headingtype) are skipped;.variants.d.tsis written when every custom value is non-augmentable;.d.tsreferences the variants file.Existing
build-themetests (prose, import-path, path-safety) still pass; ESLint clean.