nicktho/batch-promises

easily batch promises

batch-promises's Issues

Code does not pass tests.

One of the four tests does not pass:

batchPromises
    ✓ should return an array of results (315ms)
    ✓ should work with a promise returning an array (311ms)
    ✓ should batch promises (308ms)
    1) should reject on an error and halt execution

Release version with type declarations

The type declarations are sitting there in the repo, but aren't present on the 0.0.3 release. This just needs an npm release.

Elaborate more with usage example

While minor, perhaps you could elaborate a little more in the doco usage section on whether the iteratee is fired after each promise in the array, or each batch.

Also with the example it doesn't depict that the batching process is in effect. Perhaps, if a setTimeout is to be used then you could add some time comments, like:

// batchPromises(2, [1, 2, 3, 4, 5])

// Time: 100ms = 1, 2 (first batch of 2)
// Time: 200ms = 3, 4 (second batch of 2)
// Time: 300ms = 5 (last remaining batch)
// result [1, 2, 3, 4, 5]

What do you think?

Trying to understand this

Hi, if you pass this function an array of promises, would it execute them in batches?

What I mean is, if a promises array had 100 entries, would batchPromises(4, promises) wait for the first 4 to complete, before executing the next 4?

Thanks.

Batch size

Initially I thought that the batch size was the total number of batches to split the array into, instead of an explicit batch size. Perhaps this could be an additional feature/option?

For example:

batchPromises(2, [1, 2, 3, 4, 5, 6])
- batch 1 = 1, 2, 3
- batch 2 = 4, 5, 6

So with an array of 100 promises and a batch size of 2, it would batch into two chunks of 50.

Odd numbers would have to be accounted for, perhaps that could always be the last remaining batch eg.

batchPromises(2, [1, 2, 3, 4, 5])
- batch 1 = 1, 2
- batch 2 = 3, 4, 5

Interesting

I had a bit of a different idea for method batch when I wrote spex.

Just sharing ideas... ;)

Changing arguments order

Would be nice to change the arguments order in order to put the collection at the end. This would allow the following usage:

import batchPromises from 'batch-promises';

Promise.resolve([1, 2, 3, 4, 5])
.then(batchPromises.bind(null, function() { }, 4));

Note i also put the Iteratee as the first argument since we also could want to set the batch size dynamically:

import batchPromises from 'batch-promises';

Promise.all([
  4,
  [1, 2, 3, 4, 5]
])
.spread(batchPromises.bind(null, function() { }));

It could be an occasion to publish a 1.0.0 version and embrace semver at the same time ;).

Let me know if you want me to do a PR according to those changes.

TypeError #<Promise> is not a function.

Trying to run an array of promises in batch-promises results in: TypeError #<Promise> is not a function.

For example I was able to produce this with:

const batchPromises = require('batch-promises')
var promiseArr = []
for(i = 0; i < 10000; ++i) {
  promiseArr.push(new Promise((resolve) => {setTimeout(() => {resolve()}, 1000)}))
}

batchPromises(5, promiseArr, new Promise((resolve) => {
  setTimeout(() => {
    resolve()
  }, 1000)
})).then(() => {
  console.log('done')
}).catch((err) => {
  console.log(err)
})

Running this example produces the following output:

$ node test.js 
TypeError: #<Promise> is not a function
    at Array.map (native)
    at /home/CORP.INSTRUCTURE.COM/ecoan/canvas-support-utils/node_modules/batch-promises/index.js:8:38
    at process._tickCallback (internal/process/next_tick.js:103:7)
    at Function.Module.runMain (module.js:577:11)
    at startup (node.js:159:18)
    at node.js:444:3

Relevant bits of info:

$ node --version
v6.0.0
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 16.04.1 LTS
Release:    16.04
Codename:   xenial

Throw a nicer error

It would be nice to not hit a .map() is not a function error if the resolved/passed in collection is not an array.