Why not check classes?

[jtremback]

"This library will not check classes. Instead, you are encouraged to use the native instanceof operator."

Wouldn't it be easy to have this library generate the instanceof code? What's the reason not to?

[woutervh-]

Hi @jtremback

Consider the following two files:

// a.ts

class A {}
export interface B {
  a: A;
}

// b.ts

import {B} from './a';
import {is} from 'typescript-is';

is<B>({});

When I generate the code I can easily output something like obj instanceof A but A is not available at runtime. I would have to figure out how to import it (if it can even be imported, think anonymous classes, non-exported classes). I don't know a good solution to this problem.

If you can think of something, that would be great :)

[LoganDark]

@woutervh- Export something in that same file which is the is<> function, then import that and use it in the other file. That way the class stays private, but your is<> function works, everyone is happy

[woutervh-]

@LoganDark

Thanks for the suggestion.
Sounds like a plausible work around, but I get the feeling that will open up a can of worms. Just a couple of questions that immediately come to mind:

• Globals: how should I reference a class that has been declared globally, or rather: how should I detect that it was declared globally.
• Import/export style: the user's project might be configured to use CommonJS, AMD, ES6, etc. style kind of imports. If I generate some import/export statements that don't follow the same style, the compiler might get confused.
• Import/export resolution: if I generate some imports, should it point to relative path, absolute path, some vendor prefixed path? What kind of resolution is the user using? I'm not sure what the best way to generate the import paths is.
• Import/export shape: if I start putting new stuff into people's module.export, maybe their existing code will break because it does something like Object.keys(importedModule)... or who knows what else.

In general the problem is that I have to reference something at run time which can't (or with great difficulty) be guaranteed to have a reference to.
Of course we can make all kinds of assumptions and make something that works in 80% of the cases, for example with with the solution you propose, but then it should be the non-default behavior due to all the problems that it can possibly have.

[woutervh-]

Closing for house keeping.

[LoganDark]

I know this is old, but: is<SomeClass, ReferenceToAnotherClass, ReferenceToAnotherClass2> could generate instanceof checks for ReferenceToAnotherClass and ReferenceToAnotherClass2. To solve the problem of internal classes you don't have references to, you could define a function in a scope that has access to those classes, and call it from anywhere that doesn't. I think that would solve the problem very nicely.

An alternative syntax would be is<SomeClass>(ReferenceToAnotherClass, ReferenceToAnotherClass2), if either TS doesn't like is<Class, ...> or it doesn't seem right... Just a thought or two

[woutervh-]

Re-opening for discussion.

Hi @LoganDark

Maybe it could work in some cases... can you show in code how you would do this?
I can also think of some cases where it would be (almost?) impossible. For example:

const classGenerator = (/* ... */) => {
    return class { /* ... */ };
}

const y = new (classGenerator(/* ... */))();

// Later on in the code...
is<typeof y>(y); // should return `true` - but what will you write here: y instanceof ??

Maybe the better trade-off is to assume that the name of the class is available within the scope of the call to is as-is, and treat this as a requirement of the contract of is.
E.g.:

...
{
  ...
  is<X>(x); // will be implemented with `x instanceof X` - requires that `X` is available in the scope.
}

It will probably cover most cases I think, usually classes are used in such a way that the name is available in the scope, e.g.:

• Globally defined classes such as Date and RegExp.
• Custom classes imported as constants: import { X } from '...';
• Custom classes declared in the module/scope.

Like mentioned before, it will work as long as the actual class is available at the call site as-is.

[LoganDark]

That first code block is some of the most confusing and nonsensical code I've ever seen in a serious context. Are you sure that people actually do that?

Nevermind, it's just all the symbols that were confusing me. It's just a function that returns an anonymous class.

But, uh... TypeScript generates temporary variables all the time. Of course it would make sense to auto-generate variables for every argument/whatever to stop it from getting reevaluated tons of times. So, I dunno.

is<typeof y>(y) could go something like (simplified example):

((value) => {
    const temp1 = typeof y
    // ...
    if (!(value instanceof temp1)) return false
    // ...
    return true
})(y)

The fact that you're checking that something is an instance of itself is programmer error, not typescript-is error. I'm not sure why you'd need to check if y instanceof typeof y, but... uh... you do you.

Could you clarify a bit lot on what exactly is the problem there? Because I don't understand at all what you're saying.

