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

Make EventListener covariant #798

Open
creeefs opened this issue Nov 19, 2019 · 3 comments
Open

Make EventListener covariant #798

creeefs opened this issue Nov 19, 2019 · 3 comments

Comments

@creeefs
Copy link

creeefs commented Nov 19, 2019

Hello,

I opened microsoft/TypeScript#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.

@exapsy
Copy link

exapsy commented Dec 1, 2019

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.

@SantoJambit
Copy link

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

@andyvanee
Copy link

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants