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

fix: dont require index signature in event map typing #231

Merged
merged 1 commit into from
Aug 22, 2020

Conversation

forivall
Copy link
Contributor

Without this change, the following error appears when using an interface
of event listeners:

Type 'MyEventListeners' does not satisfy the constraint 'ValidEventTypes'.
  Type 'MyEventListeners' is not assignable to type '{ [x: string]: any[] | ((...args: any[]) => void); }'.
    Index signature is missing in type 'NodeRunnerEventListeners<Output>

with an interface like

interface MyEventListeners {
  log(
    content: string,
    level?: 'error' | 'warn' | 'info' | 'debug' | 'silly'
  ): void;
}

Without this change, the following error appears when using an interface
of event listeners:

Type 'MyEventListeners' does not satisfy the constraint 'ValidEventTypes'.
  Type 'MyEventListeners' is not assignable to type '{ [x: string]: any[] | ((...args: any[]) => void); }'.
    Index signature is missing in type 'NodeRunnerEventListeners<Output>
@lpinca
Copy link
Member

lpinca commented Aug 22, 2020

cc: @gfmio

Copy link
Contributor

@gfmio gfmio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I believe this is due to a recently introduced change in behaviour of theTypeScript type checker. Thanks for noticing and fixing!

@lpinca lpinca merged commit e92b667 into primus:master Aug 22, 2020
@lpinca
Copy link
Member

lpinca commented Aug 22, 2020

Thank you.

@RAnders00
Copy link

RAnders00 commented Aug 24, 2020

Maybe I'm doing it wrong but I still get the same error, even after upgrading to the new release

import EventEmitter from "eventemitter3";

interface MyEvents {
    example(param1: string, param2: number): void
}

export class Example extends EventEmitter<MyEvents> {

}

Playground Link

Type 'MyEvents' does not satisfy the constraint 'ValidEventTypes'. Type 'MyEvents' is not assignable to type '{ [x: string]: any[] | ((...args: any[]) => void) | undefined; }'. Index signature is missing in type 'MyEvents'.

@gfmio gfmio mentioned this pull request Aug 24, 2020
@gfmio
Copy link
Contributor

gfmio commented Aug 24, 2020

I've actually encountered the same issue this weekend. I had to relax the type constraints of ValidEventTypes a little bit, but the issue should be fixed once #232 has been merged.

@RAnders00 Feel free to test my branch with your code as well to be double check. I've also added a comment with a large code sample that shows the expected behaviour.

@forivall
Copy link
Contributor Author

Yeah, i'm still on typescript 3.9.7 . I didn't check if it's an issue on typescript 4+

@forivall
Copy link
Contributor Author

hmm, oddly, when i made the change to the index.d.ts directly, it worked, but now that i'm on the new version, it doesn't.

@gfmio
Copy link
Contributor

gfmio commented Aug 24, 2020

@forivall Your and my version are virtually identical. I don't have a preference. I've verified that yours works as well.

@gfmio
Copy link
Contributor

gfmio commented Aug 24, 2020

I'm also not sure what TypeScript version introduced the the issue. I tried out older TS versions, but they all seemed to fail, but I know that it did work at the time when we introduced the EventMap structure, because someone from the TypeScript team explicitly verified that it worked as well (for TypeScript 3.9).

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

Successfully merging this pull request may close these issues.

4 participants