Allow captured this in a class

[nycdotnet]

It's common for JavaScript code to capture this to a variable like self or that. One of the pain points when converting existing JavaScript functional-style "class" code to an idiomatic TypeScript class is the required chore of going through the file and replacing any instance of a captured this variable with an ambiguous this, likely breaking code that was already working; then, doing a second pass to figure out which functions need to be converted to lambdas to hopefully unbreak the code. This chore is tedious, error prone, and offers no business value; in fact it has a negative value at first. It's also a "day 1 sledgehammer" - the very people who are most likely to need to do this type of work are JavaScript developers giving TypeScript a try.

I think there's a very straight-forward solution for this: it should be legal to capture this at the top of a TypeScript class. For example:

class MyClass {
  var that = this;
  public name: string;
  public sayHello() {
    console.log(that.name);
  }
}

proposed emit:

var MyClass = (function () {
    function MyClass() {
        var that = this;
        this.sayHello = function () {
            console.log(that.name);
        };
    }
    return MyClass;
})();

The emit would work identically to how the automatic var _this = this; works when lambdas are used in a class. In the example above, that would be of type MyClass. I don't believe that it would be required to support this proposal in a defintion file except perhaps when typeof is used, in which case the name of the class could be substituted for the typeof expression. Multiple variables could plausibly be assigned to this, but it would remain illegal to declare variables in the class scope unless they are immediately assigned to this; the emit would have to be at the top of the function, above member definitions (same place as var _this = this; already is).

Having this feature would save a tremendous amount of busywork when converting large classes from JavaScript to TypeScript. I know there are a bunch of other this proposals out there, but I read through #513 and it didn't seem to have this particular idea.

Thanks very much for your efforts with TypeScript - it's such a great language to work with. I appreciate your consideration of this proposal.

[RyanCavanaugh]

Can you post an example of how this would be emitted?

[nycdotnet]

Done. Thanks, Ryan.

[mwisnicki]

Why limit assignments to this ?

[nycdotnet]

Why limit assignments to this ?

Limiting assignments to this prevents people from writing ambient code. Code in a class should be inside functions or on declarations only. If ambient code is desired, use of a module is more appropriate. At least, that was my thinking for this proposal and I think this is how TypeScript is currently designed.

[basarat]

Based on the proposed emit, it seems that perhaps these people should really just convert original code to lambda's to begin with, i.e.:

class MyClass { 
  public name: string;
  public sayHello = () => {
    console.log(this.name);
  }
}

This way :

• There is no new syntax
• They don't need to rename this to that

Alternatively this already works, but does have an ugly this.that:

class MyClass { 
  public that: MyClass = this;
  public name: string;
  public sayHello(){
    console.log(this.that.name);
  }
}

[nycdotnet]

Hey @basarat,

For this proposal, I purposefully selected an emit style to mirror the emit used with lambdas because it's a JavaScript style that the TypeScript team is already comfortable with (hopefully, because they chose it).

There is no problem with converting a two member class to use lambdas. This proposal is intended to assist people with many large/complex function-style "classes" with many members. It's far too difficult currently to do all of the refactoring required to take advantage of proper TypeScript classes. Choosing to do so requires people to risk breaking code that is already working correctly - particularly if you start getting into situations with nested callbacks. Saying "just rewrite it" doesn't fly because there's no obvious benefit.

I'm almost done with my Pluralsight video and it should hopefully be out in a few weeks. Probably a good 10 minutes in the course is spent fighting with converting a big JS function-style "class" to TypeScript just messing around with this and rewriting stuff as lambdas. I wouldn't mind if it were new code, but this was code that was already working and this was settled (as self). By converting it to use this, I broke it. It took two or three tries to get it right again after doing find and replaces, etc. If someone came to me and asked "Was all that time you just spent rewriting that function as a class worth it?" I would have a hard time saying that it was - at least not immediately. It used to be clear at a glance when we meant the class and when we meant the contextual this due to the variable name. After the "improvement" to using a proper class, we have to look at what style of function we're in to figure it out. While I think ultimately there could be a benefit, for large function-style classes it's almost best to leave them alone today and just write an interface.

With this small feature, that whole battle goes away and dealing with this in an already-working function that you're converting to a class takes all of 5 seconds. Your team can keep the capture variable they've grown accustomed to. To me, there's just so many practical benefits.

[basarat]

Saying "just rewrite it" doesn't fly because there's no obvious benefit.

Got it. So you basically want to make it easier to change the codegen to that (with ()=> semantics) without forcing them to rewrite it. Sorry I took some time to understand ❤️

With this small feature, that whole battle goes away and dealing with this in an already-working function that you're converting to a class takes all of 5 seconds

Was the old code using the revealing module pattern?

[nycdotnet]

The original code in my case was using the style demonstrated in the Knockout tutorials. For example, http://learn.knockoutjs.com/#/?tutorial=loadingsaving

[RyanCavanaugh]

Upon closer inspection, this behavior is far too subtle -- using the var here in sayHello changes it from a prototype member to an instance function. That has a ton of implications that are not immediately obvious, and it does need to be apparent from looking at a member whether it's instance or prototype.

Ideally editors would be able to provide a refactoring here to ease the JS transition path.

[nycdotnet]

