microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.61k stars 12.44k forks source link

Allow type system to represent `this` in function inputs/outputs #285

Closed fsoikin closed 9 years ago

fsoikin commented 10 years ago

The early discussions about generics had this feature outlined, but it never made it into the final version for some reason. Meanwhile, it would be extremely helpful in some cases. The use case that it most dear to my heart is Knockout:

interface Observable<T> {
   <U>( this: U, value: T ): U;
}

var viewModel = {
   Str: ko.observable( "abc" ),
   Num: ko.observable( 5 )
};

viewModel.Str( "xyz" ).Num( 10 );
basarat commented 10 years ago

Use case ported :

interface Base{
    clone():Base;
}
interface Child extends Base{}

var child:Child; 
var anotherChild = child.clone(); // Base. Should be Child.

With this:

interface Base{
    clone<this T>():T;
}
interface Child extends Base{}

var child:Child; 
var anotherChild = child.clone(); // Child

Syntax is up for debate. But I believe this should be at the generic argument list location.

danquirk commented 10 years ago

How different is this from #229? If 'this' was allowed as a parameter type we could also look at allowing it as a return type to accomplish what you want here? Ex, @basarat's example without generics:

interface Base {
    clone(): this;
}
Bartvds commented 10 years ago

+1 for this as type, it would solve some problems of writing 'chainy' API's, and with clean and semantic syntax.

It wouldn't solve #229 or the OP, which are more for callbacks and handlers with changed scope.

fsoikin commented 10 years ago

This is not different from #229 at all (as far as I can see).

I must, however, note that the example in your comment is needlessly narrowed to a special case. I would rewrite it something like this:

interface Base {
   clone<T>( this: T ): T;
   multiply<T>( this: T ): T[];
   wrap<T>( this: T ): SomeWrapper<T>;
}
fsoikin commented 10 years ago

Forgot to add an opposite example:

interface UnwrapFunction {
  <T>( this: SomeWrapper<T> ): T;
}
basarat commented 10 years ago

@danquirk that issue is more focused on this inside the function. This issue is focused on this outside the function (i.e. return value or inputs). Also putting it in generic arguments allows you to restrict the type with extends e.g. this T extends Base:

interface Base{
    clone<this T extends Base>():T;
}
interface Child extends Base{}

var child:Child; 
var anotherChild = child.clone(); // Child

For a demo of function input consider merge below (probably not a good example).

interface Base{
    merge<this T extends Base>(other:T):T;
}
danquirk commented 10 years ago

@basarat right, but if we go as far as @RyanCavanaugh suggests in that issue and consider how it affects assignability with function types then allowing it to be used in other signature positions for assignability is likely a small additional step.

basarat commented 10 years ago

suggests in that issue and consider how it affects assignability with function types then allowing it to be used in other signature positions for assignability is likely a small additional step.

@danquirk agreed this can be marked a duplicate :+1:

RyanCavanaugh commented 10 years ago

There are two very different suggestions going on here that I don't want to conflate.

The first is whether or not you can accurately represent clone-type operations:

function createClone<T>(t: T): T {
    return t; /* TODO: science! */
}
class Animal {
    clone(): this { return createClone(this); }
    //       ~~~~  
}
class Dog extends Animal {
    woof(): void { }
}
var x = new Dog();
x.clone().woof(); // Should not error; `clone` currently returns `Animal` and causes error

The other issue is the type of this inside a function body:

$('li').each(function() {
    // http://api.jquery.com/each/
    console.log(this.nane); // Should be an error
                ~~~~ type of 'this' should be HTMLElement here
});
DanielRosenwasser commented 9 years ago

I'm pretty sure that we on the team understand it, but let me just be clear on the fact that allowing this-typing on a parameter is not actually type safe.

Consider the following:

class A {
    a: number;

    // This would *clearly* be the ideal usage of 'this'-types as a parameter.
    compare(other: this): boolean {
        return this.a === other.a;
    }
}

class B extends A {
    b: number;

    // Now override A's 'compare'.
    compare(other: this): boolean {
        return this.b === other.b;
    }

}

var a1: A = new A();
var a2: A = new B();
a2.compare(a1);

So this would fully type check, but a2's type at runtime is actually B, and a2.compare will try to access a1.b which doesn't actually exist.

So really, I'd prefer that if we ever did this, we restricted this as a return type.

saschanaz commented 9 years ago

It seems that it can be a problem even without this-typing. This code is basically same with yours and does not cause any compiler error.

class A {
    a = 1; // default

    // This would *clearly* be the ideal usage of 'this'-types as a parameter.
    compare(other: A): boolean {
        return this.a === other.a;
    }
}

class B extends A  {
    b = 2; // default

    // Now override A's 'compare'.
    compare(other: B): boolean {
        if (!("b" in this) || !("b" in other))
            throw new Error("It seems there is an error...");
        return this.b === other.b;
    }

}

var a1: A = new A();
var a2: A = new B();
a2.compare(a1); // This still throws a runtime error but not a compile-time error.
RyanCavanaugh commented 9 years ago

I don't think that example is coherent either way, since B behaviorally breaks the substitution principle. It's not correct for it to subclass A and add exception cases to compare.

nathggns commented 9 years ago

Defining the type of this within a function body is covered by #1985

emanuelpalm commented 9 years ago

@DanielRosenwasser Only allowing this for return types is still very useful. I'd say it's entirely justifiable as long as the compiler outputs a decent error message when trying to use it elsewhere.

I would propose the keyword to be This, with capital T, just to differentiate it from the property this.

Any news on when this feature might land in a stable release?

bluong commented 9 years ago

Would very much like an update into this issue

DanielRosenwasser commented 9 years ago

Hey @bluong, I think we're still interested in it, but ES6 work has dominated much of our time. We're still also lacking a solid proposal.

pmccloghrylaing commented 9 years ago

I'd love to see this as a return type for chaining methods implemented - it would greatly improve the node.js type definitions, which currently don't support things like:

readStream.on('error', ...).pipe(writeStream);

The biggest issue for this as a return type is inheritance - chaining methods need to be differentiated from clone-like. My suggestion would be to implement this as a return for chaining methods first, e.g.

class Chainer {
    doSomething(...) : this {
        // do something
        return this;
    }
}

Classes with chaining methods can be safely inherited from while maintaining this as the return type. So any methods specifying this as a return type need to return this and nothing else.

For clone-like methods maybe a generic this<T> would work to show that the particular implementation returns T, so must be overridden in derived classes.

class Cloner {
    clone() : this<Cloner> {
        return new Cloner();
    }
}
class DerivedCloner extends Cloner {
    clone() : this<DerivedCloner> {
        return new DerivedCloner();
    }
}

It should be possible for these return types to be used to override each other, as well as specifying the base class or derived class as the return type (instead of this).

hypery2k commented 9 years ago

any news on this issue?

danquirk commented 9 years ago

@hypery2k the most recent proposal is https://github.com/Microsoft/TypeScript/issues/3694. We have a few different issues tracking the various ways people want to specify this types. Suffice to say we know it's a top request but it's also complicated to support correctly :)

DmitryEfimenko commented 9 years ago

+1

mhegazy commented 9 years ago

this is now tracked by #3694