Smarter types for fromEvent

[waterplea]

Feature Request

Is your feature request related to a problem? Please describe. When you create Observable with fromEvent you have to manually define generic and in received event target type is not inferred from element we provided although to my knowledge DOM event target is always the same element you listen event on. You have to typecast.

Describe the solution you'd like Here's a proof of concept for a better typed s solution: https://stackblitz.com/edit/angular-typed-fromevent It's only for traditional DOM events overload

Describe alternatives you've considered I've been using wrapper function from example above in my project. Could be great if it was unnecessary and it was built-in in RxJS.

Additional context I'm willing to work on a PR but I might need assistance with other overloads as I'm not that familiar with them.

[cartant]

The problem is that GlobalEventHandlersEventMap is declared in TypeScript's dom library - see here - but fromEvent isn't just for DOM events. It works with Node events too - see here.

AFAICT, using GlobalEventHandlersEventMap in the type declarations would necessitate the inclusion the dom library in environments that don't involve the DOM. And that's the principle blocker for this.

[waterplea]

Is it possible to break TypeScript apart and only use particular parts of it? What would be the benefits of doing so since it installs altogether and only exists in declaration files that take no space and do not make it into compiled sources?

[cartant]

TypeScript is already broken apart into libraries that contains specific type declarations - the types for said libraries are included by adding the libraries via TypeScript's lib compiler option.

To get the GlobalEventHandlersEventMap, the dom library needs to be specified. And that is the problem. If that type is used in the declaration for fromEvent, Node developers will have to specify dom in their lib compiler option. And that will also have the effect of introducing the DOM types for APIs like setInterval that differ from APIs with the same name in @types/node. Specifying dom will also introduce types for APIs that do not exist in the Node environment.

In short, that has been a problem before and some effort was made to remove the dependency on the dom library from the fromEvent type declarations - see #3566 and the related issue.

An alternative approach could be to provide an additional overload signature to fromEvent - in a separate module or package - using declaration merging. That is, you wouldn't need to implement a wrapping function, you could just provide an alternative signature that used the GlobalEventHandlersEventMap type and required the dom library to be specified. It would look something like this:

declare module "rxjs/internal/observable/fromEvent" {
  export function fromEvent</* whatever */>(/* whatever */): /* whatever */;
}

TypeScript would then attempt to match the merged signature before is matched any of the signatures declared in the RxJS codebase for fromEvent.

[niklas-wortmann]

If there is really no way to work around this, and you are still interested in releasing it in a node module, this could go to rxjs-web. It's still in an alpha version due to the fact that I am a slacker and need to implement tests :D

[cartant]

The only way that I can see it working in the RxJS codebase is if there are separate import locations for DOM and non-DOM type declarations. With the former requiring the dom TypeScript library to be specified in the consuming application's tsconfig.json.

[devanshj]

FWIW: #4891, rxjs-from-emitter (type-safe fromEvent not only for DOM but literally any event emitter)

[chrisguttandin]

Hi everyone, I just stumbled upon this. What do you think about using TypeScript's template strings to infer the event types?

I used that technique to implement the on() method of subscribable-things. The downside is that it requires TypeScript v4.1+. Not sure if RxJS v7 will require that anyway.

This is how it could look when applied to fromEvent:

type EventHandler<T, U extends Event = Event> = ThisType<T> & ((event: U) => void);

type EventTargetWithPropertyHandler<T extends EventTarget, U extends string, V extends Event> = {
    [P in U as `on${P}`]: null | EventHandler<T, V>;
};

type EventType<T extends EventTarget, U extends string> = T extends EventTargetWithPropertyHandler<T, U, infer V> ? V : Event;

declare function fromEvent<T extends EventTarget, U extends string>(target: T, eventName: U): Observable<EventType<T, U>>;

It basically works by looking up the event by it's type. It searches for the property handler with the same name and then uses the type definition of that handler.

When using it like this ...

fromEvent(messagePort, 'message');

... TypeScript will infer the event type from the type definition of messagePort.onmessage.

The type definition would need to be modified a bit to work without the pre-defined EventTarget and Event types but I think it should still work as intended.

It will unfortunately not help to improve the types for events when writing code for Node.js.

[cartant]

The downside is that it requires TypeScript v4.1+. Not sure if RxJS v7 will require that anyway.

TypeScript 4.2 will be the minimum-supported version.

What do you think about using TypeScript's template strings to infer the event types?

Maybe. There are some other things that need to be fixed before this could be done - see https://github.com/ReactiveX/rxjs/pull/6208. Specifically, the FromEventTarget union type needs to be replaced with multiple overload signatures.

[thynson]

6214 actually cause much more typing error for valid code, due to Typescript's inability to resolve function overload that contains union typed parameters.

I would suggest to separate fromEvent to fromDomEvent, fromNodeEvent(or fromEventEmitter), etc., and use option object instead of function overloading to cover each case.

[waterplea]

Would https://github.com/ReactiveX/rxjs/issues/5512#issuecomment-812878583 by @chrisguttandin be a reasonable idea now, that minimal TypeScript is supporting this?

[Neosoulink]

Wow, what a story stream ;), I was about to create an issue about this.

+1 for @waterplea, @chrisguttandin proposition might be a reasonable approach 📌

cc @cartant