1

Does anyone know, why the following code does not work?

interface EventBase {
  source: object;
}

interface ClickEvent extends EventBase {
  data: number;
}

interface ButtonEventMap {
  'click': ClickEvent;
}

class Button {
  declare eventMap: ButtonEventMap;

  trigger<K extends keyof this['eventMap']>(type: K, event: Omit<this['eventMap'][K], 'source'>) {
    // ...
  }

  click() {
    this.trigger('click', { // <-- Error
      data: 1
    });
  }
}

TypeScript (4.8.2) complains with the following error: Error:(21, 27) TS2345: Argument of type '{ data: 1; }' is not assignable to parameter of type 'Omit<this["eventMap"]["click"], "source">'.

It works correctly, if I replace this with Button. But there are quite a few methods with a similar pattern and I don't want to override these methods in every sub class.

trigger<K extends keyof Button['eventMap']>(type: K, event: Omit<Button['eventMap'][K], 'source'>)

It also works if I don't use Omit. But then I need to specify source, which is not what I want.

trigger<K extends keyof this['eventMap']>(type: K, event: this['eventMap'][K])

The same problem happens with other custom types like Partial.

trigger<K extends keyof this['eventMap']>(type: K, event: Partial<this['eventMap'][K]>)

Interestingly, it works with Partial if ClickEvent does not extend EventBase anymore.

Any ideas what's going on?

