From e1c277d44f1ad8a1f817a1975fb3fa2347ab8cc2 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 3 Oct 2024 09:34:44 +0200 Subject: [PATCH 1/2] 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 | 65 +++++++++++++++++-- 6 files changed, 107 insertions(+), 9 deletions(-) diff --git a/src/diff/index.js b/src/diff/index.js index ae2d77ffc2..9cb2aa3158 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 = null; + } } 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..8ce10b8013 100644 --- a/test/browser/render.test.js +++ b/test/browser/render.test.js @@ -1,5 +1,5 @@ import { setupRerender } from 'preact/test-utils'; -import { createElement, render, Component, options } from 'preact'; +import { createElement, render, Component, options, Fragment } from 'preact'; import { setupScratch, teardown, @@ -305,6 +305,28 @@ describe('render()', () => { expect(scratch.innerHTML).to.equal('Testing, huh! How is it going?'); }); + it('should render strings delimited by fragments text content', () => { + render( + + Foo + Bar + Baz + , + scratch + ); + expect(scratch.innerHTML).to.equal('FooBarBaz'); + + render( + + Baz + Bar + Foo + , + scratch + ); + expect(scratch.innerHTML).to.equal('BazBarFoo'); + }); + it('should render arrays of mixed elements', () => { render(getMixedArray(), scratch); expect(scratch.innerHTML).to.equal(mixedArrayHTML); @@ -1676,10 +1698,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 +1835,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; + }); }); From df6c646a9470033011a0354962a60865e28312e8 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Fri, 1 Nov 2024 09:36:19 +0100 Subject: [PATCH 2/2] Update test/browser/fragments.test.js --- test/browser/fragments.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/browser/fragments.test.js b/test/browser/fragments.test.js index 13e64f1092..0a95df9ae1 100644 --- a/test/browser/fragments.test.js +++ b/test/browser/fragments.test.js @@ -1325,7 +1325,6 @@ describe('Fragment', () => { ); }); - // TODO it('should support moving Fragments between beginning and end', () => { const Foo = ({ condition }) => (