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

"The 'this' context of type 'Foo | Bar' is not assignable to method's 'this' of type 'Foo'" with a this constraint that shouldn't have any effect #54407

Closed
Jym77 opened this issue May 26, 2023 · 5 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@Jym77
Copy link

Jym77 commented May 26, 2023

Bug Report

🔎 Search Terms

The this context of type is not assignable to methods this of type

Found #23304, #28777, and #51198 which all seem somewhat different.

🕗 Version & Regression Information

Passing in TS 3.5.1, failing in TS  3.6.3 and above (including nightly 5.2.0-dev.20230526)

  • This changed between versions 3.5.1 and 3.6.3
    (haven't tried intermediate versions since they are not in the Playground)
    (not seeing anything obvious in the 3.6 release notes

⏯ Playground Link

Playground link with relevant code

💻 Code

class Foo {
  foo = 1;

  public ok(): void {}

  public ko(this: Foo): void {}

  public overload_ok(): void;
  public overload_ok(this: Foo): void;
  public overload_ok(): void {}

  public overload_ko(this: Foo): void;
  public overload_ko(): void;
  public overload_ko(): void {}
}

class Bar {
  public ok(): void {}

  public ko(): void {}

  public overload_ko(): void {}

  public overload_ok(): void {}
}

function f(x: Foo | Bar): void {
  x.ok();
  x.ko();                     // error
  // The 'this' context of type 'Foo | Bar' is not assignable to method's 'this' of type 'Foo'. 
  // Property 'foo' is missing in type 'Bar' but required in type 'Foo'.

  x.overload_ok();
  x.overload_ko();     // error
  // The 'this' context of type 'Foo | Bar' is not assignable to method's 'this' of type 'Foo'.
  // Type 'Bar' is not assignable to type 'Foo'.
}

function g(x: Foo | Bar): void {
  if (x instanceof Foo) {
    x.ok();
    x.ko();
    x.overload_ok();
    x.overload_ko();
  } else {
    x.ok();
    x.ko();
    x.overload_ok();
    x.overload_ko();
  }
}

🙁 Actual behavior

The calls to x.ko() and x.overload_ko() throw type errors in f.

🙂 Expected behavior

So, the signatures of ko and ok are slightly different (in Foo), but since ko is already a method on the class Foo, it should only be called when this is an instance of Foo anyway, so it feels like the "this constraint" shouldn't do anything.

The overload_* cases feel even weirder. In overload_ok, the first overload is definitely a catch all, so it is the only one used (as far as I understand), and since it is the same as the ok signature, everything works.
But in overload_ko, the first overload is the same as the ko signature and fails for the same reason, but the second overload (which is the same as the ok signature and thus should work) is not even tried (my IDE also says it is never called). So, it seems that TS treats the first overload (of overload_ko) as a catch all case and stops looking further… except it is actually not behaving the same way. TS even tries to be helpful adding:

public overload_ko(): void {}
       ~~~~~~~~~~~
The call would have succeeded against this implementation, but implementation signatures of overloads are not externally visible.

Since the implementation signature is exactly the same as the second overload, this shows that TS totally ignored the second overload, yet doesn't treat the first one as being fully a catch all case, since stuff breaks with the first one that would work with the second.


g is a workaround where forcing the narrowing and breaking the union in advance resolves the problem. However, having to WET my code is not really doable in practice; and having both the "then" and "else" branchs with the exact same code is such an anti-pattern that it makes my programmer lizard brain scream 🙈


Of course, there may be something I do not really understand in what the "this constraint" actually does. This looks very weird from where I stand now.

@fatcerberus
Copy link

I think this is just a design limitation in that the compiler mostly sees this the same as any other parameter; if instead of this, you had a regular parameter where one class took a Foo and the other any—passing a Foo | Bar without first differentiating the union wouldn’t be safe then and the error would be correct. The only thing that makes this case safe by construction is that the effective this happens to be directly correlated with the (computed) type of x.ko, making this basically a variant of #30581

@Jym77
Copy link
Author

Jym77 commented Jun 5, 2023

@fatcerberus But in that case, overload_ko should work (the first overload fails because of the "extra" this parameter, but the second overload succeed).
The fact that the second overload there is not even looked at by TS seems to indicate that this is not treated as a regular parameter.

@fatcerberus
Copy link

Yeah, you're right. I tried this:

  public overload_ko_param(x: Foo): void;
  public overload_ko_param(x: any): void;
  public overload_ko_param(x: any): void {}

with an equivalent method on Bar, and the call succeeded, passing x as the argument. I won't claim to know exactly what's going on, but it looks like when you have a union of classes and call a method on it, TS unions the parameter types to decide whether it's compatible (note: Foo | any reduces to any). But for this it seems like it just ignores the ones without an explicit this when constructing that union, so it ends up with this: Foo and disallows the call.

@fatcerberus
Copy link

Changing the versions of overload_ko without a this to this: any indeed fixes the error, so there might be something to my hypothesis above.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 14, 2023
@RyanCavanaugh
Copy link
Member

From TS's analysis perspective, this code isn't distinguishable from this

class Foo {
  foo = 1;
  public ok(): void {}
  public ko(this: Foo): void {}
}

class Bar {
  bar = 2;
  public ok(): void {}
  public ko(): void {}
}

function f(x: Foo | Bar, y: Foo | Bar): void {
  x.ok();
  x.ko.call(y);
}

which actually is unsound.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants