From 84dcf7417fc7178703b708ad2621bca5d01bc05c Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Mon, 11 Nov 2024 07:42:26 +0100 Subject: [PATCH] Recreate unkeyed functional components when they change position. (#4550) --- src/diff/children.js | 6 +++- test/browser/render.test.js | 71 +++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/src/diff/children.js b/src/diff/children.js index ba2730478c..fe04b5fed6 100644 --- a/src/diff/children.js +++ b/src/diff/children.js @@ -397,9 +397,13 @@ function findMatchingIndex( // remainingOldChildren > 1 if the oldVNode is not already used/matched. Else // if the oldVNode was null or matched, then there could needs to be at least // 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. let shouldSearch = + (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 || diff --git a/test/browser/render.test.js b/test/browser/render.test.js index 96bef10bb0..cac7c7c255 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1786,6 +1786,77 @@ describe('render()', () => { ); }); + // #2949 + it('should not swap unkeyed chlildren', () => { + class X extends Component { + constructor(props) { + super(props); + this.name = props.name; + } + render() { + return

{this.name}

; + } + } + + function Foo({ condition }) { + return ( +
+ {condition ? '' : } + {condition ? : ''} +
+ ); + } + + render(, scratch); + expect(scratch.textContent).to.equal('A'); + + render(, scratch); + expect(scratch.textContent).to.equal('B'); + + render(, scratch); + expect(scratch.textContent).to.equal('A'); + }); + + it('should retain state for inserted children', () => { + class X extends Component { + constructor(props) { + super(props); + this.name = props.name; + } + render() { + return

{this.name}

; + } + } + + function Foo({ condition }) { + // We swap the prop from A to B but we don't expect this to + // reflect in text-content as we are testing whether the + // state is retained for a skew that matches the original children. + // + // We insert which should amount to a skew of -1 which should + // make us correctly match the X component. + return condition ? ( +
+ + +
+ ) : ( +
+ +
+ ); + } + + render(, scratch); + expect(scratch.textContent).to.equal('A'); + + render(, scratch); + expect(scratch.textContent).to.equal('A'); + + render(, scratch); + expect(scratch.textContent).to.equal('A'); + }); + it('handle shuffled (stress test)', () => { function randomize(arr) { for (let i = arr.length - 1; i > 0; i--) {