-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
Typings: Improve $slots and $scopedSlots type to prevent unchecked access to undefined #8946
Conversation
Thank you for making this PR. However including In the TypeScript, index signature is not guarantees whether the referenced key is exists. This is similar that an array type can be return const obj: Record<string, number> = { foo: 1, bar: 2 }
obj.baz // -> undefined
const arr: number[] = [1, 2, 3]
arr[5] // -> undefined Some case that Including const obj: Record<string, number | undefined> = { foo: 1, bar: undefined }
const a: Record<string, number | undefined> = { a: 1 }
// will be `(number | undefined)[]` even though `undefined` will be never appeared
const b = Object.values(a) |
Hey, thank you for the quick reaction.
Actually, I believe that it effectively does guarantee the following: It tells the TypeScript compiler that every possible key is permitted and (thats the important part) that all the values of the given object are guaranteed to have actual content of the type Let me give you an example to hopefully better illustrate what is happening with the current type definition: Let's first look at the current type definition for interface Vue {
readonly $scopedSlots: { [key: string]: ScopedSlot };
} Now, in a vue component we try to call the default slot function: this.$scopedSlots.default( props ); TypeScript won't complain about this code. But at runtime, Now if we tweak the type definition like that: interface Vue {
readonly $scopedSlots: { [key: string]: ScopedSlot | undefined };
} This is what happens: It says <div>
{this.$scopedSlots.default ?
this.$scopedSlots.default( props ) :
'Fallback content'
}
</div> So, to sum it up: Right now the TypeScript compiler won't complain about slot access because of the current type defintition for $scopedSlots. Because the content that is referenced by the key is defined to always be a ScopedSlot, TypeScript will happily allow a function call even if no slot function is present. It does not account for the fact that at runtime a slot is not necessarily defined. I hope this explanation better illustrates why I believe this PR is an improvement of the current typing. |
types/vue.d.ts
Outdated
@@ -28,7 +28,7 @@ export interface Vue { | |||
readonly $children: Vue[]; | |||
readonly $refs: { [key: string]: Vue | Element | Vue[] | Element[] }; | |||
readonly $slots: { [key: string]: VNode[] }; |
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.
Non-scoped slots can be undefined too.
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 for pointing that out! Will add that to this PR..
I see your point. As scoped slot and slot optional, it may be better to have I was thinking scoped slot and slot always there if the object key is exists but it was incorrect as they can be blank. Not sure how actual implementation is but it sounds good to have |
declare $slots option as potentially undefined to enable stricter TS checks
You missed the one in |
@KaelWD thanks |
…cess to undefined (vuejs#8946) * fix(types): Declare $scopedSlots as potentially undefined to enable stricter TS checks * fix(types): Fix tests * fix(types): declare $slots option as potentially undefined declare $slots option as potentially undefined to enable stricter TS checks
…cess to undefined (vuejs#8946) * fix(types): Declare $scopedSlots as potentially undefined to enable stricter TS checks * fix(types): Fix tests * fix(types): declare $slots option as potentially undefined declare $slots option as potentially undefined to enable stricter TS checks
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
When working directly with render functions in TypeScript / tsx, currently the following code is unsafe and unchecked by the ts compiler:
The change in this PR makes TypeScript complain about the unsafe access accordingly and the code has to be expressed in a way that prevents calling an undefined function, e.g.: