Inference fails for return-type of type-union between class and generic type

[mindplay-dk]

Type inference seems to fail when the return-type is a union between a class and a generic type.

I was hoping to use this pattern to be able to decorate an object (not a class) with traits/behaviors on a per-instance basis, in a type-safe manner, something along the lines of Scala's with operator.

Best explained by example:

class Fud {}

interface Decorated {
    foo(): string;
}

function decorate<T>(object: T) : T|Decorated {
    object['foo'] = function () {
        return 'Hello';
    };
    return <T|Decorated> object;
}

var test = decorate(new Fud());

console.log(test.foo()); // ERROR

So anything that goes through the decorate() function comes out decorated with a new interface, which is made possible using a union between a variable type and some interface.

Hovering over the call to decorate(), the return-type is correctly reported as Decorated | Fud - however, when the result is assigned to the test variable, the inferred type is Fud, and the attempt to call test.foo() generates the error "property foo does not exist on type Fud".

Am I missing something? Is there some reason this shouldn't work?

[danquirk]

The compiler attempts to collapse union types down at various times as a performance optimization when the constituent types aren't meaningfully different (ex the trivial case is when the union type T|T becomes just T). So while decorate's type is Decorated|Fud when assigned to test the union is collapsed to only Fud. If your type definition for Fud contains any members so that Fud and Decorated are meaningful definitions (ie have members) and distinct then you will see that test has the union type you expect:

class Fud { bar: string; }
...
console.log(test.foo()); // ERROR: "Property 'foo' does not exist on type Fud|Decorated"

In almost all cases empty type definitions lead to a bad time and should be considered an anti-pattern.

[mindplay-dk]

The compiler attempts to collapse union types down at various times as a performance optimization when the constituent types aren't meaningfully different

Okay, so it's being collapsed because Fud has an empty interface, I guess?

If I add a method to Fud that should makes the class and interface "substantially different", like so:

class Fud {
    bar(): string {
        return "BAR";
    }
}

interface Decorated {
    foo(): string;
}

function decorate<T>(object: T) : T|Decorated {
    object['foo'] = function () {
        return 'Hello';
    };
    return <T|Decorated> object;
}

var test = decorate(new Fud());

console.log(test.foo()); // ERROR

If I hover over test now, I see Decorated | Fud, so the type union isn't being collapsed now - however, I still get an error saying "property foo does not exist on type Decorated | Fud".

What gives?

[danquirk]

Right, because foo only exists on Decorated, not Fud, so we can't safely say you can access foo through an arbitrary instance of a Decorated|Fud. Unions only allow you to access members that are present in all constituent union type parts, not properties that exist in at least one constituent part. Obviously in your case you know you've patched in a foo member but from the type system's perspective it has no idea. To do this you'd need a better definition for Decorated. Probably what you'd actually want is something like:

interface Decorated<T> extends T {
    foo(): string;
}

but we don't allow you to write this sort of declaration unfortunately. What you're writing is something closer to mixins, see:

#517 (comment)
#727
#1256

The workarounds available now are casting to the type you actually expect, or to use index access to get at the property:

var test = decorate(new Fud());
var r = test['foo'](); // no error but r is 'any'

[mindplay-dk]

Unions only allow you to access members that are present in all constituent union type parts, not properties that exist in at least one constituent part

Oh. Is that how it normally works? In PHP (with php-doc) a type-annotation like Decorated|Fud means basically "either Decorated, or Fud, or both" - I always understood this to be the norm, and the most (or only) useful way to merge types.

I would have thought the same to be the case in JS, where it's common to have an object of either one type or the other - so you'd have to perform run-time checks to see if it's actually one type, or the other, or both.

To me, merging types in this manner is just sort of weird, really... because the result of this type merge operation is neither X, nor Y, nor both - it's some weird hybrid type comprised of the methods they happen to have in common. I don't understand the usefulness of this. If I wanted two types to have something in common, I would have them both implement an interface, so that what they have in common is well-defined.

I always took the vertical pipe as meaning "type alternatives" - maybe PHP (php-doc) got it wrong?

[danquirk]