[LoganDark]

By the way, that's not exactly what was intended... I think my original (right after issue close) post was a bit weird, but...

class SomeClass { /* ... */ }

interface Thingy {
    someClass: SomeClass
    // ...
}

is<Thingy, SomeClass>({someClass: SomeClass()})

This is what I mean.

Optionally:

is<Thingy>({someClass: SomeClass()}, SomeClass)

however, this would break many many things, especially since you can't infer which types to generate instanceof checks for without the function being called immediately (therefore making it impossible for the programmer to say, for example, checkIfThingy = is<Thingy, SomeClass>), so I much prefer the type-argument version.

Of course, there's always a case like this:

interface Thingy {
    class: SomeClass
    class2: SomeClass2
}

// in a scope without SomeClass2...
is<Thingy, SomeClass>({class: SomeClass(), class2: (class {})() /* haha */})

where the instanceof check for class2 would not be generated. So, that would probably pass the check. Nothing you or anyone can do about it, that's 100% programmer error and not your problem(™). although just as TS IDEs support autocomplete, you could check for public members of the class.

Built-in classes like Date and RegExp should always have instanceof checks generated, if you're paranoid about this breaking existing code make it an opt-in config option.

[woutervh-]

Hi @LoganDark

The point I was trying to make with is<typeof y>(y) is that y could be replaced with anything (but should return true in case it is y). And even if it's a programmer error, it can (will) still happen, so it should be accounted for. Maybe this clarifies the problem even better:

const classGenerator = (/* ... */) => {
    return class { /* ... */ };
};

const y = new (classGenerator(/* ... */))();

function example (arg: any) {
  is<typeof y>(arg); // Find implementation for `is<typeof y>`
}

The idea of putting typeof y into a variable is interesting in the case of is<typeof y>, and I think it would be a suitable solution for this specific case. But again, there are so many more cases that should be accounted for, that would require a new solution each time, so I don't see this as a feasible solution because it would take a lot of effort and add a lot of complexity.

The idea of putting the runtime classes as an argument to the is function is also interesting. It basically moves the responsibility of finding the runtime classes from the library to the user. I think this is a better approach than the previous one. The user will run into the same problems as the problems I described before which are a challenge to the library, but we're basically saying "now it's your problem, not ours". I am however wondering how we can force the users to supply the correct classes at runtime. E.g.:

interface Thingy {
  class: SomeClass;
}

// We should require that the 2nd argument is really of type `typeof SomeClass`
is<Thingy>({}, SomeClass); // Allowed!
is<Thingy>({}, SomeOtherClass); // Not allowed!

And I'm also wondering how to determine the order of the parameters or the structure of the parameters which supply the runtime classes.

[LoganDark]

Your first code block is still trivially solvable by putting typeof y in a variable... I STRONGLY prefer this to passing arguments to the function at runtime, because verifying this at runtime will be impossible. You'd have to verify the runtime arguments at compile time like TypeScript does with everything else, and even change the function generated based on those arguments. All around I really don't think it's a good idea.

What I'm trying to get at with is<Thingy, SomeClass> is that SomeClass ends up being an expression that references that class, allowing you to easily put it in a variable.

Thinking a little further, I believe you should put that variable right outside the function so it isn't re-evaluated every time it's called. You can use regular old {} blocks for this for scoping, or use an IIFE.

