Skip to content
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

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5656357
feat: register event listener with asynchronous iterator protocol - c…
lorenzofox3 Jan 2, 2018
78cbcd6
keep single line block formatting
lorenzofox3 Jan 2, 2018
b50ec91
fix xo and test setting
lorenzofox3 Jan 3, 2018
c904739
enable xo
lorenzofox3 Jan 3, 2018
c951511
removed useless eslint flag comments
lorenzofox3 Jan 3, 2018
d9b617a
split async iterator behavior to different file
lorenzofox3 Jan 3, 2018
cac49ab
Revert "split async iterator behavior to different file"
novemberborn Jan 4, 2018
ade1383
Merge branch 'master' into pr/20
novemberborn Jan 4, 2018
a4d414d
Implement async iterator protocol without generators, fix tests
novemberborn Jan 4, 2018
a6a3476
Fix linting error
novemberborn Jan 6, 2018
e91559c
Simplify next() implementation
novemberborn Jan 6, 2018
c4ca867
Handle edge case where return() is called by an earlier listener for …
novemberborn Jan 6, 2018
e18c260
Implement return() according to spec
novemberborn Jan 6, 2018
edb7ac5
Remove unnecessary tsconfig files in test fixtures
novemberborn Jan 6, 2018
756b47a
Change how TypeScript is loaded in types test
novemberborn Jan 6, 2018
7ef2983
Return async iterator from .events(), not .on()
novemberborn Jan 6, 2018
463c4e6
Implement .anyEvent()
novemberborn Jan 6, 2018
3f9aa37
Tweak AVA's Babel options
novemberborn Jan 6, 2018
ef596e6
Support running tests without for-await-of transpilation
novemberborn Jan 6, 2018
361cddd
Fix for-await transpilation
novemberborn Jan 7, 2018
735ee10
Ensure async iterators return non-promise values
novemberborn Jan 7, 2018
6d73663
Merge branch 'master' into pr/20
novemberborn Jan 20, 2018
0afc521
Tweak iterator implementation now that scheduling is more consistent
novemberborn Jan 20, 2018
4cdcd6f
Remove wrongly placed documentation
novemberborn Jan 20, 2018
94f54c0
Separate iterator production
novemberborn Jan 20, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 30 additions & 2 deletions index.js
Expand Up @@ -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);
Copy link
Collaborator

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 after await 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.

Copy link
Contributor Author

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.

}
}
/* eslint-enable no-constant-condition */
/* eslint-enable no-await-in-loop */
} finally {
off();
}
}

module.exports = class Emittery {
constructor() {
this._events = new Map();
Expand All @@ -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') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check the number of arguments passed, rather than the listener type? It seems a little too easy to accidentally create an asynchronous iterator otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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) {
Expand Down
15 changes: 14 additions & 1 deletion package.json
Expand Up @@ -50,19 +50,32 @@
"ava": "*",
"babel-cli": "^6.26.0",
"babel-core": "^6.26.0",
"babel-eslint": "^8.1.2",
"babel-plugin-transform-async-generator-functions": "^6.24.1",
"babel-plugin-transform-async-to-generator": "^6.24.1",
"babel-plugin-transform-es2015-spread": "^6.22.0",
"codecov": "^3.0.0",
"delay": "^2.0.0",
"nyc": "^11.3.0",
"xo": "*"
},
"xo": {
"parser": "babel-eslint"
},
"babel": {
"plugins": [
"transform-async-to-generator",
"transform-es2015-spread"
"transform-es2015-spread",
"transform-async-generator-functions"
],
"presets":[
"@ava/stage-4"
]
},
"ava": {
"require": "babel-register",
"babel": "inherit"
},
"nyc": {
"reporter": [
"html",
Expand Down
22 changes: 22 additions & 0 deletions readme.md
Expand Up @@ -47,6 +47,28 @@ Returns an unsubscribe method.

Using the same listener multiple times for the same event will result in only one method call per emitted event.

If you use the method with only the first argument, it will return an asynchronous iterator. Your listener will therefore be the loop body and you'll be able to
unsubscribe to the event simply by breaking the loop.

```Javascript
const emitter = new Emittery();
//subscribe
(async function () {
for await (let t of emitter.on('foo')) {
if(t >10){
break;//unsubscribe
}
console.log(t);
}
})();

let count = 0;
setInterval(function () {
count++;
emitter.emit('foo', count);
}, 1000);
```

##### listener(data)

#### off(eventName, [listener])
Expand Down
22 changes: 22 additions & 0 deletions test/_run.js
Expand Up @@ -37,6 +37,28 @@ module.exports = Emittery => {
t.is(emitter._events.get('🦄').size, 1);
});

test('on() - async iterator', async t => {
const fixture = '🌈';
const emitter = new Emittery();
setTimeout(() => {
emitter.emit('🦄', fixture);
}, 300);
const iterator = emitter.on('🦄');
const {value, done} = await iterator.next();
t.deepEqual(done, false);
t.deepEqual(value, fixture);
});

test('on() - async iterator (queued)', async t => {
const fixture = '🌈';
const emitter = new Emittery();
const iterator = emitter.on('🦄');
emitter.emit('🦄', fixture);
const {value, done} = await iterator.next();
t.deepEqual(done, false);
t.deepEqual(value, fixture);
});

test('off()', t => {
const emitter = new Emittery();
const listener = () => {};
Expand Down