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

typing a function that's compatible with StrictEventEmitter types #1

Closed
brendankenny opened this issue Mar 26, 2018 · 6 comments
Closed

Comments

@brendankenny
Copy link

brendankenny commented Mar 26, 2018

First, thanks for the library and the blog post! A lot of that kind of TS information is pretty diffuse, especially with the spec/handbook lagging behind on the newish stuff, so it's much appreciated.

I'm attempting to use strict-event-emitter-types from JS (via TS's jsdoc support) and it works surprisingly well. However I am running into a mismatch issue trying to type a function with the same signature as one of the StrictEventEmitter methods, which then passes on its arguments to a StrictEventEmitter. e.g.

let ee: StrictEventEmitter<EventEmitter, Events> = new EventEmitter;

function proxiedOn(eventName, cb) {
  // do stuff based on eventName/cb but don't alter them

  ee.on(eventName, cb);
}

If I manually type proxiedOn() to what seems reasonable, I get type errors when passing to ee.on().

I see the same issue in pure typescript (with --strict), so we can look at just that to avoid any current bugs in jsdoc support. Here's one of the variations that all suffer from the same error (scroll right to see full inline type):

import StrictEventEmitter from 'strict-event-emitter-types';
import { EventEmitter } from 'events';
interface Position { x: number, y: number };
interface Events {
  move: Position,
  done: void
}
let ee: StrictEventEmitter<EventEmitter, Events> = new EventEmitter;

function proxiedOn<E extends keyof Events>(eventName: E, cb: Events[E] extends void ? () => void : (m: Events[E], ...args: any[]) => void): void {
  // do stuff to events

  ee.on(eventName, cb); // error
}

proxiedOn() passes all the same tests (including failing on failure cases) as ee.on(), but the types are incompatible:

error TS2345: Argument of type 'E' is not assignable to parameter of type '"done"'.
  Type '"done" | "move"' is not assignable to type '"done"'.
    Type '"move"' is not assignable to type '"done"'.

It appears it may be due to the fact that StrictEventEmitter.on is defined with an overload instead of both void and non-void event/cb pairs, but I'm out of my depth TS-wise here (and AFAICT overloading isn't supported via jsdocs (yet?)).

Do you have any insight? Thanks!

@brendankenny
Copy link
Author

Ah, it's possible this is a compiler bug. See microsoft/TypeScript#22860 and fix in microsoft/TypeScript#22869 -- "we measure variance for type parameters of generic class and interface types, any type parameter referenced in a conditional type ends up being considered invariant, and that's what's causing the errors."

It's possible it's still not flexible enough to support this case. I'll check when typescript@next gets pushed tonight.

@brendankenny
Copy link
Author

Nightly did not fix this, turns out it's a more fundamental issue with unions vs overloaded functions: microsoft/TypeScript#1805 and microsoft/TypeScript#14107

That leaves rewriting OverriddenMethods to use unions instead of overloads, however I don't think emit is possible since it differs in arity and I can't find a way to express that with a union that TS still type checks :S

So that leaves the unfortunate ee.emit('event', undefined)

Still interested if you have any thoughts on how to make this work, otherwise this issue can be closed.

@bterlson
Copy link
Owner

The named export StrictBroadcast has the signature you want, check the bottom of the readme for usage instructions. Inside of strictBroadcast, note that you will have to use control flow guards to invoke the methods safely or else use type assertions/any (which is somewhat fine IMO if the body is trivial). See here for an example: https://github.com/bterlson/strict-event-emitter-types/blob/master/test/test.ts#L51.

@bterlson
Copy link
Owner

bterlson commented Apr 2, 2018

@brendankenny going to close this for now but let me know if you have further questions!

@bterlson bterlson closed this as completed Apr 2, 2018
@brendankenny
Copy link
Author

Thanks! That is what I needed (it's not ideal, but variadic types might eventually fix things).

I assumed StrictBroadcast was strict typing for some other event pattern I didn't care about, so I skipped right over the explanation. So thanks for being kind about that :)

@bterlson
Copy link
Owner

bterlson commented Apr 3, 2018

@brendankenny it's not your fault, the name is really bad but I couldn't think of a better one. Let me know if you have ideas!

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

2 participants