Fix indexing when moving nodes (#192)

This commit fixing the diffing bug where we were using the wrong node
index when moving siblings under certain conditions.

crates/percy-dom/src/diff.rs

@@ -456,6 +456,11 @@ fn generate_patches_for_children<'a, 'b>(
     let mut old_tracked_indices = TrackedImplicitlyKeyableIndices::default();
     let mut new_tracked_indices = TrackedImplicitlyKeyableIndices::default();
 
+    let mut old_child_indices_of_preserved_keys = vec![];
+
+    let node_idx_of_first_child = ctx.next_old_node_idx();
+    ctx.increment_old_node_idx(old_element.children.len());
+
     for (idx, new_child) in new_element.children.iter().enumerate() {
         let implicit_key = new_tracked_indices.get_and_increment(new_child);
         let new_key = node_key(new_child, implicit_key);
@@ -466,9 +471,6 @@ fn generate_patches_for_children<'a, 'b>(
         }
     }
 
-    let node_idx_of_first_child = ctx.next_old_node_idx();
-    ctx.increment_old_node_idx(old_element.children.len());
-
     for (idx, old_child) in old_element.children.iter().enumerate() {
         let implicit_key = old_tracked_indices.get_and_increment(old_child);
         let old_key = node_key(old_child, implicit_key);
@@ -483,8 +485,6 @@ fn generate_patches_for_children<'a, 'b>(
         }
     }
 
-    let mut old_child_indices_of_preserved_keys = vec![];
-
     for new_child_idx in 0..new_element.children.len() {
         let key = new_node_keys.get(&new_child_idx);
         let Some(key) = key else {
@@ -577,7 +577,7 @@ fn generate_patches_for_children<'a, 'b>(
                 let old_child_idx = key.and_then(|key| key_to_old_child_idx.get(&key));
 
                 if let Some(old_child_idx) = old_child_idx {
-                    let old_idx = parent_old_node_idx + 1 + *old_child_idx as u32;
+                    let old_idx = node_idx_of_first_child + *old_child_idx as u32;
 
                     insert_before_or_move.push(InsertBeforeOrMoveBefore::MoveBefore(old_idx));
 
@@ -1111,6 +1111,53 @@ mod tests {
         .test();
     }
 
+    /// Verify that if we have two siblings, and each sibling has their own children, we can remove
+    /// the first sibling and then move one second sibling's children to be before another one of
+    /// the second sibling's children.
+    #[test]
+    fn remove_first_sibling_that_has_children_then_move_child_of_second_sibling() {
+        DiffTestCase {
+            old: html! {
+                <div>
+                  // span gets removed
+                  <span>
+                    <source />
+                    <source />
+                  </span>
+
+                  <div>
+                    // link gets moved to before the br
+                    <link />
+                    <param />
+                    <area />
+                    <br />
+                  </div>
+                </div>
+            },
+            new: html! {
+                <div>
+                  <div>
+                    <param />
+                    <area />
+                    <link />
+                    <br />
+                  </div>
+                </div>
+            },
+            expected: vec![
+                Patch::RemoveChildren {
+                    parent_old_node_idx: 0,
+                    to_remove: vec![1],
+                },
+                Patch::MoveNodesBefore {
+                    anchor_old_node_idx: 8,
+                    to_move: vec![5],
+                },
+            ],
+        }
+        .test();
+    }
+
     /// Verify that we can add an attribute to a node.
     #[test]
     fn add_attributes() {

crates/percy-dom/src/patch/apply_patches.rs

@@ -75,7 +75,6 @@ pub fn patch<N: Into<Node>>(
         }
     }
 
-    // FIXME: Remove patches as an argument.. just trying to track down a bug..
     overwrite_events(new_vnode, root_events_node, virtual_events);
 
     Ok(())

crates/virtual-node/src/event/virtual_events.rs

@@ -322,7 +322,7 @@ impl VirtualEventNode {
 
     /// Remove a child node from it's siblings.
     pub fn remove_node_from_siblings(&mut self, child: &Rc<RefCell<VirtualEventNode>>) {
-        let mut child = child.borrow_mut();
+        let mut child = &mut *child.borrow_mut();
         let is_first_sibling = child.previous_sibling.is_none();
         let is_last_sibling = child.next_sibling.is_none();
 
@@ -335,10 +335,7 @@ impl VirtualEventNode {
             parent.children.as_mut().unwrap().last_child = child.previous_sibling.clone().unwrap();
         }
 
-        match (
-            child.previous_sibling.clone().as_mut(),
-            child.next_sibling.as_mut(),
-        ) {
+        match (child.previous_sibling.as_mut(), child.next_sibling.as_mut()) {
             (Some(previous), Some(next)) => {
                 previous.borrow_mut().next_sibling = Some(next.clone());
                 next.borrow_mut().previous_sibling = Some(previous.clone());