-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Implement zipAll (Issue #458) #1195
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
Conversation
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the critical change in logic, as @leebyron pointed out in an earlier pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work on this - mostly looks great aside from a few small improvements.
If you don't get to these soon I'll attempt to manually patch them in
return iteratorValue( | ||
type, | ||
iterations++, | ||
zipper.apply(null, steps.map(function (s) { return s.value; })) | ||
zipper.apply(null, steps.map(function (s) { return s.done ? undefined : s.value; })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When s.done
is true, s.value
is guaranteed to be undefined
, so is this change necessary?
@@ -1848,18 +1851,22 @@ function zipWithFactory(keyIter, zipper, iters) { | |||
var steps; | |||
if (!isDone) { | |||
steps = iterators.map(function (i) { return i.next(); }); | |||
isDone = steps.some(function (s) { return s.done; }); | |||
if (zipAll) { isDone = steps.every(function (s) { return s.done; }); } | |||
else { isDone = steps.some(function (s) { return s.done; }); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a ternary for this:
isDone = zipAll ? steps.every(...) : steps.some(...)
Squashed commit of the following: commit 0701ba1e23e280e68921ac9b3941445e7bfa54ea Author: Travis CI <github@fb.com> Date: Fri Sep 29 15:40:12 2017 -0700 Fix spacing and follow ups commit 03d0691de5d663e9877c6e6584ac0659a273cf12 Merge: 07329da f1d3bee Author: Travis CI <github@fb.com> Date: Fri Sep 29 15:15:11 2017 -0700 Merge branch 'zipAll' of https://github.com/arjans/immutable-js into arjans-zipAll commit f1d3bee Author: Aditya K <adityavkk@gmail.com> Date: Thu Mar 30 17:06:56 2017 -0400 run build commit ed0aa0c Author: arjans <arjansingh1989@gmail.com> Date: Thu Mar 30 20:29:12 2017 +0000 Remove redundant boolean cast. commit 03a5f43 Author: arjans <arjansingh1989@gmail.com> Date: Thu Mar 30 20:26:10 2017 +0000 remove parens in single param arrow function commit 6c0e643 Author: arjans <arjansingh1989@gmail.com> Date: Thu Mar 30 20:24:33 2017 +0000 fix linter warnings about commas, let/var, and missing whitespaces commit 2751375 Author: arjans <arjansingh1989@gmail.com> Date: Thu Mar 30 19:56:04 2017 +0000 add documentation and type signatures in flow commit 66d01cc Author: Aditya K <adityavkk@gmail.com> Date: Wed Mar 29 20:09:01 2017 -0400 remove redundant variables in zipWithFactory commit da399c9 Author: Arjan Singh <arjansingh1989@gmail.com> Date: Wed Mar 29 17:06:37 2017 -0700 Get tests working for zipAll commit 4403c04 Author: Aditya K <adityavkk@gmail.com> Date: Wed Mar 29 19:16:47 2017 -0400 add conditional zipSize calculation for zipAll in zipWithFactory commit e84cebb Merge: 58b00c9 2884e76 Author: Aditya K <adityavkk@gmail.com> Date: Wed Mar 29 18:54:01 2017 -0400 Merge branch 'zipAll' of https://github.com/arjans/immutable-js into zipAll commit 58b00c9 Author: Aditya K <adityavkk@gmail.com> Date: Wed Mar 29 18:53:43 2017 -0400 move to const and let in __iteratorUncached commit 2884e76 Author: Arjan Singh <arjansingh1989@gmail.com> Date: Wed Mar 29 15:53:26 2017 -0700 Remove extra spaces. commit c0c8fdc Author: Arjan Singh <arjansingh1989@gmail.com> Date: Wed Mar 29 15:52:38 2017 -0700 Add zipAll to CollectionImpl commit faa2261 Author: Aditya K <adityavkk@gmail.com> Date: Wed Mar 29 18:43:25 2017 -0400 add zipAll handling in __iteratorUncached for zipSequence commit 492cb01 Author: Aditya K <adityavkk@gmail.com> Date: Wed Mar 29 18:39:00 2017 -0400 add zipAll flag to __iteratorUncached in zipSequence
…ings * facebook/master: (48 commits) 4.0.0-rc.3 Use latest immutable.js build on website Only build docs on tagged releases (immutable-js#1321) Relicense as MIT (immutable-js#1320) Fix rendering issue on tall screens Merge immutable-js#1285 Do not throw from hasIn (immutable-js#1319) Add more flow tests for Records Upgrade prettier and jest (immutable-js#1318) Merge immutable-js#1193 Minor flow record fixes Merges immutable-js#1195 fixed immutable-js#1290 by removing relative position from star count (immutable-js#1317) Fix deploy script Adds a script which automatically builds a #npm branch (immutable-js#1316) Use .github directory Merges immutable-js#1314 Flow config ignore all node_modules Update to latest version of flow (immutable-js#1312) fixed immutable-js#1313 by fixing two borked reducers (immutable-js#1315) ...
Implements
zipAll
Unlike
zip
,zipAll
continues zipping until the longest collection is exhausted. Missing values from shorter collections are filled withundefined
.This PR builds on some earlier work done in a previous PR