From 242a5435b617b171f8a05751259bb1e4346f0fe7 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 3 Oct 2024 09:34:44 +0200 Subject: [PATCH] Revive single text-child hot path --- src/diff/index.js | 10 ++++- test/browser/fragments.test.js | 7 +++- test/browser/keys.test.js | 2 +- .../lifecycles/componentWillUnmount.test.js | 20 +++++++++ test/browser/placeholders.test.js | 12 ++++++ test/browser/render.test.js | 41 +++++++++++++++++-- 6 files changed, 84 insertions(+), 8 deletions(-) diff --git a/src/diff/index.js b/src/diff/index.js index ae2d77ffc2..1857e10e44 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -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 ( @@ -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 = ''; diff --git a/test/browser/fragments.test.js b/test/browser/fragments.test.js index 7eb0b0924d..13e64f1092 100644 --- a/test/browser/fragments.test.js +++ b/test/browser/fragments.test.js @@ -1325,6 +1325,7 @@ describe('Fragment', () => { ); }); + // TODO it('should support moving Fragments between beginning and end', () => { const Foo = ({ condition }) => (
    @@ -1392,8 +1393,10 @@ describe('Fragment', () => { 'rendering from false to true' ); expectDomLogToBe([ - '
      450123.appendChild(
    1. 4)', - '
        501234.appendChild(
      1. 5)' + '
          450123.insertBefore(
        1. 0,
        2. 4)', + '
            045123.insertBefore(
          1. 1,
          2. 4)', + '
              014523.insertBefore(
            1. 2,
            2. 4)', + '
                012453.insertBefore(
              1. 3,
              2. 4)' ]); }); diff --git a/test/browser/keys.test.js b/test/browser/keys.test.js index 8d5a185b91..5d7deb0595 100644 --- a/test/browser/keys.test.js +++ b/test/browser/keys.test.js @@ -354,7 +354,7 @@ describe('keys', () => { render(, scratch); expect(scratch.textContent).to.equal('abcd', 'swap back'); expect(getLog()).to.deep.equal( - ['
                  acbd.insertBefore(
                1. c,
                2. d)'], + ['
                    acbd.insertBefore(
                  1. b,
                  2. c)'], 'swap back' ); }); diff --git a/test/browser/lifecycles/componentWillUnmount.test.js b/test/browser/lifecycles/componentWillUnmount.test.js index 5aa0a841fb..6c8fca9450 100644 --- a/test/browser/lifecycles/componentWillUnmount.test.js +++ b/test/browser/lifecycles/componentWillUnmount.test.js @@ -68,5 +68,25 @@ describe('Lifecycle methods', () => { render(, 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
                    ; + } + } + + render( +
                    + +
                    , + scratch + ); + render(
                    Text
                    , scratch); + }); }); }); diff --git a/test/browser/placeholders.test.js b/test/browser/placeholders.test.js index 0fbdb6f181..edcff30e72 100644 --- a/test/browser/placeholders.test.js +++ b/test/browser/placeholders.test.js @@ -474,4 +474,16 @@ describe('null placeholders', () => { expect(getLog()).to.deep.equal(['
                    Test3.remove()']); expect(ref).to.have.been.calledOnce; }); + + it('should properly mount and unmount text nodes with placeholders', () => { + function App({ show = false }) { + return
                    {show && 'Hello'}
                    ; + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                    Hello
                    '); + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                    '); + }); }); diff --git a/test/browser/render.test.js b/test/browser/render.test.js index 96bef10bb0..1a4054b29a 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1676,10 +1676,10 @@ describe('render()', () => { expect(getLog()).to.deep.equal([ '
                    .appendChild(#text)', '
                    1352640.insertBefore(
                    11,
                    1)', - '
                    111352640.insertBefore(
                    1,
                    5)', - '
                    113152640.insertBefore(
                    6,
                    0)', - '
                    113152460.insertBefore(
                    2,
                    0)', - '
                    113154620.insertBefore(
                    5,
                    0)', + '
                    111352640.insertBefore(
                    3,
                    1)', + '
                    113152640.insertBefore(
                    4,
                    5)', + '
                    113145260.insertBefore(
                    6,
                    5)', + '
                    113146520.insertBefore(
                    2,
                    5)', '
                    .appendChild(#text)', '
                    113146250.appendChild(
                    9)', '
                    .appendChild(#text)', @@ -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
                    {textChild ? 'Hello' : [, b]}
                    ; + } + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                    Childb
                    '); + expect(proto.componentDidMount).to.have.been.calledOnce; + expect(proto.componentWillUnmount).to.have.not.been.called; + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                    Hello
                    '); + expect(proto.componentDidMount).to.have.been.calledOnce; + expect(proto.componentWillUnmount).to.have.been.calledOnce; + + render(, scratch); + expect(scratch.innerHTML).to.equal('
                    Childb
                    '); + expect(proto.componentDidMount).to.have.been.calledTwice; + expect(proto.componentWillUnmount).to.have.been.calledOnce; + }); });