diff --git a/compat/src/internal.d.ts b/compat/src/internal.d.ts index 8cbbdc93ba..d3a3af5f6d 100644 --- a/compat/src/internal.d.ts +++ b/compat/src/internal.d.ts @@ -33,7 +33,6 @@ export interface FunctionComponent

extends PreactFunctionComponent

{ export interface VNode extends PreactVNode { $$typeof?: symbol; preactCompatNormalized?: boolean; - _mask?: [number, number]; } export interface SuspenseState { @@ -45,4 +44,5 @@ export interface SuspenseComponent _pendingSuspensionCount: number; _suspenders: Component[]; _detachOnNextRender: null | VNode; + _mask?: [number, number]; } diff --git a/compat/src/suspense.js b/compat/src/suspense.js index c9385d2478..8ee35a14e0 100644 --- a/compat/src/suspense.js +++ b/compat/src/suspense.js @@ -191,23 +191,14 @@ Suspense.prototype.componentWillUnmount = function () { * @param {import('./internal').SuspenseState} state */ Suspense.prototype.render = function (props, state) { - let vnode = this._vnode; - if (!vnode._mask) { - let root = vnode; - while (root._parent) root = root._parent; - - root = root._mask || (root._mask = [0, 0]); - vnode._mask = [root[1]++, 0]; - } - if (this._detachOnNextRender) { // When the Suspense's _vnode was created by a call to createVNode // (i.e. due to a setState further up in the tree) // it's _children prop is null, in this case we "forget" about the parked vnodes to detach - if (vnode._children) { + if (this._vnode._children) { const detachedParent = document.createElement('div'); - const detachedComponent = vnode._children[0]._component; - vnode._children[0] = detachedClone( + const detachedComponent = this._vnode._children[0]._component; + this._vnode._children[0] = detachedClone( this._detachOnNextRender, detachedParent, (detachedComponent._originalParentDom = detachedComponent._parentDom) diff --git a/compat/test/browser/suspense-hydration.test.jsx b/compat/test/browser/suspense-hydration.test.jsx index 32f1b85a9b..84bcb0a510 100644 --- a/compat/test/browser/suspense-hydration.test.jsx +++ b/compat/test/browser/suspense-hydration.test.jsx @@ -4,9 +4,7 @@ import React, { hydrate, Fragment, Suspense, - lazy, memo, - useId, useState } from 'preact/compat'; import { logCall, getLog, clearLog } from '../../../test/_util/logCall'; @@ -17,7 +15,6 @@ import { } from '../../../test/_util/helpers'; import { ul, li, div } from '../../../test/_util/dom'; import { createLazy, createSuspenseLoader } from './suspense-utils'; -import { renderToString, renderToStringAsync } from 'preact-render-to-string'; import { vi } from 'vitest'; /* eslint-env browser */ @@ -77,218 +74,129 @@ describe('suspense hydration', () => { } }); - it('is stable for async Suspense siblings resolving in different orders', async () => { - const getIds = html => - Object.fromEntries( - [...html.matchAll(/([AB])<\/span>/g)].map( - ([, id, name]) => [name, id] - ) - ); - - async function renderWithResolveOrder(order) { - const loaders = {}; - - function Field({ name }) { - const id = useId(); - return {name}; - } - - const createLazy = name => - lazy( - () => - new Promise(resolve => { - loaders[name] = () => - resolve({ default: () => }); - }) - ); - - const A = createLazy('A'); - const B = createLazy('B'); - const rendered = renderToStringAsync( -

- - - - - - -
- ); - - await Promise.resolve(); - order.some(name => loaders[name]()); - - return getIds(await rendered); - } + it('should leave DOM untouched when suspending while hydrating', () => { + scratch.innerHTML = '
Hello
'; + clearLog(); - const ordered = await renderWithResolveOrder(['A', 'B']); - const reversed = await renderWithResolveOrder(['B', 'A']); + const [Lazy, resolve] = createLazy(); + hydrate( + + + , + scratch + ); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(scratch.innerHTML).to.equal('
Hello
'); + expect(getLog()).to.deep.equal([]); + clearLog(); - expect(new Set(Object.values(ordered)).size).to.equal(2); - expect(new Set(Object.values(reversed)).size).to.equal(2); - expect(reversed).to.deep.equal(ordered); + return resolve(() =>
Hello
).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal('
Hello
'); + expect(getLog()).to.deep.equal([]); + clearLog(); + }); }); - it('is stable for nested async Suspense siblings resolving in different orders', async () => { - const getIds = html => - Object.fromEntries( - [...html.matchAll(/([AB])<\/span>/g)].map( - ([, id, name]) => [name, id] - ) - ); + it('Should not crash when oldVNode._children is null during shouldComponentUpdate optimization', () => { + const originalHtml = '
Hello
'; + scratch.innerHTML = originalHtml; + clearLog(); - async function renderWithResolveOrder(order) { - const loaders = {}; + class ErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } - function Field({ name }) { - const id = useId(); - return {name}; + static getDerivedStateFromError() { + return { hasError: true }; } - const createLazy = name => - lazy( - () => - new Promise(resolve => { - loaders[name] = () => - resolve({ default: () => }); - }) - ); + render() { + return this.props.children; + } + } - const A = createLazy('A'); - const B = createLazy('B'); - const rendered = renderToStringAsync( - - -
- - - - + const [Lazy, resolve] = createLazy(); + function App() { + return ( + + + + ); - - await Promise.resolve(); - order.some(name => loaders[name]()); - - return getIds(await rendered); } - const ordered = await renderWithResolveOrder(['A', 'B']); - const reversed = await renderWithResolveOrder(['B', 'A']); - - expect(ordered).to.deep.equal({ A: 'P1-0', B: 'P2-0' }); - expect(reversed).to.deep.equal(ordered); - }); - - it('does not leak Suspense useId masks across abandoned renderToString attempts', () => { - const idsIn = html => [...html.matchAll(/P\d+-\d+/g)].map(([id]) => id); - - function Field() { - return {useId()}; - } + hydrate(, scratch); + rerender(); // Flush rerender queue to mimic what preact will really do + expect(scratch.innerHTML).to.equal(originalHtml); + expect(getLog()).to.deep.equal([]); + clearLog(); - function Suspends() { - throw Promise.resolve(); + let i = 0; + class ThrowOrRender extends React.Component { + shouldComponentUpdate() { + return i === 0; + } + render() { + if (i === 0) { + i++; + throw new Error('Test error'); + } + return
Hello
; + } } - const tree = () => ( - <> - - - - - - - - ); - - const first = idsIn(renderToString(tree())); - expect(first).to.deep.equal(['P0-0', 'P1-0']); - - expect(() => - renderToString( - - - - ) - ).to.throw(/renderToStringAsync/); - - expect(idsIn(renderToString(tree()))).to.deep.equal(first); + return resolve(ThrowOrRender).then(() => { + rerender(); + expect(scratch.innerHTML).to.equal(originalHtml); + clearLog(); + }); }); - it('keeps deeply nested Suspense useId masks compact', async () => { - function Field() { - const id = useId(); - return field; - } - - const Wrapper = ({ children }) => children; - let child = ( - - - - ); - - for (let i = 0; i < 10; i++) { - child = {child}; - } - - const html = await renderToStringAsync( - {child} - ); - - expect(html).to.equal('field'); - }); + it('does not crash when a hydrated suspended component bails out with shouldComponentUpdate', () => { + scratch.innerHTML = '
ssr
'; + clearLog(); - it('keeps nested Suspense ids distinct from parent useId calls', async () => { - const ids = []; + const promise = new Promise(() => {}); + let update; - function Field() { - ids.push(useId()); - return field; - } + class Suspender extends React.Component { + shouldComponentUpdate() { + return false; + } - function Wrapper() { - ids.push(useId()); - return ( - - - - ); + render() { + throw promise; + } } - await renderToStringAsync( - - - - ); + class App extends React.Component { + constructor(props) { + super(props); + this.state = { tick: 0 }; + update = () => this.setState({ tick: this.state.tick + 1 }); + } - expect(ids[0]).to.equal('P0-0'); - expect(ids[1]).to.equal('P1-0'); - }); + render() { + return ( + loading}> + + + ); + } + } - it('should leave DOM untouched when suspending while hydrating', () => { - scratch.innerHTML = '
Hello
'; - clearLog(); + hydrate(, scratch); + expect(scratch.innerHTML).to.equal('
ssr
'); - const [Lazy, resolve] = createLazy(); - hydrate( - - - , - scratch - ); - rerender(); // Flush rerender queue to mimic what preact will really do - expect(scratch.innerHTML).to.equal('
Hello
'); + update(); + expect(() => rerender()).not.to.throw(); + expect(scratch.innerHTML).to.equal('
ssr
'); expect(getLog()).to.deep.equal([]); clearLog(); - - return resolve(() =>
Hello
).then(() => { - rerender(); - expect(scratch.innerHTML).to.equal('
Hello
'); - expect(getLog()).to.deep.equal([]); - clearLog(); - }); }); it('should leave DOM untouched when suspending while hydrating', () => { diff --git a/src/diff/index.js b/src/diff/index.js index be7a4781b5..9ef5b04596 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -375,21 +375,20 @@ export function diff( newVNode._component._excess = oldDom; } newVNode._dom = oldDom; - } else { - if (excessDomChildren != NULL) { - for (let i = excessDomChildren.length; i--; ) { - removeNode(excessDomChildren[i]); - } + } else if (excessDomChildren != NULL) { + for (let i = excessDomChildren.length; i--; ) { + removeNode(excessDomChildren[i]); } - markAsForce(newVNode); } } else { newVNode._dom = oldVNode._dom; - if (!newVNode._children && oldVNode._children) { - newVNode._children = oldVNode._children; - } - if (!e.then) markAsForce(newVNode); } + + if (newVNode._children == NULL) { + newVNode._children = oldVNode._children || []; + } + + if (!e.then) markAsForce(newVNode); options._catchError(e, newVNode, oldVNode); } } else {