Skip to content

Adds empty object narrowing to type guards #4051

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

Closed
wants to merge 5 commits into from

Conversation

tinganho
Copy link
Contributor

Fixes #4029.

Void narrowing only affects union types:

let x: A | B;
if (isA(x)) {
}
else if(isB(x)) {
}
else {
   x// is void
}

// if (isA(x)) { // isA(...) returns A and the narrowing type has the value of A so narrow to void type.
//
// }
// else if (isB(x)) { // narrow to A
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean "to B"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x is on the else clause. So it removes B from the union set.

if (!(type.flags & TypeFlags.Union)) {
// Get the original type again because we don't want to store it on a variable higher
// up to minimize memory allocation.
let originalType = getTypeOfSymbol(symbol);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just get originalType at the beginning with type itself.

@RyanCavanaugh
Copy link
Member

We'd prefer to use {} (emptyObjectType) instead of void when all constituents have been exhausted.

@tinganho
Copy link
Contributor Author

tinganho commented Aug 5, 2015

Ok just changed it to an empty object type. What's the reason behind {}?

@tinganho tinganho changed the title Adds void narrowing to type guards Adds empty object narrowing to type guards Aug 5, 2015
//
// let x: A | B | C;
//
// if (isB(x)) { // narrow x to A.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean A | C?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I meant A. It remove B from A(which is A).

@tinganho
Copy link
Contributor Author

tinganho commented Nov 6, 2015

Is there more to be done here?

@DanielRosenwasser
Copy link
Member

Looks like this was fixed by @sandersn in #5427.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants