Skip to content

Incorrect inferred function return type with destructuring assignments #56771

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

Open
byanofsky opened this issue Dec 13, 2023 · 5 comments Β· May be fixed by #56875
Open

Incorrect inferred function return type with destructuring assignments #56771

byanofsky opened this issue Dec 13, 2023 · 5 comments Β· May be fixed by #56875
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@byanofsky
Copy link

πŸ”Ž Search Terms

"return type destructure"
"destructuring assignment type"

πŸ•— Version & Regression Information

  • This changed between versions 3.3.3 and 3.5.1

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.3.2#code/C4TwDgpgBASlC8UDeAoK6oEEBcy0YIENcByTE-dAXygB8oA7AVwBsW6omGATCAMwCWDCNxRUUfLgGNgAgPYMofBgB4AKlAgAPYBB4BnWAD4AFMGxqAlLg14CAJwjAm9xUhqFDagNxiUKAHoAKhQASUUIQikACyhdfWAoKU9oACMIFjkAdwAaKFCSbihPfSYAW2hgaOhHZ1c48Gg5PiVpWQU4uSh0qAADJBxkYigyEioqXoA6FAAJbIgANwh7POT9aAAWTnXDXgT7JhkXIQBzYp446IFDQjBIQntDYC7ePkJWRKqapxdFUEhOn0YFMUEEAv4AgEoABhFJQACMgSh0Lk9kcMhYICgQj4yyejSgzT6jlKLGAk0wvWKhn6gyQw1G416KCkCgSUBJHwQSgYJgGuHppHI40svk5ZIpk0I3igkKgADkust7KiIci4QAmJEw1Ho4CY7EMXGPBoAomYeHUvqCkbC5mshjsi3c5R8ukM4VUSwU3wWqUyuWKzRo1Xa2HrKAAZjDuogGKxOLxpqaLUwkat-Q9Y3tbMS-KwGpoiFd+ZtjI8hgdCVFKEwGv9sqhQeVobl4c22vCrLRcf1CaNSf+KYLGdLWZo9GYbA4XFeQhEExZueQgzTRZ5boF469vsjDblAFEQ-ZcAB5VIAK172MMYDk+n0AlSBt6U5YVNRfVn-Hn3F6QA

πŸ’» Code

type R = {
    A: {
        a: 'A'
    } | null | undefined
}
function fn<T extends R>(t:T): T  {
    return {} as T;
}

/*
In each test case below, I'd assume the return type of function to be `{A: {a: 'A'}}`.
However, case 4 uses destructuring and this appears to default the return type to `R`.
*/

// Case 1
// Correctly infers type of `result.A` as `{A: {a: 'A'}}`
const result = fn({A: {a: 'A'}});
result.A.a; // No error

// Case 2
// Correctly infers type of A1 as `{a: 'A'}`
const A1 = fn({A: {a: 'A'}}).A;
A1.a; // No error

// Case 3
// Correctly infers type of A3 as `{a: 'A'}`
const {A: A2} = fn({A: {a: 'A'}} as const);
A2.a; // No error

// Case 4
// Incorrectly infers type of A2 as `{A: {a: 'A'} | null | undefined}`
const {A: A3} = fn({A: {a: 'A'}});
A3.a; // Error: Object is possibly `null` or `undefined`

πŸ™ Actual behavior

In case 4, accessing A3.a results in error "Object is possibly null or undefined".

Appears that the inferred type of A3 is {a: 'A'} | null | undefined. I suspect when we destructure, the return type of fn is inferred as R instead of the narrowed generic type variable T.

πŸ™‚ Expected behavior

Expect no errors. Should be able to access A3.a, similar cases 1, 2, and 3. Expectation is that destructuring assignment shouldn't modify inferred return type of function.

Additional information about the issue

Current workarounds:

  • Use as const appended to param (see case 3)
  • Use const with generic type variable in function signature (function fn<const T extends R>(t: T): T;)
  • Avoid destructuring assignments (case 1 & 2)
@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Dec 13, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Dec 13, 2023
@Andarist
Copy link
Contributor

Destructuring creates a contextual type of { A: any } here. This is used to contextually type your object literal and thus your inner a property widens to string (since it's kinda contextually typed by any). This creates an inferred candidate { A: { a: string; }; } for T and since it's not assignable to R it gets replaced by the constraint (R) as the final inferred type.

IIRC @andrewbranch was working on improvements in this area in the past so maybe he'd have some ideas how a fix for this could look like

@Andarist
Copy link
Contributor

Andarist commented Dec 13, 2023

It might also be worth noticing that other cases with the same contextual type of { A: any } kinda end up with the same problem. It's just that in other scenarios it's harder to observe this. An example (TS playground):

type R = {
  A:
    | {
        a: "A";
      }
    | null
    | undefined;
};
function fn<T extends R>(t: T): T {
  return {} as T;
}

function test(): { A: any } {
  return fn({ A: { a: "A" } });
  //     ^? function fn<R>(t: R): R
}

@fatcerberus
Copy link

fatcerberus commented Dec 14, 2023

This creates an inferred candidate { A: { a: string; }; } for T and since it's not assignable to R ...

Wait, then why isn't the call itself an error?! Do you mean to tell me a widens to string for the purposes of inference without widening the object literal itself? That seems a bit... surprising

@Andarist
Copy link
Contributor

Do you mean to tell me a widens to string for the purposes of inference without widening the object literal itself? That seems a bit... surprising

Yes, this is roughly how it works under the hood. Arguments during inference are checked multiple times - with different contextual types.

So one of the first passes in inferTypeArguments checks this argument with a contextual type of T. That however gets instantiated to { A: any} by getApparentTypeOfContextualType in checkObjectLiteral.

Later, once the compiler settles~ on the inferred type, it re-checks the same argument in getSignatureApplicabilityError - but this time the contextual type is what got inferred, so R (since the inference candidate didn't satisfy the constraint, the constraint type itself got assigned as the inferred type). This time it's OK - the argument typechecks fine, that property inside it doesn't reuse the widened type from the prior pass and this time it retains its literalness because the contextual type (R) makes it to.

@Andarist
Copy link
Contributor

Some extra data points. In one of the last PRs that touched this, we can see this test case:

declare function pick<O, T extends keyof O>(keys: T[], obj?: O): Pick<O, T>;
const _    = pick(['b'], { a: 'a', b: 'b' }); // T: "b"
const {  } = pick(['b'], { a: 'a', b: 'b' }); // T: "b" | "a" ??? (before fix)

Both infer T as "b" with this PR in - however, that could be just because, IIRC, it starts to ignore empty binding patterns. So let's add an element to this:

declare function pick<O, T extends keyof O>(keys: T[], obj?: O): Pick<O, T>;
const { b } = pick(['b'], { a: 'a', b: 'b' }); // T: "b"

It's still ok πŸ‘

If we add a constraint to this object then it still works fine:

declare function pick<O extends { a?: "a"; b?: "b" }, T extends keyof O>(
  keys: T[],
  obj?: O,
): Pick<O, T>;
const { b } = pick(["b"], { a: "a", b: "b" }); // T: "b"

but now if we make that constraint nullable it gives us incorrect results:

declare function pick<O extends { a?: "a"; b?: "b" } | null, T extends keyof O>(
  keys: T[],
  obj?: O,
): Pick<O, T>;
const { b } = pick(["b"], { a: "a", b: "b" }); // T: "a" | "b"

I assume that this is the same problem but a different manifestation of it.

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

Successfully merging a pull request may close this issue.

4 participants