-
-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update VariantProp
type merging logic for Compose
interface
#276
base: main
Are you sure you want to change the base?
Conversation
@lunelson is attempting to deploy a commit to the cva Team on Vercel. A member of the Team first needs to authorize it. |
Thanks @lunelson! Bit stretched for time at the moment but I'll try to get around to testing this. Really appreciate the contribution 🙏 |
packages/cva/src/index.ts
Outdated
@@ -74,7 +82,7 @@ type CVAVariantShape = Record<string, Record<string, ClassValue>>; | |||
type CVAVariantSchema<V extends CVAVariantShape> = { | |||
[Variant in keyof V]?: StringToBoolean<keyof V[Variant]> | undefined; | |||
}; | |||
type CVAClassProp = | |||
export type CVAClassProp = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for adding the export
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @joe-bell thanks for the quick review, and sorry for the slow response, I was on the road after I submitted this, until today.
I must have added this when I was tinkering around, in order to use this type in a scratch test file; I'll remove it.
I also realized after the fact that I could possibly add some type-testing coverage, since you have vitest set up here; and that I hadn't written my commit message with the conventional format. Thoughts?
- update Compose interface to use new MergeVariantProp utility type - clean up some other types
506d2ad
to
b61bf93
Compare
Any plans to merge? Seems like a must have feature for typescript ppl |
Description
This PR primarily addresses the following issue, updating the variant prop merging logic that is applied by the
Compose
interface, so that the union of available variant prop names is correct.#256
There are also some secondary simplifications of types, and application of
unknown
in place ofany
.What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).