TypeScript knows the type of pretty much every expression, so with the template-argument syntax (which basically configures the function's structure before it gets called!! which is important) you can easily verify that the passed classes occur somewhere within the interface you're checking for.

[hmil]

Hi there,

Would you be open to reconsider this entire argument from scratch?

tl;dr: Just have is<ClassType> behave exactly like is<InterfaceType> and forget about instanceof altogether.

---

I think you've got the concept of class type and class constructor mixed up here.

In this expression, Foo is the constructor of the Foo class:

doSomethingWith(Foo)

In this expression however, Foo is a type reference referencing the instance type of the Foo class

doSomethingWith<Foo>()

In the latter, it doesn't matter if Foo references a class or an interface or an inline type, the code must work the same way.

This is what it means for TypeScript to be a structurally typed language.

---

The class syntax in TypeScript is tricky. It creates both a type and a variable, but the relationship between those is far from obvious.

The variable is the constructor. The type is the instance type. Effectively we have:

const Foo: typeof Foo = { new(): Foo }

The type Foo is not the type of Foo (the runtime symbol), it is the type of the instance created by Foo.

An instanceof check for x only asserts that ConstructorType<typeof x> === typeof Foo which is very different from the assertion that typeof x === Foo.
That is why instanceof checks must never be mixed up with regular type checks.

---

If this library should have an instanceof utility (maybe it should not, but let's assume it should for the sake of argument), then it should be a different operator than the operators that check for interface types.
You'd expect to have

is<Foo>(bar); // Structural checking
isInstanceOf(Foo, bar); // constructor checking

I am not even suggesting the syntax isInstanceOf<Foo>(bar) because it is in complete violation with the semantics of TypeScript. bar cannot possibly be an instance of the type Foo because it's a type and not a constructor. If you really want a template syntax, then the correct version would be isInstanceOf<typeof Foo>(bar), but even that has issues.

---

Now let's go back to our inital question: what to do with classes?

It should make no difference if Foo is an interface or a class type in this case either:

is<Foo>(candidate)

I was surprised to see that typescript-is choked on this code. As a matter of fact I wasn't even aware that the type I was checking was a class type. I thought it was an interface. The point is: it should not even matter if it's a class or an interface. This is TypeScript, only the structure matters.

---

Sorry for the wall of text. This discussion has already been going on for a while and I figured I'd need to bring serious arguments to the table in order to change your minds.

Bottom line is: Can we just have classes work the same way as interfaces?

[woutervh-]

Hi @hmil

I think the suggestion you propose to treat it the same as an interface, and not bother with instanceof, is good.

What do you think of these options?

1. Only enable this through an option,
2. Enable it by default.
3. Enable it by default, but give a warning in the compiler output.

I'm wondering what everyone else thinks of this, because in my mind I don't want to give users the false impression that it will check for classes, and then it turns out that the user thought they had a certain class, but they don't.

[hmil]

Either option 2 or 3 seem good to me. Or maybe 3, but give an option to turn off the warning for people who "know what they're doing".

One major use case I did not cite in my message above is that Java devs coming to TypeScript will tend to use classes as DTOs (because they're used to POJOs). This situation seem to happen quite often, at least in my experience.

Obviously it'd be great if other people could comment on this issue to give their opinion.

[acomagu]

This is TypeScript, only the structure matters.

In my opinion, but actually nobody want to treat non-Foo object as Foo instance. If we really want, we can create new interface extending the class. It's enough to make possible to accept such interfaces for typescript-is. I think it's better to crash when it face classes, because it's bigger problem that I cannot notice the type is including classes.

I +1 the solution giving class as arguments. like

is<Thingy>({}, SomeClass);

Or using decorator like other library is also good solution.

interface Thingy {
    @classType(SomeClass)
    someClass: SomeClass;
}

[Venryx]

I think the suggestion you propose to treat it the same as an interface, and not bother with instanceof, is good.

What do you think of these options?

1. Only enable this through an option,
2. Enable it by default.
3. Enable it by default, but give a warning in the compiler output.

Option 2 sounds best to me (though option 1 and 3 are also fine).

The reason I don't have issue with is<Class>(classInstance) checking for mere shape-matching by default, is that this behavior is the natural assumption for me: if I wanted to check for prototype matching, I would just have used instanceof. (in other words: "I got this library for shape-checking, not prototype checking -- as that's already easy to do myself. So why isn't it letting me shape-check against a type, just because that type is defined through a class!?")

Granted, my "natural assumption" described above is apparently not shared by all developers (eg. @acomagu), and I can understand why.

Thus, I am fine with any of the three options -- just so long as there's a way to achieve the described-above behavior in a project-wide/set-it-and-forget-it way. (though option 2 is still my preferred, with just a note in the readme saying that if you use the is function with classes as the passed type, it checks by shape, not by prototype)

EDIT: And for people who want to make sure an object is matching both by shape and prototype (if option 2 is implemented), they can just do: is<ClassX>(obj) && obj instanceof ClassX

This gives them higher assurance that the object will interact as expected, since not only is the prototype correct, but the properties are also the correct types and shape.

This level of assurance is not possible by merely inserting an instanceof ClassX check, since in some cases a property might be modified to be an invalid type (for example, through bad conversion of data from input-elements), while the object's prototype remains "correct".