Merge pull request #5183 from preactjs/perf/v11-retained-subtree-bailout

Avoid traversing retained subtrees

Author: JoviDeCroock

src/diff/children.js

@@ -126,21 +126,19 @@ export function diffChildren(
 			firstChildDom = newDom;
 		}
 
-		let shouldPlace = childVNode._flags & INSERT_VNODE;
-		if (shouldPlace || oldVNode._children === childVNode._children) {
+		if (childVNode._flags & INSERT_VNODE) {
 			oldDom = insert(
 				childVNode,
 				oldDom,
 				parentDom,
-				shouldPlace,
 				oldVNode._original == NULL
 			);
 
 			// When a matched VNode is physically moved via INSERT_VNODE, its old
 			// _dom pointer becomes a stale positional reference. Clear it so that
 			// getDomSibling (called from nested diffs) won't return this stale
 			// reference and mis-place subsequent DOM nodes. See #5065.
-			if (shouldPlace && oldVNode._dom) {
+			if (oldVNode._dom) {
 				oldVNode._dom = NULL;
 			}
 		} else if (typeof childVNode.type == 'function' && result !== UNDEFINED) {
@@ -351,11 +349,10 @@ function constructNewChildrenArray(
  * @param {VNode} parentVNode
  * @param {PreactElement} oldDom
  * @param {PreactElement} parentDom
- * @param {number} shouldPlace
  * @param {boolean} isMounting
  * @returns {PreactElement}
  */
-function insert(parentVNode, oldDom, parentDom, shouldPlace, isMounting) {
+function insert(parentVNode, oldDom, parentDom, isMounting) {
 	// Note: VNodes in nested suspended trees may be missing _children.
 	if (typeof parentVNode.type == 'function') {
 		// Root children live in another container, they never move with the
@@ -369,23 +366,21 @@ function insert(parentVNode, oldDom, parentDom, shouldPlace, isMounting) {
 				// children's _parent pointer to point to the newVNode (parentVNode
 				// here).
 				children[i]._parent = parentVNode;
-				oldDom = insert(children[i], oldDom, parentDom, shouldPlace, false);
+				oldDom = insert(children[i], oldDom, parentDom, false);
 			}
 		}
 
 		return oldDom;
 	} else if (parentVNode._dom != oldDom) {
-		if (shouldPlace) {
-			if (oldDom && parentVNode.type && !oldDom.parentNode) {
-				oldDom = getDomSibling(parentVNode);
-			}
+		if (oldDom && parentVNode.type && !oldDom.parentNode) {
+			oldDom = getDomSibling(parentVNode);
+		}
 
-			if (HAS_MOVE_BEFORE_SUPPORT && !isMounting) {
-				// @ts-expect-error This isn't added to TypeScript lib.d.ts yet
-				parentDom.moveBefore(parentVNode._dom, oldDom);
-			} else {
-				parentDom.insertBefore(parentVNode._dom, oldDom || NULL);
-			}
+		if (HAS_MOVE_BEFORE_SUPPORT && !isMounting) {
+			// @ts-expect-error This isn't added to TypeScript lib.d.ts yet
+			parentDom.moveBefore(parentVNode._dom, oldDom);
+		} else {
+			parentDom.insertBefore(parentVNode._dom, oldDom || NULL);
 		}
 		oldDom = parentVNode._dom;
 	}

src/diff/index.js

@@ -232,6 +232,11 @@ export function diff(
 						commitQueue.push(c);
 					}
 
+					// Skip over the retained subtree without traversing it; the
+					// `result` branch in diffChildren picks this up as the next
+					// oldDom.
+					oldDom = getDomSibling(oldVNode);
+
 					break outer;
 				}
 

test/browser/lifecycles/shouldComponentUpdate.test.jsx

@@ -88,6 +88,57 @@ describe('Lifecycle methods', () => {
 			expect(ShouldNot.prototype.render).toHaveBeenCalledOnce();
 		});
 
+		it('should not traverse a retained subtree after bailing out', () => {
+			const Inner = () => (
+				<Fragment>
+					<span>A</span>
+					<span>B</span>
+				</Fragment>
+			);
+
+			class Bailout extends Component {
+				shouldComponentUpdate() {
+					return false;
+				}
+
+				render() {
+					return <Inner />;
+				}
+			}
+
+			render(
+				<div>
+					<Bailout value={0} />
+					<span>C</span>
+				</div>,
+				scratch
+			);
+
+			const root = scratch._children;
+			const bailoutVNode = root._children[0]._children[0];
+			const innerVNode = bailoutVNode._children[0];
+			const children = innerVNode._children;
+			let reads = 0;
+			bailoutVNode._children[0] = new Proxy(innerVNode, {
+				get(target, property, receiver) {
+					const value = Reflect.get(target, property, receiver);
+					if (value === children) reads++;
+					return value;
+				}
+			});
+
+			render(
+				<div>
+					<Bailout value={1} />
+					<span>C</span>
+				</div>,
+				scratch
+			);
+
+			expect(scratch.textContent).to.equal('ABC');
+			expect(reads).to.equal(0);
+		});
+
 		it('should reorder non-updating text children', () => {
 			const rows = [
 				{ id: '1', a: 5, b: 100 },