From 4fa6e860a176985e03fda969b2b435fe168f31c2 Mon Sep 17 00:00:00 2001
From: Matthew Berry <me@mattrb.com>
Date: Wed, 23 Oct 2024 09:51:08 -0700
Subject: [PATCH] feat!: make insertions during iteration safe (#295)

This change addresses some unexpected behavior when inserting nodes into
a container. For example, assume you have a container containing two
class nodes `.foo.baz`. While iterating that container with `each`,
assume you are processing `.foo` at index 0. Today, the behavior is as
follows:

- `insertBefore(.baz, .bar)` => the next callback is to `.baz` at idx 3
- `insertAfter(.foo, .bar)` => the next callback is to `.baz` at idx 3
- `prepend(.bar)` => the next callback is to `.foo` again at idx 1

With this change, the behavior is the following, respectively:

- the next callback is to `.bar` at idx 2
- the next callback is to `.bar` at idx 2
- the next callback is to `.baz` at idx 3

The newly added tests demonstrate this behavior. I've also removed the
old "container#each (safe iteration)" test, as it now creates an
infinite loop. I'd argue that this is the expected behavior now, though.
---
 src/__tests__/container.mjs | 56 +++++++++++++++++++++++++++++--------
 src/selectors/container.js  |  7 +++--
 2 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/src/__tests__/container.mjs b/src/__tests__/container.mjs
index 28c4c3e..e852aac 100644
--- a/src/__tests__/container.mjs
+++ b/src/__tests__/container.mjs
@@ -35,20 +35,52 @@ test('container#each', (t) => {
     t.deepEqual(indexes, [0, 1]);
 });
 
-test('container#each (safe iteration)', (t) => {
-    let out = parse('.x, .y', (selectors) => {
-        selectors.each((selector) => {
-            selector.parent.insertBefore(
-                selector,
-                parser.className({value : 'b'})
-            );
-            selector.parent.insertAfter(
-                selector,
-                parser.className({value : 'a'})
-            );
+test('container#each (safe iteration w/ insertBefore)', (t) => {
+    let indexes = [];
+    let out = parse('.x,.z', (selectors) => {
+        selectors.each((selector, index) => {
+            if (index === 0) {
+                selectors.insertBefore(
+                    selectors.at(1),
+                    parser.className({ value: 'y' })
+                );
+            }
+            indexes.push(index);
+        });
+    });
+    t.deepEqual(out, '.x,.y,.z');
+    t.deepEqual(indexes, [0, 1, 2]);
+});
+
+test('container#each (safe iteration w/ prepend)', (t) => {
+    let indexes = [];
+    let out = parse('.y,.z', (selectors) => {
+        selectors.each((selector, index) => {
+            if (index === 0) {
+                selectors.prepend(parser.className({ value: 'x' }));
+            }
+            indexes.push(index);
+        });
+    });
+    t.deepEqual(out, '.x,.y,.z');
+    t.deepEqual(indexes, [0, 2]);
+});
+
+test('container#each (safe iteration w/ insertAfter)', (t) => {
+    let indexes = [];
+    let out = parse('.x,.z', (selectors) => {
+        selectors.each((selector, index) => {
+            if (index === 0) {
+                selectors.insertAfter(
+                    selector,
+                    parser.className({ value: 'y' })
+                );
+            }
+            indexes.push(index);
         });
     });
-    t.deepEqual(out, '.b,.x,.a,.b, .y,.a');
+    t.deepEqual(out, '.x,.y,.z');
+    t.deepEqual(indexes, [0, 1, 2]);
 });
 
 test('container#each (early exit)', (t) => {
diff --git a/src/selectors/container.js b/src/selectors/container.js
index ae213df..2575dde 100644
--- a/src/selectors/container.js
+++ b/src/selectors/container.js
@@ -18,6 +18,9 @@ export default class Container extends Node {
     prepend (selector) {
         selector.parent = this;
         this.nodes.unshift(selector);
+        for ( let id in this.indexes ) {
+            this.indexes[id]++;
+        }
         return this;
     }
 
@@ -82,7 +85,7 @@ export default class Container extends Node {
         let index;
         for ( let id in this.indexes ) {
             index = this.indexes[id];
-            if ( oldIndex <= index ) {
+            if ( oldIndex < index ) {
                 this.indexes[id] = index + 1;
             }
         }
@@ -100,7 +103,7 @@ export default class Container extends Node {
         let index;
         for ( let id in this.indexes ) {
             index = this.indexes[id];
-            if ( index <= oldIndex ) {
+            if ( index >= oldIndex ) {
                 this.indexes[id] = index + 1;
             }
         }