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

Inconsistencies in type checking functions with optional destructured parameters #17667

Closed
trevorade opened this issue Aug 7, 2017 · 8 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@trevorade
Copy link

trevorade commented Aug 7, 2017

I encountered unexpected and inconsistent errors when attempting to write a function declaration for a function with an optional destructured parameter with default property values when using strictNullChecks.

It's possible that I wrote my function declaration incorrect but I couldn't find a good example for writing function declarations with an optional destructured parameter with default property values. Regardless, the inconsistency in behavior described in this issue is still a bug AFAICT.

TypeScript Version: 2.4.0 / nightly (2.5.0-dev.201xxxxx)

Code
Bug demo (enable strictNullChecks to see error):
https://www.typescriptlang.org/play/#src=const%20want%20%3D%20(%7Ba%20%3D%201%2C%20b%20%3D%202%7D%20%3D%20%7B%7D)%20%3D%3E%20%7B%7D%3B%0D%0Awant()%3B%0D%0Awant(%7B%7D)%3B%0D%0Awant(%7Ba%3A%203%7D)%3B%0D%0Awant(%7Ba%3A%203%2C%20b%3A%204%7D)%3B%0D%0Awant(%7Bb%3A%205%7D)%3B%0D%0A%0D%0Alet%20have%3A%20(nums%3F%3A%20%7Ba%3F%3Anumber%2C%20b%3F%3Anumber%7D)%20%3D%3E%20void%3B%0D%0Ahave%20%3D%20want%3B%0D%0Ahave%20%3D%20(%7Ba%20%3D%201%2C%20b%20%3D%202%7D%20%3D%20%7B%7D)%20%3D%3E%20%7B%7D%3B%20%2F%2F%20Same%20value%20as%20want.%0D%0Ahave()%3B%0D%0Ahave(%7B%7D)%3B%0D%0Ahave(%7Ba%3A%203%7D)%3B%0D%0Ahave(%7Ba%3A%203%2C%20b%3A%204%7D)%3B%0D%0Ahave(%7Bb%3A%205%7D)%3B%0D%0A

let have: (nums?: {a?:number, b?:number}) => void;
      have = ({a = 1, b = 2} = {}) => {};  // A. Results in error.
const want = ({a = 1, b = 2} = {}) => {};
have = want;                               // B. This works fine.

Expected behavior:
The lines denoted by A. and B. both work (preferable).
OR
The lines denoted by A. and B. fail in the same way.

Actual behavior:
The line denoted by A. results in errors:

Type '{ a?: number | undefined; b?: number | undefined; } | undefined' has no property 'a' and no string index signature.
Type '{ a?: number | undefined; b?: number | undefined; } | undefined' has no property 'b' and no string index signature.

The line denoted by B. works.

@DanielRosenwasser DanielRosenwasser added the Bug A bug in TypeScript label Aug 27, 2017
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.6 milestone Aug 27, 2017
@whatisaphone
Copy link

I wonder if #17080 is related to this?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 10, 2017

have = want; fails now with --strictFunctionTypes. seems like we are not adding | undefined for optional parameters correctly for want.
so there are two issues here, not one.

@trevorade
Copy link
Author

I mean, at the end of the day, what I want to be able to do is write something like this:

let adder: (nums?: {a?:number, b?:number}) => number;
if (useImplementation1) {
  adder = ({a = 1, b = 2} = {}) => a + b; 
} else {
  adder = someOtherFunctionOfThisType;
}

The have = want thing is a hack to work around the issue.

If my declaration for adder is off, I'm happy to change it, I just don't know how to write it such that the TypeScript compiler is happy.

@sandersn
Copy link
Member

I think what @mhegazy is saying is that you found two bugs:

A. A contextually-typed binding pattern doesn't check initialised properties to the type correctly. It should understand the structure-with-initialiser well enough to narrow out the undefineds from the contextual type.
B. A non-contextually-typed binding pattern should add | undefined to properties that it knows are optional because they have initialisers. Right now it only gets marked as optional.

@sandersn
Copy link
Member

In the meantime, both bugs can be worked around by adding an explicit type annotation to the destructured parameter itself: const want = ({ a = 1, b = 2 }: { a?: number, b?: number} = {}) => {};

@Lazarus535
Copy link
Contributor

I have fixed both issues present here and the changes are just about ready.
Still working on some nice testcases and stuff.
Expect a PR in the next days. Just saying, so that nobody starts putting work in. :-D

Lazarus535 added a commit to Lazarus535/TypeScript that referenced this issue Jan 26, 2018
Notes about the bug:
There were in fact two bugs, as was initially suggested by @Mhehazy.
The first lead to properties not being found inside optional object
binding patterns.
The second lead to a function not being assignable to another when it
actually should have.
Both bugs were fixed.

Addes more testcases.
@Lazarus535 Lazarus535 mentioned this issue Jan 26, 2018
@Lazarus535
Copy link
Contributor

PR is out! :-)

@jakebailey
Copy link
Member

Just looking at old issues; this one was fixed by #37309 (and #37271 was the same just years later).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants