Repository: preactjs/preact-render-to-string
Pull request #413 (2025-01-16 17:03:04)

Fix rendering waterfall of suspended components in a single Suspense boundary
I ran into an issue that is very similar to https://github.com/preactjs/preact-render-to-string/pull/335, where multiple suspended children in the same Suspense boundary throw a promise that bubbles out of the render functions, resulting in `Error: the promise {} was thrown, throw an Error :)`.

```jsx
const promise = renderToStringAsync(
	<ul>
		<Suspense fallback={null}>
			<SuspenderOne>
				<li>one</li>
			</SuspenderOne>
			<SuspenderTwo>
				<li>two</li>
			</SuspenderTwo>
			<SuspenderThree>
				<li>three</li>
			</SuspenderThree>
		</Suspense>
	</ul>
);

suspendedOne.promise.then(() => { void suspendedTwo.resolve();});
suspendedTwo.promise.then(() => { void suspendedThree.resolve();});

suspendedOne.resolve();
```

This only occurs when there are 3+ direct children in a Suspense boundary, rendered async, where the Promises resolve one by one, which is why the test case added by #335 doesn't catch this.
Staggering the Promise resolves makes a key difference: with the existing test, rendering suspends on the Promise thrown in SuspenderOne, after which all three promises are resolved, and then rendering continues: SuspenderTwo and SuspenderThree never throw a promise.

https://github.com/preactjs/preact-render-to-string/blob/81e7da321da925dbd8eba345ecbd46b1cf456b69/src/index.js#L546-L559
The issue arises here due to the use of `.then(onFulfilled, onRejected)` instead of `.then(onFulfilled).catch(onRejected)`: when the onFulfilled callback throws an error here, it won't be caught.
But after changing that in the code (which makes all the tests pass) I realised that block could also be simplified to just
```js
const renderNestedChildren = () => {
	try {
		const result = _renderToString(
			rendered,
			context,
			isSvgMode,
			selectValue,
			vnode,
			asyncMode,
			renderer
		);
		return vnode._suspended
			? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR
			: result;
	} catch (e) {
		if (!e || typeof e.then != 'function') throw e;
		
		return e.then(renderNestedChildren);
	}
};

return error.then(renderNestedChildren);
```
Without the additional inline closure.

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

Latest commit: b87bea99dcb3e50f726cd78ff2f90d999242b758

**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/async-waterfall?filename=.changeset/real-oranges-cry.md&value=---%0A%22preact-render-to-string%22%3A%20patch%0A---%0A%0AFix%20rendering%20waterfall%20of%20suspended%20components%20in%20a%20single%20Suspense%20boundary%0A)


f0x52: It might be good to update the `should render JSX with multiple suspended direct children within a single suspense boundary` test as well, because right now the second and third children never actually suspend, as their promises are already resolved before they ever get rendered.

Patch:
diff --git a/.changeset/long-ties-brake.md b/.changeset/long-ties-brake.md
new file mode 100644
index 0000000..333694f
--- /dev/null
+++ b/.changeset/long-ties-brake.md
@@ -0,0 +1,5 @@
+---
+'preact-render-to-string': patch
+---
+
+Fix async rendering of multiple suspended components in a single Suspense boundary
diff --git a/src/index.js b/src/index.js
index f6530f4..9c69683 100644
--- a/src/index.js
+++ b/src/index.js
@@ -542,21 +542,8 @@ function _renderToString(
 						: result;
 				} catch (e) {
 					if (!e || typeof e.then != 'function') throw e;
-
-					return e.then(() => {
-						const result = _renderToString(
-							rendered,
-							context,
-							isSvgMode,
-							selectValue,
-							vnode,
-							asyncMode,
-							renderer
-						);
-						return vnode._suspended
-							? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR
-							: result;
-					}, renderNestedChildren);
+					
+					return e.then(renderNestedChildren);
 				}
 			};
 


Test patch:
diff --git a/test/compat/async.test.jsx b/test/compat/async.test.jsx
index 0ca7835..62a347d 100644
--- a/test/compat/async.test.jsx
+++ b/test/compat/async.test.jsx
@@ -254,6 +254,48 @@ describe('Async renderToString', () => {
 		expect(rendered).to.equal(expected);
 	});
 
+	it('should render JSX with multiple suspended direct children within a single suspense boundary that resolve one-after-another', async () => {
+		const {
+			Suspender: SuspenderOne,
+			suspended: suspendedOne
+		} = createSuspender();
+		const {
+			Suspender: SuspenderTwo,
+			suspended: suspendedTwo
+		} = createSuspender();
+		const {
+			Suspender: SuspenderThree,
+			suspended: suspendedThree
+		} = createSuspender();
+
+		const promise = renderToStringAsync(
+			<ul>
+				<Suspense fallback={null}>
+					<SuspenderOne>
+						<li>one</li>
+					</SuspenderOne>
+					<SuspenderTwo>
+						<li>two</li>
+					</SuspenderTwo>
+					<SuspenderThree>
+						<li>three</li>
+					</SuspenderThree>
+				</Suspense>
+			</ul>
+		);
+
+		const expected = `<ul><!--$s--><li>one</li><!--/$s--><!--$s--><li>two</li><!--/$s--><!--$s--><li>three</li><!--/$s--></ul>`;
+
+		suspendedOne.promise.then(() => { void suspendedTwo.resolve();});
+		suspendedTwo.promise.then(() => { void suspendedThree.resolve();});
+
+		suspendedOne.resolve();
+
+		const rendered = await promise;
+
+		expect(rendered).to.equal(expected);
+	});
+
 	it('should rethrow error thrown after suspending', async () => {
 		const { suspended, getResolved } = createSuspender();