Skip to content

Commit

Permalink
Require promise-returning functions when using the concurrency opti…
Browse files Browse the repository at this point in the history
…on in `PProgress.all()` (#3)

Fixes #4
  • Loading branch information
zikaari authored and sindresorhus committed Jul 11, 2018
1 parent 2a1efaa commit 8cd4e7e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 5 deletions.
15 changes: 11 additions & 4 deletions index.js
Expand Up @@ -16,12 +16,16 @@ class PProgress extends Promise {
return (...args) => {
return new PProgress((resolve, reject, progress) => {
args.push(progress);
input(...args).then(resolve, reject);
input(...args).then(resolve, reject); // eslint-disable-line promise/prefer-await-to-then
});
};
}

static all(promises, options) {
if (options && typeof options.concurrency === 'number' && !(promises.every(p => typeof p === 'function'))) {
throw new TypeError('When `options.concurrency` is set, the first argument must be an Array of Promise-returning functions');
}

return PProgress.fn(progress => {
const progressMap = new Map();
const iterator = promises[Symbol.iterator]();
Expand All @@ -31,7 +35,8 @@ class PProgress extends Promise {
};

const mapper = async () => {
const promise = iterator.next().value;
const next = iterator.next().value;
const promise = typeof next === 'function' ? next() : next;
progressMap.set(promise, 0);

if (promise instanceof PProgress) {
Expand Down Expand Up @@ -60,10 +65,12 @@ class PProgress extends Promise {
}

// We run this in the next microtask tick so `super` is called before we use `this`
Promise.resolve().then(() => {
Promise.resolve().then(() => { // eslint-disable-line promise/prefer-await-to-then
if (progress === this._progress) {
return;
} else if (progress < this._progress) {
}

if (progress < this._progress) {
throw new Error('The progress percentage can\'t be lower than the last progress event');
}

Expand Down
2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -35,6 +35,8 @@
"devDependencies": {
"ava": "*",
"delay": "^2.0.0",
"in-range": "^1.0.0",
"time-span": "^2.0.0",
"xo": "*"
}
}
4 changes: 3 additions & 1 deletion readme.md
Expand Up @@ -145,7 +145,7 @@ const allProgressPromise = PProgress.all([

Type: `Array`

Array of promises.
Array of promises or promise-returning functions, similar to [p-all](https://github.com/sindresorhus/p-all).

#### options

Expand All @@ -161,6 +161,8 @@ Number of concurrently pending promises.

To run the promises in series, set it to `1`.

When this option is set, the first argument must be an array of promise-returning functions.


## Related

Expand Down
44 changes: 44 additions & 0 deletions test.js
@@ -1,5 +1,7 @@
import test from 'ava';
import delay from 'delay';
import timeSpan from 'time-span';
import inRange from 'in-range';
import PProgress from '.';

const fixture = Symbol('fixture');
Expand Down Expand Up @@ -101,3 +103,45 @@ test('PProgress.all()', async t => {
undefined
]);
});

test('PProgress.all() with concurrency = 1', async t => {
const fixtureFn = PProgress.fn(async (input, progress) => {
progress(0.16);
await delay(50);
progress(0.55);
await delay(50);
return input;
});

const fixtureFn2 = PProgress.fn(async (input, progress) => {
progress(0.41);
await delay(50);
progress(0.93);
await delay(50);
return input;
});

// Should throw when first argument is array of promises instead of promise-returning functions
t.throws(() => PProgress.all([fixtureFn(fixture), fixtureFn2(fixture)], {
concurrency: 1
}), TypeError);

const end = timeSpan();
const p = PProgress.all([
() => fixtureFn(fixture),
() => fixtureFn2(fixture)
], {
concurrency: 1
});

p.onProgress(progress => {
t.true(progress >= 0 && progress <= 1);
});

t.deepEqual(await p, [
fixture,
fixture
]);

t.true(inRange(end(), 200 /* 4 delays of 50ms each */, 250 /* reasonable padding */));
});

0 comments on commit 8cd4e7e

Please sign in to comment.