Consistently reduce union types#2729
Conversation
Conflicts: src/compiler/types.ts tests/baselines/reference/APISample_compile.js tests/baselines/reference/APISample_compile.types tests/baselines/reference/APISample_linter.js tests/baselines/reference/APISample_linter.types tests/baselines/reference/APISample_transform.js tests/baselines/reference/APISample_transform.types tests/baselines/reference/APISample_watcher.js tests/baselines/reference/APISample_watcher.types
There was a problem hiding this comment.
Split each branch onto its own line:
return type.flags & TypeFlags.Union ?
getPropertiesOfUnionType(<UnionType>type) :
getPropertiesOfObjectType(type);There was a problem hiding this comment.
or, preferably:
return type.flags & TypeFlags.Union
? getPropertiesOfUnionType(<UnionType>type)
: getPropertiesOfObjectType(type);There was a problem hiding this comment.
Fits just fine on one line, I'd prefer to keep it that way.
There was a problem hiding this comment.
The problem with long single lines is github:
It's actually harder to read because github is optimized for 80 columns.
|
Does this fix: #1953 as well? If not, is there something we can do while we're addressing consistent reduction to also address the nondeterminism issue as well? |
|
No, this isn't related to #1953. We need a different solution for that. |
There was a problem hiding this comment.
Is it at all significant that this case moved to the end of the function? To my reading, it is not.
There was a problem hiding this comment.
No, it is not significant, but I like handling the union case after the object case (as it is less common).
|
Do we also need to force subtype reduction in applyToContextualType? |
|
👍 |
|
@JsonFreeman Regarding subtype reduction in applyToContextualType... No, I don't think we want to do that. When a contextual type is a union type, keeping the all constituents of the union gives us more information. For example, say you have a contextual type A | B, where B is a subtype of A, for some object literal. If B declares some property foo, then keeping B around allows us to contextually type a foo property in the object literal. |
|
Your argument about applyToContextualType makes sense. However, we cannot take advantage of that capability consistently, because the fact is that normally we do collapse the union types when we create them. So in many cases, by the time we get to applyToContextualType, it is already too late and we've lost some of the properties. |
|
@JsonFreeman I think we just need to modify the spec to document exactly when subtype reduction occurs. Or, conversely, when it doesn't occur. The latter may be easier as the only time we defer subtype reduction is in an explicitly written type. |
|
And instantiation, but I guess that is a process that's not explained in the spec. Yes, I think it makes sense to spec subtype reduction, just like how we spec "depends on" for type aliases. |
|
👍 |
Consistently reduce union types
Union types are now consistently reduced to the smallest possible set before members (properties or signatures) are accessed.
Fixes #2610.