Claudio Guglielmo
  • 188
  • 5
  • The `this` type is an implicit generic type parameter that gets specified only when the instance is calling a method. So the compiler will perform its generics analysis which complains in different situations from specific types (both such analyses are somewhat unsound and incomplete, so they can be fooled). On the face of it, your `click()` implementation is unsafe because I could write `class SubButton extends Button { declare eventMap: { click: ClickEvent & { x: string } }; }` and then `new SubButton().click()` will pass `{data: 1}` to something expecting `{data: number; x: string}`. – jcalz Sep 08 '22 at 13:25
  • If that fully addresses your question I could write up a detailed answer explaining it. If not, what am I missing? – jcalz Sep 08 '22 at 13:26
  • Thank you for your fast reply. Unfortunately, this does not address my question. If you add a SubButton and extend ClickEvent, you would also need to override the click method and add the new x-property to the triggered event which will guarantee, a listener receives the correct data. `class SubButton extends Button { declare eventMap: { click: ClickEvent & { x: string } }; override click() { this.trigger('click', { data: 1, x: 'other' }); } } ` The problem that I have is, that it works correctly without `Omit<>` but I need it to work with Omit. – Claudio Guglielmo Sep 09 '22 at 08:46
  • But the compiler is technically correct to complain about the implementation of `click()` because nothing guarantees that a subclass will override `click()` (the "need" to do so is not represented in the code). When you say "it works correctly" without `Omit<>`, you mean that the compiler does not complain, but it's also unsafe in the same way but the compiler doesn't notice it. If you want to work around it you can do so in various ways, ([example](https://tsplay.dev/mZjD1m)), but if your question is "why is this happening" I've told you. – jcalz Sep 09 '22 at 14:07
  • I can write up an answer explaining why the compiler cares about the unsoundness with `this` types more than it does with specific types, and show the unsoundness. I could probably end up writing a whole book about unsoundness in TypeScript, but in order to limit the scope of the question and answer, could you precisely delineate the question? Is it "why the following code does not work" as your first line says? Or is it "how can I get this to work" as you seem to imply with your latest comment? And if you still think I'm missing something about your question, please explain. Thanks! – jcalz Sep 09 '22 at 14:10
  • I wanted to know why it behaves like this to find a better solution than our current workaround which is to not use this and override trigger in every subclass instead (https://github.com/eclipse-scout/scout.rt/blob/features/suite/23.1/typescript/eclipse-scout-core/src/events/EventEmitter.ts). But it seems that I won't be able to fully understand why it compiles without Omit but not with Omit, so I'd be happy if you could show us your ideas to work around it. – Claudio Guglielmo Sep 12 '22 at 08:10

1 Answers1

1

I think it's safe to say that it's not easy to understand why certain forms with the polymorphic this type work and why other ones fail; see microsoft/TypeScript#36505 which is still marked as "needs investigation" because the TS dev lead isn't sure exactly what is going on in a very similar situation.

The general issue is that this is an implicitly defined generic type parameter, and therefore has the same benefits and drawbacks as generic type parameters. A benefit is that it can represent lots of different types "at once", such as automatically adjusting to be the type of subclass instances. A drawback is that it can be hard for the compiler to accurately see when a specific value is or is not assignable to it.

Sometimes the compiler is too strict, and won't let you assign a value to it even though you are 100% sure it is safe. Other times it is too lenient, and allows you to assign a value to it even though there are some cases where it would lead to runtime errors. And sometimes it does both. I don't know how useful it is to try to tease out exactly which pitfalls your code triggers. If you're very interested you might file an issue in the TypeScript GitHub repo, but there's a good chance it will be closed as a Design Limitation.

First let's show that it is technically unsafe to use the this type the way you're trying to use it. There's nothing wrong with the following at the type level, assuming you can narrow K to "click" if type is equal to "click" (which doesn't happen by itself, but there is a suggestion at microsoft/TypeScript#33014 to allow this):

class SubButton extends Button {
  declare eventMap: { click: ClickEvent & { x: string } };

  trigger<K extends keyof this['eventMap']>(
    type: K, 
    event: Omit<this['eventMap'][K], 'source'>) {
    if (type === "click") {
      const e = event as Omit<this['eventMap']['click'], 
        'source'>; // should be safe-ish
      console.log(e.x.toUpperCase())
    }
  }
}

I've made the eventMap property narrower in the subclass than in the superclass, which is okay according to TypeScript's structural subtyping. I've overriden trigger() with an implementation of the same call signature as the one in the superclass, which again, is fine. Inside that override, if type is "click", I access event as the narrowed ClickEvent type, because this['eventMap']['click'] should be a subtype of ClickEvent & { x: string }. And I have not overridden click(), which is again, fine according to the type system. It's not like click is an abstract method or anything that must be overridden according to the type system.

And now if you run this:

new SubButton().click(); // ERROR! e.x is undefined

you get a runtime error. Because the superclass click() implementation passes the wrong event type to trigger().

It may well be that SubButton is improperly written according to your requirements, but these requirements are not encoded in the types of Button, so we can't expect the compiler to know about them.

The point of this section is to show that it is reasonable for the compiler to issue a warning when you try to pass { data: 1 } as a value of type Omit<this['eventMap'][K], 'source'>. If we can come up with a way to prevent this warning from happening that might meet your needs, but the safety hole is still there, and you'll need to be careful.

Here is one workaround. The compiler will let you widen a value of the polymorphic this type to one of the type of the class instance. Widening is often type safe, but due to variance (see Difference between Variance, Covariance, Contravariance and Bivariance in TypeScript) you can end up widening and making things unsafe (e.g., it is "safe" to assign a value of type {x: string} to a variable of type {x: string | number}, but it is unsafe to assign an x property of type number back into that variable). So it will suppress the error:

click() {
  const thiz: Button = this;
  thiz.trigger('click', {
    data: 1 // okay
  });
}

I've widened this of type this to thiz of type Button. And then I can call thiz.trigger('click', {data: 1}) with no error. After all, Omit<Button["eventMap"]["click"], "source"> is just Omit<ClickEvent, "source">, and {data: 1} is assignable to that.

So there you go. We've managed to suppress the error, as desired.

But, and by now I've belabored the point, it's still just as unsafe as it was before. It's not much better than a type assertion in that regard. If you are careful and only subclass Button in "good" ways, you will be fine. If you are not careful and produce a SubButton like I've shown, you will get a runtime error that the compiler failed to predict. Only you know which one of these situations is more likely in your code base.

Playground link to code

jcalz
  • 264,269
  • 27
  • 359
  • 360