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

Flag for strict default function this types for call-site and assignability checking (--strictThis) #7968

Open
mattmccutchen opened this issue Apr 8, 2016 · 14 comments
Labels
Add a Flag Any problem can be solved by flags, except for the problem of having too many flags Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@mattmccutchen
Copy link
Contributor

#6018 was originally going to add a --strictThis flag that made function this types default to void or the enclosing class (instead of any) for purposes of call-site and assignability checking, but that functionality was dropped (details). This is a follow-up suggestion for the dropped functionality, whatever the flag ends up being named.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Apr 8, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Apr 8, 2016

Please see #7689 for the relevant desing discussion

@jcalz
Copy link
Contributor

jcalz commented Aug 31, 2017

I just ran into a problem where the obvious solution would be that methods without an explicit this parameter default it to the this type. Searching brought me here. So the main problem is... performance? In any case, I hope a --strictThis option makes it into TS sometime (adding my 👍 ).

@flash1293
Copy link

Maybe we can revive this thing as I believe it would bring a lot of benefit.

Regarding the "backwards compatibility" problem - is this due to class-this context for methods which in reality should stay any because they are allowed to be called from different contexts? If yes, --strictThis could only implicitly set the context to class-this for methods which have a typescript implementation and actually use this in the method body. DefinitelyTyped could adapt step by step and pure typescript code bases could use the improved type check right away.

@tejasmanohar
Copy link

tejasmanohar commented Nov 21, 2018

I also hope this is added back. I just started using Typescript last week, and this is basically the only "type error" I've run into (and repeatedly for that matter) in my all Typescript codebase. I hit it frequently, as I am using an ES6 classes as a router/controller in my Express app, e.g. app.use(this.someClassFunction). When I see failing tests with an error like cannot access property someClassFunction of undefined, I just change the declaration syntax to ES7 function bind syntax class X { constructor() {} x: (req, res) => {} } and the issue is fixed, but it would be really nice to know about that at compile time instead of at runtime.

I've been impressed with Typescript in general but was very surprised to hear that it's not checked at compile time since I can't think of another typed language, where such an error is possible.

@JojOatXGME
Copy link

Hi, I was surprised that strict type checking for the this parameter is currently that limited and would like to see enhancements in future versions. Since the problem caused by the absence of type checking made it into the FAQ, it seems to be supising for many users.

As already mentioned in previous comments, the issues #3694 and #6018 have already proposed some changes and some of them were already implemented in the pull requests #4910 and #6739. Since there is a long history, I found it difficult to get an overview about the situation. I therefore tried to summarize it below. I hope that I did not mix up anything. At the bottom, I bring up my own thoughts about the current situation.

Implemented

✔️ Polymorphic this type

Proposed by #3694 and implemented by #4910. It has added a special type which represents the actual type of this which might be a subclass.

class C {
  newInstance(): this {
    // Error because 'this' my be a subclass.
    return new C();
  }
}

class D extends C {
  run() {
    // Works
    let x: D = this.newInstance();
  }
}

Within method bodies, this now has type this (as default).

class C {
  f() {
    // `this` and therefore `x` have type `this`. In previous versions, they
    // would have type `C`.
    let x = this;
  }
}

// However, for the users of the method, `this` is still considered to has
// type `any`. Even so it might cause a runtime error, the following compiles
// because `null` is assignable to `any`.
const c = new C();
c.f.call(null);

Note that TypeScript looses the “it might be a subtype” semantic when you are outside of the class.

class C {
  f(arg: this) {
    // Error because the method might require a subtype of C.
    this.f(new C()); // <-- Error
  }
}

declare let c: C;
// The method might still require a subtype of C but TypeScript does not
// have this kind of semantic outside of the class.
c.f(new C()); // <-- Compiles

✔️ Type specifications for this parameter

Proposed by #3694, refined by #6018, and implemented by #6739. There were proposals for different syntaxes but I only show the implemented one.

function f(this: string) {
  // A function that expects a string for the `this` parameter.
}

Note that this also works when a function is called as constructor. It then also specifies the type of the created object.

interface F {}
function F(this: F) {}
let f = new F(); // `f` has type `F`

✔️ Type checking for this parameter on call sites

Proposed by #3694, resumed by #6018, and implemented by #6739. It's about actually checking the argument for the this parameter.

declare function f(this: string): any;
const obj = {f};
obj.f(); // <-- Error, `obj` is not a `string`

Note that #3694 alternatively suggested that we could already fail when a function is assigned to an object that does not satisfy the this parameter. Then, the second line of the code snipped would already fail. However, it was never mentioned again (and I think it would be strange).

✔️ --noImplicitThis

Implemented in #6739 as replacement for --strictThis which will be described later. The --noImplicitThis causes a compilation error when you access this but the type of this is implicitly set to any. This can happen when you access this in the function body or when you use new.

function f() {
  this.x; // <-- Error because type of this is unspecified.
}

function C() {}
const c = new C(); // Error because type for `this` in `C` unspecified.

However, note that it does not check call sites of such methods. Since the default type for this is inconsistent inside and outside of the method body, the following still compiles without errors.

class C {
  f() {
    // Fine because `this` has type `this`.
    this.f();
  }
  g() {}
}

const c = new C();
// Fine because `C.f` has type `(this: any) => void`.
c.f.call(null);

✔️ Modified assignability checks

Proposed by #6018 and implemented by #6739. It allows assigning anything to the this parameter if the this parameter is void. Usually, only undefined (or null) can be assigned to void.

function f(this: void): void {}
const obj = {f};

// Passes `obj` to `void` but still compiles.
obj.f();
// Assigns `string` to `void` but still compiles.
const g: (this: string) => void = f;

✔️ Contextually typed this

Proposed by #6018 and implemented by #6739. In some cases, TypeScript will infer the type for this from the context. For example, when you directly assign a new function to a variable that has a declared type. The inferred type will only be used in the method body but not on call sites.

declare let x: (this: string) => void;
x = function () {
  // `this` has type `string`.
};

If the type for this is not specified by the target, this is contextually typed to the object where it is assigned to.

const obj = {
  x: 0,
  f: function () {
    // Ok, `this` is contextually typed as `typeof obj`.
    this.x;
  },
}

Proposed

📘 New implicit type for this parameter (--strictThis)

Proposed by a comment in #3694 and further specified by #6018 but not yet implemented. The proposal is about adding a new compiler option like --strictThis which changes the default type for this from any to either this or void.

class C {
  f() {
    // Works since `this` is of type `this`, just as in current versions.
    this.g();
  }
  g() {}
}

const c = new C();
// Fails because `C.f` is of type `(this: C) => void`.
// Currently, TypeScript considers the type to be `(this: any) => void`.
c.f.call(null);

The type shall be this for method-style declarations and void for function-style declarations. Currently, it is always this in the method body, and any outside of the method body.

// type: (this: void) => void
function f() {}

interface I {
  // type: (this: void) => any
  g: () => any;
  // type: (this: this) => void
  h();
}

const obj = {
  // type: (this: typeof obj) => void
  // (However, I think it should be `(this: void) => void` as I describe later)
  g: function() {},
  // type: (this: typeof obj) => void
  h(): void {},
}

According to #7689, it has not been implemented because

  • it decreases performance by 5% to 10%, and
  • the difference for method-style declarations and function-style declarations might not be obvious for some users. (As a user that does not have that much experience with TypeScript, I want to mention that I kind of expected this difference until I found out that it does not exist.)

My thoughts

🔷 Safe method invocation still missing

A lot of stuff have already been implemented, but we are still kind of missing safe method invocations. For me, safe method invocations actually look like one of the most important motivations behind the topic. As of today, we only get safe method invocations if we always explicitly specify the type of this.

function schedule(callback: (this: void) => void) {}
class C {
  data = 10;
  f(this: C) {}
}
const c = new C();
schedule(c.f); // Error

The problem: If we miss the type specification for either schedule or f, everything compiles without errors. Additionally, one of both places might not be in our control. For example, setTimeout does not specify the type of this for the callback. Therefore, in many situations, users are not able to get safe method invocations even if they explicitly specify the type of this for all their functions and methods.

🔷 Use unknown instead of void

A few weeks ago, I started a project with TypeScript. At one point, I was quite confused that the following assignment works even so, it contradicts the description of void.

declare let x: (this: string) => any;
declare let y: (this: void) => any;
x = y;

The snipped compiles due to the modified assignability checks mentioned above. If we would use this: unknown instead of this: void, the special handling in the assignability checks would not be necessary, and it would not confuse new users like me. However, we can probably not revert the special handling since it would break a lot of existing code. However, if we want to change this in the long term, TypeScript could generate a warning whenever this is specified as void and recommend using unknown instead.

🔷 Revert part of Contextually typed this

I think an important objective is that the type for this is mostly consistent between the call-site and the method body. Whenever it becomes inconsistent, it enables incorrect usage. With this objective in mind, we should revert the contextual typing when the type is not specified.

const obj = {
  x: 0,
  f: function () {
    // Currently: Ok, `this` is contextually typed as `typeof obj`
    // Proposed: Error, because `this` is `void` (or `unknown`)
    this.x;
  },
  g() {
    // Continues to work sine it uses a method-style declaration
    this.x;
  }
};
// Currently: Ok, `obj.f` is typed as `(this: any) => void`
// Proposed (1): Ok, `obj.f` is typed as `(this: unknown) => void`
// Proposed (2): Error, `obj.f` is typed as `(this: void) => void`
obj.f.call(null);
// Currently: Ok, `obj.g` is typed as `(this: any) => void`
// Proposed: Error, `obj.g` is typed as `(this: typeof obj) => void`
obj.g.call(null);

interface I {
  x: number;
  f: (this: this) => void;
}
const obj2: I = {
  x: 0,
  f: function () {
    // Ok, `this` is contextually typed as `ThisType<I['f']>`
    // So, it does not change to the current behavior.
    this.x;
  },
};

I think the proposal in #6018 was kind of inconsistent in this regard.

// Copy from the proposal
interface I {
  f: (n: number) => string; // this: void
  g(n: number): string; // this: this
}
// Also from the proposal (but I removed a member)
let o = {
  data: 12,
  g: function() {   // this is inferred from the contextual type
    console.log(this.data); 
  }
}
// However, I think we should use the same rules as for interfaces and
// look at the style of the declaration.
let p = {
  data: 12,
  g() {   // `this` has type of `p`
    console.log(this.data); 
  },
  h: function() {   // `this` has type `void` (or `unknown`)
    console.log(this.data); // Error
  },
}

🔷 Avoiding performance hit

If I understood the conversations correctly, whenever we use this in the facade of a type, the type becomes generic, which then causes a loss in performance. As an approximation, we could avoid the usage of this in the facade but use the most specific concrete type we know.

class B {
  private x = 0;
  f() { // Has type `(this: B) => void` instead of `(this: this) => void`
    // Within the method body, we still consider `this` to have type `this`
  }
}
class C extends B {
  private y = 0;
  g() {} // Has type `(this: C) => void` instead of `(this: this) => void`
}
const c = new C();
// Compiles since due to the approximation, `this` has type `B` instead of `C`
c.f.call(new B());
// However, most cases like the following two are detected.
c.f.call(null); // Error
setTimeout(c.f, 0); // Error

@akomm
Copy link

akomm commented May 10, 2021

Maybe a partial solution with some usefulness would be to check this whenever it was defined explicitly at source and only then. If the "full" solution with implicit this is not viable.

@WorldSEnder
Copy link

Even if the correct deduction of the this type is not viable, at least "strict" should imply that the implicit coercion of this types is disallowed. The fact that

class Foo {
    _a = 5;
    f() { console.log(this._a); }
}
let foo = new Foo();
let cb: () => void = foo.f;
cb(); // oops

compiles under strict1 is just very unfortunate and leads to a lot of subtle errors that could be easily avoided even without a perfect deduction of the this type.

Footnotes

  1. Even changing f to mention an explict this type, as f(this: Foo) allows the assignment of foo.f to cb: (this: any) => void

@ljharb
Copy link
Contributor

ljharb commented May 16, 2023

I think I ran into this as well - https://www.typescriptlang.org/play?#code/MYGwhgzhAEAKCmAnCB7AdtA3gKGtNYAtvAFzQQAuiAlmgOYDcu0w6liArsBSogBQFiZdrToBKLMzwUAFtQgA6QfGgBefEXhM8AX2YQwATwBymvhJx48rNKhDwFIFHT6z5SzWO3Q9e+xWgAIxRAtXx4AHc4JFQ0PgByACEQ+K9sf2gONGCsgBMw4MCFAxNNJiyctFzzJiA - if you hover over the sayName on the second to last line, it says "Person.sayName()", which is wrong, because it implies it's a static method when it's actually an instance method.

@HolgerJeromin
Copy link
Contributor

which is wrong, because it implies it's a static method when it's actually an instance method.

Currently TS shows tooltip of instance properties/methods vs static properties/methods exactly the same.
Which could be optimized, but is unrelated to this item.

@kubernegit
Copy link

I hope that this feature will someday be included into the release :)

@ssalbdivad
Copy link

ssalbdivad commented Jan 13, 2024

I ran into an issue that I'd love to avoid in the future via a flag like this:

class Foo {
	fighters = []

	popFighter() {
		return this.fighters.pop()
	}
}

class Bar {
	foo = new Foo()

	get popFighter() {
		// this ends up being the Bar instance here when called in JS
		// if the user doesn't know it needs to be bound here (like I didn't)
		return this.foo.popFighter
	}
}

const bar = new Bar()
// runtime error: cannot read properties of undefined (reading 'pop')
bar.popFighter() //?

class FooWithThis {
	fighters = []

        // can also use `this: this`
	popFighter(this: FooWithThis) {
		return this.fighters.pop()
	}
}

class Bar2 {
	foo = new FooWithThis()

	get popFighter() {
		// this ends up being the Bar instance here when called in JS
		// if the user doesn't know it needs to be bound here (like I didn't)
		return this.foo.popFighter
	}
}

const bar2 = new Bar2()
// TS will actually warn us here that the `this` context is wrong as long as it was explicitly typed 😍
bar2.popFighter()

// The problem is it's hard to know how a method on a class might be referenced like this when the
// class is being written.

TS Playground

I assume the actual implementation work involved must be relatively limited since TS can already identify this problem if the type is manually specified (which I was very impressed with!)? Could probably help catch quite a few of JS's notorious this-related gotchas.

@ljharb
Copy link
Contributor

ljharb commented Jan 13, 2024

Why would you use a getter there instead of popFighter() { return this.foo.popFighter(); } tho?

@ssalbdivad
Copy link

@ljharb This is a repro meant to be minimal not realistic.

@JojOatXGME
Copy link

Maybe it helps if I put everything I learned earlier in the form of a proposal. Note that I am not really an active TypeScript user at the moment, but I still think this is an important issue and I would like to see it fixed when I return to TypeScript eventually. It would be interesting to know if this proposal would have a chance of being accepted if someone created a Pull Request.

Problem Statement (Motivation)

The type of this is practically unchecked in many scenarios. This causes runtime errors which could otherwise be detected by TypeScript. These types of runtime errors often happen in combination with callbacks. They are common enough that they have their own entry in the FAQ of TypeScript.

Back when class components were still a thing in React, the official React documentation also spent a large portion of Handling Events discussing this problem as part of their introduction.

The option --noImplicitThis already tries to mitigate this issue, but --noImplicitThis doesn't work for method-style function declarations. Unfortunately, method-style declarations are often the more natural syntax for functions that are using the this parameter. Besides, many function types used for callbacks do not declare the type of this (e.g. setTimeout). This often makes declaring the type of this in function definitions ineffective, which makes --noImplicitThis ineffective as well.

This proposal introduces a compiler flag that allows the compiler to detect these types of errors at compile time. While the runtime error was an aftereffect (which was often delayed), with this proposal the compiler can pinpoint the problematic location in the source code at compile time.

Example Code with setTimeout

The following code runs into TypeError: this is undefined at the end of the second update. The first update runs through without issues. This proposal introduces the option --strictThis which causes a compile error at the first argument of setTimeout.

class LiveData {

  constructor() {
    this.update();
  }

  update() {
    // updae data ...
    setTimeout(this.update, 1000);
  }
}
Example Code with React (old style class component)

The following code runs into TypeError: this is undefined within handleClick when a user clicks on the toggle button. This proposal introduces the option --strictThis which causes a compile error in the render function.

class Toggle extends React.Component {
  constructor(props) {
    super(props);
    this.state = {isToggleOn: true};
  }

  handleClick() {
    this.setState(prevState => ({
      isToggleOn: !prevState.isToggleOn
    }));
  }

  render() {
    return (
      <button onClick={this.handleClick}>
        {this.state.isToggleOn ? 'ON' : 'OFF'}
      </button>
    );
  }
}
Example Code with Explicit Type Specification

Even when specifying the type of this explicitly, TypeScript may not detect the error. The following example compiles without error even with strict type checking enabled. The code fails to print Hello World! and prints undefined instead.

class MyClass {
  private text = "Hello World!";

  logText(this: MyClass) {
    console.log(this.text);
  }
}

const instance = new MyClass();
setTimeout(instance.logText, 1);

Open in Playground

As long as either logText or setTimeout does not specify the type explicitly, the current TypeScript compiler does not detect the error. As of today, functions like setTimeout do usually not specify the type of this in their declarations. Even if you try the specify the type everywhere, this is also something which can easily be forgotten.

History

The type of this has been the topic of many issues over the years. The first issue I am aware of is #229. Later, proposal #3694 explicitly declared “Safe method invocations” as part of the motivation. Its implementation in #4910 was eventually narrowed down. The topic was picked up again by #6018, but its implementation in #6739 was also narrowed down eventually.

While #4910 and #6739 introduced many of the necessary fundamentals, the goal of “Safe method invocations” is still not achieved in practice today. Two issues make type-checking of this ineffective:

  1. Implicit use of this: any in function type-declarations.
    (These function-type declarations are often defined by libraries and used for callbacks.)
  2. The implicit type of this is inconsistent inside and outside of functions.
    (Inside method-style functions, TypeScipt implicitly declares this as having the polymorphic this type. Outside the function, the type is declared as this: any.)

Concerns that led to narrowing down the proposals included some resistance to introducing new compiler flags which might not work well with existing declarations in libraries (including DefinitelyTyped). Another concern was the impact on the compiler performance, but it might have been specific to the previous solution. Some resistance to introducing too many changes at once might also have played a role.

Goals

  • Make strict type checking of this effective on call-sites.
    • Provide a (mostly) consistent type for the this parameter inside and outside of functions.
    • Make void the default type of this in function types.

Changes

New compiler option --strictThis

The proposal adds --strictThis as a new compiler option. You may use it by specifying --strictThis as a command line argument or by adding "strictThis": true below "compilerOptions" in the tsconfig.json.

The new option changes the implicit default from this: any to this: void in function-style declarations and function types. In method-style declarations, the default changes from this: any to the type of the surrounding structure. Consider the following code in TypeScript.

