Repository: immutable-js/immutable-js
Pull request #2038 (2024-12-09 10:37:55)

Fix issue 1915
Hello!
This is a fix for a 2 years old issue: https://github.com/immutable-js/immutable-js/issues/1915
Let me know if you have any question or feel some stuffs should be done differently.
Cheers

jdeniau: Hi @alexvictoor .

Can you rebase onto the main branch ? It seems that you have included a lot of commits 
alexvictoor: I messed up a little bit with my commit but there is only one...

On Mon, Dec 9, 2024 at 11:40 AM Julien Deniau ***@***.***>
wrote:

> Hi @alexvictoor <https://github.com/alexvictoor> .
>
> Can you rebase onto the main branch ? It seems that you have included a
> lot of commits
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/immutable-js/immutable-js/pull/2038#issuecomment-2527551743>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AADKXNANQ4HA565NJSSTHV32EVXTPAVCNFSM6AAAAABTIQP7LGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRXGU2TCNZUGM>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>


Patch:
diff --git a/src/Iterator.js b/src/Iterator.js
index e563ff576..3e0eb9cf8 100644
--- a/src/Iterator.js
+++ b/src/Iterator.js
@@ -29,7 +29,8 @@ Iterator.prototype[ITERATOR_SYMBOL] = function () {
 };
 
 export function iteratorValue(type, k, v, iteratorResult) {
-  const value = type === 0 ? k : type === 1 ? v : [k, v];
+  const value =
+    type === ITERATE_KEYS ? k : type === ITERATE_VALUES ? v : [k, v];
   iteratorResult
     ? (iteratorResult.value = value)
     : (iteratorResult = {
diff --git a/src/Operations.js b/src/Operations.js
index f50857a06..a7c2d7a8e 100644
--- a/src/Operations.js
+++ b/src/Operations.js
@@ -13,8 +13,8 @@ import {
   IndexedCollection,
 } from './Collection';
 import { isCollection } from './predicates/isCollection';
-import { isKeyed } from './predicates/isKeyed';
-import { isIndexed } from './predicates/isIndexed';
+import { IS_KEYED_SYMBOL, isKeyed } from './predicates/isKeyed';
+import { IS_INDEXED_SYMBOL, isIndexed } from './predicates/isIndexed';
 import { isOrdered, IS_ORDERED_SYMBOL } from './predicates/isOrdered';
 import { isSeq } from './predicates/isSeq';
 import {
@@ -587,6 +587,100 @@ export function skipWhileFactory(collection, predicate, context, useKeys) {
   return skipSequence;
 }
 
+class ConcatSeq extends Seq {
+  constructor(iterables) {
+    this._wrappedIterables = iterables.flatMap(iterable => {
+      if (iterable._wrappedIterables) {
+        return iterable._wrappedIterables;
+      }
+      return [iterable];
+    });
+    this.size = this._wrappedIterables.reduce((sum, iterable) => {
+      if (sum !== undefined) {
+        const size = iterable.size;
+        if (size !== undefined) {
+          return sum + size;
+        }
+      }
+    }, 0);
+    this[IS_KEYED_SYMBOL] = this._wrappedIterables[0][IS_KEYED_SYMBOL];
+    this[IS_INDEXED_SYMBOL] = this._wrappedIterables[0][IS_INDEXED_SYMBOL];
+    this[IS_ORDERED_SYMBOL] = this._wrappedIterables[0][IS_ORDERED_SYMBOL];
+  }
+
+  __iterateUncached(fn, reverse) {
+    if (this._wrappedIterables.length === 0) {
+      return;
+    }
+
+    if (reverse) {
+      return this.cacheResult().__iterate(fn, reverse);
+    }
+
+    let iterableIndex = 0;
+    const useKeys = isKeyed(this);
+    const iteratorType = useKeys ? ITERATE_ENTRIES : ITERATE_VALUES;
+    let currentIterator = this._wrappedIterables[iterableIndex].__iterator(
+      iteratorType,
+      reverse
+    );
+
+    let keepGoing = true;
+    let index = 0;
+    while (keepGoing) {
+      let next = currentIterator.next();
+      while (next.done) {
+        iterableIndex++;
+        if (iterableIndex === this._wrappedIterables.length) {
+          return index;
+        }
+        currentIterator = this._wrappedIterables[iterableIndex].__iterator(
+          iteratorType,
+          reverse
+        );
+        next = currentIterator.next();
+      }
+      const fnResult = useKeys
+        ? fn(next.value[1], next.value[0], this)
+        : fn(next.value, index, this);
+      keepGoing = fnResult !== false;
+      index++;
+    }
+    return index;
+  }
+
+  __iteratorUncached(type, reverse) {
+    if (this._wrappedIterables.length === 0) {
+      return new Iterator(iteratorDone);
+    }
+
+    if (reverse) {
+      return this.cacheResult().__iterator(type, reverse);
+    }
+
+    let iterableIndex = 0;
+    let currentIterator = this._wrappedIterables[iterableIndex].__iterator(
+      type,
+      reverse
+    );
+    return new Iterator(() => {
+      let next = currentIterator.next();
+      while (next.done) {
+        iterableIndex++;
+        if (iterableIndex === this._wrappedIterables.length) {
+          return next;
+        }
+        currentIterator = this._wrappedIterables[iterableIndex].__iterator(
+          type,
+          reverse
+        );
+        next = currentIterator.next();
+      }
+      return next;
+    });
+  }
+}
+
 export function concatFactory(collection, values) {
   const isKeyedCollection = isKeyed(collection);
   const iters = [collection]
@@ -618,22 +712,7 @@ export function concatFactory(collection, values) {
     }
   }
 
-  let concatSeq = new ArraySeq(iters);
-  if (isKeyedCollection) {
-    concatSeq = concatSeq.toKeyedSeq();
-  } else if (!isIndexed(collection)) {
-    concatSeq = concatSeq.toSetSeq();
-  }
-  concatSeq = concatSeq.flatten(true);
-  concatSeq.size = iters.reduce((sum, seq) => {
-    if (sum !== undefined) {
-      const size = seq.size;
-      if (size !== undefined) {
-        return sum + size;
-      }
-    }
-  }, 0);
-  return concatSeq;
+  return new ConcatSeq(iters);
 }
 
 export function flattenFactory(collection, depth, useKeys) {


Test patch:
diff --git a/__tests__/concat.ts b/__tests__/concat.ts
index 596b1c3fc..6cc58ebcf 100644
--- a/__tests__/concat.ts
+++ b/__tests__/concat.ts
@@ -1,10 +1,9 @@
-import { is, List, Seq, Set } from 'immutable';
+import { List, Seq, Set } from 'immutable';
 
 describe('concat', () => {
   it('concats two sequences', () => {
     const a = Seq([1, 2, 3]);
     const b = Seq([4, 5, 6]);
-    expect(is(a.concat(b), Seq([1, 2, 3, 4, 5, 6]))).toBe(true);
     expect(a.concat(b).size).toBe(6);
     expect(a.concat(b).toArray()).toEqual([1, 2, 3, 4, 5, 6]);
   });
@@ -181,4 +180,42 @@ describe('concat', () => {
     expect(i.get(-4)).toBe(-9);
     expect(i.get(-5, 888)).toBe(888);
   });
+
+  it('should iterate on many concatenated sequences', () => {
+    let meta = Seq();
+
+    for (let i = 0; i < 10000; ++i) {
+      meta = meta.concat(i) as Seq<unknown, unknown>; // TODO fix typing
+    }
+
+    expect(meta.toList().size).toBe(10000);
+  });
+
+  it('should handle iterator on many concatenated sequences', () => {
+    const nbLoops = 10000;
+    let meta = Seq();
+    for (let i = 1; i < nbLoops; i++) {
+      meta = meta.concat(i) as Seq<unknown, unknown>; // TODO fix typing
+    }
+    const it = meta[Symbol.iterator]();
+    let done = false;
+    let i = 0;
+    while (!done) {
+      const result = it.next();
+      i++;
+      done = !!result.done;
+    }
+    expect(i).toBe(nbLoops);
+  });
+
+  it('should iterate on reverse order on concatenated sequences', () => {
+    let meta = Seq([1]);
+    meta = meta.concat(42);
+    const it = meta.reverse()[Symbol.iterator]();
+    const result = it.next();
+    expect(result).toEqual({
+      done: false,
+      value: 42,
+    });
+  });
 });