-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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(runtime-core): return boolean type when use defineProps
to set optional boolean prop
#7116
Conversation
…optional boolean prop
❌ Deploy Preview for vuejs-coverage failed.
|
@@ -13,16 +13,20 @@ describe('defineProps w/ type declaration', () => { | |||
// type declaration | |||
const props = defineProps<{ | |||
foo: string | |||
bool?: boolean |
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.
Consider this case
boolAndUndefined: boolean | undefined
{ | ||
[K in keyof TypeProps as TypeProps[K] extends boolean | undefined | ||
? K | ||
: never]-?: TypeProps[K] |
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.
: never]-?: TypeProps[K] | |
: never]-?: NotUndefined<TypeProps[K]> |
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.
Without NotUndefined
, the '-?' will still remove 'undefined'.
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.
Actually, it won't when there's only boolean | undefined
and no ?:
interface T {
boolAndUndefined: boolean | undefined
}
type Test = {
[K in keyof T]-?: T[K]
}
// type Test = {
// boolAndUndefined: boolean | undefined;
// }
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, modified.
}>() | ||
// explicitly declared type should be refined | ||
expectType<string>(props.foo) | ||
// the Boolean absent props will be cast to false | ||
expectType<boolean>(props.bool) |
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.
This implementation will lead it cannot jump to the declaration by clicking bool
of props.bool
(props.foo
also).
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.
I'll create a new PR with another implementation.
@godxiaoji Thank you very much for the PR - @sxzz came up with a better alternative in #7619 that retains the ability to jump to definition on props properties, but we really appreciate your contribution! |
Is my pleasure. @yyx990803 I have a few pr, free also trouble to review #7133 #6827 #6773 #6765. |
… type of defineProps (vuejs#7619) close vuejs#7116 fix vuejs#5847 fix vuejs#7487
… type of defineProps (vuejs#7619) close vuejs#7116 fix vuejs#5847 fix vuejs#7487
fix #5847 fix #7487
The results are as follows: