Skip to content

Commit

Permalink
Revert unkeyed no-search
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Dec 11, 2024
1 parent 5c9625a commit cccb676
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
10 changes: 5 additions & 5 deletions src/diff/children.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ||
Expand All @@ -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];
Expand Down
40 changes: 39 additions & 1 deletion test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 <p>{this.name}</p>;
}
}

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

render(<Foo />, scratch);
expect(scratch.textContent).to.equal('1AC');
expect(calls).to.deep.equal(['1', 'A', 'C']);

render(<Foo condition />, scratch);
expect(scratch.textContent).to.equal('1BC');
expect(calls).to.deep.equal(['1', 'A', 'C', 'B']);

render(<Foo />, 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) {
Expand Down

0 comments on commit cccb676

Please sign in to comment.