fix(patch): childNodes shifting during modification

aidenybai · Aug 10, 2021 · 011cad2 · 011cad2
1 parent 223e1f7
commit 011cad2
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 10 deletions.
diff --git a/src/__test__/patch.spec.ts b/src/__test__/patch.spec.ts
@@ -1,7 +1,6 @@
 import { createElement } from '../createElement';
 import { m, INSERT, UPDATE, DELETE } from '../m';
 import { patch } from '../patch';
-// import { patch } from '../patch';
 import { VFlags, VNode, VProps } from '../structs';
 
 const h = (tag: string, props?: VProps, ...children: VNode[]) =>
@@ -132,4 +131,27 @@ describe('.patch', () => {
     );
     expect(el.childNodes.length).toEqual(0);
   });
+
+  it('should used keyed algorithm when flag is passed', () => {
+    const el = document.createElement('ul');
+    const list1 = ['foo', 'bar', 'baz'];
+    const newVNode1 = m(
+      'ul',
+      undefined,
+      list1.map((item) => m('li', { key: item }, [item])),
+      VFlags.ONLY_KEYED_CHILDREN,
+    );
+    patch(el, newVNode1, m('ul', undefined, undefined, VFlags.NO_CHILDREN));
+    expect(el).toEqual(createElement(newVNode1));
+
+    const list2 = ['foo', 'baz', 'bar'];
+    const newVNode2 = m(
+      'ul',
+      undefined,
+      list2.map((item) => m('li', { key: item }, [item])),
+      VFlags.ONLY_KEYED_CHILDREN,
+    );
+    patch(el, newVNode2, newVNode1);
+    expect(el).toEqual(createElement(newVNode2));
+  });
 });
diff --git a/src/patch.ts b/src/patch.ts
@@ -83,8 +83,10 @@ export const patchChildren = (
           break;
       }
     }
-  } else if (keyed) {
-    // Keyed reconciliation algorithm adapted from [Fre](/~https://github.com/yisar/fre)
+  } else if (keyed && oldVNodeChildren.length > 0) {
+    const childNodes = [...el.childNodes];
+
+    // Keyed reconciliation algorithm originally adapted from [Fre](/~https://github.com/yisar/fre)
     let oldHead = 0;
     let newHead = 0;
     let oldTail = oldVNodeChildren.length - 1;
@@ -114,7 +116,7 @@ export const patchChildren = (
     } else if (newHead > newTail) {
       // There are no dirty new children: [X, Y, Z], []
       while (oldHead <= oldTail) {
-        el.removeChild(el.childNodes[oldTail--]);
+        el.removeChild(childNodes[oldTail--]);
       }
     } else {
       const keyMap = {};
@@ -123,21 +125,24 @@ export const patchChildren = (
       }
       while (newHead <= newTail) {
         const newVNodeChild = <VElement>newVNodeChildren[newTail];
-        const i = keyMap[newVNodeChild.key!];
-        if (i && newVNodeChild.key === (<VElement>oldVNodeChildren[i]).key) {
+        const oldVNodePosition = keyMap[newVNodeChild.key!];
+        if (
+          oldVNodePosition &&
+          newVNodeChild.key === (<VElement>oldVNodeChildren[oldVNodePosition]).key
+        ) {
           // Determine move for child that moved: [X, A, B, C] -> [A, B, C, X]
-          const child = el.removeChild(el.childNodes[i]);
-          el.insertBefore(child, el.childNodes[newTail--]);
+          const child = el.removeChild(childNodes[oldVNodePosition]);
+          el.insertBefore(child, childNodes[newTail--]);
           delete keyMap[newVNodeChild.key!];
         } else {
           // VNode doesn't exist yet: [] -> [X]
-          el.insertBefore(createElement(newVNodeChild, false), el.childNodes[newTail--]);
+          el.insertBefore(createElement(newVNodeChild, false), childNodes[newTail--]);
         }
       }
 
       for (const key in keyMap) {
         // VNode wasn't found in new vnodes, so it's cleaned up: [X] -> []
-        el.removeChild(el.childNodes[keyMap[key]]);
+        el.removeChild(childNodes[keyMap[key]]);
       }
     }