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

Suggestion: Type guard generic types #4742

Closed
dsherret opened this issue Sep 11, 2015 · 16 comments
Closed

Suggestion: Type guard generic types #4742

dsherret opened this issue Sep 11, 2015 · 16 comments
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@dsherret
Copy link
Contributor

Would it be good to have type guarding on generic types? Or was this decided against?

Example 1

class Parent {
    prop1: string;
}

class Child extends Parent {
    childProp: string;
}

var t: Parent = new Child();

if (t instanceof Child) {
    t.childProp; // no error
}

function myFunction<T extends Parent>(item: T) {
    if (item instanceof Child) {
        item.childProp; // error
    }
}

I realize this example could be rewritten without generics. It's just an example :)

Example 2

Here's a scenario I came across when creating a wrapper class:

interface NedbItem {
    _id: string;
}

function isNedbItem(item: any): item is NedbItem {
    return item != null && typeof item._id === "string";
}

class DatabaseWrapper<T> {
    getItems() {
        const itemsFromDatabase: T[] = .....;

        itemsFromDatabase.forEach(i => this.stripNedbID(i));

        return itemsFromDatabase;
    }

    private stripNedbID(item: T) {
        if (isNedbItem(item))
        {
            delete item._id; // error, _id does not exist on T
        }
    }
}

