-
Notifications
You must be signed in to change notification settings - Fork 388
added generic for EventEmitter to allow Custom EventKeys like an enum… #134
Conversation
|
||
export class EventEmitter { | ||
export class EventEmitter<E = EventKey> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So <E = EventKey>
turns EventEmitter
into a generic class with a type variable that defaults to EventKey
if not provided? (I don't know TS, so I'm speculating! 😄)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is ture. If no generic type if given, EventKey
is used. But is there is any given, E
will turn to need the given value. Typescript genric works like any other generic languages.
I' ll test providing no value to the generic type resulting is need a type of KeyEvent
and providing eg. an enum type will result in requireing a typeof of enum given to the generic pattern.
You have to know that this default type KeyEvent
will only allow a backwards compatiblity at typescript >= 2.3. but typescript ^2.4 is at current stable.
I hope this meight help
EventEmitter.d.ts
Outdated
removeAllListeners(event?: EventKey): this; | ||
emitEvent(event: EventKey, args?: any[]): this; | ||
trigger(event: EventKey, args?: any[]): this; | ||
emit(event: EventKey): this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this actually work though, like if you said "my event key type is now MyType", you could then call .addEventListener(somethingOfMyType, fn)
, which would type check, but throw an exception. Right? You can pass any type you want as a key, so I'm not sure how this added functionality helps.
Had a look, yeah, whatever you pass is expected to either be a regular expression or a string. Anything else will be coerced to a string. I don't see the benefit for letting users change the key type. The only thing that should type check should be a string or a regular expression, as is the case right now.
Line 80 in 6f211f4
response = events[evt] || (events[evt] = []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would not throw an exception but it would thorow an error while trying to compile (or transpile) typescript. js code it not infected beaause the .d.ts file it not loaded.
the added functionallity meight be the folllowing:
export enum Events {
CoolEvent, EvenCoolerEvent
}
export class CoolClass extends EventEmitter<Events> {
constructor() {
this.emit(Events.CoolEvent); // this meight work because of the enum type in generic
}
}
currently if i try to extends EventEmitter the required param of emit()
or other function is EventKey
and EventKey is not an Enum.
So you now can type whateher type you whant as EventKey
.
You need to know that Typescript handles Enumerations as numeric value to transpile down to javascript. That means if you have an enum enum TestEnum { cool}
and you type TestEnum.cool
it will result in a numeric value of 0 because Typescript uses an object with an string index to find out the value (wich is numeric). so an enum ist not a string or RegExp.
I'll hope this helped!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be more clear about the type checking, you must know that everything about the REAL type checking is linke you the the contributors implemeted it. typescript is a layer above javascript that doesnt change javascript code or its behaviour. so to change the *.d.ts file everything in the *.js file will behave like it is originally written. just the tsc (typescript compiler or transpiler) will notice about the changes.
Sorry...
I can't amend my commits through this :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good. Just had to resolve a conflict.
In v5.2.2, I hope it works! |
Thanks! |
Changed the typescript definition file with backwards compatibility to allow generic pattern for event names link enums.
I have not changed any js code, so i haven' t run any tests.
My impl. of this .d.ts works