Skip to content
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

Fix #25172: Add narrowing for unknown with triple equals #26941

Merged

Conversation

jack-williams
Copy link
Collaborator

@jack-williams jack-williams commented Sep 6, 2018

Fixes #25172.

Narrows an identifier of type unknown when strict compared (===) with a value of primitive, non-primitive, or object type.

  • Comparing with a primitive/non-primitive type narrows to the type of the operand, retaining any literal types.
  • Comparing with an object type narrows to the default non-primitive type object.

Limitations.

  • Does not work with == because we would need to consider coercion rules.
  • Does not work with intersection or union:
    • Intersection: Branding tricks like number & "age" make it hard to tell the actual runtime type. We could inherit the intersection type but this might violate branding expectations.
    • Union: To narrow from a union would require traversing the union and collecting the set possible target types. The performance impacts of this aren't clear, and I'm not sure how often this happens anyway.

@RyanCavanaugh RyanCavanaugh added this to the Community milestone Sep 17, 2018
@jack-williams jack-williams force-pushed the narrow-unknown-with-triple-equals branch from c2ab8f6 to a8cc939 Compare September 18, 2018 22:55
@jack-williams
Copy link
Collaborator Author

Shall I revive this and remerge master?

@jack-williams
Copy link
Collaborator Author

cc @RyanCavanaugh

Not many people seem to have run into this recently, but if you're still interested in adding the feature just shout and I can check it works with master.

@RyanCavanaugh
Copy link
Member

@jack-williams please revive, I'm back from leave now and am trying to get the PR backlog moving again. A fresh rebase with master would be great

@RyanCavanaugh RyanCavanaugh self-assigned this Jan 25, 2019
@jack-williams jack-williams force-pushed the narrow-unknown-with-triple-equals branch from a8cc939 to 6f30537 Compare January 25, 2019 09:52
@jack-williams
Copy link
Collaborator Author

@RyanCavanaugh done!

Copy link
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

LGTM - great testcase btw. Will merge on Tuesday after the release branch finalizes

@jack-williams
Copy link
Collaborator Author

Great, and thank you!

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 25, 2019

Technically a breaking change, better bump the major version of TS 🙄

declare const u: unknown;
if (u === 0) {
    let m = u;
    // Now an error
    m = "";
}

(don't actually change anything)

@RyanCavanaugh RyanCavanaugh merged commit 0df89cc into microsoft:master Jan 30, 2019
@jack-williams jack-williams deleted the narrow-unknown-with-triple-equals branch January 30, 2019 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants