Skip to content

Fix exceptions on empty tuple errors#17311

Merged
weswigham merged 3 commits into
microsoft:masterfrom
weswigham:fix-indexed-exception
Jul 20, 2017
Merged

Fix exceptions on empty tuple errors#17311
weswigham merged 3 commits into
microsoft:masterfrom
weswigham:fix-indexed-exception

Conversation

@weswigham
Copy link
Copy Markdown
Member

Fixes #17266
Fixes #17303

Our type to string function expected the node builder to always yield a type when errors were ignored, when for empty tuples it still wasn't returning nodes for an empty tuple type. Now it does.

@weswigham weswigham mentioned this pull request Jul 19, 2017
@weswigham
Copy link
Copy Markdown
Member Author

I don't know how to restart the .NET CI builds like I can travis; but the build is functional again.

Copy link
Copy Markdown
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Approved with one not to fix.

Comment thread src/compiler/checker.ts Outdated
@@ -2623,8 +2623,9 @@ namespace ts {
}
if (!context.encounteredError && !(context.flags & NodeBuilderFlags.AllowEmptyTuple)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Invert the predicate here to make it more readable and put return undefined at the end.

@sandersn
Copy link
Copy Markdown
Member

One NIT to fix.

Also, I think that Jenkins might respond to commands in comments, if you know who to @ and you have permissions and you know the command.
@RyanCavanaugh probably knows the answer to these but it's probably not worth it.

@pmunin
Copy link
Copy Markdown

pmunin commented Aug 4, 2017

Thanks for the fix! I see it was merged to master, but I still experience this on latest version of Typescript (2.4.2) that I installed via npm. When can we expect it to be updated in NPM?

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