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

Conditional Type is assignable to a discriminated union, yet not working with type guards as expected. #23985

Closed
DavidKDeutsch opened this issue May 8, 2018 · 5 comments
Labels
Suggestion An idea for TypeScript

Comments

@DavidKDeutsch
Copy link

(Apologies if this is a duplicate; I suspect that it is after seeing so many related questions being marked "behaving as designed," but am having a hard time figuring out exactly which issue this may be duplicating).

TypeScript Version: 2.8.3

Search Terms: discriminated union, conditional generic, etc...

Code

interface Foo {
    hasADate: true;
    obj: Date;
}

interface Bar {
    hasADate: false;
    obj: number;
}

type FooBar = Foo | Bar;
type GenericFooBar<T> = T extends Function ? Foo : Bar;

function testing<T>(doesntWork: GenericFooBar<T>) {
    if (doesntWork.hasADate === true) {
        return doesntWork.obj.getDate(); // ERROR: arg.obj is still type number | Date
    }

    let works: FooBar = doesntWork;
    if (works.hasADate === true) {
        return works.obj.getDate();     // WORKS: arg.obj is type Date
    }
}

Expected behavior:
Inside of the doesntWork.hasADate === true type guard, doesntWork.obj should be narrowed to a Date

Actual behavior:
doesntWork.obj is still a number | Date. The compiler does allow me to assign doesntWork to works (whose type is the respective discriminated union), and with that variable everything works as expected. It seems odd that the compiler recognizes that GenericFooBar<T> extends Foo | Bar (as it allows the assignment), yet doesn't work with the discriminated type guard.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels May 8, 2018
@mhegazy
Copy link
Contributor

mhegazy commented May 8, 2018

This is no different from:

function testing<T extends Foo | Bar>(doesntWork: T){
    if (doesntWork.hasADate === true) {
        return doesntWork.obj.getDate(); // ERROR: arg.obj is still type number | Date
    }
}

The underlying issue here is narrowing does not work on higher order types. what we need here is to first enable narrowing to work on T and in the true branch to be T extends Foo and in the false branch T extends Bar.

#22348 is the start of this change.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label May 8, 2018
@ahejlsberg
Copy link
Member

Adding to what @mhegazy just said... Typically, conditional types are useful in return types where they can depend on type parameters that in turn are used in parameter types such that inferences can be made for them. As you have it written in your example, we can't make any inferences for T and therefore you'd have to always specify it manually--which sort of defeats the purpose.

@DavidKDeutsch
Copy link
Author

@mhegazy, thanks for that code snippet; I was so sure this had to do with conditional types that I never thought to try a simpler case like that one.

@KSXGitHub
Copy link
Contributor

@DavidKDeutsch Why did you closed this issue? I think this is a good suggestion.

@DavidKDeutsch
Copy link
Author

@KSXGitHub I closed it because I had not realized that my particular problem really didn't have anything to do with the new conditional type functionality; it's a pre-existing issue with generics in general (as @mhegazy demonstrated for me), and is already being worked on in #22348. I didn't want to clutter the open issues list with a duplicate (I figured it was a duplicate of something when I first posted it, but didn't know what that "something" was).

That being said, if @mhegazy thinks it is a useful ticket to keep open, he or I can reopen it. I'm just trying to be a good citizen :)

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants