-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix boolean literal types #10577
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
Merged
Merged
Fix boolean literal types #10577
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
} | ||
function h({ prop = "baz" }: StringUnion) {} | ||
>h : ({prop}: StringUnion) => void | ||
>prop : "foo" | "bar" | "baz" |
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.
The initializer is completely ignoring the type annotation in this case. I would say this is undesirable.
@mhegazy Want to take a look? I think we need this one in 2.0. |
👍 |
merged in release-2.0 in 2cd4d20 |
6 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #10432.
This PR introduces two changes:
boolean
we now consider it a literal type context. Previously we specifically excluded this case.Some background on the changes.
Since
boolean
is identical to the union typetrue | false
, when an expression has the contextual typeboolean
we really should consider it a literal type context (because the contextual type is a union of unit types). Before this PR we specifically excluded that case in the name of backwards compatibility. With the fix we no longer do. However, that causes another issue to appear:In the example above, the expression
cond ? false : undefined
is contextually typed by typeboolean
because of the default value on the left hand side. That causes us to consider the expression a literal type context, and we therefore use typefalse
for thefalse
literal. So, the type ofx
on the right hand side becomesfalse | undefined
. Now, when we destructurex
we require the default value to be of a type assignable tofalse | undefined
. Butboolean
isn't.The issue really is that when a binding element has a default value, we ought to give it a union type of the destructuring type and the default type. With this PR we do. The downside is that you might sometimes end up with a union type when an error might have been more appropriate.
@mhegazy @DanielRosenwasser You might want to take a look.