Incorrect union type narrowing with literal types

[DCzajkowski]

Bug Report

🔎 Search Terms

narrowing, union types, if branches.

🕗 Version & Regression Information

• This is not a crash
• This changed between versions 4.1.5 and 4.4.2 (works better, but still wrong)
• This is the behavior in every version I tried, and I reviewed the FAQ
• I was unable to test this on prior versions because I used the playground

⏯ Playground Link

Playground Link

💻 Code

type ResponseFolder =
    | {
          id: string;
      }
    | {
          id: string;
          version: number;
      };

declare const response: ResponseFolder[];

type GenericFolder =
    | {
          id: string;
          version?: undefined;
      }
    | {
          id: string;
          version: 2;
      };

/**
 * This fails as expected ('version' of type `number` is not assignable to type `2`).
 */

const folders1: GenericFolder[] = response.map((folder) => {
    return folder;
});

/**
 * This does not fail in 4.1.5 (which is wrong, it's the same as above), but fails in 4.4.2.
 */

const folders2: GenericFolder[] = response.map((folder) => {
    if ("version" in folder && folder.version === 2) {
        return folder;
    }

    return folder;
});

/**
 * This does not fail in 4.4.2 (which is wrong, it's the same as `folders1`, just written differently).
 */

const folders3: GenericFolder[] = response.map((folder) => {
    if ("version" in folder) {
        return folder;
    }

    return folder;
});

🙁 Actual behavior

folders3 assignment does not throw a compile error.

🙂 Expected behavior

folders3 should fail on compilation, because version of type number | <missing prop> should not be assignable to version of type 2 | optional undefined.

[MartinJohns]

I believe this is a duplicate of #34975.

Also, "version" in folder can return true if the folder has an explicit undefined property named version, e.g. { version: undefined }.

[DCzajkowski]

I think the main problem in the case of this issue @MartinJohns is return folder; and return "version" in folder ? folder : folder; are identical, but TS interprets them differently.

[andrewbranch]

This actually looks separate from the well-known in unsoundness... in the if block, folder shows up as { id: string, version: number }, and after the block it shows up as { id: string }. The unsoundness we expect is already apparent in that narrowing. But there’s something much more subtle going on here. The return type of the map callback is contextually typed by the Folder[] annotation on the const declaration, but its return type is not, in the end, Folder. The return type comes from the subtype reduction of the two return statements: { id: string, version: number } | { id: string } reduces to { id: string }, so the return of the map function is { id: string }[] which is assignable to Folder[].

I guess the unsoundness really at play here is just the one about aliasing and excess properties: playground. I don’t think this issue tells us anything we didn’t already know, but this was a pretty insidious example and it took me a minute to figure it out. It’s definitely not great. @MartinJohns got any more related/duplicates we can cross-reference with my analysis? 😁

[DCzajkowski]

Oh, I get it now. Thank you for the in-depth analysis @andrewbranch. This was very invaluable to me.

As I understand in these cases it's safer to just type the callback's return type, not const's type (or both for a good measure).

const folders = response.map((folder): Folder => { /* ... */ })

In my mind this issue can be closed 🙂

[typescript-bot]

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes. If you're still waiting on a response, questions are usually better suited to stackoverflow.