From 49bd318b3a17348ed53610f0a62ab9448e0c2b57 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 13 Feb 2025 17:41:10 +0100 Subject: [PATCH] Comment denoted hydration (#4636) * Remove unused imports * Comment denoted hydration * Make it work * Golfies --- .../test/browser/suspense-hydration.test.js | 292 ++++++++++++++---- src/diff/index.js | 53 +++- src/internal.d.ts | 1 + 3 files changed, 284 insertions(+), 62 deletions(-) diff --git a/compat/test/browser/suspense-hydration.test.js b/compat/test/browser/suspense-hydration.test.js index b364cf41c6..befba4972f 100644 --- a/compat/test/browser/suspense-hydration.test.js +++ b/compat/test/browser/suspense-hydration.test.js @@ -723,61 +723,17 @@ describe('suspense hydration', () => { }); }); - // Currently not supported. Hydration doesn't set attributes... but should it - // when coming back from suspense if props were updated? - it.skip('should hydrate and update attributes with latest props', () => { - const originalHtml = '

Count: 0

Lazy count: 0

'; - scratch.innerHTML = originalHtml; - clearLog(); - - /** @type {() => void} */ - let increment; - const [Lazy, resolve] = createLazy(); - function App() { - const [count, setCount] = useState(0); - increment = () => setCount(c => c + 1); - - return ( - -

Count: {count}

- - - ); - } - - hydrate(, scratch); - rerender(); // Flush rerender queue to mimic what preact will really do - expect(scratch.innerHTML).to.equal(originalHtml); - // Re: DOM OP below - Known issue with hydrating merged text nodes - expect(getLog()).to.deep.equal(['