It sounds like PHP is using it to mean a combination of union and intersection types. The goal for TypeScript's union types is to increase expressivity but also increase safety.

The result of the operation isn't some hybrid type, it is what you think: a type that is either an A or a B. What follows from that is the question of what can safely be accessed off of instances of types like this. You could say it's simply unsafe to access anything until you 'pick' which part of the union type this actually is. But that's unnecessarily restrictive. We can guarantee that it is safe to access a property that is on A and B (assuming compatible types of the respective properties). As you noted if you want to access something that is only available on A or only available on B then some additional checks are required to do so safely/correctly. This is what type guards are for in TypeScript. When you write your runtime checks the type system can interpret what is happening and narrow the union to only the relevant part(s).

var test = decorate(new Fud());
// if this was allowed it's entirely likely you just wrote code that will be a null ref exception
test.bar().toString(); // error
// this is what you needed to write anyway to make sure your code wasn't buggy
if(test instanceof Fud) {
    test.bar(); // ok, now we're sure this is something that has a ```bar``` member
}

The value of union types would be far lower if there was no checking for valid property access, assignability, etc.

[mindplay-dk]

I know it's late to bring this up, but...

The choice of operator here, the vertical pipe, is inconsistent both with JSDoc and Google Closure Compiler, where e.g. number|string is referred to as a "union", but means exactly what I thought it meant - basically type alternatives, "X or Y".

The use of the vertical pipe in JS and in general means "or", but the meaning of it in TS is more like "and", e.g. a type containing interfaces present in "X and Y", so something like X&Y would probably have been more consistent?

Neither JSDoc, Google Closure annotations or php-doc even provide a type operation like the one implemented by Typescript - what's referred to as a "type union" in these established standards is not what I was expecting, and I'm pretty sure I won't be the first to react with surprise.

I am hard pressed to think of a scenario where I would ever use this feature. All I can think is that X|Y is more like a pseudo-interface that magically contains the public members those two types have in common, and I can't think of any reason why I would not simply define an interface for that? Relying on this dynamically generated common interface seems risky, given a lot of possible scenarios, such as the name of a method (or it's arguments) getting changed in one class, and suddenly the resulting types are something else entirely.

I don't think anyone has ever asked for the type operation implemented here to be added to either JSDoc, Closure compiler or php-doc, and I think with good reason - in JS (or PHP) variables do not have types, so having alternative types assigned to the same variable (or property or parameter) is going to be a common scenario, while types that have some public members in common is, well, already covered by interfaces in Typescript and PHP; it isn't necessary.

Type alternatives, on the other hand, occur all the time in existing JS code, and decoration (as per my example above) is quite common as well - but it would appear there is still no way to describe those type relationships. (?)

I would be surprised if I'm the only person who perceives this as a bug - it's a pretty big departure from something that is already well-known and established in other JS annotation systems.

The value of union types would be far lower if there was no checking for valid property access, assignability, etc.

I get your point, in theory - but in practice, if you have alternative types, you expect to have to do some type assertions before operating on them, and it rarely turns out to be a problem at all.

My bigger point is that this is inconsistent with established tools - it feels awkward.

And it doesn't address a very real and frequent scenario in JS, where objects get decorated with more properties and methods at run-time. Happens everywhere, all the time. We already had interfaces which can describe common properties just fine, and in recent versions you don't even have to explicitly implement interfaces, so there is already a much better way to handle this case.

I don't know when I would use this feature or why. Maybe I'm missing something?

[danquirk]

The choice of operator here, the vertical pipe, is inconsistent both with JSDoc and Google Closure Compiler, where e.g. number|string is referred to as a "union", but means exactly what I thought it meant - basically type alternatives, "X or Y".

No, JSDoc, Google Closure, and TypeScript all have the same meaning for |, along with many functional languages that also have the concept of union type using |. This definition is precisely why you see the behavior you do. Because it is X or Y, but not both, it is unsafe to do anything to it that isn't safe to do to both X and Y.

var x: string|number;
var y = 1;
y = x; // error, unsafe
var z = y * x; // error, unsafe

