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

Extracting Types from Base Class #35

Closed
alexlafroscia opened this issue Sep 30, 2022 · 3 comments
Closed

Extracting Types from Base Class #35

alexlafroscia opened this issue Sep 30, 2022 · 3 comments

Comments

@alexlafroscia
Copy link

I'm running into a problem with trying to correctly type a utility function when using the "base class" version of TypedEmitter.

My application has an on helper that wraps EventEmitter#on but returns a function that calls EventEmitter#off for you:

export function on<Events extends EventMap, Event extends keyof Events>(
    emitter: TypedEmitter<Events>,
    event: Event,
    listener: Events[Event]
): () => void {
    emitter.on(event, listener);

    return () => {
        emitter.off(event, listener);
    };
}

This works great under certain circumstances; namely, variables that are directly declared to be of some TypedEmitter<Events> type

type Events = {
  foo: () => void;
}

const myEmitter: TypedEmitter<Events> = new EventEmitter();
// full type-checking; only valid event names are suggested for the second argument
on(myEmitter, 'foo', () => {});

// the following results, correctly, in a type error because `"bar"` is not a valid event
on(myEmitter, 'bar', () => {});

However, in many parts of my codebase, I'm using the ability to extend a class from EventEmitter as a TypedEmitter, which breaks my on helper

class MyEmitter extends (EventEmitter as new () => TypedEmitter<Events>) {}

const myEmitter: TypedEmitter<Events> = new EventEmitter();
// works fine, but no more completion for the supported events or typed for listener arguments
on(myEmitter, 'foo', () => {});

// no longer results in a type error
on(myEmitter, 'bar', () => {});

I think this is related, conceptually, to #24 but I was unable to use the examples in there to alter my on helper so that it would respect the types supported by the class.

Do you have any ideas on how to type a helper function so that it is able to "see the valid events" when using the base-class approach?

@alexlafroscia
Copy link
Author

alexlafroscia commented Sep 30, 2022

I have tried playing with an EventsFor type (again, borrowed from #24) that extracts the supported events, in the hopes that that would help illuminate what's going on

export type EventsFor<T extends TypedEmitter<any>> =
    T extends TypedEmitter<infer EventMap>
      ? keyof EventMap
      : never;

This works great with variables declared directly as a TypedEmitter<Events>

// knows that only "foo" is a valid option
const event: EventsFor<TypedEmitter<Events>> = "foo";

but again, it breaks down for a class extending EventEmitter as a TypedEmitter:

// allows any `string | number`; this should error but does not
const event: EventsFor<MyEmitter> = 42;

@alexlafroscia
Copy link
Author

Thanks to some help from a co-worker, I was able to get to a version of TypedEventEmitter that works with my helper function!

Basically, the issue ended up being that TypedEventEmitter did not have it's own reference to the Events type, so my on function had no way to look it up. That could be resolved by adding an optional member to the interface, of the Events type

interface TypedEventEmitter<Events extends EventMap> {
+ __events_ref?: Events;
}

Leaking this in the API of any TypedEventEmitter is not ideal, but we can use a declared class instead of an interface to fix that

- interface TypedEventEmitter<Events extends EventMap> {
+ declare class TypedEventEmitter<Events extends EventMap> {
- __events_ref?: Events;
+ private __events_ref?: Events;
}

Without any other changes, my on helper can now extract the right event-type information from my classes that extend from EventEmitter as a TypedEventEmitter!

@alexlafroscia
Copy link
Author

I'm going to close this issue, since my problem is resolved; I don't know if that's a change you'd want to make to this library, but hopefully this helps resolve an issue for someone down the road!

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

1 participant