-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix(types): handle union types in generic parameter #2794
Conversation
✅ Deploy Preview for pinia-playground ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for pinia-official canceled.
|
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2 #2794 +/- ##
==========================================
+ Coverage 89.66% 90.11% +0.44%
==========================================
Files 14 14
Lines 1335 1335
Branches 219 221 +2
==========================================
+ Hits 1197 1203 +6
+ Misses 137 131 -6
Partials 1 1 ☔ View full report in Codecov by Sentry. |
export type StoreToRefs<SS extends StoreGeneric> = _ToStateRefs<SS> & | ||
ToRefs<PiniaCustomStateProperties<StoreState<SS>>> & | ||
_ToComputedRefs<StoreGetters<SS>> | ||
export type StoreToRefs<SS extends StoreGeneric> = SS extends unknown |
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.
Why does this fix the problem? Isn't x extends unknown
always true? I want to add a note comment for my future self and other contributors
TS is sometimes so weird 🫠
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.
When conditional types act on a generic type, they become distributive when given a union type (Distributive Conditional Types).
In this case, it will always resolve to the first branch because unknown
is the top type in TypeScript.
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.
Thanks! This makes me wonder if there isn't another bug up the chain of types. But this is already good enough!
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.
Thanks a lot!
export type StoreToRefs<SS extends StoreGeneric> = _ToStateRefs<SS> & | ||
ToRefs<PiniaCustomStateProperties<StoreState<SS>>> & | ||
_ToComputedRefs<StoreGetters<SS>> | ||
export type StoreToRefs<SS extends StoreGeneric> = SS extends unknown |
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.
Thanks! This makes me wonder if there isn't another bug up the chain of types. But this is already good enough!
close #2785