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
Implement listener subscription as async iterator protocol #20
Changes from 5 commits
5656357
78cbcd6
b50ec91
c904739
c951511
d9b617a
cac49ab
ade1383
a4d414d
a6a3476
e91559c
c4ca867
e18c260
edb7ac5
756b47a
7ef2983
463c4e6
3f9aa37
ef596e6
361cddd
735ee10
6d73663
0afc521
4cdcd6f
94f54c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,29 @@ function assertEventName(eventName) { | |
} | ||
} | ||
|
||
async function * iterator(emitter, eventName) { | ||
const queue = []; | ||
const off = emitter.on(eventName, data => { | ||
queue.push(data); | ||
}); | ||
|
||
try { | ||
/* eslint-disable no-constant-condition */ | ||
/* eslint-disable no-await-in-loop */ | ||
while (true) { | ||
if (queue.length > 0) { | ||
yield queue.shift(); | ||
} else { | ||
yield await emitter.once(eventName); | ||
} | ||
} | ||
/* eslint-enable no-constant-condition */ | ||
/* eslint-enable no-await-in-loop */ | ||
} finally { | ||
off(); | ||
} | ||
} | ||
|
||
module.exports = class Emittery { | ||
constructor() { | ||
this._events = new Map(); | ||
|
@@ -24,8 +47,13 @@ module.exports = class Emittery { | |
|
||
on(eventName, listener) { | ||
assertEventName(eventName); | ||
this._getListeners(eventName).add(listener); | ||
return this.off.bind(this, eventName, listener); | ||
|
||
if (typeof listener === 'function') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this check the number of arguments passed, rather than the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you wish. Although it is a public method with a specific contract so I think this alternative is better. Otherwise it means I'll need to check for "arguments" which is not correct in strict mode; Or use the spread operator on the signature which makes the contract less obvious in my opinion |
||
this._getListeners(eventName).add(listener); | ||
return this.off.bind(this, eventName, listener); | ||
} | ||
|
||
return iterator(this, eventName); | ||
} | ||
|
||
off(eventName, listener) { | ||
|
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 shouldn't yield,
queue
will contain the data of the next event afterawait
returns.Alternatively you could await a new promise, and expose its
resolve()
method to the listener created above. That way you don't need to add an additional listener.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.
you are right the yield is superfluous.
However I don't think exposing the resolve of the new promise on each iteration is a good idea: As the listener above is created once I would need to create a binding in its closure and reassign this binding to the resolve of the new promise on each iteration. That seems a bit awkward and unsafe, or I am missing something.