Skip to content

Commit

Permalink
Revive single text-child hot path
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Nov 1, 2024
1 parent 07dc9f3 commit 242a543
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 8 deletions.
10 changes: 9 additions & 1 deletion src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,6 @@ function diffElementNodes(
}
}

// If the new vnode didn't have dangerouslySetInnerHTML, diff its children
if (newHtml) {
// Avoid re-applying the same '__html' if it did not changed between re-render
if (
Expand All @@ -499,6 +498,15 @@ function diffElementNodes(
}

newVNode._children = [];
} else if (
typeof newChildren === 'string' &&
typeof oldProps.children === 'string'
) {
if (newChildren !== oldProps.children) {
oldVNode._children[0]._dom.data = newChildren;
newVNode._children = oldVNode._children;
oldVNode._children = [];
}
} else {
if (oldHtml) dom.innerHTML = '';

Expand Down
7 changes: 5 additions & 2 deletions test/browser/fragments.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,7 @@ describe('Fragment', () => {
);
});

// TODO
it('should support moving Fragments between beginning and end', () => {
const Foo = ({ condition }) => (
<ol>
Expand Down Expand Up @@ -1392,8 +1393,10 @@ describe('Fragment', () => {
'rendering from false to true'
);
expectDomLogToBe([
'<ol>450123.appendChild(<li>4)',
'<ol>501234.appendChild(<li>5)'
'<ol>450123.insertBefore(<li>0, <li>4)',
'<ol>045123.insertBefore(<li>1, <li>4)',
'<ol>014523.insertBefore(<li>2, <li>4)',
'<ol>012453.insertBefore(<li>3, <li>4)'
]);
});

Expand Down
2 changes: 1 addition & 1 deletion test/browser/keys.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ describe('keys', () => {
render(<List values={values} />, scratch);
expect(scratch.textContent).to.equal('abcd', 'swap back');
expect(getLog()).to.deep.equal(
['<ol>acbd.insertBefore(<li>c, <li>d)'],
['<ol>acbd.insertBefore(<li>b, <li>c)'],
'swap back'
);
});
Expand Down
20 changes: 20 additions & 0 deletions test/browser/lifecycles/componentWillUnmount.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,25 @@ describe('Lifecycle methods', () => {
render(<Foo />, scratch);
render(null, scratch);
});

it('should only remove dom after componentWillUnmount was called with single text child', () => {
class Foo extends Component {
componentWillUnmount() {
expect(document.getElementById('foo')).to.not.equal(null);
}

render() {
return <div id="foo" />;
}
}

render(
<div>
<Foo />
</div>,
scratch
);
render(<div>Text</div>, scratch);
});
});
});
12 changes: 12 additions & 0 deletions test/browser/placeholders.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -474,4 +474,16 @@ describe('null placeholders', () => {
expect(getLog()).to.deep.equal(['<div>Test3.remove()']);
expect(ref).to.have.been.calledOnce;
});

it('should properly mount and unmount text nodes with placeholders', () => {
function App({ show = false }) {
return <div>{show && 'Hello'}</div>;
}

render(<App show />, scratch);
expect(scratch.innerHTML).to.equal('<div>Hello</div>');

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<div></div>');
});
});
41 changes: 37 additions & 4 deletions test/browser/render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1676,10 +1676,10 @@ describe('render()', () => {
expect(getLog()).to.deep.equal([
'<div>.appendChild(#text)',
'<div>1352640.insertBefore(<div>11, <div>1)',
'<div>111352640.insertBefore(<div>1, <div>5)',
'<div>113152640.insertBefore(<div>6, <div>0)',
'<div>113152460.insertBefore(<div>2, <div>0)',
'<div>113154620.insertBefore(<div>5, <div>0)',
'<div>111352640.insertBefore(<div>3, <div>1)',
'<div>113152640.insertBefore(<div>4, <div>5)',
'<div>113145260.insertBefore(<div>6, <div>5)',
'<div>113146520.insertBefore(<div>2, <div>5)',
'<div>.appendChild(#text)',
'<div>113146250.appendChild(<div>9)',
'<div>.appendChild(#text)',
Expand Down Expand Up @@ -1813,4 +1813,37 @@ describe('render()', () => {
);
}
});

it('should correctly transition from multiple children to single text node and back', () => {
class Child extends Component {
componentDidMount() {}
componentWillUnmount() {}
render() {
return 'Child';
}
}

const proto = Child.prototype;
sinon.spy(Child.prototype, 'componentDidMount');
sinon.spy(Child.prototype, 'componentWillUnmount');

function App({ textChild = false }) {
return <div>{textChild ? 'Hello' : [<Child />, <span>b</span>]}</div>;
}

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<div>Child<span>b</span></div>');
expect(proto.componentDidMount).to.have.been.calledOnce;
expect(proto.componentWillUnmount).to.have.not.been.called;

render(<App textChild />, scratch);
expect(scratch.innerHTML).to.equal('<div>Hello</div>');
expect(proto.componentDidMount).to.have.been.calledOnce;
expect(proto.componentWillUnmount).to.have.been.calledOnce;

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<div>Child<span>b</span></div>');
expect(proto.componentDidMount).to.have.been.calledTwice;
expect(proto.componentWillUnmount).to.have.been.calledOnce;
});
});

0 comments on commit 242a543

Please sign in to comment.