-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
refactor: migrate lib/events.js into ES6 #3012
Conversation
@@ -24,7 +24,9 @@ var BaseLauncher = function (id, emitter) { | |||
Object.keys(EventEmitter.prototype).forEach(function (method) { | |||
this[method] = EventEmitter.prototype[method] | |||
}, this) | |||
KarmaEventEmitter.call(this) | |||
|
|||
this.bind = KarmaEventEmitter.prototype.bind.bind(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.
This trick was needed because of cannot calls KarmaEventEmitter without new operator
like error. I have plan to do further refactoring, and transform BaseLauncher into regular ES6 class.
lib/events.js
Outdated
|
||
eventsToBuffer.forEach(function (eventName) { | ||
var listener = genericListener.bind(null, eventName) | ||
eventsToBuffer.forEach(eventName => { |
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.
We seem to have -- mostly -- been using parens even for the one-argument case:
eventsToBuffer.forEach((eventName) => {
This form is more instantly readable as a function and makes adding/removing args easy.
(A couple of places)
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.
Ok, I understand and will improve it.
What do you think about update eslint configuration with arrow-parens rule to catch such cases? I can do pr for that.
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.
sounds great!
Within this pull request I moved lib/events into ES6 class. In next iteration I'm planning to migrate pseudo-classes that extends
KarmaEventEmitter
into classes.