Move the strongly-typed Typed types into the main types

[airhorns]

I think the extra type is unnecessary, and it's easier to maintain only one of them!

Fixes #67

[airhorns]

Ok, since we're ok with a breaking change, updated this to:

• default to having a loosely typed event data map accepting any event name and any event argument
• allow overriding the event data map with only one type, where each key is an event name and each value is undefined if that event has no listener arguments and otherwise is the type of the argument
• fix linting issues

Thanks for the feedback!

[sindresorhus]

Maybe make a type alias for type EventNames = keyof EventData?

[airhorns]

I opted not to do that because to create type aliases from operators on the generics within class definitions like this, you have to use more generic types with defaults. To the best of my knowledge, you can't just do:

class Foo<T> {
  type U = keyof T;
  // ...
}

you have to do

class Foo<T, U = keyof T> {
  // ...
}

For types that are meant to be consumed by end users, which Emittery definitely is, I always find this confusing. The type you are going to use has more type parameters than you are supposed to pass, and without looking at the code or knowing that the code is using this trick, it seems unclear. So, I'd say its better to be verbose in the class body and keep the top level user facing type interface clean, but happy to change if you feel strongly.

[airhorns]

Just realized I did this already for the DatalessEvents type so I am being kind of inconsistent, oops. I will move this up in the same way.

[airhorns]

AH I remember why this wasn't possible now -- fancy typesystem reasons. Because of the strange no-type-aliases-only-derived-generic-type-defaults thing above, the generic type will usually be the keyof EventNames type, but could be instantiated with a different type if the user passed it in. For that reason, TypeScript can't for sure, always access EventNames[Name], because Name only defaults to keyof EventNames, but could be something else. So, it doesn't type check, so I can't really do it. Sorry for all the confusion!

[sindresorhus]

@mmkal Looks good to you now?

[mmkal]

Lgtm - made a comment re tests. Couple of other questions I wouldn't consider blocking but worth asking since this is will be a breaking release:

• should the .Typed alias be removed from js too to avoid confusion?
• is there a plan to rewrite in typescript and use tsc to generate the d.ts file? If doing that might yield other small breaking changes it'd be nice for users if they got bundled.

[sindresorhus]

should the .Typed alias be removed from js too to avoid confusion?

Yes

[sindresorhus]

is there a plan to rewrite in typescript and use tsc to generate the d.ts file? If doing that night yield other small breaking changes it'd be nice for users if they got bundled.

If anyone wanna do the work, sure. But it's not something I plan to work on.

[sindresorhus]

@airhorns Can you do #69 (comment) ?

[airhorns]

Yeah, while adding the extended tests I noticed some other issues, still trying to work through those.

[airhorns]

Ok, I think this is ready for rereview!

[sindresorhus]

Make sure you run $ npx xo --fix locally and fix the remaining issues.

[sindresorhus]

I did 48ea6ca to help you enforce the correct style.

[airhorns]

Ok, corrected all my style issues and the latent typescript lint issues, which are unrelated to this PR but resulted from the xo version bump.

[sindresorhus]

Looks good. Thank you :)