Merge pull request #1531 from developit/improve-fragments-childDom-golf

Remove getFirstOldDom (-42 B)
preactjs · Apr 12, 2019 · 7f368b4 · 7f368b4
2 parents c08ee16 + 0e271e9
commit 7f368b4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 39 deletions.
diff --git a/src/diff/children.js b/src/diff/children.js
@@ -32,6 +32,30 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
 
 	let oldChildrenLength = oldChildren.length;
 
+	// Only in very specific places should this logic be invoked (top level `render` and `diffElementNodes`).
+	// I'm using `EMPTY_OBJ` to signal when `diffChildren` is invoked in these situations. I can't use `null`
+	// for this purpose, because `null` is a valid value for `oldDom` which can mean to skip to this logic
+	// (e.g. if mounting a new tree in which the old DOM should be ignored (usually for Fragments).
+	if (oldDom == EMPTY_OBJ) {
+		oldDom = null;
+		if (excessDomChildren!=null) {
+			for (i = 0; i < excessDomChildren.length; i++) {
+				if (excessDomChildren[i]!=null) {
+					oldDom = excessDomChildren[i];
+					break;
+				}
+			}
+		}
+		else {
+			for (i = 0; i < oldChildrenLength; i++) {
+				if (oldChildren[i] && oldChildren[i]._dom) {
+					oldDom = oldChildren[i]._dom;
+					break;
+				}
+			}
+		}
+	}
+
 	for (i=0; i<newChildren.length; i++) {
 		childVNode = newChildren[i] = coerceToVNode(newChildren[i]);
 		oldVNode = index = null;

diff --git a/src/diff/index.js b/src/diff/index.js
@@ -54,7 +54,7 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
 			if (newVNode._children.length) {
 				dom = newVNode._children[0]._dom;
 
-				// If lastChild is a Fragment, use _lastDomChild, else use _dom
+				// If the last child is a Fragment, use _lastDomChild, else use _dom
 				p = newVNode._children[newVNode._children.length - 1];
 				newVNode._lastDomChild = p._lastDomChild || p._dom;
 			}
@@ -286,8 +286,7 @@ function diffElementNodes(dom, newVNode, oldVNode, context, isSvg, excessDomChil
 				dom.multiple = newProps.multiple;
 			}
 
-			const oldDom = getFirstOldDom(oldVNode, excessDomChildren);
-			diffChildren(dom, newVNode, oldVNode, context, newVNode.type==='foreignObject' ? false : isSvg, excessDomChildren, mounts, ancestorComponent, oldDom);
+			diffChildren(dom, newVNode, oldVNode, context, newVNode.type==='foreignObject' ? false : isSvg, excessDomChildren, mounts, ancestorComponent, EMPTY_OBJ);
 			diffProps(dom, newProps, oldProps, isSvg);
 		}
 	}
@@ -388,34 +387,3 @@ function catchErrorInComponent(error, component) {
 	}
 	throw error;
 }
-
-/**
- * Determine which currently attached DOM node to use when beginning to
- * diff the children of a VNode
- * @param {import('../internal').VNode} oldVNode The old VNode whose children
- * are about to be diffed
- * @param {import('../internal').PreactElement[]} excessDomChildren If hydrating,
- * the currently attached DOM elements that are being hydrated
- * @returns {import('../internal').PreactElement | Text | undefined}
- */
-export function getFirstOldDom(oldVNode, excessDomChildren) {
-
-	/** @type {import('../internal').VNode[]} */
-	let oldChildren = oldVNode!=null && oldVNode!=EMPTY_OBJ && oldVNode._children || EMPTY_ARR;
-
-	let i;
-	if (excessDomChildren!=null) {
-		for (i = 0; i < excessDomChildren.length; i++) {
-			if (excessDomChildren[i]!=null) {
-				return excessDomChildren[i];
-			}
-		}
-	}
-	else {
-		for (i = 0; i < oldChildren.length; i++) {
-			if (oldChildren[i] && oldChildren[i]._dom) {
-				return oldChildren[i]._dom;
-			}
-		}
-	}
-}
diff --git a/src/render.js b/src/render.js
@@ -1,5 +1,5 @@
 import { EMPTY_OBJ, EMPTY_ARR } from './constants';
-import { commitRoot, getFirstOldDom } from './diff/index';
+import { commitRoot } from './diff/index';
 import { diffChildren } from './diff/children';
 import { createElement, Fragment } from './create-element';
 import options from './options';
@@ -15,11 +15,8 @@ export function render(vnode, parentDom) {
 	let oldVNode = parentDom._prevVNode;
 	vnode = createElement(Fragment, null, [vnode]);
 
-	const excessDomChildren = oldVNode ? null : EMPTY_ARR.slice.call(parentDom.childNodes);
-	const oldDom = getFirstOldDom(oldVNode, excessDomChildren);
-
 	let mounts = [];
-	diffChildren(parentDom, parentDom._prevVNode = vnode, oldVNode, EMPTY_OBJ, parentDom.ownerSVGElement!==undefined, excessDomChildren, mounts, vnode, oldDom);
+	diffChildren(parentDom, parentDom._prevVNode = vnode, oldVNode, EMPTY_OBJ, parentDom.ownerSVGElement!==undefined, oldVNode ? null : EMPTY_ARR.slice.call(parentDom.childNodes), mounts, vnode, EMPTY_OBJ);
 	commitRoot(mounts, vnode);
 }