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

typed-emitter compatibility #4

Closed
huan opened this issue Jul 27, 2020 · 8 comments
Closed

typed-emitter compatibility #4

huan opened this issue Jul 27, 2020 · 8 comments
Labels
as-designed Working as designed

Comments

@huan
Copy link

huan commented Jul 27, 2020

Hi @devanshj ,

Thank you very much for your great module! I run into the same issue as you before today. (see: wechaty/redux#4)

After performed my google-fu, I found your issue ReactiveX/rxjs#4891, and here, where I believe it will be my final destination. ;^)

I'm using the TypedEmitter which is really nice to type my emitter classes, when I use fromEmitter() with the typedEmitter, I found that if we have more than one event, unfortunately we will get an unknown typing with the returned Observable, which I believe it is a bug.

Minimum Reproducible Code

import { EventEmitter }   from 'events'
import TypedEventEmitter  from 'typed-emitter'

import { fromEmitter } from 'rxjs-from-emitter'

interface Events {
  foo: (n: number) => void
  bar: (s: string) => void
}
const TypedEmitter = EventEmitter as new () => TypedEventEmitter<Events>

const emitter = new TypedEmitter()

const $ = fromEmitter(emitter).event('foo')
// const $: Observable<unknown>

If we remove bar in the Events interface, then the $ will be able to be inference right: const $: Observable<number>

I tried to dive into your code to fix it, however, the Typing code is more than I expected.

Would really appreciate it if we can get our fromEmitter works with typedEmitter, thank you for your time for my issue, and looking forward to seeing your reply.

@devanshj
Copy link
Owner

devanshj commented Jul 27, 2020

Hi Huan!

The behavior is actually expected and is caused because of the way types are written keeping a limitation of TypeScript in mind. The limitation is that inference breaks when you have generics involved in a function (as TypedEventEmitter uses them on all the methods), an example (demo):

type Identity = <T>(x: T) => T;
type Test = Identity extends (x: string) => infer T ? T : never;
// Test is expected to be `string` but is `unknown`

Now I can solve this but it would require having typed-emitter as a dependency, but that still would only solve the problem only for people who use typed-emitter not for others who have emitters with generics. So unfortunately supporting typed-emitter out of the box magically is a non-goal for rxjs-from-emitter.

But that being said I can still solve you're problem in a much much simpler way. I believe you want fromEvent to work with TypedEventEmitter right? Here's the solution:

// utils/from-typed-emitter.ts
import { EventEmitter } from 'events';
import { fromEvent, Observable } from "rxjs";
import TypedEventEmitter from "typed-emitter";

type FromTypedEvent = 
  < Emitter extends TypedEventEmitter<any>
  , EventName extends keyof Events
  , Events = Emitter extends TypedEventEmitter<infer T> ? T : never
  >(emitter: Emitter, event: EventName ) =>
      Observable<ObservedValue<Events[EventName] extends (...args: infer A) => any ? A : never>>

type ObservedValue<A extends unknown[]> =
  A["length"] extends 0 ? void :
  A["length"] extends 1 ? A[0] :
  A

export const fromTypedEvent = fromEvent as FromTypedEvent;

Now you can simply use it as you would use fromEvent example:

import { fromTypedEvent } from "./utils/from-typed-emitter";
import TypedEventEmitter from "typed-emitter";

interface Events {
  foo: (n: number) => void
  bar: (s: string, x: number) => void,
  baz: () => void
}
const TypedEmitter = EventEmitter as new () => TypedEventEmitter<Events>
const emitter = new TypedEmitter()

fromTypedEvent("", "") // error
fromTypedEvent(emitter, "") // error
fromTypedEvent(emitter, "foo") // Observable<number>
fromTypedEvent(emitter, "bar") // Observable<[string, number]>
fromTypedEvent(emitter, "baz") // Observable<void>

So if you're using typed-emitter, you actully don't need rxjs-from-emitter because it's made for emitters that are actually typed in a hardcoded way like this.

One day when we no longer have that TypeScript limitation rxjs-from-emitter will be able to support typed-emitter without doing anything special.

I hope this helps! I'm closing this as I said it's a non-goal. Thanks for the interest tho!

@devanshj devanshj changed the title fromEmitter(typedEmitter).event('foo') returns Observable<unknown> typed-emitter compatibility Jul 27, 2020
@devanshj devanshj added the as-designed Working as designed label Jul 27, 2020
@devanshj
Copy link
Owner

Opened #5 for tracking any improvements in this space. Likely there will be none till we don't have that TypeScript limitation resolved.

@huan
Copy link
Author

huan commented Jul 28, 2020

Hi Devanshj,

You are so kind for responding to my question with so detailed analytic explanation!

I understand the problem is caused by a limitation of the current version of TypeScript now, and thank you very much for you great solution code, it works like a charm!

Cheers, and have a great day!

@huan
Copy link
Author

huan commented Jul 28, 2020

Oops, I found that our solution code does not work when we do a extends with the TypedEmitter:

class Test extends TypedEmitter {}

import { EventEmitter } from 'events'

import { fromTypedEvent } from './from-typed-event'
import TypedEventEmitter from 'typed-emitter'

interface Events {
  foo: (n: number) => void
}
const TypedEmitter = EventEmitter as new () => TypedEventEmitter<Events>

// We declare a Test class
//      |
//      v
class Test extends TypedEmitter {}

const emitter = new Test()

fromTypedEvent(emitter, '')
// Oops! No error

fromTypedEvent(emitter, 'foo')
// Oops! Observable<void>

I'm trying to see how to fix it now...

@devanshj
Copy link
Owner

You're welcome! I'll look into helping you with a solution in some time

@devanshj
Copy link
Owner

devanshj commented Jul 28, 2020

I can't really figure out why it's not exactly working right now, but you can use this workaround:

import { EventEmitter } from 'events';
import { fromEvent, Observable } from "rxjs";
import OriginalTypedEventEmitter from "typed-emitter";

type TypedEventEmitter<T> = 
  & OriginalTypedEventEmitter<T>
  & { __events: T } // <-- only change

type FromTypedEvent = 
  < Emitter extends TypedEventEmitter<any>
  , EventName extends keyof Events
  , Events = Emitter extends TypedEventEmitter<infer T> ? T : never
  >(emitter: Emitter, event: EventName ) =>
      Observable<ObservedValue<Events[EventName] extends (...args: infer A) => any ? A : never>>

type ObservedValue<A extends unknown[]> =
  A["length"] extends 0 ? void :
  A["length"] extends 1 ? A[0] :
  A

export const fromTypedEvent = fromEvent as FromTypedEvent;


interface Events {
  foo: (n: number) => void
}
const TypedEmitter = EventEmitter as any as new () => TypedEventEmitter<Events>
class Test extends TypedEmitter {}
const emitter = new Test()

fromTypedEvent(emitter, '') // error
fromTypedEvent(emitter, 'foo') // Observable<number>

@huan
Copy link
Author

huan commented Jul 28, 2020

Thank you so much for your solution, it works like a charm.

Appreciate it!

@devanshj
Copy link
Owner

You're welcome!

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

No branches or pull requests

2 participants