Skip to content

Filter undefined only in binding patterns in parameters w/initialiser #38116

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

Merged
merged 1 commit into from
Apr 22, 2020

Conversation

sandersn
Copy link
Member

#37309 removes undefined from the type of any binding pattern whose parent has an initialiser. But this is only correct when the initialiser is for a parameter. Here's a counter-example:

declare let x: { s: string } | undefined;
const { s } = x;

This PR removes undefined from the type of a binding pattern only when
the binding pattern's parent is a parameter. This fixes the regression
from 3.8. However, it's still not the ideal fix; we should be able to
use control flow to solve this problem. Consider:

const { s }: { s: string } | undefined = { s: 'hi' }
declare function f({ s }: { s: string } | undefined = { s: 'hi' }): void

Neither line should have an error, but the first does in 3.8 and after
this change.

Fixes #38110

initialiser. But this is only correct when the initialiser is for a
parameter. For example:

```ts
declare let x: { s: string } | undefined;
const { s } = x;
```

This PR removes undefined from the type of a binding pattern only when
the binding pattern's parent is a parameter. This fixes the regression
from 3.8. However, it's still not the ideal fix; we should be able to
use control flow to solve this problem. Consider:

```ts
const { s }: { s: string } | undefined = { s: 'hi' }
declare function f({ s }: { s: string } | undefined = { s: 'hi' }): void
```

Neither line should have an error, but the first does in 3.8 and after
this change.
@sandersn sandersn merged commit f248567 into master Apr 22, 2020
@sandersn sandersn deleted the filter-undefined-only-from-binding-elts-in-params branch April 22, 2020 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible regression in 3.9-beta when destructuring from a union including undefined
4 participants