Skip to content
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

preserve order, make variadic and handle falsy values in concat #1436

Merged
merged 6 commits into from Jun 23, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/concat.js
@@ -1,5 +1,5 @@
import concat from './internal/concat';
import doParallel from './internal/doParallel';
import doLimit from './internal/doLimit';
import concatLimit from './concatLimit';

/**
* Applies `iteratee` to each item in `coll`, concatenating the results. Returns
Expand All @@ -26,4 +26,4 @@ import doParallel from './internal/doParallel';
* // files is now a list of filenames that exist in the 3 directories
* });
*/
export default doParallel(concat);
export default doLimit(concatLimit, Infinity);
29 changes: 25 additions & 4 deletions lib/concatLimit.js
@@ -1,5 +1,7 @@
import doParallelLimit from './internal/doParallelLimit';
import concat from './internal/concat';
import noop from 'lodash/noop';
import wrapAsync from './internal/wrapAsync';
import slice from './internal/slice';
import mapLimit from './mapLimit';

/**
* The same as [`concat`]{@link module:Collections.concat} but runs a maximum of `limit` async operations at a time.
Expand All @@ -14,9 +16,28 @@ import concat from './internal/concat';
* @param {number} limit - The maximum number of async operations at a time.
* @param {AsyncFunction} iteratee - A function to apply to each item in `coll`,
* which should use an array as its result. Invoked with (item, callback).
* @param {Function} [callback(err)] - A callback which is called after all the
* @param {Function} [callback] - A callback which is called after all the
* `iteratee` functions have finished, or an error occurs. Results is an array
* containing the concatenated results of the `iteratee` function. Invoked with
* (err, results).
*/
export default doParallelLimit(concat);
export default function(coll, limit, iteratee, callback) {
callback = callback || noop;
var _iteratee = wrapAsync(iteratee);
mapLimit(coll, limit, function(val, callback) {
_iteratee(val, function(err /*, ...args*/) {
if (err) return callback(err);
return callback(null, slice(arguments, 1));
});
}, function(err, mapResults) {
var result = [];
var _concat = Array.prototype.concat;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can move this var to the top level scope.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, thank you!

for (var i = 0; i < mapResults.length; i++) {
if (mapResults[i]) {
result = _concat.apply(result, mapResults[i]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One note I just realized about this implementation is that it is breaking for ArrayLike objects:

var result = {0: 'foo', 1: 'bar', 2: 'baz', length: 3};
// previously
[].concat(result); // [{0: 'foo', 1: 'bar', 2: 'baz', length: 3}]
// now
Array.prototype.concat.apply([], args); // ['foo', 'bar', 'baz']
Nevermind, sorry, this will actually run
Array.prototype.concat.apply([], [args]); // [{0: 'foo', 1: 'bar', 2: 'baz', length: 3}]

keeping the previous behaviour.

}
}

return callback(err, result);
});
}
6 changes: 3 additions & 3 deletions lib/concatSeries.js
@@ -1,5 +1,5 @@
import concat from './internal/concat';
import doSeries from './internal/doSeries';
import doLimit from './internal/doLimit';
import concatLimit from './concatLimit';

/**
* The same as [`concat`]{@link module:Collections.concat} but runs only a single async operation at a time.
Expand All @@ -19,4 +19,4 @@ import doSeries from './internal/doSeries';
* containing the concatenated results of the `iteratee` function. Invoked with
* (err, results).
*/
export default doSeries(concat);
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this the last use of doSeries?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup. From the history, it looks like all of the other doSeries were converted to doLimit(<function_name>Limit, 1); when the *Series functions were implemented in terms of *Limit here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's time to delete it, then!

export default doLimit(concatLimit, 1);
11 changes: 0 additions & 11 deletions lib/internal/concat.js

This file was deleted.

8 changes: 0 additions & 8 deletions lib/internal/doSeries.js

This file was deleted.