in JS (or PHP) variables do not have types

They absolutely do have types. That you have not written them explicitly is a very different matter. If JS variables actually did not have types then things like TypeScript and Flow couldn't even exist. The fact that a single variable could be multiple types, or a type that can change at runtime, is precisely why we need union types to describe them statically, or intersection types to describe their dynamic behavior.

having alternative types assigned to the same variable (or property or parameter) is going to be a common scenario

Yes, this is precisely what union types are for and why they were commonly requested.

I am hard pressed to think of a scenario where I would ever use this feature.

See previous discussions/requests here:
#14
#186
#805
Some common libraries that use this:
https://github.com/borisyankov/DefinitelyTyped/blob/master/jquery/jquery.d.ts
https://github.com/borisyankov/DefinitelyTyped/blob/master/knockout/knockout.d.ts
You can find a variety of other .d.ts on DefinitelyTyped that are using | in comments (in JSDoc comments no less) for things that are any which need to be updated to union types for better expressivity/safety (node.d.ts, durandal.d.ts, etc).

I get your point, in theory - but in practice, if you have alternative types, you expect to have to do some type assertions before operating on them, and it rarely turns out to be a problem at all.

This is not what we've heard from our users or experienced ourselves in a variety of languages with this feature. More generally, we could remove all sorts of type safety checks by this logic. This argument feels like 'in practice, just remember to always use the correct types correctly and you don't need a type checker.'

And it doesn't address a very real and frequent scenario in JS, where objects get decorated with more properties and methods at run-time

Yes, that is a different feature, my earlier reply linked to a few issues already tracking suggested ways to implement it (mixins/decorators/etc). It is definitely on our radar.

[mindplay-dk]

in JS (or PHP) variables do not have types

They absolutely do have types

Just to clarify (so I don't sound like an idiot) that was a misphrase - what I meant to say, was that variables and members don't have defined types, meaning it's totally OK to do things like var i = 123; i = "abc"; in either JS or PHP, same for properties, etc... of course variables have types :-)

JSDoc, Google Closure, and TypeScript all have the same meaning for |

Could have sworn I had done this in JSDoc at least. Guess not. So PHP got it wrong again. That's shocking. (but not really.)

Anyway, I mucked about with type unions some more last night, and I do see their usefulness now - although I still don't agree with magically generating an interface based on whatever members happen to have the same signature, this is a feature I can simply avoid using. It's still useful as far as allowing more than one type as argument to a function, and in conjunction with instanceof, I can play it safe and not rely on overlapping members.

Thank you for your patience! I really appreciate it.

Typescript remains the most awesome thing since sliced bread :-)

[NoelAbrahams]

The case for using a union type as declared is quite limited:

var stringOrNumber: string|number = 'string';

// Not very useful
stringOrNumber.toString();

The real utility comes from type guards

if(typeof stringOrNumber === 'string'){
    stringOrNumber.replace();
}

I think there are still a few bugs to be fixed and extensions to be implemented. However, in practice we have found union types to be indispensable.

[danquirk]

Happy to talk through the issues :) I definitely get where you're coming from and in particular the decorator/mixin scenario is not handled by this stuff so it can seem odd if that was the direction you started from.

although I still don't agree with magically generating an interface based on whatever members happen to have the same signature

Don't think of it as magically generating a new interface. It's just about allowing the easiest possible use of a union type when it can be provided safely. It's annoying to have to write those runtime checks but sometimes it's a necessity. But there are cases where they're simply unnecessary to do certain things given the intersection of functionality of the union parts. With the examples we've looked at in this thread it's not necessarily super useful because you have fairly disjoint union parts (ex things like string|number, string|string[], etc) but it can be more useful if there's some overlap. Consider:

class Animal {
    name: string;
}

class Cat extends Animal {
    name: string;
    meow() {}
}

class Dog extends Animal {
    name: string;
    bark() {}
} 

var cog: Cat|Dog;
console.log(cog.name); // ok, good, why should I have had to disambiguate Cat vs Dog to do this?
if(cog instanceof Cat) {
    cog.meow(); // this on the other hand required more code to be safe
}

