Repository navigation
Conversation
takeWhile's __iterate callback returned `predicate() && ...`, and __iterate only stops on `false`. A predicate returning null, 0 or '' did not stop iteration, so later matching values leaked through. For example List(['dog', 'frog', 'cat', 'hat', 'god']) .takeWhile(x => x.match(/o/)) returned List [ "dog", "frog", undefined, undefined, "god" ]. zip/zipAll reversed each input on its own for reverse iteration, which misaligns values when the inputs have different sizes. Stack builds from a reversed iteration, so Stack([1, 2]).zipAll(Stack([3, 4, 5])) returned [[undefined, 3], [1, 4], [2, 5]]. Zip forward and walk the result backwards instead. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01NQDmA3mwGfL83pbg9WQ6ey
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran the code examples from the website docs and the type definitions against the current
mainbuild. Two of them printed something other than what the docs say. In both cases the docs are right and the library is wrong. This PR fixes both bugs insrc/Operations.jsand adds regression tests.1.
takeWhiledoes not stop when the predicate returns a falsy value that is notfalseThe
takeWhileexample on theCollection.KeyedandSeq.Keyedpages is:On
mainit returnsList [ "dog", "frog", undefined, undefined, "god" ]. The live REPL on the List, Map, Set, Seq.Set and Collection.Set pages runs the same code, so it shows that wrong result today.The cause is in
takeWhileFactory. The__iteratecallback returnspredicate(...) && ++iterations && fn(...).__iterateonly stops when the callback returns exactlyfalse. When the predicate returnsnull,0,''orundefined, iteration keeps going and later matching values are passed through. The__iteratorpath already uses!predicate(...), so spreading the same Seq gives the right answer andtoArray()does not:The TypeScript signature says the predicate returns a
boolean, but plain JS callers, and the docs example itself, return truthy or falsy values.filter,skipWhileand the iterator path oftakeWhilealready accept that. The fix returns an explicitfalsewhen the predicate result is falsy.2. Reversed
zip/zipAllmisaligns values when the collections have different sizesThe
Stack#zipAllexample in the docs is:On
mainit returnsStack [ [ undefined, 3 ], [ 1, 4 ], [ 2, 5 ] ].Stack([1, 2, 3]).zip(Stack([4, 5]))also returns[ [ 2, 4 ], [ 3, 5 ] ]instead of[ [ 1, 4 ], [ 2, 5 ] ].The cause is in
zipWithFactory. For reverse iteration it reverses each input on its own and zips the reversed inputs. That only works when all inputs have the same size.Stackbuilds itself by iterating its source in reverse, so it always hits this. A reversed lazy zip shows it too:The fix zips forward and then walks the result backwards when
reverseis set. This is the same idea astakeWhileandskipWhile, which callcacheResult()for reverse iteration. Keys in reverse order stay0, 1, 2, ...like before.Tests
__tests__/ListJS.js:takeWhilewith a predicate that returnsnullor0, checked through a List and through a Seq withtoArray(),Array.from()andcount(). I put it in the JS test file because the TypeScript signature asks for aboolean, so the docs example does not type-check in a.tstest.__tests__/zip.ts: reversedzipandzipAllwith different sizes, andStack#zip/Stack#zipAllwith different sizes.All three new tests fail on
mainand pass with this change. I rannpm run lint,npm run type-check,npm run build,CI=1 npm run test:unit(tests the built file) andnpm run test:types, and they pass.Both bugs are also present on the
6.xbranch insrc/operations/factories.ts. I can port the fix there if you want. I did not add a CHANGELOG entry since recent outside PRs left that to you, but I can add one under "Unreleased".