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

Narrow from 'any' in most situations #10334

Merged
merged 5 commits into from
Aug 15, 2016

Conversation

yortus
Copy link
Contributor

@yortus yortus commented Aug 15, 2016

Supersedes #10319 which has weird CI build problems.

This PR implements narrowing from any as described in #9999 (comment).

NB: this is a breaking change.

Fixes #9999, #10000, #8677

Checklist:

  • Narrowing to primitives continues to work as it does today
  • Narrowing via instanceof or a user-defined type predicate will narrow unless the narrowed-to type is exactly Function or exactly Object (in which case the expression remains of type any)
  • No commandline switch - this is the behavior in all cases
  • Successfully run jake runtests locally
  • There are new or updated unit tests validating the change

yortus added 5 commits August 14, 2016 19:42
instanceof and user-defined typeguards narrow from 'any' unless the narrowed-to type is exactly 'Object' or 'Function'. This is a breaking change.
@msftclas
Copy link

Hi @yortus, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

@yortus
Copy link
Contributor Author

yortus commented Aug 15, 2016

Existential crisis averted. Clearly it's because this PR title starts with an uppercase 'N'.

@RyanCavanaugh
Copy link
Member

👍 and an extra 🍺 for updating the comments in the testcases


else if (err instanceof Error) {
err.message;
err.massage; // ERROR: Property 'massage' does not exist on type 'Error'
Copy link
Member

Choose a reason for hiding this comment

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

If only errors came with massages.

@DanielRosenwasser DanielRosenwasser merged commit 78cea2a into microsoft:master Aug 15, 2016
@DanielRosenwasser
Copy link
Member

Thanks for the change along with those test cases!

@yortus
Copy link
Contributor Author

yortus commented Aug 15, 2016

Thanks for the guidance.

@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Aug 17, 2016
@mhegazy mhegazy added this to the TypeScript 2.0.1 milestone Aug 17, 2016
@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
Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants