Compute minimal child moves with a longest increasing subsequence

src/diff/children.js

@@ -180,6 +180,10 @@ function constructNewChildrenArray(
 
 	let skew = 0;
 
+	/** Whether any matched child was found far from its skewed index, i.e. a
+	 * real reorder happened and we need to compute the minimal set of moves. */
+	let moved = false;
+
 	newParentVNode._children = new Array(newChildrenLength);
 	for (i = 0; i < newChildrenLength; i++) {
 		// @ts-expect-error We are reusing the childVNode variable to hold both the
@@ -289,38 +293,74 @@ function constructNewChildrenArray(
 			if (typeof childVNode.type != 'function') {
 				childVNode._flags |= INSERT_VNODE;
 			}
-		} else if (matchingIndex != skewedIndex) {
-			// When we move elements around i.e. [0, 1, 2] --> [1, 0, 2]
-			// --> we diff 1, we find it at position 1 while our skewed index is 0 and our skew is 0
-			//     we set the skew to 1 as we found an offset.
-			// --> we diff 0, we find it at position 0 while our skewed index is at 2 and our skew is 1
-			//     this makes us increase the skew again.
-			// --> we diff 2, we find it at position 2 while our skewed index is at 4 and our skew is 2
-			//
-			// this becomes an optimization question where currently we see a 1 element offset as an insertion
-			// or deletion i.e. we optimize for [0, 1, 2] --> [9, 0, 1, 2]
-			// while a more than 1 offset we see as a swap.
-			// We could probably build heuristics for having an optimized course of action here as well, but
-			// might go at the cost of some bytes.
-			//
-			// If we wanted to optimize for i.e. only swaps we'd just do the last two code-branches and have
-			// only the first item be a re-scouting and all the others fall in their skewed counter-part.
-			// We could also further optimize for swaps
+		} else {
+			// Matched children are candidates for the minimal-move pass below;
+			// MATCHED on a _new_ vnode is cleared in diffChildren's placement loop.
+			childVNode._flags |= MATCHED;
+
+			// The skew adjustments keep findMatchingIndex's search centered for
+			// shift patterns (insertions/removals at the front). A match further
+			// than one position away means children were genuinely reordered:
+			// flag it so the minimal set of moves is computed below. Off-by-one
+			// matches provably keep matched old indices in increasing order, so
+			// no moves are needed for them.
 			if (matchingIndex == skewedIndex - 1) {
 				skew--;
 			} else if (matchingIndex == skewedIndex + 1) {
 				skew++;
-			} else {
+			} else if (matchingIndex != skewedIndex) {
 				if (matchingIndex > skewedIndex) {
 					skew--;
 				} else {
 					skew++;
 				}
 
-				// Move this VNode's DOM if the original index (matchingIndex) doesn't
-				// match the new skew index (i + new skew)
-				// In the former two branches we know that it matches after skewing
-				childVNode._flags |= INSERT_VNODE;
+				moved = true;
+			}
+		}
+	}
+
+	if (moved) {
+		// Children were reordered: mark the minimal set of matched (MATCHED flag)
+		// children for insertion by finding the longest increasing subsequence of
+		// old indices (patience sorting). Children on the subsequence stay in
+		// place, all others get INSERT_VNODE. `_index` still holds the
+		// matchingIndex here.
+		/** @type {number[]} tails[x] is the smallest old index ending an increasing subsequence of length x+1 */
+		let tails = [];
+		/** @type {number[]} length of the longest increasing subsequence ending at child i */
+		let lisLengths = [];
+		for (i = 0; i < newChildrenLength; i++) {
+			childVNode = newParentVNode._children[i];
+			if (childVNode && childVNode._flags & MATCHED) {
+				// Binary search for the insertion point, keeping the pass at
+				// O(n log n) even for pathological reorders.
+				let lo = 0,
+					hi = tails.length;
+				while (lo < hi) {
+					const mid = (lo + hi) >> 1;
+					if (tails[mid] < childVNode._index) {
+						lo = mid + 1;
+					} else {
+						hi = mid;
+					}
+				}
+				tails[lo] = childVNode._index;
+				lisLengths[i] = lo + 1;
+			}
+		}
+
+		// `skew` is dead after the main loop; reuse it as the remaining
+		// subsequence length while walking backwards. Likewise `i` is left at
+		// newChildrenLength by the loop above.
+		skew = tails.length;
+		while (i--) {
+			if (lisLengths[i]) {
+				if (lisLengths[i] == skew) {
+					skew--;
+				} else {
+					newParentVNode._children[i]._flags |= INSERT_VNODE;
+				}
 			}
 		}
 	}

test/browser/fragments.test.jsx

@@ -803,9 +803,8 @@ describe('Fragment', () => {
 		expect(scratch.innerHTML).to.equal(htmlForFalse);
 		expectDomLogToBe(
 			[
-				'<div>barHellobeep.insertBefore(<div>bar, <div>beep)',
-				'<div>Hellobarbeep.appendChild(<div>Hello)',
-				'<div>barbeepHello.appendChild(<div>bar)'
+				'<div>fooHellobeep.insertBefore(<div>beep, <div>foo)',
+				'<div>beepfooHello.insertBefore(<div>Hello, <div>foo)'
 			],
 			'rendering true to false'
 		);
@@ -817,8 +816,9 @@ describe('Fragment', () => {
 		expect(scratch.innerHTML).to.equal(htmlForTrue);
 		expectDomLogToBe(
 			[
-				'<div>beepHellofoo.appendChild(<div>Hello)',
-				'<div>boopfooHello.appendChild(<div>boop)'
+				'<div>beepHellofoo.insertBefore(<div>foo, <div>beep)',
+				'<div>foobeepHello.insertBefore(<div>foo, <div>beep)',
+				'<div>foobeepHello.insertBefore(<div>Hello, <div>beep)'
 			],
 			'rendering false to true'
 		);
@@ -1594,9 +1594,8 @@ describe('Fragment', () => {
 		expectDomLogToBe(
 			[
 				'<div>boop.remove()',
-				'<div>barHellobeep.insertBefore(<div>bar, <div>beep)',
-				'<div>Hellobarbeep.appendChild(<div>Hello)',
-				'<div>barbeepHello.appendChild(<div>bar)'
+				'<div>fooHellobeep.insertBefore(<div>beep, <div>foo)',
+				'<div>beepfooHello.insertBefore(<div>Hello, <div>foo)'
 			],
 			'rendering from true to false'
 		);
@@ -1611,8 +1610,9 @@ describe('Fragment', () => {
 		);
 		expectDomLogToBe(
 			[
-				'<div>beepHellofoo.appendChild(<div>Hello)',
-				'<div>boopfooHello.appendChild(<div>boop)',
+				'<div>beepHellofoo.insertBefore(<div>foo, <div>beep)',
+				'<div>foobeepHello.insertBefore(<div>foo, <div>beep)',
+				'<div>foobeepHello.insertBefore(<div>Hello, <div>beep)',
 				'<div>.appendChild(#text)',
 				'<div>fooHelloboop.appendChild(<div>boop)'
 			],
@@ -1680,8 +1680,8 @@ describe('Fragment', () => {
 		);
 		expectDomLogToBe(
 			[
-				'<div>barHellobeepbeepbeep.insertBefore(<div>bar, <div>beep)',
-				'<div>Hellobarbeepbeepbeep.appendChild(<div>Hello)',
+				'<div>fooHellobeepbeepbeep.appendChild(<div>Hello)',
+				'<div>barbeepbeepbeepHello.appendChild(<div>Hello)',
 				'<div>barbeepbeepbeepHello.appendChild(<div>bar)'
 			],
 			'rendering from true to false'
@@ -1697,8 +1697,8 @@ describe('Fragment', () => {
 		);
 		expectDomLogToBe(
 			[
-				'<div>beepbeepbeepHellofoo.appendChild(<div>Hello)',
-				'<div>beepbeepbeepfooHello.insertBefore(<div>foo, <div>beep)',
+				'<div>beepbeepbeepHellofoo.insertBefore(<div>foo, <div>beep)',
+				'<div>foobeepbeepbeepHello.insertBefore(<div>foo, <div>beep)',
 				'<div>foobeepbeepbeepHello.insertBefore(<div>Hello, <div>beep)'
 			],
 			'rendering from false to true'

test/browser/keys.test.jsx

@@ -351,7 +351,7 @@ describe('keys', () => {
 		render(<List values={['b', 'a']} />, scratch);
 		expect(scratch.textContent).to.equal('ba');
 
-		expect(getLog()).to.deep.equal(['<ol>ab.appendChild(<li>a)']);
+		expect(getLog()).to.deep.equal(['<ol>ab.insertBefore(<li>b, <li>a)']);
 	});
 
 	it('should swap existing keyed children in the middle of a list efficiently', () => {
@@ -367,7 +367,7 @@ describe('keys', () => {
 		render(<List values={values} />, scratch);
 		expect(scratch.textContent).to.equal('acbd', 'initial swap');
 		expect(getLog()).to.deep.equal(
-			['<ol>abcd.insertBefore(<li>b, <li>d)'],
+			['<ol>abcd.insertBefore(<li>c, <li>b)'],
 			'initial swap'
 		);
 
@@ -378,11 +378,176 @@ describe('keys', () => {
 		render(<List values={values} />, scratch);
 		expect(scratch.textContent).to.equal('abcd', 'swap back');
 		expect(getLog()).to.deep.equal(
-			['<ol>acbd.insertBefore(<li>c, <li>d)'],
+			['<ol>acbd.insertBefore(<li>b, <li>c)'],
 			'swap back'
 		);
 	});
 
+	it('should displace multiple keyed children to the end efficiently', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		values.push(...values.splice(0, 3));
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('defghijabc');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdefghij.appendChild(<li>a)',
+			'<ol>bcdefghija.appendChild(<li>b)',
+			'<ol>cdefghijab.appendChild(<li>c)'
+		]);
+	});
+
+	it('should not displace when the suffix after the match is shorter than the jump', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		// Swap two far apart children; only the two swapped children are out of
+		// order, so only those two may move.
+		[values[1], values[8]] = [values[8], values[1]];
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('aicdefghbj');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdefghij.insertBefore(<li>i, <li>b)',
+			'<ol>aibcdefghj.insertBefore(<li>b, <li>j)'
+		]);
+	});
+
+	it('should move the shorter suffix when more than half the list is displaced', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdef');
+
+		// Displacing 4 of 6 children: moving the two-child suffix is the
+		// minimal set of moves.
+		values.push(...values.splice(0, 4));
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('efabcd');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdef.insertBefore(<li>e, <li>a)',
+			'<ol>eabcdf.insertBefore(<li>f, <li>a)'
+		]);
+	});
+
+	it('should displace multiple keyed children to the end while the list grows', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		values.push(...values.splice(0, 3));
+		values.push('k');
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('defghijabck');
+		expect(getLog()).to.deep.equal([
+			'<ol>abcdefghij.appendChild(<li>a)',
+			'<ol>bcdefghija.appendChild(<li>b)',
+			'<ol>cdefghijab.appendChild(<li>c)',
+			'<li>.appendChild(#text)',
+			'<ol>defghijabc.appendChild(<li>k)'
+		]);
+	});
+
+	it('should displace multiple keyed children to the end while another is removed', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		values.push(...values.splice(0, 3));
+		values.splice(values.indexOf('j'), 1);
+		clearLog();
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('defghiabc');
+		expect(getLog()).to.deep.equal([
+			'<li>j.remove()',
+			'<ol>abcdefghi.appendChild(<li>a)',
+			'<ol>bcdefghia.appendChild(<li>b)',
+			'<ol>cdefghiab.appendChild(<li>c)'
+		]);
+	});
+
+	it('should displace keyed children to the end repeatedly', () => {
+		const values = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
+		const expectedDisplaceLogs = [
+			[
+				'<ol>abcdefghij.appendChild(<li>a)',
+				'<ol>bcdefghija.appendChild(<li>b)',
+				'<ol>cdefghijab.appendChild(<li>c)'
+			],
+			[
+				'<ol>defghijabc.appendChild(<li>d)',
+				'<ol>efghijabcd.appendChild(<li>e)',
+				'<ol>fghijabcde.appendChild(<li>f)'
+			],
+			[
+				'<ol>ghijabcdef.appendChild(<li>g)',
+				'<ol>hijabcdefg.appendChild(<li>h)',
+				'<ol>ijabcdefgh.appendChild(<li>i)'
+			]
+		];
+
+		render(<List values={values} />, scratch);
+		expect(scratch.textContent).to.equal('abcdefghij');
+
+		for (let n = 0; n < 3; n++) {
+			values.push(...values.splice(0, 3));
+			clearLog();
+
+			render(<List values={values} />, scratch);
+			expect(scratch.textContent).to.equal(values.join(''));
+			expect(getLog()).to.deep.equal(expectedDisplaceLogs[n], `round ${n}`);
+		}
+	});
+
+	it('should keep text siblings correct around displaced keyed children', () => {
+		const content = condition => (
+			<ol>
+				{condition
+					? [
+							<li key="a">a</li>,
+							<li key="b">b</li>,
+							<li key="c">c</li>,
+							'mid',
+							<li key="d">d</li>,
+							<li key="e">e</li>
+						]
+					: [
+							<li key="c">c</li>,
+							'mid',
+							<li key="a">a</li>,
+							<li key="b">b</li>,
+							<li key="d">d</li>,
+							<li key="e">e</li>
+						]}
+			</ol>
+		);
+
+		render(content(true), scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<ol><li>a</li><li>b</li><li>c</li>mid<li>d</li><li>e</li></ol>'
+		);
+
+		clearLog();
+		render(content(false), scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<ol><li>c</li>mid<li>a</li><li>b</li><li>d</li><li>e</li></ol>'
+		);
+	});
+
 	it('should move keyed children to the end of the list', () => {
 		const values = ['a', 'b', 'c', 'd'];
 
@@ -463,7 +628,7 @@ describe('keys', () => {
 			'<ol>jihgfabcde.insertBefore(<li>e, <li>a)',
 			'<ol>jihgfeabcd.insertBefore(<li>d, <li>a)',
 			'<ol>jihgfedabc.insertBefore(<li>c, <li>a)',
-			'<ol>jihgfedcab.appendChild(<li>a)'
+			'<ol>jihgfedcab.insertBefore(<li>b, <li>a)'
 		]);
 	});
 

test/browser/lifecycles/shouldComponentUpdate.test.jsx

@@ -1068,7 +1068,7 @@ describe('Lifecycle methods', () => {
 			items: [7, 6, 5, 4, 3, 2, 1],
 			expectedLog: [
 				'<div>7634521.insertBefore(<div>5, <div>3)',
-				'<div>7653421.insertBefore(<div>3, <div>2)'
+				'<div>7653421.insertBefore(<div>4, <div>3)'
 			]
 		});
 	});