Skip to content

Commit

Permalink
Recreate unkeyed functional components when they change position. (#4550
Browse files Browse the repository at this point in the history
)
  • Loading branch information
JoviDeCroock authored Nov 11, 2024
1 parent 3a95fc1 commit 84dcf74
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/diff/children.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
71 changes: 71 additions & 0 deletions test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <p>{this.name}</p>;
}
}

function Foo({ condition }) {
return (
<div>
{condition ? '' : <X name="A" />}
{condition ? <X name="B" /> : ''}
</div>
);
}

render(<Foo />, scratch);
expect(scratch.textContent).to.equal('A');

render(<Foo condition />, scratch);
expect(scratch.textContent).to.equal('B');

render(<Foo />, 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 <p>{this.name}</p>;
}
}

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 <span /> which should amount to a skew of -1 which should
// make us correctly match the X component.
return condition ? (
<div>
<span />
<X name="B" />
</div>
) : (
<div>
<X name="A" />
</div>
);
}

render(<Foo />, scratch);
expect(scratch.textContent).to.equal('A');

render(<Foo condition />, scratch);
expect(scratch.textContent).to.equal('A');

render(<Foo />, scratch);
expect(scratch.textContent).to.equal('A');
});

it('handle shuffled (stress test)', () => {
function randomize(arr) {
for (let i = arr.length - 1; i > 0; i--) {
Expand Down

0 comments on commit 84dcf74

Please sign in to comment.