If I read the coffeescript correctly this method will sequentialize all iterations by invoking the next callback when the previous one is resolved. Isn't it possible to execute all iterations in parallel with a Promise.all-like approach?

@jorrit that would be easier to do now, at least:

var results = [];

cursor.eachAsync(function (row) {
  results.push(asyncRowHandler(row.id));
  // as long as we don't return the promise
  // the next row will be read immediately
})
.then(function () {
  return Promise.all(results).then(function (data) { /* ... */ });
});

Personally I see no benefit in eachAsync supporting a callback that returns Promises when it serializes these promises. Maybe make it configurable with an options parameter?

/cc @deontologician @Tryneus

はい

I would make me (the testing guy) particularly happy if you added appropriate tests to test/rql_test/connections/cursor.mocha

@larkost I already copied the cursor.each tests and made them work for cursor.eachAsync, but I'll definitely add a couple more since those are fairly sparse.

Maybe make it configurable with an options parameter?

I like this interface. It seems to make the common case easy. When I think of iteration with each, I think of them executing in order. Given that you can just collect them and run them asynchronously if you want, it seems ok not to provide an option.

It looks good to me overall. I've played with the code a bit and it works like I'd expect. Since this is an api change, I'd like to let it breath for a bit before merging it. Maybe get some opinions from other js guys that hang around here like @neumino and @joaojeronimo ...

@larkost just added tests covering all the cases outlined in my opening comment and added a "stop iteration early" test to cursor.each. Let me know if that doesn't suffice.

@deontologician somewhat unrelated to this PR, the original cursor.each implementation appears to read an extra row off the cursor after iteration has already been stopped. I added a fix for that and the tests still pass, but I wanted to make sure I wasn't overlooking something intentional.

Looks like it was part of removing hasNext() in commit 190c8c2 . It's possible reading one more is for ensuring the cursor has retrieved everything there is off the server, but I haven't looked too closely yet at the code.

Ok, that makes me pretty certain it was a bug. We weren't even checking the result of that last (unintentional) iteration, so I don't see how it could impact anything.

@deontologician -- Adding a new method seems reasonable to me and I can see how useful it can be.

Maybe there's a better name than eachAsync since people may thing that each is synchronous, but I have to admit that I can't come up with a better name.
Bluebird's doc calls map(... {concurrency: 1}) a static map, but it doesn't sound good :)

Ok, I'm ready to merge this, but I was wondering what people thought of the name "eachPromise" for the name. I think I agree with @neumino that since both each implementations are async, it might be misleading to call it eachAsync. @marshall007 thoughts? (or a better name)

@deontologician I think at the next opportunity to introduce breaking changes (i.e. 2.2) we should consider changing the current signature of cursor.each so that it can once again support both styles:

cursor.each(function(err, row), function()) // current

cursor.each(function(row, cb), function(err)) // new callback-style
cursor.each(function(row)) // new promise-style

I wouldn't mind waiting to merge all this at once under cursor.each. If you want to go ahead and get it released, I think the -Async suffix is fairly standard and I definitely prefer that over eachPromise.

https://github.com/petkaantonov/bluebird/blob/master/API.md#promisification

Notice examples like fs.readFileAsync in their docs even though fs.readFile is asynchronous as well.

Hmm. Alright, in that case I think we should just merge this as-is then. Breaking backwards compatibility in something this core is something I would expect in a major release (like 3.0). I think adding the extra method is a good compromise in the meantime.