I guess I'm not sure the preferred alternative you have in mind. We could have just made any property access on these types an error but that's not really better is it?

[mindplay-dk]

Don't think of it as magically generating a new interface. It's just about allowing the easiest possible use of a union type when it can be provided safely

I get that, but I don't agree that this is safe - which members happen to overlap is just cause and effect, it's not dependable, and we already have automatic (duck) type-casting for interfaces, so I don't see the utility at all.

In your example, I would have never used Cat|Dog as the type-hint, since clearly, what is relevant here, is that they are both Animals with names. If Cat and Dog had something in common that wasn't common to all Animals, I would add another interface to describe what they have in common, rather than relying on a union, which could fall apart at any moment - there is no contract and hence no guarantee that members which happen to overlap now will continue to do so in the future.

It just seems like a poor alternative to interfaces, especially with auto duck casting (or whatever the correct term is) enabling us to specify a requirement only on the consumer side, rather than having to explicitly implement an interface.

We could have just made any property access on these types an error but that's not really better is it?

Yes and no.

Using unions in this way, nothing is precisely defined on neither the consumer or provider side - I don't see the value of that. It doesn't help me solve a problem, and doesn't help me writer better or more self-documenting code.

All I can see this feature doing is helping me avoid using features that could have made my code better and safer.

I'm not a fan. Sorry :-)

[mindplay-dk]

To clarify with examples - if Dog and Cat were both animals, I would write:

...

var cog: Animal;
console.log(cog.name);
if(cog instanceof Cat) {
    cog.meow(); // this on the other hand required more code to be safe
}

In this case, the code indicates that we are interested in common traits of Animals, whereas the other way, it suggests we are interested in either a Cat or Dog specifically, which doesn't seem to matter here.

If there was no common base class, I would do this:

class Cat {
    name: string;
    meow() {}
}

class Dog {
    name: string;
    bark() {}
} 

interface Named {
    name: string;
}

var cog: Named;
console.log(cog.name);
if(cog instanceof Cat) {
    cog.meow(); // this on the other hand required more code to be safe
}

In this case, the code explains which trait we're interested in, so at least the consumer side indicates what aspect of a Dog or Cat it is interested in.

I'm pretty sure I will be using unions only to clarify in cases where only certain specific types are allowed - I don't think I could find a case where I wouldn't also want the code to explain why those types are allowed.

Did somebody ask for this feature? Where was it used in real life?

[danquirk]

I get that, but I don't agree that this is safe - which members happen to overlap is just cause and effect, it's not dependable, and we already have automatic (duck) type-casting for interfaces, so I don't see the utility at all.

It's precisely as safe as accessing any other members off of well defined types. If you change the types involved in a way that it no longer becomes safe to access it without disambiguating the union cases then the original access will become an error.

It just seems like a poor alternative to interfaces

function doStuff(x: string|number|myOtherType) {
    console.log("The value is: " + x.toString());
    // do some stuff, pass x to other functions, etc
}

The correct parameter type is not toStringable or Object. And I do not want to have to write 3 type guards or cast to any just to be allowed to call toString.

If you just want to write a ton of interface definitions for every intersection of functionality in your program you are certainly more than welcome to do that. For many people this is both onerous and unnecessary, if not actually imprecise.

var cog: Named;
console.log(cog.name);
if(cog instanceof Cat) {
    cog.meow(); // this on the other hand required more code to be safe
}

Take this code and put it in a function called WalkCatOrDog. What is the parameter type of cog? It is not Named. That is imprecise. The function is not intended to take anything with a name (like a function typed object). It is also not Animal, because I'm writing a function specific to ex how you walk a cat or dog. There's no reason for me to have to duplicate the code to log the name in every type guard. What if Animal has 6 properties? Are you going to write N interfaces for every combination of those 6 properties that could be shared through a union of multiple Animal types?

Did somebody ask for this feature? Where was it used in real life?

I already pointed out multiple threads where people asked for this and multiple libraries already using this feature to increase type safety and expressiveness. You certainly don't have to use this feature if you don't like it but it's been a top request for us for a long time.