Skip to content

Add async iterator interface to cursors - #371

Open
mxstbr wants to merge 1 commit into
neumino:masterfrom
mxstbr:async-iterators
Open

mxstbr wants to merge 1 commit into
neumino:masterfrom
mxstbr:async-iterators

Conversation

@mxstbr

@mxstbr mxstbr commented Mar 12, 2018

Copy link
Copy Markdown

See #370, this patch adds a cursor.asyncIterator method which returns an async iterator:

const feed = yield r.db().table().changes().run();

const iterator = feed.asyncIterator();

for await (var change of iterator) {
  console.log(change);
}
console.log('Done!');

I added iterall as a dependency since I need to use it in the tests anyway, but if you want me to I can move it to the devDependencies and inline the $$asyncIterator polyfill snippet. (it's like 3 lines)

@mxstbr

mxstbr commented Mar 12, 2018

Copy link
Copy Markdown
Author

Wercker failed even though all tests passed 🤔

Comment thread lib/cursor.js
(error.message.match(/You cannot call `next` on a closed/) === null)) {
return iter.throw(error);
} else {
return iter.return();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mxstbr What other errors might occur? why stop the iterations for only the above errors?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good question, I copied that from the other places where self._next() is called. Not sure why it's that specific, honestly!

@sagivf sagivf Mar 13, 2018 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suppose it makes sense, since any other errors you might want to ignore where as these don't make sense to continue.

This does raise a question though. With callbacks you get the error and can ignore or handle it.
In this case you wont get it. maybe it should be a part of the return iter.return(err).

This also makes me really wonder what errors are possible... I'll ask around.

@segphault

Copy link
Copy Markdown

I came here to do this and was pleasantly surprised to find a PR already open. Now that async iterators are supported natively in node, this would definitely be nice to have, ideally without polyfill. Would it be possible to use the [Symbol.asyncIterator] protocol so that it's possible to pass the cursor itself directly into the for-await-of loop?

@mxstbr

mxstbr commented Apr 25, 2018

Copy link
Copy Markdown
Author

Would it be possible to use the [Symbol.asyncIterator] protocol so that it's possible to pass the cursor itself directly into the for-await-of loop?

Already possible!

@thelinuxlich

Copy link
Copy Markdown

@neumino ?

@thelinuxlich

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants