-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Union subtype reduction #4537
Union subtype reduction #4537
Conversation
@JsonFreeman Thought this might interest you! |
// is considered known if the object type is empty and the check is for assignability, if the object type has | ||
// index signatures, or if the property is actually declared in the object type. In a union or intersection | ||
// type, a property is considered known if it is known in any constituent type. | ||
function isKnownProperty(type: Type, name: string): 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.
Perhaps isExpectedProperty
would better describe this.
Whoa, interesting! That's true about fresh object literals failing the subtype test for subtype reduction. I have two general questions:
|
👍 |
function removeDuplicateTypes(types: Type[]) { | ||
let i = types.length; | ||
function removeSubtypes(types: Type[]) { | ||
var i = types.length; |
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.
Use let
It's mostly a backwards compatibility thing. We've seen several examples of real world code that was broken by the new behavior. The breaks all had to do with call signatures disappearing because a union type didn't get reduced to a single supertype. Of course it doesn't hurt that this new (or perhaps I should say old) way of doing things is also simpler in that there isn't a new kind of type relationship we have to document and reason about.
Consider this example: var a = [{ x: 1 }, { x: 2, y: 2 }]; // ({ x: number } | { x: number, y: number })[] Because the elements have fresh object literal types they aren't subtypes of each other, so the element type of the array literal is a union type. As we widen the type of the array literal we make sure _not_ to perform subtype reduction again as that would collapse the union when the object literal types lose their freshness. So, the full type survives as the type of |
@ahejlsberg Got it. Thanks for the explanation! So the difference is that noSubtypeReduction is true for the call to getUnionType in getWidenedType? That would seem to be the trap where subtype reduction is dangerous. The scheme of aggregating the signatures of a union type is remaining as is, correct? I think it's still good for union types that didn't collapse under subtype reduction. |
This PR rolls back some of the changes related to union types introduced in #3823. Specifically, we now again use subtype reduction as the primary means of removing redundant constituent types in union types. This improves backwards compatibility without impacting the strict object literal assignment checking that was the primary purpose of #3823.
It turns out that subtype reduction isn't at odds with strict object literal assignment checks as long as we are a bit more selective about where we perform the reduction. Specifically, because fresh object literals are subtypes only of object types with an exact matching set of properties (the fresh object literals can have neither too many nor too few properties), we get pretty much the same behavior from subtype checks as we did from the new deduplication algorithm in #3823.