Skip to content

Safe keyword alternative to "this" #30203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
5 tasks done
john-larson opened this issue Mar 3, 2019 · 5 comments
Closed
5 tasks done

Safe keyword alternative to "this" #30203

john-larson opened this issue Mar 3, 2019 · 5 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript

Comments

@john-larson
Copy link

Search Terms

this keyword

Suggestion

“this” keyword in Javascript is context dependent, which means its value depends on the calling function. And this is one of the culprits of most subtle and latent run-time errors in Javascript.

The first design goal stated by Typescript in its official documentation is:

Statically identify constructs that are likely to be errors.
https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

When “this” is used in callback functions or in functions given to forEach as argument, IDE rightfully cannot raise any compile-time errors, giving us the false sense of security, but we get run-time errors because “this” is undefined.

There seem to be two work-arounds:

  1. Using arrow functions
  2. Using .bind(this) syntax

In accordance with the first and foremost design goal of Typescript, I propose a new keyword that will be the alternative of "this" and will always point to the instance of the class.

Examples

Here is a sample ts code:

class RequestManager{

    private successMessage = "Xhr successful.";

    makeRequest() {
        var oReq = new XMLHttpRequest();
        oReq.addEventListener("load", this.responseHandler);
        oReq.open("GET", "www.google.com");
        oReq.send();
    }

    private responseHandler() {
        window.alert(this.successMessage);
    }
}

var reqManager = new RequestManager();
reqManager.makeRequest();

This is what it transpiles to:

var RequestManager = /** @class */ (function () {
    function RequestManager() {
        this.successMessage = "Xhr successful.";
    }
    RequestManager.prototype.makeRequest = function () {
        var oReq = new XMLHttpRequest();
        oReq.addEventListener("load", this.responseHandler);
        oReq.open("GET", "www.google.com");
        oReq.send();
    };
    RequestManager.prototype.responseHandler = function () {
        window.alert(this.successMessage);
    };
    return RequestManager;
}());
var reqManager = new RequestManager();
reqManager.makeRequest();

And in the following piece of code, I added a "self" variable to always point to the class instance:

var RequestManager = /** @class */ (function () {
    var self;
    function RequestManager() {
    	self = this;
        self.successMessage = "Xhr successful.";
    }
    RequestManager.prototype.makeRequest = function () {
        var oReq = new XMLHttpRequest();
        oReq.addEventListener("load", self.responseHandler);
        oReq.open("GET", "www.google.com");
        oReq.send();
    };
    RequestManager.prototype.responseHandler = function () {
        window.alert(self.successMessage);
    };
    return RequestManager;
}());
var reqManager = new RequestManager();
reqManager.makeRequest();

The new keyword that will be alternative to "this" can create such a variable when transpiling.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@dragomirtitian
Copy link
Contributor

dragomirtitian commented Mar 3, 2019

Adding new keyword in expressions would contradict this guideline:

This wouldn't change the runtime behavior of existing JavaScript code

Adding new syntax to JS expressions is not something the Typescript team does lightly as new developments in ECMAScript might end up conflicting with these.

There is also currently a perfectly decent workaround to this issue, use arrow functions:

class RequestManager{

    private successMessage = "Xhr successful.";

    makeRequest() {
        var oReq = new XMLHttpRequest();
        oReq.addEventListener("load", this.responseHandler);
        oReq.open("GET", "www.google.com");
        oReq.send();
    }

    private responseHandler = ()  => {
        window.alert(this.successMessage);
    }
}

var reqManager = new RequestManager();
reqManager.makeRequest();

Or use bind in the constructor (constructor() { this.responseHandler = this.responseHandler.bind(this) }), or when sending the function to addEventListener (oReq.addEventListener("load", this.responseHandler.bind(this));)

I feel particularly the first version is not any more burdensome than the proposed new keyword.

@j-oliveras
Copy link
Contributor

Your proposed code has a problem: only allows a single instance of the class RequestManager (I changed the code inside the class prototype methods to run it on node):

var count = 0;
var RequestManager = /** @class */ (function () {
    var self;
    function RequestManager() {
    	self = this;
        self.successMessage = "Xhr successful: " + count;
        count++;
    }
    RequestManager.prototype.makeRequest = function () {
        self.responseHandler();
    };
    RequestManager.prototype.responseHandler = function () {
        console.log(self.successMessage);
    };
    return RequestManager;
}());
var reqManager = new RequestManager();
var reqManager2 = new RequestManager();

reqManager.makeRequest();
reqManager2.makeRequest();

Will output:

Xhr successful: 1
Xhr successful: 1

Anyway, this will contradict the This wouldn't change the runtime behavior of existing JavaScript code guideline.

@j-oliveras
Copy link
Contributor

This issue seems the round two of your issue #30073.

@RyanCavanaugh answered you there: see #513.

@john-larson
Copy link
Author

john-larson commented Mar 3, 2019

@dragomirtitian I already mentioned the work-arounds in my original post but my point is, isn't it the goal of Typescript to write statically checked code so you will not face run-time errors for things that the compiler can check? And I think "this" is on top of that list. And we should not be resorting to work-arounds for such a fundamental construct of the language. Just assume you forgot to use an arrow function or a .bind(). Compiler will not be able to raise an error and you will encounter the error in run-time. Isn't this the exact kind of problem Typescript is trying to solve?

@j-oliveras I would like to point out that in #30073, I was proposing that "this" inside a class should always reference the class instance but as @RyanCavanaugh mentioned, this would contradict the current behavior of "this" in current Javascript classes. That is why I proposed a new keyword in my new proposal.
And regarding the problem in my sample code, you are right. I am able to accomplish the desired behavior this way:

var count = 0;
var RequestManager = function () {
    var self = this;
    self.successMessage = "Xhr successful: " + count;
    count++;

    this.makeRequest = function () {
        responseHandler();
    };

    var responseHandler = function () {
        console.log(self.successMessage);
    };
};
var reqManager = new RequestManager();
var reqManager2 = new RequestManager();

reqManager.makeRequest();
reqManager2.makeRequest();

But using an iife as in the code that Typescript transpiles to, it fails as you mentioned. I would appreciate it if you could suggest a solution that will work with the code that Typescript transpiles to right now.

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints labels Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants