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

Destructuring assignment makes any types when it skips some elements (regression) #20872

Closed
falsandtru opened this issue Dec 23, 2017 · 2 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@falsandtru
Copy link
Contributor

cc @andy-ms

TypeScript Version: 2.7.0-dev.20171223

Code

const [, a = ''] = ''.match('') || [];

Expected behavior:

a is string.

Actual behavior:

a is any.

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Dec 23, 2017
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.7 milestone Dec 23, 2017
@weswigham
Copy link
Member

weswigham commented Dec 29, 2017

@ahejlsberg The bisect indicates that 25af351 introduced this regression as part of #19671. I think the change is unintentional, in this instance? Before, when we saw RegExpMatchArray on the LHS of || and [any, ""] on the RHS, we did a best-choice thing that eliminated the tuple type from the union (neither is a subtype of the other thanks to tuples and any, I think, but the tuple is assignable to the regexp match array, so was eliminated before). Now that it is preserved, when we go to get the index type for the union, we see a tuple with an any member (created from the shape of the destructuring pattern) and happily state that the numeric index type must thusly be any. I think a decent fix that doesn't revert our behavior back to the best-choice mechanic is to stop using unknownType for positions indicated by OmittedExpressions when building the tuple type in the first place in checkArrayLiteral (since those broaden the index type of the tuple type created to any), and instead use emptyObjectType.

@falsandtru
Copy link
Contributor Author

Also see #20495

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jan 8, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

4 participants