Fix useId stability across async Suspense (#5108)

Assisted-By: devx/1b997072-d89c-4f6a-a18f-9525b3ef7a9e

Author: JoviDeCroock

compat/src/internal.d.ts

@@ -34,6 +34,7 @@ export interface FunctionComponent<P = {}> extends PreactFunctionComponent<P> {
 export interface VNode<T = any> extends PreactVNode<T> {
 	$$typeof?: symbol | string;
 	preactCompatNormalized?: boolean;
+	_mask?: [number, number];
 }
 
 export interface SuspenseState {

compat/src/suspense.js

@@ -198,14 +198,23 @@ 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 (this._vnode._children) {
+		if (vnode._children) {
 			const detachedParent = document.createElement('div');
-			const detachedComponent = this._vnode._children[0]._component;
-			this._vnode._children[0] = detachedClone(
+			const detachedComponent = vnode._children[0]._component;
+			vnode._children[0] = detachedClone(
 				this._detachOnNextRender,
 				detachedParent,
 				(detachedComponent._originalParentDom = detachedComponent._parentDom)

compat/test/browser/suspense-hydration.test.jsx

@@ -4,7 +4,9 @@ import React, {
 	hydrate,
 	Fragment,
 	Suspense,
+	lazy,
 	memo,
+	useId,
 	useState
 } from 'preact/compat';
 import { logCall, getLog, clearLog } from '../../../test/_util/logCall';
@@ -15,6 +17,7 @@ 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';
 
 /* eslint-env browser, mocha */
 describe('suspense hydration', () => {
@@ -73,6 +76,196 @@ describe('suspense hydration', () => {
 		}
 	});
 
+	it('is stable for async Suspense siblings resolving in different orders', async () => {
+		const getIds = html =>
+			Object.fromEntries(
+				[...html.matchAll(/<span id="([^"]+)">([AB])<\/span>/g)].map(
+					([, id, name]) => [name, id]
+				)
+			);
+
+		async function renderWithResolveOrder(order) {
+			const loaders = {};
+
+			function Field({ name }) {
+				const id = useId();
+				return <span id={id}>{name}</span>;
+			}
+
+			const createLazy = name =>
+				lazy(
+					() =>
+						new Promise(resolve => {
+							loaders[name] = () =>
+								resolve({ default: () => <Field name={name} /> });
+						})
+				);
+
+			const A = createLazy('A');
+			const B = createLazy('B');
+			const rendered = renderToStringAsync(
+				<div>
+					<Suspense fallback={null}>
+						<A />
+					</Suspense>
+					<Suspense fallback={null}>
+						<B />
+					</Suspense>
+				</div>
+			);
+
+			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(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);
+	});
+
+	it('is stable for nested async Suspense siblings resolving in different orders', async () => {
+		const getIds = html =>
+			Object.fromEntries(
+				[...html.matchAll(/<span id="([^"]+)">([AB])<\/span>/g)].map(
+					([, id, name]) => [name, id]
+				)
+			);
+
+		async function renderWithResolveOrder(order) {
+			const loaders = {};
+
+			function Field({ name }) {
+				const id = useId();
+				return <span id={id}>{name}</span>;
+			}
+
+			const createLazy = name =>
+				lazy(
+					() =>
+						new Promise(resolve => {
+							loaders[name] = () =>
+								resolve({ default: () => <Field name={name} /> });
+						})
+				);
+
+			const A = createLazy('A');
+			const B = createLazy('B');
+			const rendered = renderToStringAsync(
+				<Suspense fallback={null}>
+					<Suspense fallback={null}>
+						<A />
+					</Suspense>
+					<Suspense fallback={null}>
+						<B />
+					</Suspense>
+				</Suspense>
+			);
+
+			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 <i>{useId()}</i>;
+		}
+
+		function Suspends() {
+			throw Promise.resolve();
+		}
+
+		const tree = () => (
+			<>
+				<Suspense fallback={null}>
+					<Field />
+				</Suspense>
+				<Suspense fallback={null}>
+					<Field />
+				</Suspense>
+			</>
+		);
+
+		const first = idsIn(renderToString(tree()));
+		expect(first).to.deep.equal(['P0-0', 'P1-0']);
+
+		expect(() =>
+			renderToString(
+				<Suspense fallback={null}>
+					<Suspends />
+				</Suspense>
+			)
+		).to.throw(/renderToStringAsync/);
+
+		expect(idsIn(renderToString(tree()))).to.deep.equal(first);
+	});
+
+	it('keeps deeply nested Suspense useId masks compact', async () => {
+		function Field() {
+			const id = useId();
+			return <span id={id}>field</span>;
+		}
+
+		const Wrapper = ({ children }) => children;
+		let child = (
+			<Suspense fallback={null}>
+				<Field />
+			</Suspense>
+		);
+
+		for (let i = 0; i < 10; i++) {
+			child = <Wrapper>{child}</Wrapper>;
+		}
+
+		const html = await renderToStringAsync(
+			<Suspense fallback={null}>{child}</Suspense>
+		);
+
+		expect(html).to.equal('<span id="P1-0">field</span>');
+	});
+
+	it('keeps nested Suspense ids distinct from parent useId calls', async () => {
+		const ids = [];
+
+		function Field() {
+			ids.push(useId());
+			return <span id={ids[1]}>field</span>;
+		}
+
+		function Wrapper() {
+			ids.push(useId());
+			return (
+				<Suspense fallback={null}>
+					<Field />
+				</Suspense>
+			);
+		}
+
+		await renderToStringAsync(
+			<Suspense fallback={null}>
+				<Wrapper />
+			</Suspense>
+		);
+
+		expect(ids[0]).to.equal('P0-0');
+		expect(ids[1]).to.equal('P1-0');
+	});
+
 	it('should leave DOM untouched when suspending while hydrating', () => {
 		scratch.innerHTML = '<div>Hello</div>';
 		clearLog();