TypeScript-DOM-lib-generator icon indicating copy to clipboard operation
TypeScript-DOM-lib-generator copied to clipboard

Make EventListener covariant

Open Clee681 opened this issue 4 years ago • 3 comments

Hello,

I opened https://github.com/microsoft/TypeScript/pull/35211 without realizing that the lib.dom.d.ts file was a generated file. I'm copying the PR description here to see if people think it is a good idea. If so, I'd appreciate some direction to get this implemented given I'm not familiar with how these types are being generated after looking through the source code.


Original Typescript PR 35211 Description

Currently, the EventListener interface is invariant. As a result, functions that pass a subtype of Event fail to compile (e.g. see the CustomEvent and MessageEvent examples below).

CustomEvent handler

obj.addEventListener("customEvt", (e: CustomEvent) => {});
Type '(e: CustomEvent<any>) => void' is not assignable to type 'EventListener'.
    Types of parameters 'e' and 'evt' are incompatible.
        Type 'Event' is missing the following properties from type 'CustomEvent<any>': detail, initCustomEvent  TS2345

MessageEvent handler

eventSource.addEventListener("messageEvt", (e: MessageEvent) => {});
Type '(e: MessageEvent) => void' is not assignable to type 'EventListener'.
    Types of parameters 'e' and 'evt' are incompatible.
        Type 'Event' is missing the following properties from type 'MessageEvent': data, lastEventId, origin, ports, source  TS2345

As a result, it seems like a lot of people resort to using the as EventListener type assertion.

evtSource.addEventListener("messageEvt", ((e: MessageEvent) => {}) as EventListener);

Instead of having users resort to the as operator, I'm proposing we introduce a generic type variable that extends Event to the EventListener interface. We could then pass the appropriate sub-type to the addEventHandler functions for the respective objects (e.g. EventSource below).

interface EventSource extends EventTarget {
    addEventListener(type: string, listener: EventListenerOrEventListenerObject<MessageEvent>, options?: boolean | AddEventListenerOptions): void;
}

The net result would be to allow users to simply do:

evtSource.addEventListener("messageEvt", (e: MessageEvent) => {});

11/19/19: Note that the PR currently only implements the changes for the EventSource interface. I'd like to get some feedback before porting the change to every other object.

Clee681 avatar Nov 19 '19 22:11 Clee681

The solution

evtSource.addEventListener("messageEvt", ((e: MessageEvent) => {}) as EventListener);

Doesnt work for me either, when I use it with React.MouseEvent. I dont know if it really works in any other case at that point.

exapsy avatar Dec 01 '19 17:12 exapsy

@exapsy React.MouseEvent is not meant to be used with EventSource. I think your issue is a different one.

SantoJambit avatar Apr 06 '20 14:04 SantoJambit

I have encountered this issue using EventSource with custom message types as in this tutorial: https://javascript.info/server-sent-events#event-types

I am working around this by redefining the types of EventSource addEventListener and removeEventListener to always receive MessageEvent rather than EventListenerOrEventListenerObject as they currently do.

https://github.com/microsoft/TypeScript/blob/9f70d498f250429ab388f4b34507ecd0f554feb3/lib/lib.dom.d.ts#L5396

https://github.com/microsoft/TypeScript/blob/9f70d498f250429ab388f4b34507ecd0f554feb3/lib/lib.dom.d.ts#L5398

--- lib.dom.d.ts	2020-06-23 15:11:30.000000000 -0600
+++ patched.d.ts	2020-06-23 15:11:12.000000000 -0600
@@ -21,8 +21,9 @@
     readonly CLOSED: number;
     readonly CONNECTING: number;
     readonly OPEN: number;
+
     addEventListener<K extends keyof EventSourceEventMap>(type: K, listener: (this: EventSource, ev: EventSourceEventMap[K]) => any, options?: boolean | AddEventListenerOptions): void;
-    addEventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions): void;
+    addEventListener(type: string,listener: (this: EventSource, ev: MessageEvent) => any, options?: boolean | AddEventListenerOptions): void;
     removeEventListener<K extends keyof EventSourceEventMap>(type: K, listener: (this: EventSource, ev: EventSourceEventMap[K]) => any, options?: boolean | EventListenerOptions): void;
-    removeEventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions): void;
+    removeEventListener(type: string, listener: MessageEvent, options?: boolean | EventListenerOptions): void;
 }

I think in the case of messages from an EventSource it makes sense that any event other than open and error will be a MessageEvent, but I'm aware that this doesn't solve general case...

andyvanee avatar Jun 23 '20 21:06 andyvanee