Skip to content

Commit

Permalink
remove _nextDom resetting as it collides with nested fragment switching
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Sep 23, 2022
1 parent a868b02 commit 3fddb48
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 16 deletions.
11 changes: 0 additions & 11 deletions src/diff/children.js
Expand Up @@ -205,17 +205,6 @@ export function diffChildren(
// Remove remaining oldChildren if there are any.
for (i = oldChildrenLength; i--; ) {
if (oldChildren[i] != null) {
if (
typeof newParentVNode.type == 'function' &&
oldChildren[i]._dom != null &&
oldChildren[i]._dom == newParentVNode._nextDom
) {
// If the newParentVNode.__nextDom points to a dom node that is about to
// be unmounted, then get the next sibling of that vnode and set
// _nextDom to it
newParentVNode._nextDom = getDomSibling(oldParentVNode, i + 1);
}

unmount(oldChildren[i], oldChildren[i]);
}
}
Expand Down
78 changes: 73 additions & 5 deletions test/browser/fragments.test.js
Expand Up @@ -237,7 +237,9 @@ describe('Fragment', () => {
expectDomLogToBe([
'<div>.appendChild(#text)',
'<div>122.insertBefore(<div>1, <span>1)',
'<span>1.remove()'
'<span>1.remove()',
'<div>122.appendChild(<span>2)',
'<div>122.appendChild(<span>2)'
]);
});

Expand Down Expand Up @@ -2658,7 +2660,7 @@ describe('Fragment', () => {
render(<App condition={false} />, scratch);

expect(scratch.innerHTML).to.equal(div([div(1), div('A')]));
expectDomLogToBe(['<div>2.remove()']);
expectDomLogToBe(['<div>2.remove()', '<div>1A.appendChild(<div>A)']);
});

it('should efficiently unmount nested Fragment children', () => {
Expand Down Expand Up @@ -2702,7 +2704,12 @@ describe('Fragment', () => {
render(<App condition={false} />, scratch);

expect(scratch.innerHTML).to.equal(div([div(1), div('A'), div('B')]));
expectDomLogToBe(['<div>2.remove()', '<div>3.remove()']);
expectDomLogToBe([
'<div>2.remove()',
'<div>3.remove()',
'<div>1AB.appendChild(<div>A)',
'<div>1BA.appendChild(<div>B)'
]);
});

it('should efficiently place new children and unmount nested Fragment children', () => {
Expand Down Expand Up @@ -2751,7 +2758,9 @@ describe('Fragment', () => {
'<div>.appendChild(#text)',
'<div>123AB.insertBefore(<div>4, <div>2)',
'<div>2.remove()',
'<div>3.remove()'
'<div>3.remove()',
'<div>14AB.appendChild(<div>A)',
'<div>14BA.appendChild(<div>B)'
]);
});

Expand Down Expand Up @@ -2799,7 +2808,66 @@ describe('Fragment', () => {
'<div>123AB.insertBefore(<span>1, <div>1)',
'<div>2.remove()',
'<div>3.remove()',
'<div>1.remove()'
'<div>1.remove()',
'<div>1AB.appendChild(<div>A)',
'<div>1BA.appendChild(<div>B)'
]);
});

it('should swap nested fragments correctly', () => {
let swap;
class App extends Component {
constructor(props) {
super(props);
this.state = { first: true };
}

render() {
if (this.state.first) {
return (
<Fragment>
{
<Fragment>
<p>1. Original item first paragraph</p>
</Fragment>
}
<p>2. Original item second paragraph</p>
<button onClick={(swap = () => this.setState({ first: false }))}>
Click me
</button>
</Fragment>
);
}
return (
<Fragment>
<p>1. Second item first paragraph</p>
<Fragment>
<p>2. Second item second paragraph</p>
<div />
</Fragment>
<button onClick={(swap = () => this.setState({ first: true }))}>
Click me
</button>
</Fragment>
);
}
}

render(<App />, scratch);
expect(scratch.innerHTML).to.equal(
'<p>1. Original item first paragraph</p><p>2. Original item second paragraph</p><button>Click me</button>'
);

swap();
rerender();
expect(scratch.innerHTML).to.equal(
'<p>1. Second item first paragraph</p><p>2. Second item second paragraph</p><div></div><button>Click me</button>'
);

swap();
rerender();
expect(scratch.innerHTML).to.equal(
'<p>1. Original item first paragraph</p><p>2. Original item second paragraph</p><button>Click me</button>'
);
});
});

0 comments on commit 3fddb48

Please sign in to comment.