TS: Custom event names in addEventListener methods

[rozsival]

Bug Report

Naja currenctly exposes dispatchEvent method in its public API. This is incredibly useful when creating custom extensions with their own events and corresponding event listeners (see an example extension).

Since Naja itself extends EventTarget it should be possible to add a listener for any event (string), not just those declared in NajaEventsMap as this declaration should only be considered an overload of the original one in EventTarget.

This is not currently working with TSC, I can only use event names as keyof NajaEventsMap. Any other string besides those defined in that particular interface results in compile error.

Example

naja.addEventListener('dependentSelectBoxLoaded', e => {});

Result

TS2345: Argument of type '"dependentSelectBoxLoaded"' is not assignable to parameter of type 'keyof NajaEventMap'.

Environment

• naja:2.1.4
• node:12.19.0
• typescript:4.2.3

To be honest, I see no reason why this should not work as its pretty common to overload method declarations for more specific cases. However, I am not able to make this work. Could it be an issue with TS declarations after build or TSC itself?

[jiripudil]

Hello,

this is partly on purpose, I consider the finite set of dispatched events to be a part of the public API that is not open for external extension, as I'd personally make a separate EventTarget for custom events, much like some of Naja's core components are EventTargets even though they could easily dispatch the event on naja, mainly to clearly separate the responsibilities. The dispatchEvent method being exposed is an implementation detail given by the unfortunate fact that everything is public in JavaScript.

That being said, who am I to judge what's right and what's wrong 🤷 I see it might be convenient and understand the argument that it effectively breaks the exposed public API of EventTarget, so I'm not against widening the types.

I believe changing the declaration to something like this should help, could you please try that?

diff --git a/src/Naja.ts b/src/Naja.ts
--- a/src/Naja.ts
+++ b/src/Naja.ts
@@ -200,8 +200,8 @@
        return data;
    }

-   declare public addEventListener: <K extends keyof NajaEventMap>(type: K, listener: TypedEventListener<Naja, NajaEventMap[K]>, options?: boolean | AddEventListenerOptions) => void;
-   declare public removeEventListener: <K extends keyof NajaEventMap>(type: K, listener: TypedEventListener<Naja, NajaEventMap[K]>, options?: boolean | AddEventListenerOptions) => void;
+   declare public addEventListener: <K extends keyof NajaEventMap | string>(type: K, listener: TypedEventListener<Naja, K extends keyof NajaEventMap ? NajaEventMap[K] : CustomEvent>, options?: boolean | AddEventListenerOptions) => void;
+   declare public removeEventListener: <K extends keyof NajaEventMap | string>(type: K, listener: TypedEventListener<Naja, K extends keyof NajaEventMap ? NajaEventMap[K] : CustomEvent>, options?: boolean | AddEventListenerOptions) => void;
 }

 export interface Extension {

[rozsival]

Hey @jiripudil,

apologies for late response.

I can confirm those updated typings work. If that's ok with you I would be grateful for such extension. On the other hand, I completely get your point and don't want to push you into some change that doesn't fit in the way you develop this awesome library. The final decision is yours to make.

Thank you!

[jiripudil]

🚀 in 2.1.5 :)