Use suspense boundary vnode as parent for subtree re-render (#408)

* use suspense boundary vnode as parent for re-render

* update internal types for renderChild

* add changeset

* add test to check for VNode circular references inside suspense boundary renderChild

* add useId with Suspense test

* remove unused import

Author: f0x52

.changeset/friendly-numbers-hang.md

@@ -0,0 +1,5 @@
+---
+'preact-render-to-string': patch
+---
+
+Fix issue where subtree re-render for Suspense boundaries caused a circular reference in the VNode's parent

src/index.js

@@ -480,13 +480,13 @@ function _renderToString(
 			return str;
 		} catch (error) {
 			if (!asyncMode && renderer && renderer.onError) {
-				let res = renderer.onError(error, vnode, (child) =>
+				let res = renderer.onError(error, vnode, (child, parent) =>
 					_renderToString(
 						child,
 						context,
 						isSvgMode,
 						selectValue,
-						vnode,
+						parent,
 						asyncMode,
 						renderer
 					)

src/internal.d.ts

@@ -1,4 +1,4 @@
-import { ComponentChildren, VNode } from 'preact';
+import { ComponentChildren, ComponentChild, VNode } from 'preact';
 
 interface Suspended {
 	id: string;
@@ -15,7 +15,7 @@ interface RendererErrorHandler {
 		this: RendererState,
 		error: any,
 		vnode: VNode<{ fallback: any }>,
-		renderChild: (child: ComponentChildren) => string
+		renderChild: (child: ComponentChildren, parent: ComponentChild) => string
 	): string | undefined;
 }
 

src/lib/chunked.js

@@ -75,7 +75,7 @@ function handleError(error, vnode, renderChild) {
 	const promise = error.then(
 		() => {
 			if (abortSignal && abortSignal.aborted) return;
-			const child = renderChild(vnode.props.children);
+			const child = renderChild(vnode.props.children, vnode);
 			if (child) this.onWrite(createSubtree(id, child));
 		},
 		// TODO: Abort and send hydration code snippet to client

test/compat/render-chunked.test.js

@@ -1,9 +1,11 @@
 import { h } from 'preact';
 import { expect } from 'chai';
 import { Suspense } from 'preact/compat';
+import { useId } from 'preact/hooks';
 import { renderToChunks } from '../../src/lib/chunked';
 import { createSubtree, createInitScript } from '../../src/lib/client';
 import { createSuspender } from '../utils';
+import { VNODE, PARENT } from '../../src/lib/constants';
 
 describe('renderToChunks', () => {
 	it('should render non-suspended JSX in one go', async () => {
@@ -66,4 +68,85 @@ describe('renderToChunks', () => {
 			'</div>'
 		]);
 	});
+
+	it('should encounter no circular references when rendering a suspense boundary subtree', async () => {
+		const { Suspender, suspended } = createSuspender();
+
+		const visited = new Set();
+		let circular = false;
+
+		function CircularReferenceCheck() {
+			let root = this[VNODE];
+			while (root !== null && root[PARENT] !== null) {
+				if (visited.has(root)) {
+					// Can't throw an error here, _catchError handler will also loop infinitely
+					circular = true;
+					break;
+				}
+				visited.add(root);
+				root = root[PARENT];
+			}
+			return <p>it works</p>;
+		}
+
+		const result = [];
+		const promise = renderToChunks(
+			<div>
+				<Suspense fallback="loading...">
+					<Suspender>
+						<CircularReferenceCheck />
+					</Suspender>
+				</Suspense>
+			</div>,
+			{ onWrite: (s) => result.push(s) }
+		);
+
+		suspended.resolve();
+		await promise;
+
+		if (circular) {
+			throw new Error('CircularReference');
+		}
+
+		expect(result).to.deep.equal([
+			'<div><!--preact-island:16-->loading...<!--/preact-island:16--></div>',
+			'<div hidden>',
+			createInitScript(1),
+			createSubtree('16', '<p>it works</p>'),
+			'</div>'
+		]);
+	});
+
+	it('should support using useId hooks inside a suspense boundary', async () => {
+		const { Suspender, suspended } = createSuspender();
+
+		function ComponentWithId() {
+			const id = useId();
+			return <p>id: {id}</p>;
+		}
+
+		const result = [];
+		const promise = renderToChunks(
+			<div>
+				<ComponentWithId />
+				<Suspense fallback="loading...">
+					<Suspender>
+						<ComponentWithId />
+					</Suspender>
+				</Suspense>
+			</div>,
+			{ onWrite: (s) => result.push(s) }
+		);
+
+		suspended.resolve();
+		await promise;
+
+		expect(result).to.deep.equal([
+			'<div><p>id: P0-0</p><!--preact-island:24-->loading...<!--/preact-island:24--></div>',
+			'<div hidden>',
+			createInitScript(1),
+			createSubtree('24', '<p>id: P0-0</p>'),
+			'</div>'
+		]);
+	});
 });

test/compat/stream-node.test.js

@@ -66,10 +66,10 @@ describe('renderToPipeableStream', () => {
 		const result = await sink.promise;
 
 		expect(result).to.deep.equal([
-			'<div><!--preact-island:17-->loading...<!--/preact-island:17--></div>',
+			'<div><!--preact-island:33-->loading...<!--/preact-island:33--></div>',
 			'<div hidden>',
 			createInitScript(),
-			createSubtree('17', '<p>it works</p>'),
+			createSubtree('33', '<p>it works</p>'),
 			'</div>'
 		]);
 	});

test/compat/stream.test.js

@@ -82,10 +82,10 @@ describe('renderToReadableStream', () => {
 		const result = await sink.promise;
 
 		expect(result).to.deep.equal([
-			'<div><!--preact-island:24-->loading...<!--/preact-island:24--></div>',
+			'<div><!--preact-island:40-->loading...<!--/preact-island:40--></div>',
 			'<div hidden>',
 			createInitScript(),
-			createSubtree('24', '<p>it works</p>'),
+			createSubtree('40', '<p>it works</p>'),
 			'</div>'
 		]);
 	});