Skip to content

Optional destructured parameters do not work with default values #17080

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

Closed
jqyu opened this issue Jul 10, 2017 · 4 comments
Closed

Optional destructured parameters do not work with default values #17080

jqyu opened this issue Jul 10, 2017 · 4 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@jqyu
Copy link

jqyu commented Jul 10, 2017

TypeScript Version: 2.4.0

Code

The following code incorrectly type checks:

interface Attrs {
    foo?: number;
    bar?: number;
}

function unsafe({ foo, bar }: Attrs = {}): number {
    return foo * bar.toString().length + 1;
}

Expected behavior:
This should fail to type check since foo and bar may both be of type undefined

Actual behavior:

Match behaviour of omitting default value:

function failsToTypecheck({ foo, bar }: Attrs): number {
    return foo * bar.toString().length + 1;
}

Or behaviour of omitting destructuring:

function failsToTypecheck(attrs: Attrs = {}): number {
    return attrs.foo * attrs.bar.toString().length + 1;
}
@mhegazy mhegazy added the Needs Investigation This issue needs a team member to investigate its status. label Aug 28, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Aug 28, 2017

@sandersn we talked about this one before, any thoughts here?

@whatisaphone
Copy link

Here's another example of this that I wrote up before searching for an existing issue:

interface Foo {
  readonly bar?: number;
}

function performFoo({ bar }: Foo = {}) {
  useBar(bar);  // `bar` should have type `number | undefined`, so this shouldn't compile.
}

function useBar(bar: number) {
  alert('The number is: ' + bar);
}

performFoo();

We've started using this pattern in our code, so I'm also interested in a fix for this.

@whatisaphone
Copy link

If someone takes up fixing this, here is another potentially useful test case:

interface Foo {
  readonly bar?: number;
}

function performFoo({ bar = null }: Foo = {}) {
  useBar(bar);  // `bar` should have type `number | null`, so this call shouldn't compile.
}

function useBar(bar: number | undefined) {
  alert('The number is: ' + bar);
}

performFoo();

@mhegazy mhegazy added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Oct 10, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Oct 10, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.7, TypeScript 2.8 Jan 9, 2018
@Lazarus535
Copy link
Contributor

Lazarus535 commented Jan 22, 2018

I fixed the issue in PR #21328
Update: Added testcases, signed CLA
Update2: Issue #17667 is not (totally) fixed by this. Only the part with the optionality (inclusion of undefined).

Lazarus535 added a commit to Lazarus535/TypeScript that referenced this issue Jan 22, 2018
microsoft#17080
Added testcases from the Github bugreport (all working as intended now).
Signed CLA.
Lazarus535 added a commit to Lazarus535/TypeScript that referenced this issue Jan 22, 2018
Fixed the two requested changes.
1) Deleting the file "pull_request_template.md"
2) Declaring functions in tests, instead of defining
Lazarus535 added a commit to Lazarus535/TypeScript that referenced this issue Jan 22, 2018
Readded untouched pull_request_template.md
sandersn added a commit that referenced this issue Jan 22, 2018
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jan 24, 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

5 participants