Repository: preactjs/preact-render-to-string
Pull request #405 (2025-01-04 19:42:34)

renderToPipeableStream: Expose errors through onError, no longer emit un-catchable error event on internal stream
Fixes https://github.com/preactjs/preact-render-to-string/issues/404

Changes error and abort handling in `renderToPipeableStream` so the stream no longer emits an uncatchable error event. Instead errors are passed to `options.onError` if provided. If there is no error callback it's rethrown, though maybe that's not ideal since it happens in the async chain, so can't really be caught either. Alternatively this could just `console.error`, so it doesn't get silently swallowed entirely? 

Also added an optional `reason` argument for `stream.abort()`, defaulting to `new Error(The render was aborted by the server without a reason.)` inspired by React. Previously this was just `new Error('aborted')`, though this was never exposed to consumers.
Abort still destroys the stream, and also calls the `onError` callback (if defined) with the reason

changeset-bot[bot]: ###  🦋  Changeset detected

Latest commit: 310e1bb067ee93776e805eddb737ebe51a266408

**The changes in this PR will be included in the next version bump.**

<details><summary>This PR includes changesets to release 1 package</summary>

  | Name                    | Type  |
| ----------------------- | ----- |
| preact-render-to-string | Patch |

</details>

Not sure what this means? [Click here  to learn what changesets are](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add another changeset to this PR](https://github.com/f0x52/preact-render-to-string/new/streaming-errors?filename=.changeset/smooth-pots-sniff.md&value=---%0A%22preact-render-to-string%22%3A%20patch%0A---%0A%0ArenderToPipeableStream%3A%20Expose%20errors%20through%20onError%2C%20no%20longer%20emit%20un-catchable%20error%20event%20on%20internal%20stream%0A)



Patch:
diff --git a/.changeset/friendly-numbers-hang.md b/.changeset/friendly-numbers-hang.md
new file mode 100644
index 0000000..4e8fe4b
--- /dev/null
+++ b/.changeset/friendly-numbers-hang.md
@@ -0,0 +1,6 @@
+---
+'preact-render-to-string': patch
+---
+
+Ensure that the `_parent` is kept around across multiple suspensions and avoid circular references.
+In doing so our `useId` hook should always output unique ids during renderingToString.
diff --git a/.changeset/little-rocks-doubt.md b/.changeset/little-rocks-doubt.md
new file mode 100644
index 0000000..2b41c74
--- /dev/null
+++ b/.changeset/little-rocks-doubt.md
@@ -0,0 +1,5 @@
+---
+'preact-render-to-string': patch
+---
+
+Ensure the renderToStream types of `/stream` and `/stream-node` accept a generic for the props of the passed in VNode
diff --git a/.changeset/mighty-keys-admire.md b/.changeset/mighty-keys-admire.md
new file mode 100644
index 0000000..5af460d
--- /dev/null
+++ b/.changeset/mighty-keys-admire.md
@@ -0,0 +1,5 @@
+---
+'preact-render-to-string': patch
+---
+
+renderToPipeableStream: expose errors through onError, no longer emit un-catchable error event on internal stream
diff --git a/package-lock.json b/package-lock.json
index e4d3c80..6e40cb4 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
 	"name": "preact-render-to-string",
-	"version": "6.5.11",
+	"version": "6.5.12",
 	"lockfileVersion": 3,
 	"requires": true,
 	"packages": {
 		"": {
 			"name": "preact-render-to-string",
-			"version": "6.5.11",
+			"version": "6.5.12",
 			"license": "MIT",
 			"devDependencies": {
 				"@babel/plugin-transform-react-jsx": "^7.12.12",
@@ -14,7 +14,7 @@
 				"@babel/register": "^7.12.10",
 				"@changesets/changelog-github": "^0.4.1",
 				"@changesets/cli": "^2.18.0",
-				"baseline-rts": "npm:preact-render-to-string@6.5.11",
+				"baseline-rts": "npm:preact-render-to-string@latest",
 				"benchmarkjs-pretty": "^2.0.1",
 				"chai": "^4.2.0",
 				"check-export-map": "^1.3.1",
diff --git a/src/index.js b/src/index.js
index a617432..ff4c774 100644
--- a/src/index.js
+++ b/src/index.js
@@ -443,7 +443,6 @@ function _renderToString(
 					return EMPTY_STR;
 				} finally {
 					if (afterDiff) afterDiff(vnode);
-					vnode[PARENT] = null;
 
 					if (ummountHook) ummountHook(vnode);
 				}
@@ -473,20 +472,19 @@ function _renderToString(
 
 			if (afterDiff) afterDiff(vnode);
 			// when we are dealing with suspense we can't do this...
-			vnode[PARENT] = null;
 
 			if (options.unmount) options.unmount(vnode);
 
 			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
 					)
@@ -688,9 +686,6 @@ function _renderToString(
 
 	if (afterDiff) afterDiff(vnode);
 
-	// TODO: this was commented before
-	vnode[PARENT] = null;
-
 	if (ummountHook) ummountHook(vnode);
 
 	// Emit self-closing tag for empty void elements:
diff --git a/src/internal.d.ts b/src/internal.d.ts
index 8d2959f..b4bbec8 100644
--- a/src/internal.d.ts
+++ b/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;
 }
 
diff --git a/src/lib/chunked.js b/src/lib/chunked.js
index 38e6a26..cd1686d 100644
--- a/src/lib/chunked.js
+++ b/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
diff --git a/src/stream-node.d.ts b/src/stream-node.d.ts
index 5cdc0e5..990147b 100644
--- a/src/stream-node.d.ts
+++ b/src/stream-node.d.ts
@@ -8,12 +8,12 @@ interface RenderToPipeableStreamOptions {
 }
 
 interface PipeableStream {
-	abort: () => void;
+	abort: (reason?: unknown) => void;
 	pipe: (writable: WritableStream) => void;
 }
 
-export function renderToPipeableStream(
-	vnode: VNode,
+export function renderToPipeableStream<P = {}>(
+	vnode: VNode<P>,
 	options: RenderToPipeableStreamOptions,
 	context?: any
 ): PipeableStream;
diff --git a/src/stream-node.js b/src/stream-node.js
index dcd6f8f..bffa20e 100644
--- a/src/stream-node.js
+++ b/src/stream-node.js
@@ -44,7 +44,12 @@ export function renderToPipeableStream(vnode, options, context) {
 			stream.end();
 		})
 		.catch((error) => {
-			stream.destroy(error);
+			stream.destroy();
+			if (options.onError) {
+				options.onError(error);
+			} else {
+				throw error;
+			}
 		});
 
 	Promise.resolve().then(() => {
@@ -52,9 +57,15 @@ export function renderToPipeableStream(vnode, options, context) {
 	});
 
 	return {
-		abort() {
+		/**
+		 * @param {unknown} [reason]
+		 */
+		abort(reason = new Error('The render was aborted by the server without a reason.')) {
 			controller.abort();
-			stream.destroy(new Error('aborted'));
+			stream.destroy();
+			if (options.onError) {
+				options.onError(reason);
+			}
 		},
 		/**
 		 * @param {import("stream").Writable} writable
diff --git a/src/stream.d.ts b/src/stream.d.ts
index c786d96..1ab9aa5 100644
--- a/src/stream.d.ts
+++ b/src/stream.d.ts
@@ -4,7 +4,7 @@ interface RenderStream extends ReadableStream<Uint8Array> {
 	allReady: Promise<void>;
 }
 
-export function renderToReadableStream(
-	vnode: VNode,
+export function renderToReadableStream<P = {}>(
+	vnode: VNode<P>,
 	context?: any
 ): RenderStream;


Test patch:
diff --git a/test/compat/render-chunked.test.js b/test/compat/render-chunked.test.js
index b043b8f..502507f 100644
--- a/test/compat/render-chunked.test.js
+++ b/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,126 @@ 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-1</p>'),
+			'</div>'
+		]);
+	});
+
+	it('should support using multiple useId hooks inside multiple suspense boundaries', async () => {
+		const { Suspender, suspended } = createSuspender();
+		const { Suspender: Suspender2, suspended: suspended2 } = 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>
+				<Suspense fallback="loading...">
+					<Suspender2>
+						<ComponentWithId />
+					</Suspender2>
+				</Suspense>
+			</div>,
+			{ onWrite: (s) => result.push(s) }
+		);
+
+		suspended.resolve();
+		suspended2.resolve();
+		await promise;
+
+		expect(result).to.deep.equal([
+			'<div><p>id: P0-0</p><!--preact-island:33-->loading...<!--/preact-island:33--><!--preact-island:36-->loading...<!--/preact-island:36--></div>',
+			'<div hidden>',
+			createInitScript(1),
+			createSubtree('33', '<p>id: P0-1</p>'),
+			createSubtree('36', '<p>id: P0-2</p>'),
+			'</div>'
+		]);
+	});
 });
diff --git a/test/compat/stream-node.test.js b/test/compat/stream-node.test.js
index 5a030f7..5783887 100644
--- a/test/compat/stream-node.test.js
+++ b/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:47-->loading...<!--/preact-island:47--></div>',
 			'<div hidden>',
 			createInitScript(),
-			createSubtree('17', '<p>it works</p>'),
+			createSubtree('47', '<p>it works</p>'),
 			'</div>'
 		]);
 	});
diff --git a/test/compat/stream.test.js b/test/compat/stream.test.js
index 7d50aa7..09f6134 100644
--- a/test/compat/stream.test.js
+++ b/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:54-->loading...<!--/preact-island:54--></div>',
 			'<div hidden>',
 			createInitScript(),
-			createSubtree('24', '<p>it works</p>'),
+			createSubtree('54', '<p>it works</p>'),
 			'</div>'
 		]);
 	});