Skip to content

Commit fcb8d7c

Browse files
authored
Revert unkeyed no-search (#4604)
1 parent eba80f7 commit fcb8d7c

File tree

2 files changed

+44
-6
lines changed

2 files changed

+44
-6
lines changed

src/diff/children.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,6 @@ function findMatchingIndex(
387387
) {
388388
const key = childVNode.key;
389389
const type = childVNode.type;
390-
let x = skewedIndex - 1;
391-
let y = skewedIndex + 1;
392390
let oldVNode = oldChildren[skewedIndex];
393391

394392
// We only need to perform a search if there are more children
@@ -401,11 +399,11 @@ function findMatchingIndex(
401399
// 1 (aka `remainingOldChildren > 0`) children to find and compare against.
402400
//
403401
// If there is an unkeyed functional VNode, that isn't a built-in like our Fragment,
404-
// we should not search as we risk re-using state of an unrelated VNode.
402+
// we should not search as we risk re-using state of an unrelated VNode. (reverted for now)
405403
let shouldSearch =
406-
(typeof type != 'function' || type === Fragment || key) &&
404+
// (typeof type != 'function' || type === Fragment || key) &&
407405
remainingOldChildren >
408-
(oldVNode != null && (oldVNode._flags & MATCHED) == 0 ? 1 : 0);
406+
(oldVNode != null && (oldVNode._flags & MATCHED) == 0 ? 1 : 0);
409407

410408
if (
411409
oldVNode === null ||
@@ -416,6 +414,8 @@ function findMatchingIndex(
416414
) {
417415
return skewedIndex;
418416
} else if (shouldSearch) {
417+
let x = skewedIndex - 1;
418+
let y = skewedIndex + 1;
419419
while (x >= 0 || y < oldChildren.length) {
420420
if (x >= 0) {
421421
oldVNode = oldChildren[x];

test/browser/render.test.js

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1800,7 +1800,7 @@ describe('render()', () => {
18001800
});
18011801

18021802
// #2949
1803-
it('should not swap unkeyed chlildren', () => {
1803+
it.skip('should not swap unkeyed chlildren', () => {
18041804
class X extends Component {
18051805
constructor(props) {
18061806
super(props);
@@ -1830,6 +1830,44 @@ describe('render()', () => {
18301830
expect(scratch.textContent).to.equal('A');
18311831
});
18321832

1833+
// #2949
1834+
it.skip('should not swap unkeyed chlildren', () => {
1835+
const calls = [];
1836+
class X extends Component {
1837+
constructor(props) {
1838+
super(props);
1839+
calls.push(props.name);
1840+
this.name = props.name;
1841+
}
1842+
render() {
1843+
return <p>{this.name}</p>;
1844+
}
1845+
}
1846+
1847+
function Foo({ condition }) {
1848+
return (
1849+
<div>
1850+
<X name="1" />
1851+
{condition ? '' : <X name="A" />}
1852+
{condition ? <X name="B" /> : ''}
1853+
<X name="C" />
1854+
</div>
1855+
);
1856+
}
1857+
1858+
render(<Foo />, scratch);
1859+
expect(scratch.textContent).to.equal('1AC');
1860+
expect(calls).to.deep.equal(['1', 'A', 'C']);
1861+
1862+
render(<Foo condition />, scratch);
1863+
expect(scratch.textContent).to.equal('1BC');
1864+
expect(calls).to.deep.equal(['1', 'A', 'C', 'B']);
1865+
1866+
render(<Foo />, scratch);
1867+
expect(scratch.textContent).to.equal('1AC');
1868+
expect(calls).to.deep.equal(['1', 'A', 'C', 'B', 'A']);
1869+
});
1870+
18331871
it('should retain state for inserted children', () => {
18341872
class X extends Component {
18351873
constructor(props) {

0 commit comments

Comments
 (0)