Count: .appendChild(#text)']); - clearLog(); - - increment(); - rerender(); - - expect(scratch.innerHTML).to.equal( - '

Count: 1

Lazy count: 0

' - ); - expect(getLog()).to.deep.equal([]); - clearLog(); - - return resolve(({ count }) => ( -

Lazy count: {count}

- )).then(() => { - rerender(); - expect(scratch.innerHTML).to.equal( - '

Count: 1

Lazy count: 1

' - ); - // Re: DOM OP below - Known issue with hydrating merged text nodes - expect(getLog()).to.deep.equal(['

Lazy count: .appendChild(#text)']); - clearLog(); - }); - }); - - // Currently not supported, but I wrote the test before I realized that so - // leaving it here in case we do support it eventually - it.skip('should properly hydrate suspense when resolves to a Fragment', () => { - const originalHtml = ul([li(0), li(1), li(2), li(3), li(4), li(5)]); + it('should properly hydrate suspense when resolves to a Fragment', () => { + const originalHtml = ul([ + li(0), + li(1), + '', + li(2), + li(3), + '', + li(4), + li(5) + ]); const listeners = [ sinon.spy(), @@ -809,8 +765,8 @@ describe('suspense hydration', () => { scratch ); rerender(); // Flush rerender queue to mimic what preact will really do - expect(scratch.innerHTML).to.equal(originalHtml); expect(getLog()).to.deep.equal([]); + expect(scratch.innerHTML).to.equal(originalHtml); expect(listeners[5]).not.to.have.been.called; clearLog(); @@ -839,4 +795,228 @@ describe('suspense hydration', () => { expect(listeners[5]).to.have.been.calledTwice; }); }); + + it('should properly hydrate suspense when resolves to a Fragment without children', () => { + const originalHtml = ul([ + li(0), + li(1), + '', + '', + li(2), + li(3) + ]); + + const listeners = [sinon.spy(), sinon.spy(), sinon.spy(), sinon.spy()]; + + scratch.innerHTML = originalHtml; + clearLog(); + + const [Lazy, resolve] = createLazy(); + hydrate( + + + 0 + 1 + + + + + + 2 + 3 + + , + scratch + ); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(getLog()).to.deep.equal([]); + expect(scratch.innerHTML).to.equal(originalHtml); + expect(listeners[3]).not.to.have.been.called; + + clearLog(); + scratch.querySelector('li:last-child').dispatchEvent(createEvent('click')); + expect(listeners[3]).to.have.been.calledOnce; + + return resolve(() => null).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal(originalHtml); + expect(getLog()).to.deep.equal([]); + clearLog(); + + scratch + .querySelector('li:nth-child(2)') + .dispatchEvent(createEvent('click')); + expect(listeners[1]).to.have.been.calledOnce; + + scratch + .querySelector('li:last-child') + .dispatchEvent(createEvent('click')); + expect(listeners[3]).to.have.been.calledTwice; + }); + }); + + it('Should hydrate a fragment with multiple children correctly', () => { + scratch.innerHTML = '

Hello
World!
'; + clearLog(); + + const [Lazy, resolve] = createLazy(); + hydrate( + + + , + scratch + ); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(scratch.innerHTML).to.equal( + '
Hello
World!
' + ); + expect(getLog()).to.deep.equal([]); + clearLog(); + + return resolve(() => ( + <> +
Hello
+
World!
+ + )).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal( + '
Hello
World!
' + ); + expect(getLog()).to.deep.equal([]); + + clearLog(); + }); + }); + + it('Should hydrate a fragment with no children correctly', () => { + scratch.innerHTML = '
Hello world
'; + clearLog(); + + const [Lazy, resolve] = createLazy(); + hydrate( + <> + + + +
Hello world
+ , + scratch + ); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(scratch.innerHTML).to.equal( + '
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + clearLog(); + + return resolve(() => null).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal( + '
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + + clearLog(); + }); + }); + + it('Should hydrate a fragment with no children correctly deeply', () => { + scratch.innerHTML = + '
Hello world
'; + clearLog(); + + const [Lazy, resolve] = createLazy(); + const [Lazy2, resolve2] = createLazy(); + hydrate( + <> + + + + + + + +
Hello world
+ , + scratch + ); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(scratch.innerHTML).to.equal( + '
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + clearLog(); + + return resolve(p => p.children).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal( + '
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + + clearLog(); + return resolve2(() => null).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal( + '
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + + clearLog(); + }); + }); + }); + + it('Should hydrate a fragment with multiple children correctly deeply', () => { + scratch.innerHTML = + '

I am

Fragment
Hello world
'; + clearLog(); + + const [Lazy, resolve] = createLazy(); + const [Lazy2, resolve2] = createLazy(); + hydrate( + <> + + + + + + + +
Hello world
+ , + scratch + ); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(scratch.innerHTML).to.equal( + '

I am

Fragment
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + clearLog(); + + return resolve(p => p.children).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal( + '

I am

Fragment
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + + clearLog(); + return resolve2(() => ( + <> +

I am

+ Fragment + + )).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal( + '

I am

Fragment
Hello world
' + ); + expect(getLog()).to.deep.equal([]); + + clearLog(); + }); + }); + }); }); diff --git a/src/diff/index.js b/src/diff/index.js index d68045ed7f..20037b55e9 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -69,8 +69,8 @@ export function diff( // If the previous diff bailed out, resume creating/hydrating. if (oldVNode._flags & MODE_SUSPENDED) { isHydrating = !!(oldVNode._flags & MODE_HYDRATE); - oldDom = newVNode._dom = oldVNode._dom; - excessDomChildren = [oldDom]; + excessDomChildren = oldVNode._component._excess; + oldDom = newVNode._dom = oldVNode._dom = excessDomChildren[0]; } if ((tmp = options._diff)) tmp(newVNode); @@ -261,7 +261,7 @@ export function diff( let renderResult = isTopLevelFragment ? tmp.props.children : tmp; if (isTopLevelFragment) { - tmp.props.children = null; + tmp.props.children = NULL; } oldDom = diffChildren( @@ -293,15 +293,56 @@ export function diff( // if hydrating or creating initial tree, bailout preserves DOM: if (isHydrating || excessDomChildren != NULL) { if (e.then) { + let shouldFallback = true, + commentMarkersToFind = 0, + done = false; + newVNode._flags |= isHydrating ? MODE_HYDRATE | MODE_SUSPENDED : MODE_SUSPENDED; - while (oldDom && oldDom.nodeType == 8 && oldDom.nextSibling) { - oldDom = oldDom.nextSibling; + newVNode._component._excess = []; + for (let i = 0; i < excessDomChildren.length; i++) { + let child = excessDomChildren[i]; + if (child == NULL || done) continue; + + // When we encounter a boundary with $s we are opening + // a boundary, this implies that we need to bump + // the amount of markers we need to find before closing + // the outer boundary. + // We exclude the open and closing marker from + // the future excessDomChildren but any nested one + // needs to be included for future suspensions. + if (child.nodeType == 8 && child.data == '$s') { + if (commentMarkersToFind > 0) { + newVNode._component._excess.push(child); + } + commentMarkersToFind++; + shouldFallback = false; + excessDomChildren[i] = NULL; + } else if (child.nodeType == 8 && child.data == '/$s') { + commentMarkersToFind--; + if (commentMarkersToFind > 0) { + newVNode._component._excess.push(child); + } + done = commentMarkersToFind === 0; + oldDom = excessDomChildren[i]; + excessDomChildren[i] = NULL; + } else if (commentMarkersToFind > 0) { + newVNode._component._excess.push(child); + excessDomChildren[i] = NULL; + } + } + + if (shouldFallback) { + while (oldDom && oldDom.nodeType == 8 && oldDom.nextSibling) { + oldDom = oldDom.nextSibling; + } + + excessDomChildren[excessDomChildren.indexOf(oldDom)] = NULL; + newVNode._component._excess.push(oldDom); } - excessDomChildren[excessDomChildren.indexOf(oldDom)] = NULL; newVNode._dom = oldDom; } else { for (let i = excessDomChildren.length; i--; ) { diff --git a/src/internal.d.ts b/src/internal.d.ts index b306c4a011..71d99e35a3 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -162,6 +162,7 @@ export interface Component

extends preact.Component { constructor: ComponentType

; state: S; // Override Component["state"] to not be readonly for internal use, specifically Hooks + _excess?: PreactElement[]; _dirty: boolean; _force?: boolean; _renderCallbacks: Array<() => void>; // Only class components