-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Possible internal interface update #778
Comments
I'm not really interested in changing the API right now, but you do have a very interesting observation that each = (arr, iterator, cb) => eachLimit(arr, Infinity, iterator, cb)
eachSeries = (arr, iterator, cb) => eachLimit(arr, 1, iterator, cb) We could simplify our internals if we implemented |
A quick test reveals that the performance impact is negligible, but it changes the behavior of the functions subtly in ways that cause tests to break. I really like the simplification, but this would be a v2 change. |
I think that methods Sync API var count = 0;
async.whilst(
function () { return count < 5; },
function (callback) {
count++;
setTimeout(callback, 1000);
},
function (err) {}
); Async API var count = 0;
async.whilst(
// Just check that arguments.length > 0
function (callback) {
return callback(null, count < 5);
},
function (callback) {
count++;
setTimeout(callback, 1000);
},
function (err) {}
); |
Relying on the arity of the function is too brittle -- functions with optional args won't have an accurate |
👍 |
Has anyone started working on this? I'm happy to take this on. |
Go for it. I also just created a 2.x branch for things we're tracking for |
I think I'd prefer leaving it as is and implementing |
Instead of having three functions
How about just having each take an optional object argument at the end
To run series have options be
{series: true}
To run with limit have options be
{limit: 2}
The
series
option could even be removed as its equivalent to{limit: 1}
I think this would help simplify the library as the number of functions will be reduced but the power will still be there and additional options can easily be added.
This could be done as a non-breaking change as
eachSeries
andeachLimit
are deprecated andeach
is extended to support the optionaloptions
argument.The text was updated successfully, but these errors were encountered: