-
Notifications
You must be signed in to change notification settings - Fork 226
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
Reduce type constraints #232
Conversation
… new type helpers
…rove type inference
Here's an example: import eventemitter3 from "eventemitter3";
interface MyEventMap {
anEvent: () => void;
anotherEvent: (a: number, b: string) => void;
yetAnotherEvent: [string, number, boolean];
invalid: string;
}
const ee = new eventemitter3.EventEmitter<MyEventMap>();
ee.emit("anEvent");
ee.emit("anotherEvent", 1, "");
ee.emit("yetAnotherEvent", "", 0, true);
ee.emit("invalid"); // works, but will suggest any[] for the event args
ee.emit("string"); // throws type error
const ee2 = new eventemitter3.EventEmitter<string>();
ee2.emit("anything"); // allows any event name and set of args
const ee3 = new eventemitter3.EventEmitter<"anEvent" | "anotherEvent">();
ee3.emit("anEvent"); // allows the "anEvent" event and any[] set of args
ee3.emit("anotherEvent"); // allows the "anotherEvent" event and any[] set of args
ee3.emit("invalid"); // throws type error |
alternatively (diff generated using diff --git a/node_modules/eventemitter3/index.d.ts b/node_modules/eventemitter3/index.d.ts
index 769b427..5519d4c 100644
--- a/node_modules/eventemitter3/index.d.ts
+++ b/node_modules/eventemitter3/index.d.ts
@@ -94,7 +94,7 @@ declare namespace EventEmitter {
export type ValidEventTypes =
| string
| symbol
- | { [K in string | symbol]: any[] | ((...args: any[]) => void) | undefined };
+ | object;
export type EventNames<T extends ValidEventTypes> = T extends string | symbol
? T
@@ -113,7 +113,7 @@ declare namespace EventEmitter {
K extends EventNames<T>
> = T extends string | symbol
? (...args: any[]) => void
- : (...args: ArgumentMap<Exclude<T, string | symbol>>[K]) => void;
+ : (...args: ArgumentMap<Exclude<T, string | symbol>>[Extract<K, keyof T>]) => void;
export type EventArgs<
T extends ValidEventTypes, |
I don't use TypeScript so I don't know which one is better. I'll wait until tomorrow to keep discussion going. I would like to reach consensus on a single fix. |
Here's my cleaner version, which doesn't go through |
@forivall can you open a PR? |
Thank you. |
This PR fixes the same issue that #231 was addressing. The PR didn't properly fix the issue.
The underlying problem was that TypeScript now requires an index signature for object types as currently used in the type constraint for
ValidEventMaps
.The PR relaxes the type constraint to allow arbitrary object types. The event emitter will recognise all keys
K
of the object typeT
as event names.The existing behaviour does not change: If the type
T[K]
is an array type, it is used as the type of the event args. If it is a function type, the parameter type of the function becomes the type of the event args.The new behaviour is that for keys where
T[K]
is neither a function nor an array type, the argument type for the eventK
now becomesany[]
.This fixes the bug and the relaxation should offer an acceptable compromise.