Preserve literal types in contextual unions#19966
Preserve literal types in contextual unions#19966weswigham merged 14 commits intomicrosoft:masterfrom
Conversation
b08779b to
491e464
Compare
491e464 to
9b2a760
Compare
|
@ahejlsberg I've updated this for post-dynamic-names merge; would you like to take a look over it again so we can merge and fix #19837 and #16457? |
| return shouldUseLiteralType(declaration) ? type : getWidenedLiteralType(type); | ||
| } | ||
|
|
||
| function shouldUseLiteralType(declaration: VariableLikeDeclaration) { |
There was a problem hiding this comment.
shouldKeepLiteralType may be a better name since it only matters if you already have a literal type.
| type = declaration.initializer ? | ||
| getUnionType([type, checkExpressionCached(declaration.initializer)], /*subtypeReduction*/ true) : | ||
| type; | ||
| return shouldUseLiteralType(declaration) ? type : getWidenedLiteralType(type); |
There was a problem hiding this comment.
What are the cases that are affected by this change?
There was a problem hiding this comment.
You can see the changes at https://github.com/weswigham/TypeScript/pull/1/files
| } | ||
|
|
||
| function isLiteralLikeContextualType(contextualType: Type) { | ||
| function isLiteralLikeContextualType(candidateLiteral: Type, contextualType: Type): boolean { |
| return !!(((contextualType.flags & TypeFlags.StringLike) && maybeTypeOfKind(candidateLiteral, TypeFlags.StringLike)) || | ||
| ((contextualType.flags & TypeFlags.NumberLike) && maybeTypeOfKind(candidateLiteral, TypeFlags.NumberLike)) || | ||
| ((contextualType.flags & TypeFlags.BooleanLike) && maybeTypeOfKind(candidateLiteral, TypeFlags.BooleanLike)) || | ||
| ((contextualType.flags & TypeFlags.ESSymbolLike) && maybeTypeOfKind(candidateLiteral, TypeFlags.ESSymbolLike)) |
There was a problem hiding this comment.
Why all the extra parentheses?
There was a problem hiding this comment.
Old habit; I tend to add parens around any inner binary expression since internalizing operator precedence beyond simple PEMDAS isn't something I, personally, bother to think about. They're gone now~
|
@ahejlsberg Ran DT tests - Fixes 7 instances of #19837, and changes the error text of 4 tests to include or not include literals as appropriate for the new rules, same as RWC (introducing no new errors). |
|
@weswigham we had an offline discussion today as a result of #20279. the main cause of the memory load is generating unions of literal types and doing sub-type reduction on them. the inclusion of this change exasperates the issue further. @ahejlsberg has a change to limit the inference of literal types to only when the contextual type has a literal type from the same family, and that seems to alleviate the issue for these large json-like arrays/objects with a contextual type. but obviously this is going the opposite direction of this PR. I now believe your original proposal for including the literal type in the contextual type is a better approach, and we should go with that instead. @ahejlsberg seems to have changed his mind on that issue as well after looking into #20279. |
108a851 to
a39515f
Compare
Done.
I couldn't find a PR for that, so I built it into this one, since it was a smallish change in the same place as I've been working in here. I'll add a test for #20279 hopefully shortly. |
|
I've added a test for #20279 and confirmed that with the proposed tweaks, this fixes it. |
e61f47d to
300816f
Compare
|
@ahejlsberg This has now been merged with master and is now just the change for preserving literal types in unions generated via contextual typing (the original proposal the fix the original issues). |
|
@ahejlsberg |
Fixes #16457, replaces #19587, in which this code started getting reviewed.
Adds a test for #20279.
Also fixes #19837.
Also fixes #16457.