Interfaces and classes considered equal

[jrieken]

TypeScript Version:

1.8.0

Code

// A self-contained demonstration of the problem follows...
class A {
    constructor(a: number, b: boolean) {

    }
    foo() {

    }
    bar: boolean;
}

function doIt(a: A) {
    if (a instanceof A) {

    }
}

doIt(new A(1, true));

doIt({ foo() { }, bar: false }); // expect type check error

Expected behavior:

The second to doIt should raise a type check error.

Actual behavior:

It does not. I assume it was a conscious decision and I wonder why? This break assumptions the instanceof operator makes and has led to subtle bugs on our side.

[kitsonk]

TypeScript is a structural typing system, not a nominal one. 😄 Therefore anything that shares the same structure is considered type equality.

[jrieken]

I understand that, but a class has a constructor and a prototype which I would expect to be present in the structure of a class but not in the structure of a object literal

[RyanCavanaugh]

You can add a private or protected property to a class if you don't want this to happen accidentally.

[jrieken]

@RyanCavanaugh Doesn't that mean that in effect I can never rely on instanceof esp when using OR-typing and type guards as propagated by TypeScript? I think that design choice should be revisited.

function doIt(a: A | number) {

    if (a instanceof A) {

    } else if (typeof a === 'number') {

   } else {
      // what now? Could be A'ish or random stuff
   }
}

[mhegazy]

Some history, initially instanceOf checks were nominal. i.e. in side the true branch, the type is narrowed to the class, but outside it nothing happens, as we can not be sure that it is not of the type A but not created by the constructor A.

We then got feedback about users thinking of instanceof as a type check not a constructor check. and when you have something that is A|B you expect the else of if (x instnceof A) to mean it is a B.
@RyanCavanaugh did some research on what common patterns we see in the wild (see #1719 (comment)) and that reinforced the idea that the constructor and the type are thought of as the same.

So, though i agree that instanceof is a nominal check, it seems that most users do not think of the difference between nominal and structural checks when testing for types that much.

As for your last example, I had a discussion about this with @ahejlsberg yesterday. We think the correct behavior is to narrow to {} in the else branch. this way instanceof type guards work similar to typeof and undefined/null guards.

[jrieken]

For us the problem is twofold. Take the CompletionItem provider we ask extension writers to implement. The interface states that you should CompletionItems[], which is a class, or a CompletionList. Since we want to protect against bad extensions we check the returned values like so:

            let list: CompletionList;
            if (Array.isArray(value)) {
                list = new CompletionList(value);
            } else if (value instanceof CompletionList) {
                list = value;
                defaultSuggestions.incomplete = list.isIncomplete;
            } else if (!value) {
                // undefined and null are valid results
                return;
            } else {
                // warn about everything else
                console.warn('INVALID result from completion provider. expected CompletionItem-array or CompletionList but got:', value);
                return;
            }

Now the problem is that an extension writer doesn't get a compile error when returning a structure instead of a new'ed object but we still ignore his value since it doesn't comply to our checked. Working around this by not using instanceof but some manually property-by-property check isn't what my performance buddy would recommend.

[mhegazy]

I would say the check in value instanceof CompletionList does not match the interface declaration for provideCompletionItems; the interface just says return something that looks like CompletionList, nothing in there says it has to be an instance of CompletionList. i think what you really want is a nominal marker (#202) on CompletionList that says, rely i want an instance of CompletionList class that i have implemented and not just a structural match.

[kitsonk]

Yes, wouldn't something like this solve the problem:

class CompletionList {
  private __class = 'CompletionList';
  /*...*/
}

Now no duck can quack like CompletionList except CompletionList.

[jrieken]

Well, it just makes very ugly API if I have to put these markers in every class of our system. We use classes because we see additional benefit from that (like ctor logic, etc) but it seems TypeScript is not fit for that.

[jrieken]

does not match the interface declaration for provideCompletionItems

@mhegazy That's the whole point. The interface declaration says return CompletionItem or CompletionList which both are defined as classes, not interfaces.

[kitsonk]

We use classes because we see additional benefit from that (like ctor logic, etc) but it seems TypeScript is not fit for that.

But when consuming CompletionList why do you need it to be a created instance of the class when it is structured exactly the same. Or asked another why, why do you need nominal typing? If it is shaped like CompletionList you can do every operation you want on it in a safe way.

There is good reason why TypeScript is structurally typed, because of the permissiveness of the language it is a superset of. Many people have tried to project concepts of other languages into JavaScript because of familiarity only to find out that it doesn't work that way. ES6 Classes are nothing more than a constructor functions with an assigned prototype and a bit of logic which throws if you don't use a magical keyword new.

[jrieken]

We also allow people to write extensions in Javascript (in which you don't get type checking) and I want to be save in case where a client returns something which is neither a CompletionList nor a CompletionItem array but something more or less random

[kitsonk]

I think then what you are saying is that from an interface perspective, you would be completely fine with someone providing you an object that was structurally typed like CompletionList but the easiest and only effective runtime way to ensure the structure is like CompletionList is to use instanceof, but at runtime instanceof is effectively a nominal type guard, while at design time it is a structural type guard, meaning the behaviours diverge and could give end developers in either TypeScript or the Salsa language services a false idea that their design time behaviour will match the run time behaviour.

[jrieken]

yes, well said. maybe TypeScript should emit instanceof usage to function calls that do instanceof || all_properties_match

[kitsonk]

Not that this addresses the design time/runtime behaviour of instanceof, but since we (Dojo 2) don't generally use ES6 classes (or constructor functions for that matter) in a lot of code, instead of instanceof we often use custom type guards that do enough structural checking to be able to narrow the type. Even though we except people to use Dojo 2 without TypeScript or the Salsa language services, we believe that adding significant overhead in value checking at runtime, when there are specifically tools to avoid this at design time is a burden we wouldn't want to put on the end user of an application.

I realise there maybe a difference when you are trying to execute others code and need to try to do a level of guarding against people being malicious, but considering a custom type guard to weed out random "junk" maybe sufficient (because if the TypeScript team agree about the behaviour, it will be a while before it would become available).

[mhegazy]

We have got feedback previously about users thinking of classes nominally and interfaces structurally. but i have to say there is value in making these two interchangeable, specially for ambients, the way the definition author decided to write the declarations should not dictate your runtime semantics.

I do not think we can change that now anyways, or that would be a huge breaking change. An option would be what is tracked by #202, possibly adding a tag on the classes marking them as nominal.

[mhegazy]

I will bring this issue to discussion. #7271 seems to be the same issue. so marking this one as a duplicate of #7271.