Thanks. That is a really good point, Ryan.

I think you're saying that this proposal leaves the door open to function keyworded functions behaving like lambdas and I think that's fair.

How would you feel if this were restricted to only lambda-style methods via a compiler error? That would at least give the users an explicit "to-do" list to work against, instead of manual hunt and peck. Don't forget it's impossible to know which this someone meant to refer to after uncapturing it and doing a rename.

I'm not sure how an editor could provide a reliable refactoring for this scenario without supporting capture in the language. If capturing were allowed, then "right-click, convert method to a lambda" would be pretty straight forward to implement as a refactoring.

[nycdotnet]

For example:

This code (sample 1):

class MyClass {
  var that = this;
  public name: string;
  public sayHello() {
    console.log(that.name);
  }
}

Would produce an error like this:
TypeScript Compiler error - line 5: captured 'this' is not accessible in standard class methods. Consider changing the method to a lambda or use the 'this' keyword instead. Variable 'that' is a captured 'this'.

If emit on error is allowed, it would emit this non-working JS (sample 2):

var MyClass = (function () {
    function MyClass() {
        var that = this;
    }
    MyClass.prototype.sayHello = function () {
        console.log(that.name);
    };
    return MyClass;
})();

At runtime: that is undefined on line 6.

Let's assume that this proposal (as amended to only work in lambdas) is implemented. TypeScript would raise an error in code sample 1 on line 5 and suggest that the user change the function to a lambda.

Since TypeScript gives the user an error, they know exactly where they need to refactor and what to do. Most importantly, once they finish their refactor, their code doesn't break in mysterious ways:

After refactoring (sample 3):

class MyClass {
  var that = this;
  public name: string;
  public sayHello = () => {    // now sayHello is a lambda.
    console.log(that.name);
  }
}

Emit (valid) (sample 4):

var MyClass = (function () {
    function MyClass() {
        var that = this;
        this.sayHello = function () {
            console.log(that.name);
        };
    }
    return MyClass;
})();

Let's be very clear on what this is proposing. We need a road to take a user from a JS constructor function-style "class" to a TS class. There is a current road, and the proposed one. The destination of both roads is the same. In today's TypeScript, the user eventually (and painfully) lands on code that is identical to sample 3, except that this is used instead of the capture variable name that they preferred.

Along today's road, the user is forced into the pit of despair: manual find and replace for the capture variable name to this and then having to figure out which this they really needed - contextual this or lexical this (which was formerly their familiar capture variable). Most critically: no help is provided by TypeScript in the form of errors.

This proposal (as amended) offers a way to force the users into the pit of success by providing good error messages that help the user fix their code to the correct TypeScript form. With language support implemented, this process could even be automated with tooling.

Without language support for a capture variable, any tool that would reliably and correctly implement this as a refactoring (considering complexities like nested this in multiple callbacks) would probably start to resemble the TypeScript language service anyway. If I'm wrong on that, please let me know.

I'm going to set you up as a contributor on a private repo that contains the demo project that I cooked up for my Pluralsight course (http://www.pluralsight.com/courses/typescript-practical-migration). In the course, I spend about 10% of the screen time refactoring a large JS constructor function-style "class" to a TypeScript class (module 3, clips 2 and 3), and that was even with rendering the video at 130% speed for a lot of the typing and cutting out a lot of the mechanical find/replace stuff and changing functions to lambdas. Even as a very experienced TypeScript developer and supported by unit tests, it took me 3 or 4 tries to convert every part of the code base to lambdas correctly (a few nested callbacks got me). This is all with no compile-time error support - and for code that was previously working!! The experience is just so painful.

With this enhancement to the language, I'd just have to run through an error list, plus it's valid ES6 and there are no breaking changes. That's the TypeScript promise.

Thanks, Ryan. Please consider this proposal as amended to only work with lambdas and to add a new error if a captured this is used in a standard function member.

[basarat]

first off : don't 🔫 me .... but:

class MyClass {
  var that = this;
  public name: string;
  public sayHello = () => {    // now sayHello is a lambda.
    console.log(that.name);
  }
}

Refactor to: (sample bas)

class MyClass {
  public name: string;
  public sayHello = () => {    // now sayHello is a lambda.
    console.log(that.name);         // that is a compiler error
  }
}

The compiler should guide you through the rest of the refactoring at this point?

At this point (sample bas) I can add a change that to this and change self to this style refactoring at each error location in atom-typescript if thats helpful?

[basarat]

Perhaps a better one would be:

class MyClass {
  that = MyClass;
  constructor(){
       this.that = this;
  }
  public name: string;
  public sayHello = () => {
    console.log(that.name);    // Error Here
  }
}

Supported by I meant to use this style quick fix at each error location.

[danquirk]

Definitely understand the desire to make this better. That said, if you scope your proposal to fat arrow based members does it really do what you want? If you had fat arrow functions in your JS code then aren't you targeting ES6 already in which case you'd have just written a real class rather than an ES5 style function based class? And in an ES6 class you'd can't have 'var that = this' at the top level outside the constructor either.

To me this feels like a great opportunity for an IDE to provide value in a refactoring. You could imagine a large scale 'turn this function into a class' refactoring or a smaller scale 'factor out that=this' (and the reverse).