Port skew based child diffing

src/diff/children.js

@@ -1,7 +1,6 @@
 import { diff, unmount, applyRef } from './index';
 import { createVNode, Fragment } from '../create-element';
 import { EMPTY_OBJ, EMPTY_ARR } from '../constants';
-import { getDomSibling } from '../component';
 import { isArray } from '../util';
 
 /**
@@ -36,16 +35,25 @@ export function diffChildren(
 	oldDom,
 	isHydrating
 ) {
-	let i, j, oldVNode, childVNode, newDom, firstChildDom, refs;
+	let i,
+		j,
+		oldVNode,
+		childVNode,
+		newDom,
+		firstChildDom,
+		refs,
+		skew = 0;
 
 	// This is a compression of oldParentVNode!=null && oldParentVNode != EMPTY_OBJ && oldParentVNode._children || EMPTY_ARR
 	// as EMPTY_OBJ._children should be `undefined`.
 	let oldChildren = (oldParentVNode && oldParentVNode._children) || EMPTY_ARR;
 
-	let oldChildrenLength = oldChildren.length;
+	let oldChildrenLength = oldChildren.length,
+		remainingOldChildren = oldChildrenLength,
+		newChildrenLength = renderResult.length;
 
 	newParentVNode._children = [];
-	for (i = 0; i < renderResult.length; i++) {
+	for (i = 0; i < newChildrenLength; i++) {
 		childVNode = renderResult[i];
 
 		if (
@@ -104,40 +112,22 @@ export function diffChildren(
 		childVNode._parent = newParentVNode;
 		childVNode._depth = newParentVNode._depth + 1;
 
-		// Check if we find a corresponding element in oldChildren.
-		// If found, delete the array item by setting to `undefined`.
-		// We use `undefined`, as `null` is reserved for empty placeholders
-		// (holes).
-		oldVNode = oldChildren[i];
+		let skewedIndex = i + skew;
+		const matchingIndex = findMatchingIndex(
+			childVNode,
+			oldChildren,
+			skewedIndex,
+			remainingOldChildren
+		);
 
-		if (
-			oldVNode === null ||
-			(oldVNode &&
-				childVNode.key == oldVNode.key &&
-				childVNode.type === oldVNode.type)
-		) {
-			oldChildren[i] = undefined;
+		if (matchingIndex === -1) {
+			oldVNode = EMPTY_OBJ;
 		} else {
-			// Either oldVNode === undefined or oldChildrenLength > 0,
-			// so after this loop oldVNode == null or oldVNode is a valid value.
-			for (j = 0; j < oldChildrenLength; j++) {
-				oldVNode = oldChildren[j];
-				// If childVNode is unkeyed, we only match similarly unkeyed nodes, otherwise we match by key.
-				// We always match by type (in either case).
-				if (
-					oldVNode &&
-					childVNode.key == oldVNode.key &&
-					childVNode.type === oldVNode.type
-				) {
-					oldChildren[j] = undefined;
-					break;
-				}
-				oldVNode = null;
-			}
+			oldVNode = oldChildren[matchingIndex] || EMPTY_OBJ;
+			oldChildren[matchingIndex] = undefined;
+			remainingOldChildren--;
 		}
 
-		oldVNode = oldVNode || EMPTY_OBJ;
-
 		// Morph the old element into the new one, but don't append it to the dom yet
 		diff(
 			parentDom,
@@ -164,24 +154,61 @@ export function diffChildren(
 				firstChildDom = newDom;
 			}
 
+			let isMounting =
+				isHydrating || oldVNode === EMPTY_OBJ || oldVNode._original === null;
+			let hasMatchingIndex = !isMounting && matchingIndex === skewedIndex;
+			if (isMounting) {
+				if (matchingIndex == -1) {
+					skew--;
+				}
+			} else if (matchingIndex !== skewedIndex) {
+				if (matchingIndex === skewedIndex + 1) {
+					skew++;
+					hasMatchingIndex = true;
+				} else if (matchingIndex > skewedIndex) {
+					if (remainingOldChildren > newChildrenLength - skewedIndex) {
+						skew += matchingIndex - skewedIndex;
+						hasMatchingIndex = true;
+					} else {
+						// ### Change from keyed: I think this was missing from the algo...
+						skew--;
+					}
+				} else if (matchingIndex < skewedIndex) {
+					if (matchingIndex == skewedIndex - 1) {
+						skew = matchingIndex - skewedIndex;
+					} else {
+						skew = 0;
+					}
+				} else {
+					skew = 0;
+				}
+			}
+
+			skewedIndex = i + skew;
+			hasMatchingIndex =
+				hasMatchingIndex || (matchingIndex == i && !isMounting);
+
 			if (
 				typeof childVNode.type == 'function' &&
-				childVNode._children === oldVNode._children
+				(matchingIndex !== skewedIndex ||
+					oldVNode._children === childVNode._children)
 			) {
-				childVNode._nextDom = oldDom = reorderChildren(
-					childVNode,
-					oldDom,
-					parentDom
-				);
+				oldDom = reorderChildren(childVNode, oldDom, parentDom);
+			} else if (typeof childVNode.type != 'function' && !hasMatchingIndex) {
+				oldDom = placeChild(parentDom, newDom, oldDom);
+			} else if (childVNode._nextDom !== undefined) {
+				// Only Fragments or components that return Fragment like VNodes will
+				// have a non-undefined _nextDom. Continue the diff from the sibling
+				// of last DOM child of this child VNode
+				oldDom = childVNode._nextDom;
+
+				// Eagerly cleanup _nextDom. We don't need to persist the value because
+				// it is only used by `diffChildren` to determine where to resume the diff after
+				// diffing Components and Fragments. Once we store it the nextDOM local var, we
+				// can clean up the property
+				childVNode._nextDom = undefined;
 			} else {
-				oldDom = placeChild(
-					parentDom,
-					childVNode,
-					oldVNode,
-					oldChildren,
-					newDom,
-					oldDom
-				);
+				oldDom = newDom.nextSibling;
 			}
 
 			if (typeof newParentVNode.type == 'function') {
@@ -194,14 +221,6 @@ export function diffChildren(
 				// node's nextSibling.
 				newParentVNode._nextDom = oldDom;
 			}
-		} else if (
-			oldDom &&
-			oldVNode._dom == oldDom &&
-			oldDom.parentNode != parentDom
-		) {
-			// The above condition is to handle null placeholders. See test in placeholder.test.js:
-			// `efficiently replace null placeholders in parent rerenders`
-			oldDom = getDomSibling(oldVNode);
 		}
 	}
 
@@ -218,7 +237,8 @@ export function diffChildren(
 				// If the newParentVNode.__nextDom points to a dom node that is about to
 				// be unmounted, then get the next sibling of that vnode and set
 				// _nextDom to it
-				newParentVNode._nextDom = getLastDom(oldParentVNode).nextSibling;
+
+				newParentVNode._nextDom = oldChildren[i]._dom.nextSibling;
 			}
 
 			unmount(oldChildren[i], oldChildren[i]);
@@ -236,6 +256,7 @@ export function diffChildren(
 function reorderChildren(childVNode, oldDom, parentDom) {
 	// Note: VNodes in nested suspended trees may be missing _children.
 	let c = childVNode._children;
+
 	let tmp = 0;
 	for (; c && tmp < c.length; tmp++) {
 		let vnode = c[tmp];
@@ -249,7 +270,7 @@ function reorderChildren(childVNode, oldDom, parentDom) {
 			if (typeof vnode.type == 'function') {
 				oldDom = reorderChildren(vnode, oldDom, parentDom);
 			} else {
-				oldDom = placeChild(parentDom, vnode, vnode, c, vnode._dom, oldDom);
+				oldDom = placeChild(parentDom, vnode._dom, oldDom);
 			}
 		}
 	}
@@ -276,81 +297,59 @@ export function toChildArray(children, out) {
 	return out;
 }
 
-function placeChild(
-	parentDom,
-	childVNode,
-	oldVNode,
-	oldChildren,
-	newDom,
-	oldDom
-) {
-	let nextDom;
-	if (childVNode._nextDom !== undefined) {
-		// Only Fragments or components that return Fragment like VNodes will
-		// have a non-undefined _nextDom. Continue the diff from the sibling
-		// of last DOM child of this child VNode
-		nextDom = childVNode._nextDom;
-
-		// Eagerly cleanup _nextDom. We don't need to persist the value because
-		// it is only used by `diffChildren` to determine where to resume the diff after
-		// diffing Components and Fragments. Once we store it the nextDOM local var, we
-		// can clean up the property
-		childVNode._nextDom = undefined;
-	} else if (
-		oldVNode == null ||
-		newDom != oldDom ||
-		newDom.parentNode == null
-	) {
-		outer: if (oldDom == null || oldDom.parentNode !== parentDom) {
-			parentDom.appendChild(newDom);
-			nextDom = null;
-		} else {
-			// `j<oldChildrenLength; j+=2` is an alternative to `j++<oldChildrenLength/2`
-			for (
-				let sibDom = oldDom, j = 0;
-				(sibDom = sibDom.nextSibling) && j < oldChildren.length;
-				j += 1
-			) {
-				if (sibDom == newDom) {
-					break outer;
-				}
-			}
-			parentDom.insertBefore(newDom, oldDom);
-			nextDom = oldDom;
-		}
-	}
-
-	// If we have pre-calculated the nextDOM node, use it. Else calculate it now
-	// Strictly check for `undefined` here cuz `null` is a valid value of `nextDom`.
-	// See more detail in create-element.js:createVNode
-	if (nextDom !== undefined) {
-		oldDom = nextDom;
-	} else {
-		oldDom = newDom.nextSibling;
+function placeChild(parentDom, newDom, oldDom) {
+	if (oldDom == null || oldDom.parentNode !== parentDom) {
+		parentDom.insertBefore(newDom, null);
+	} else if (newDom != oldDom || newDom.parentNode == null) {
+		parentDom.insertBefore(newDom, oldDom);
 	}
 
-	return oldDom;
+	return newDom.nextSibling;
 }
 
 /**
- * @param {import('../internal').VNode} vnode
+ * @param {import('../internal').VNode | string} childVNode
+ * @param {import('../internal').VNode[]} oldChildren
+ * @param {number} skewedIndex
+ * @param {number} remainingOldChildren
+ * @returns {number}
  */