Maybe a generic type with no constraint should be treated like an any type in this case? (Once type guards are fixed for any types -- See #4432)

This suggestion is probably very low on the priority scale.

I couldn't find this issue discussed elsewhere so sorry if it was.

@dsherret dsherret changed the title Suggestion: Type guard generics Suggestion: Type guard generic types Sep 11, 2015
@dsherret
Copy link
Contributor Author

Just thinking about this a bit more. I guess this is not allowed for the same reason that this is not allowed:

function myFunction<T>(item: T) {
    const s = item as string; // error, neither T nor string is assignable to the other
}

I guess it does make sense to not allow this the more I think about it and it won't be needed in most scenarios.

I'm closing this issue.

@ziahamza
Copy link

There are still valid cases where this can be helpful, especially when generic type extends from a union type. E.g

class Cat {    }
class Dog {  }
class Node<T extends Cat | Dog> {
     data: T = null
     print() {
          if (this.data instanceof Dog) {
                // this.data now has the type Dog inside the block
          }
     }
} 

But the example doesnt seem to work today as typescript does not specialise the generic type even if it is a subset of the type it extends from

@mhegazy
Copy link
Contributor

mhegazy commented Sep 11, 2015

there is another problem in the use of this.data; type guards do not work on property look ups only variables. that is tracked by #1260

@mhegazy mhegazy reopened this Sep 11, 2015
@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Sep 11, 2015
@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed In Discussion Not yet reached consensus labels Oct 19, 2015
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Oct 19, 2015
@RyanCavanaugh RyanCavanaugh added the Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". label Oct 19, 2015
@RyanCavanaugh
Copy link
Member

Accepting PRs.

The root cause here comes from two factors.

First is that given a (constrained) generic type parameter T extends U, we don't consider T to be assignable from a subtype of U. That rule is correct - it would be unsound to let you write code like this:

function clone<T extends Animal>(x: T) {
  return new Dog(); // Unsafe: Dog is subtype of Animal, but is not a subtype of T
}
clone(new Cat()); // Example invocation of other subtype

Second is that a type guard on an expression of type W to type V only succeeds if V is assignable to W. That rule is also correct -- typeguarding guarding a Cat with isDog should not succeed.

The rules do not combine favorably in the case of generics, though. We believe the correct fix is to take the apparent type of the generic type parameter when performing the assignability check in the second rule.

@JsonFreeman
Copy link
Contributor

Thinking about @RyanCavanaugh's two root causes, why should type guarding Cat with isDog be illegal? What if the Cat is actually Cat & Dog at runtime?

@hesselink
Copy link

I have another use case for this. Simplified:

interface A { type : string }
interface B extends A { f () : B }
function isB (x : A) : x is B {
    return x.type === "B";
} 

// Doesn't work.
function g <T extends A> (x : T) : T {
    if (isB(x))
      return x.f()
}

// Works, but lets you return an `A` if passed a `B`.
function h (x : A) : A {
    if (isB(x))
      return x.f();

The function g was written with generics to enforce that you get back the type you passed in. So you can pass in either an A or a B, but when you pass a B, you will get back a B as well. However, after type isB check, typescript doesn't refine T to be a B, so the call to f fails to type check.

@tiagoefmoraes
Copy link

Another use case:

function foo<T extends string | number>(value: T) {
	return typeof value === "number" && isNaN(value);
	// on isNaN value still is "value: T extends string | number"
}

@masaeedu
Copy link
Contributor

masaeedu commented Jul 31, 2017

It seems like it already sort-of works if I explicitly write a generic function that returns the type guard:

class Cat { meow() { console.log("meow") } }
class Dog { woof() { console.log("woof") } }

class N<T extends Cat | Dog> {
    data: T = null
    print() {
        const data = this.data

        // This is fine
        if (is(data, Dog)) {
            return data.woof()
        }

        // Unfortunately, this is not. Looks
        // like narrowing isn't working.
        // return data.meow()

        // However, you can work around the
        // problem with this:
        if (is(data, Cat)) {
            return data.meow()
        }
    }
}

function is<T, TClass>(x: T, c: new () => TClass): x is T & TClass {
    return x instanceof c
}

Unfortunately, it looks like narrowing of the union doesn't work. If I add the explicit annotation const data: Cat | Dog the error goes away, so it looks like an unfortunate interaction between union narrowing and generics. In the meantime, we can just explicitly write if-clauses for each case instead of relying on narrowing.

@thorn0
Copy link

thorn0 commented Jul 17, 2018

Why is this issue still open? All the examples here compile fine already.

@thorn0
Copy link

thorn0 commented Jul 17, 2018

What about more advanced scenarios? Like this:

function foo<T>(a: T) {
  const b = a;
  if (typeof a === 'string') {
    console.log(b.toLowerCase()); // error
  }
}

It doesn't compile, however wouldn't it be safe to assume that T is string inside the if? Is there probably another issue where it's been discussed?

@rolfvandekrol
Copy link

I found another example of this problem where this goes wrong.

type A = { kind: "a" }
type B = { kind: "b" }

const a = (a: A): void => undefined
const b = (b: B): void => undefined

const c = <C extends A | B>(c: C): void => (c.kind == "a" ? a(c) : b(c))

Of course in this simple example C could just be defined a type alias, but in the code this is extracted from I need C to be generic and then this doesn't work anymore.

@jack-williams
Copy link
Collaborator

@rolfvandekrol That is probably related to this: #21483, or at least it suffers from the same issues.

@dartess
Copy link

dartess commented Aug 26, 2020

So few answers and reactions to such an old problem... :С

A similar problem: https://stackoverflow.com/questions/63594468/type-guard-dont-work-with-generic-params

@Zzzen
Copy link
Contributor

Zzzen commented May 16, 2021

What about more advanced scenarios? Like this:

function foo<T>(a: T) {
  const b = a;
  if (typeof a === 'string') {
    console.log(b.toLowerCase()); // error
  }
}

It doesn't compile, however wouldn't it be safe to assume that T is string inside the if? Is there probably another issue where it's been discussed?

This is working as intended. Types are immutable, only variables can be narrowed, a is tested so it will have type string. You can replace generic T with concrete type string | number. @thorn0

@parzhitsky
Copy link

Playing with the OP's second example (and fixing some apparently unintended errors), I've found that it is optional part that causes the compile error. In other words, this compiles perfectly fine:

interface Doubleable {
    double(): void; // ✅ required
}

declare function isDoubleable(value: unknown): value is Doubleable;

class Wrapper<Value> {
    act(value: Value) {
        if (isDoubleable(value)) {
            value.double();
        }
    }
}

… while this doesn't:

interface Doubleable {
    double?(): void; // ❌ optional
}

declare function isDoubleable(value: unknown): value is Doubleable;

class Wrapper<Value> {
    act(value: Value) {
        if (isDoubleable(value)) {
            value.double();
//                ^^^^^^
//                Property 'double' does not exist on type 'Value'. (2339)
        }
    }
}

This happens not only in generic classes, but in generic functions as well, so I guess it is really a problem with generics.


What's weird is that the error message says "on type 'Value'". I would understand if TypeScript refused to invoke a possibly missing method, which it does in this case:

interface Doubleable {
    double?(): void;
}

function double(value: Doubleable): void {
    value.double();
//  ^^^^^^^^^^^^
//  Cannot invoke an object which is possibly 'undefined'.
}

… but in our cases it does something different.

@andrewbranch
Copy link
Member

All these examples (except #4742 (comment)) are working now 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests