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

narrowing in switches doesnt work on constrained unions #20375

Closed
zpdDG4gta8XKpMCd opened this issue Nov 30, 2017 · 9 comments · Fixed by #43183
Closed

narrowing in switches doesnt work on constrained unions #20375

zpdDG4gta8XKpMCd opened this issue Nov 30, 2017 · 9 comments · Fixed by #43183
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Milestone

Comments

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Nov 30, 2017

assuming my understanding it right: an extended union is a subset (subtype) of the original union

    declare function never(never: never): never;
    type X = 'A' | 'B' | 'C';
    void function fn<Y extends X>(y: Y): Y {
        switch (y) {
            case 'A': return y;
            case 'B': return y;
            case 'C': return y;
            default: return never(y); // <-- y expected to be never, actual Y
        }
    }

same problem with objects

    type X = { k: 'A' } | { k: 'B' } | { k: 'C' };
    void function fn<Y extends X>(y: Y): Y {
        switch (y.k) {
            case 'A': return y;
            case 'B': return y;
            case 'C': return y;
            default: return never(y); // <-- y expected to be never, actual Y
        }
    }
@masaeedu
Copy link
Contributor

Nvm earlier comment, misunderstood what you meant by "extended". The weird part here is y is being inferred as never in each of the 'A', 'B' and 'C' cases.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2017

Generics that are constrained to literal types are not treated like literals at the moment. generics have a more conservative behavior when narrowing in general since the actual type is not known, but generics with literal constraints should be behave like a union of literals.

@mhegazy mhegazy added Bug A bug in TypeScript Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Dec 2, 2017
@sandersn
Copy link
Member

sandersn commented Dec 4, 2017

In the first example, y is never in the case A/B/C branches. This, too , seems wrong.

declare var n: never;
function fn<Y extends "A" | "B" | "C">(y: Y): Y {
    switch (y) {
        case 'A': n = y; return y; // ok!
        case 'B': n = y; return y;
        case 'C': n = y; return y;
        default: n = y; return never(y); // <-- y expected to be never, actual Y
    }
}

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Jan 30, 2018
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 2.8 milestone Jan 30, 2018
@RyanCavanaugh
Copy link
Member

This is indeed super wrong

@sandersn
Copy link
Member

Upon thinking about it some more (and fixing the bug), I don't think this is actually super wrong. It's correct in an annoying way. Consider the non-type-parameter example, which the compiler correctly complains about:

function f(ab: 'a' | 'b') {
  switch(ab) {
    case 'a': return ab;
    case 'b': return ab;
    case 'c': return ab;
    default: return ab;
  }
}

case 'c' can never happen, so the compiler adds an error that 'c' not comparable to 'a' | 'b'. This is equivalent to the above example with Y='a' | 'b'; 'a' | 'b' extends 'a' | 'b' | 'c', but case 'c' will never be true. At the function's declaration we don't know which type Y will be, so case 'c' (or 'a' or 'b') might never be true. We can't know. So the correct thing is not to narrow at all, which is the current behaviour.

However, this is useless and annoying because even though case 'c' is useless with Y='a' | 'b', it's not hurting anything. I changed the narrowing code to use the base constraint of the type instead of the type itself. The PR is up at #21483

@zpdDG4gta8XKpMCd
Copy link
Author

zpdDG4gta8XKpMCd commented Jan 30, 2018

what you say is indeed true in a non-practical way, yes one cannot narrow an unknown something like we know what it is, but here instead we are narrowing in assumption that we cover all possible cases out there, so i guess it's still correct

@marshall007
Copy link

The fact that type narrowing does not work as expected in conjunction with generic constraints is very subtle and confusing behavior. I've been trying to figure out why we haven't been able to get this to work in our codebase for quite a while, thinking it was an issue with our interface definitions, before finally stumbling across this issue.

We use enum discriminator properties a lot, so this is pretty annoying to workaround. It would be great if this could be prioritized, it very much feels like a bug.

@jcalz
Copy link
Contributor

jcalz commented Apr 11, 2019

Is this different from #13995?

@ahejlsberg
Copy link
Member

This issue is now fixed in #43183.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
9 participants