-function getLastDom(vnode) {
-	if (vnode.type == null || typeof vnode.type === 'string') {
-		return vnode._dom;
-	}
+function findMatchingIndex(
+	childVNode,
+	oldChildren,
+	skewedIndex,
+	remainingOldChildren
+) {
+	const key = childVNode.key;
+	const type = childVNode.type;
+	let x = skewedIndex - 1;
+	let y = skewedIndex + 1;
+	let oldVNode = oldChildren[skewedIndex];
+
+	if (
+		oldVNode === null ||
+		(oldVNode && key == oldVNode.key && type === oldVNode.type)
+	) {
+		return skewedIndex;
+	} else if (remainingOldChildren > (oldVNode != null ? 1 : 0)) {
+		while (x >= 0 || y < oldChildren.length) {
+			if (x >= 0) {
+				oldVNode = oldChildren[x];
+				if (oldVNode && key == oldVNode.key && type === oldVNode.type) {
+					return x;
+				}
+				x--;
+			}
 
-	if (vnode._children) {
-		for (let i = vnode._children.length - 1; i >= 0; i--) {
-			let child = vnode._children[i];
-			if (child) {
-				let lastDom = getLastDom(child);
-				if (lastDom) {
-					return lastDom;
+			if (y < oldChildren.length) {
+				oldVNode = oldChildren[y];
+				if (oldVNode && key == oldVNode.key && type === oldVNode.type) {
+					return y;
 				}
+				y++;
 			}
 		}
 	}
 
-	return null;
+	return -1;
 }

test/_util/logCall.js

@@ -29,11 +29,26 @@ export function logCall(obj, method) {
 			c += serialize(args[i]);
 		}
 
-		// Normalize removeChild -> remove to keep output clean and readable
-		const operation =
-			method != 'removeChild'
-				? `${serialize(this)}.${method}(${c})`
-				: `${serialize(c)}.remove()`;
+		let operation;
+		switch (method) {
+			case 'removeChild': {
+				operation = `${serialize(c)}.remove()`;
+				break;
+			}
+			case 'insertBefore': {
+				if (args[1] === null && args.length === 2) {
+					operation = `${serialize(this)}.appendChild(${serialize(args[0])})`;
+				} else {
+					operation = `${serialize(this)}.${method}(${c})`;
+				}
+				break;
+			}
+			default: {
+				operation = `${serialize(this)}.${method}(${c})`;
+				break;
+			}
+		}
+
 		log.push(operation);
 		return old.apply(this, args);
 	};