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

Type parameters in custom type guards don't seem to have the same meaning as in their predicates #4212

Closed
zpdDG4gta8XKpMCd opened this issue Aug 7, 2015 · 19 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@zpdDG4gta8XKpMCd
Copy link

I know I am not supposed to use values of void, because one day they might be banned. But for time being I don't understand is it by design or by mistake that the isVoid guard works whereas isNonVoid doesn't in the example below:

function isVoid<a>(value: void | a): value is void {
    return undefined;
}

function isNonVoid<a>(value: void | a) : value is a {
    return undefined;
}

function foo<a>(value: void|a): void {
    if (isVoid(value)) {
        // value is void
    } else {
        // value is a
    }
}

function baz<a>(value: void|a): void {
      if (isNonVoid(value)) {
            // value is void | a
      } else {
            // value is {}
      }
}

How come that seemingly identical guard implementation breaks apart entirely?

@tinganho
Copy link
Contributor

tinganho commented Aug 7, 2015

@Aleksey-Bykov notice that you are using a type parameter a in isNonVoid and you are passing the type value | a for the type parameter a in the predicate value is a.

And at the else clause you are subtracting the type value | a from value | a which I presume will result in type {}.

@zpdDG4gta8XKpMCd
Copy link
Author

why does the value is a predicate treat void | a as a? in other words a in value is a is not the same a as in void | a.

if I understood your right how isNonVoid is defined could have been just as well written as:

function isNonVoid<a>(value: void | a) : <b>(value is b) {
    return undefined;
}

but that is at very least surprising that the knowledge about what a is is not propagated to the predicate

@zpdDG4gta8XKpMCd zpdDG4gta8XKpMCd changed the title Custom type guards work interestingly Type parameters in custom type guards don't seem to have the same meaning as in their predicates Aug 7, 2015
@wgebczyk
Copy link

wgebczyk commented Aug 7, 2015

@tinganho Please notice that this is not the first time when people sees guard expressions as working in unexpected way. Please search for all issues (open and closed) and you see that something is wrong with them. Either this construct in not natural to people or the people that have problem should not use TS as they are not target group.

@tinganho
Copy link
Contributor

tinganho commented Aug 7, 2015

@Aleksey-Bykov Sorry, after looking at it more closely it probably is a bug.

@tinganho
Copy link
Contributor

tinganho commented Aug 7, 2015

I just noticed that type parameter matching for union types does not work on the following case too:

function f<T>(x: string | T): T {
    return undefined;
}

let a: string | number;
let b = f(a); // b: string | number, while it should be number?

@zpdDG4gta8XKpMCd
Copy link
Author

Was there any luck figuring out whether it is a bug or was done by design?

@danquirk
Copy link
Member

Seems like a bug if the type predicate is intended to work the same as other type guard forms. That is, if you apply this logic (from 4.24 in the spec):

• A type guard of the form typeof x === s, where s is a string literal with the value 'string', 'number', or 'boolean',
o   when true, narrows the type of x to the given primitive type provided it is a subtype of the type of x, or, if the type of x is a union type, removes from the type of x all constituent types that aren't subtypes of the given primitive type, or
o   when false, removes the primitive type from the type of x.

it seems like the two cases in the original post should work as you expected and not how they are at the moment.

@danquirk danquirk added the Bug A bug in TypeScript label Aug 10, 2015
@tinganho
Copy link
Contributor

@danquirk If this is a bug then probably my above comment is also?

function f<T>(x: string | T): T {
    return undefined;
}

let a: string | number;
let b = f(a); // b: string | number, while it should be number?

TS doesn't seem to remove matched types before the type parameter is decided. Though I would argue that if you have more than 1 type parameter it would be very hard to decide which type the type parameter should have.

function f<T, U>(x: string | T | U): T {
    return undefined;
}

let a: string | number | boolean;
let b = f(a); // what should type of b be? 

@danquirk
Copy link
Member

It's not clear whether that's really desirable. For instance:
let c = f('hello');
Did you want c to be {} here? Or is string a valid type for T and c should be string?

@zpdDG4gta8XKpMCd
Copy link
Author

if I were you I would have banned unions with more than one bare type parameter

i mean from practical stand point f<U, T>(x: U|T) is the same as f<W>(x: W) because in both cases f has no idea what it deals with be that some U or some T or just some W

@tinganho
Copy link
Contributor

I'm not sure whether this is desirable or not to have the functionality in my previous comment. Though my point is that my described problem is the same as OP?

function baz<a>(value: void | a): void {
      if (isNonVoid(value)) {
            // value is void | a
      } else {
            // value is {}
      }
}

Notice the parameter type for isNonVoid is void | a, where a is a type parameter. It's quite similar to me described problem above. And if you don't think it is desirable. Then this feature should not be desirable for type guards too.

@zpdDG4gta8XKpMCd
Copy link
Author

I think situations like in isNonVoid are valid and should be supported just like anything else that involves a union with not more than one type parameter

unions with more than one type parameter regardless of the context should end up with a compilation error

@zpdDG4gta8XKpMCd
Copy link
Author

@ahejlsberg, there seems to be an unresolved question related to how TypeScript is designed, would you please take a look?

@sandersn
Copy link
Member

@vladima points out that this is not related to type guards, but the way that unions are inferred. In the example below, removeString(b) instantiates T as U | string, which gives a : U | string | string, not a: U | string as I would expect.

removeString's return type strips one string off a, which returns U | string in this case. That fails to compile because U | string doesn't match bar's return type U. You can see this by setting the return type of bar to U | string, which allows it compile.

declare function removeString<T>(a: T | string): T;
function bar<U>(b: U | string): U {
    return removeString(b);
}

@ahejlsberg, can you take a look at this? You made a previous fix in #1035 to fix #1011, but I think this issue is more complex than that one. Is this worth it for us to fix?

There are some related issues that might have the same root cause. I'll add references to this issue if I find any.

@sandersn
Copy link
Member

@sandersn sandersn added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Nov 17, 2015
@sandersn
Copy link
Member

@Aleksey-Bykov, @ahejlsberg's answer on issue #2264 that you filed in March is basically: "we do not "carve up" [aka destructure, ed] a union type ... but proposals are certainly welcome".

@zpdDG4gta8XKpMCd
Copy link
Author

got it, will be looking hard into it

@ahejlsberg
Copy link
Member

I working on a fix for this issue as well as #5417 and #5456 (they're all the same core issue). Stay tuned...

@ahejlsberg
Copy link
Member

Now fixed by #5738.

@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed By Design Deprecated - use "Working as Intended" or "Design Limitation" instead labels Nov 24, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

7 participants