From fcb8d7c7b74ea0dad95d04f919d06009499602e8 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 12 Dec 2024 08:01:29 +0100 Subject: [PATCH] Revert unkeyed no-search (#4604) --- src/diff/children.js | 10 +++++----- test/browser/render.test.js | 40 ++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/diff/children.js b/src/diff/children.js index 80090ec996..5053a51174 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -387,8 +387,6 @@ function findMatchingIndex( ) { const key = childVNode.key; const type = childVNode.type; - let x = skewedIndex - 1; - let y = skewedIndex + 1; let oldVNode = oldChildren[skewedIndex]; // We only need to perform a search if there are more children @@ -401,11 +399,11 @@ function findMatchingIndex( // 1 (aka `remainingOldChildren > 0`) children to find and compare against. // // If there is an unkeyed functional VNode, that isn't a built-in like our Fragment, - // we should not search as we risk re-using state of an unrelated VNode. + // we should not search as we risk re-using state of an unrelated VNode. (reverted for now) let shouldSearch = - (typeof type != 'function' || type === Fragment || key) && + // (typeof type != 'function' || type === Fragment || key) && remainingOldChildren > - (oldVNode != null && (oldVNode._flags & MATCHED) == 0 ? 1 : 0); + (oldVNode != null && (oldVNode._flags & MATCHED) == 0 ? 1 : 0); if ( oldVNode === null || @@ -416,6 +414,8 @@ function findMatchingIndex( ) { return skewedIndex; } else if (shouldSearch) { + let x = skewedIndex - 1; + let y = skewedIndex + 1; while (x >= 0 || y < oldChildren.length) { if (x >= 0) { oldVNode = oldChildren[x]; diff --git a/test/browser/render.test.js b/test/browser/render.test.js index 7230210d1d..b5ceff81c6 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1800,7 +1800,7 @@ describe('render()', () => { }); // #2949 - it('should not swap unkeyed chlildren', () => { + it.skip('should not swap unkeyed chlildren', () => { class X extends Component { constructor(props) { super(props); @@ -1830,6 +1830,44 @@ describe('render()', () => { expect(scratch.textContent).to.equal('A'); }); + // #2949 + it.skip('should not swap unkeyed chlildren', () => { + const calls = []; + class X extends Component { + constructor(props) { + super(props); + calls.push(props.name); + this.name = props.name; + } + render() { + return

{this.name}

; + } + } + + function Foo({ condition }) { + return ( +
+ + {condition ? '' : } + {condition ? : ''} + +
+ ); + } + + render(, scratch); + expect(scratch.textContent).to.equal('1AC'); + expect(calls).to.deep.equal(['1', 'A', 'C']); + + render(, scratch); + expect(scratch.textContent).to.equal('1BC'); + expect(calls).to.deep.equal(['1', 'A', 'C', 'B']); + + render(, scratch); + expect(scratch.textContent).to.equal('1AC'); + expect(calls).to.deep.equal(['1', 'A', 'C', 'B', 'A']); + }); + it('should retain state for inserted children', () => { class X extends Component { constructor(props) {