type Callback = () => void;
const obj = {
  func: function () {},
  method() {}
};

Without --strictThis and in previous versions of TypeScript, the effective types match the following declarations:

type Callback = (this: any) => void;
declare const obj: {
    func: (this: any) => void;
    method: (this: any) => void;
};

When --strictThis is enabled, the effective types instead match the following declarations:

type Callback = (this: void) => void;   // this: any -> this: void
declare const obj: {
    func: (this: void) => void;         // this: any -> this: void
    method: (this: typeof obj) => void; // this: any -> this: typeof obj
};

If the new option is enabled, --noImplicitThis becomes obsolete. If both options are enabled, --noImplicitThis shall be ignored. In all scenarios where --noImplicitThis would prevent the usage of this, --strictThis will implicitly declare this: void and therefore also effectively prevent its usage. There shall be no error if both options are enabled to allow the combination --strict, which implies --noImplicitThis.

Type of this in function types (--strictThis only)

If --strictThis is enabled, the default type of this in function types becomes void. Otherwise, TypeScript continues to use any as the default type.

type Callback = () => void;
//   ^ --strictThis disabled: (this: any) => void
//     --strictThis enabled:  (this: void) => void
Type of this in method-style declarations (--strictThis only)

The default type of this is not changed within the body of method-style declarations.

class Class {
  method() {
    type self = typeof this;
    //   ^? type self = this
    // (Just as in previous versions, independent from --strictThis)
  }
}

However, the type of the resulting function changes if --strictThis is enabled. Consider the following code.

class Class {
  method() {}
}

In previous versions of TypeScript or when --strictThis is disabled, the effective types match the following declaration:

declare class Class {
    method: (this: any) => void;
}

When --strictThis gets enabled, the effective type changes. Method style declarations will use the type of the surrounding structure as the type for this. The effective types therefore match the following declarations:

declare class Class {
    method: (this: Class) => void; // this: any -> this: Class
}

Method-style declarations in interfaces (and object literals) are equally affected by --strictThis.

interface Interface {
  // --strictThis disabled: method: (this: any) => void
  // --strictThis enabled:  method: (this: Interface) => void
  method(): void;
}

Note that the usage of the surrounding structure as the type is an approximation. Using the polymorphic this type would be more correct, but a previous implementation ran into a performance degradation (#7689). The polymorphic this type might have caused the performance degradation as it made all methods generic.

Type of this in function-style declarations

The default type of this changes to any within the body of function-style declarations when --strictThis is disabled. (It looks like this is already the case in current versions of TypeScript according to my tests on the Playground. I just list it as a change because I thought it was different in some previous version and I am not sure if I missed something.)

class Class {
  func = function() {
    type self = typeof this;
    //   ^? type self = any
    // (Wasn't it "self: this" in some previous versions?)
  }
}

When --strictThis is enabled, the default type of this changes to void.

class Class {
  func = function() {
    type self = typeof this;
    //   ^? type self = void
  }
}

This change also affects the type of the resulting function. This means the effective type of Class["func"] is (this: void) => void when --strictThis is enabled.

Future Work

  • --strictThis can be made a part of --strict, assuming the community has generally accepted this option and libraries had some time to adjust their type declarations.
  • --noImplicitThis might be deprecated in favor of --strictThis.

Note: Void vs Unknown

In the current version of TypeScript, this: void behaves mostly like this: unknown. The usages of void in this proposal could technically be replaced by unknown without changing any of the arguments. I am using this: void for consistency with previous implementations. Note that the documentation of void currently only describes its meaning when used as the return type. Also note that ThisParameterType<() => void> currently results in unknown, which is inconsistent with this proposal. It might make sense to also change the behavior of ThisParameterType when --strictThis is enabled, or alternatively change the proposal to use this: unknown as the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add a Flag Any problem can be solved by flags, except for the problem of having too many flags Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests