Skip to content
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

False Sense of Safety When Using “this” Keyword #30073

Closed
john-larson opened this issue Feb 24, 2019 · 6 comments
Closed

False Sense of Safety When Using “this” Keyword #30073

john-larson opened this issue Feb 24, 2019 · 6 comments
Labels
Duplicate An existing issue was already created

Comments

@john-larson
Copy link

john-larson commented Feb 24, 2019

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 run-time errors in Javascript.
On the other hand, Typescript gives a different meaning to it and uses “this” keyword to refer to class properties from inside class methods.

But the problem is, when “this” is used in callback functions or in functions given to foreach as argument, IDE does not 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

I believe that it should not be necessary to use these work-arounds and Typescript should be doing correct static type checking of “this” as long as we are not disabling it by using “any” etc. Because it is the main reason we are using Typescript.

I do not have much idea about the inner workings of Typescript, but I assume this could be accomplished by doing a “let self = this;” kind of assignment in the transpiling process and using this variable when “this” is used in code.

Example code (obj.fe2() works whereas obj.fe() does not)

@j-oliveras
Copy link
Contributor

If I understand correctly, probably the option noImplicitThis solve most of the problems with this that you describe:

function aaaaa() {
  this.z = 1;
}

[1, 2, 3, 4].map(function () { this.x = 0; })

Playground (enable the option under Option button).

With that option enabled, flags both this usages with 'this' implicitly has type 'any' because it does not have a type annotation..

See compiler options.

You can define the type of this inside a function with that syntax:

function aaa(this: { x: number }) {
  this.x = 1;
}

Playground.

@john-larson
Copy link
Author

Enabling noImplicitThis will only force you to define this as a function argument with its type but this does not solve the problem. Compiler will not complain but this will still be undefined for 'use strict' and window otherwise.

Here is an example:

Playground

Calling obj.fe() will not work whereas calling fe2() will work.

@john-larson john-larson changed the title False Sense of Security When Using “this” Keyword False Sense of Safety When Using “this” Keyword Feb 24, 2019
@DanielRosenwasser
Copy link
Member

Issue #7968 covers strict this checks (called --strictThis when originally implemented in #6018).

@RyanCavanaugh RyanCavanaugh added the Unactionable There isn't something we can do with this issue label Feb 25, 2019
@RyanCavanaugh
Copy link
Member

See #513 and start following links from there 😉

@john-larson
Copy link
Author

john-larson commented Feb 25, 2019

@RyanCavanaugh I certainly think this issue deserves to remain open as none of the items listed in #513 (or in #7968, #6018 for that matter) is related to what I am proposing here. I am proposing that this inside a class method should always be correctly referencing the class instance as is expected. (I say as expected because it is what the transpiler assumes without options like --strictThis and it is also what developers assume since you need to use this in order to reference class properties from inside the methods). Feel free to reference this issue from other summary issue lists but I think this issue should be reopened. Please reconsider it.

@RyanCavanaugh
Copy link
Member

I am proposing that this inside a class method should always be correctly referencing the class instance as is expected

Classes are a JavaScript feature. TypeScript does not alter the runtime behavior of JavaScript, including behavior around this.

@DanielRosenwasser DanielRosenwasser added Duplicate An existing issue was already created and removed Unactionable There isn't something we can do with this issue labels Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants