Repository: preactjs/preact-render-to-string
Pull request #376 (2024-07-13 08:15:08)

Denote suspenseful components with comment markers
Relates to https://github.com/preactjs/preact/issues/4442

This adds the comments approach described in the RFC, these comments will be used with resumed hydration to correctly allocate the DOM-children required to rehydrate a Suspense boundary. Currently this purposefully excludes streaming renders.

This needs a lot more tests and will probably lead to a major version of RTS and Preact (to be discussed).

The algorithm we'd use in Preact for this would entail us searching for the comment boundary, so in the most ideal scenario this would look like

```js
const excessDomChildren = [null, COMMENT_BEGIN,...theDomNodes, COMMENT_END];
```

but there will be time where it looks like 

```js
const excessDomChildren = [null, COMMENT_BEGIN, 'div', COMMENT_BEGIN, COMMENT_END, COMMENT_END];
```

due to adjacent suspending children, we'd have to look for 2 end nodes then so the search algorithm would have to count the amount of contained suspensions as well.


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 19ee72884653d0d06e48de7c344869f8679a3a65

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/preactjs/preact-render-to-string/new/comment-markers?filename=.changeset/slimy-squids-burn.md&value=---%0A%22preact-render-to-string%22%3A%20patch%0A---%0A%0ADenote%20suspenseful%20components%20with%20comment%20markers%0A)



Patch:
diff --git a/.changeset/happy-peas-type.md b/.changeset/happy-peas-type.md
new file mode 100644
index 0000000..fa92970
--- /dev/null
+++ b/.changeset/happy-peas-type.md
@@ -0,0 +1,5 @@
+---
+'preact-render-to-string': minor
+---
+
+Insert comment markers for suspended trees, only in renderToStringAsync
diff --git a/src/index.js b/src/index.js
index ff4c774..f6530f4 100644
--- a/src/index.js
+++ b/src/index.js
@@ -29,6 +29,8 @@ const EMPTY_ARR = [];
 const isArray = Array.isArray;
 const assign = Object.assign;
 const EMPTY_STR = '';
+const BEGIN_SUSPENSE_DENOMINATOR = '<!--$s-->';
+const END_SUSPENSE_DENOMINATOR = '<!--/$s-->';
 
 // Global state for the current render pass
 let beforeDiff, afterDiff, renderHook, ummountHook;
@@ -372,7 +374,14 @@ function _renderToString(
 
 					if (renderHook) renderHook(vnode);
 
-					rendered = type.call(component, props, cctx);
+					try {
+						rendered = type.call(component, props, cctx);
+					} catch (e) {
+						if (asyncMode) {
+							vnode._suspended = true;
+						}
+						throw e;
+					}
 				}
 				component[DIRTY] = true;
 			}
@@ -403,6 +412,7 @@ function _renderToString(
 						selectValue,
 						vnode,
 						asyncMode,
+						false,
 						renderer
 					);
 				} catch (err) {
@@ -475,6 +485,21 @@ function _renderToString(
 
 			if (options.unmount) options.unmount(vnode);
 
+			if (vnode._suspended) {
+				if (typeof str === 'string') {
+					return BEGIN_SUSPENSE_DENOMINATOR + str + END_SUSPENSE_DENOMINATOR;
+				} else if (isArray(str)) {
+					str.unshift(BEGIN_SUSPENSE_DENOMINATOR);
+					str.push(END_SUSPENSE_DENOMINATOR);
+					return str;
+				}
+
+				return str.then(
+					(resolved) =>
+						BEGIN_SUSPENSE_DENOMINATOR + resolved + END_SUSPENSE_DENOMINATOR
+				);
+			}
+
 			return str;
 		} catch (error) {
 			if (!asyncMode && renderer && renderer.onError) {
@@ -503,7 +528,7 @@ function _renderToString(
 
 			const renderNestedChildren = () => {
 				try {
-					return _renderToString(
+					const result = _renderToString(
 						rendered,
 						context,
 						isSvgMode,
@@ -512,22 +537,26 @@ function _renderToString(
 						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(
-						() =>
-							_renderToString(
-								rendered,
-								context,
-								isSvgMode,
-								selectValue,
-								vnode,
-								asyncMode,
-								renderer
-							),
-						renderNestedChildren
-					);
+					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);
 				}
 			};
 


Test patch:
diff --git a/test/compat/async.test.jsx b/test/compat/async.test.jsx
index 12e9ac5..0ca7835 100644
--- a/test/compat/async.test.jsx
+++ b/test/compat/async.test.jsx
@@ -3,6 +3,7 @@ import { h, Fragment } from 'preact';
 import { Suspense, useId, lazy, createContext } from 'preact/compat';
 import { expect } from 'chai';
 import { createSuspender } from '../utils.jsx';
+const wait = (ms) => new Promise((r) => setTimeout(r, ms));
 
 describe('Async renderToString', () => {
 	it('should render JSX after a suspense boundary', async () => {
@@ -16,7 +17,30 @@ describe('Async renderToString', () => {
 			</Suspense>
 		);
 
-		const expected = `<div class="foo">bar</div>`;
+		const expected = `<!--$s--><div class="foo">bar</div><!--/$s-->`;
+
+		suspended.resolve();
+
+		const rendered = await promise;
+
+		expect(rendered).to.equal(expected);
+	});
+
+	it('should correctly denote null returns of suspending components', async () => {
+		const { Suspender, suspended } = createSuspender();
+
+		const Analytics = () => null;
+
+		const promise = renderToStringAsync(
+			<Suspense fallback={<div>loading...</div>}>
+				<Suspender>
+					<Analytics />
+				</Suspender>
+				<div class="foo">bar</div>
+			</Suspense>
+		);
+
+		const expected = `<!--$s--><!--/$s--><div class="foo">bar</div>`;
 
 		suspended.resolve();
 
@@ -49,7 +73,7 @@ describe('Async renderToString', () => {
 			</ul>
 		);
 
-		const expected = `<ul><li>one</li><li>two</li><li>three</li></ul>`;
+		const expected = `<ul><!--$s--><li>one</li><!--$s--><li>two</li><!--/$s--><li>three</li><!--/$s--></ul>`;
 
 		suspendedOne.resolve();
 		suspendedTwo.resolve();
@@ -85,10 +109,102 @@ describe('Async renderToString', () => {
 			</ul>
 		);
 
-		const expected = `<ul><li>one</li><li>two</li><li>three</li></ul>`;
+		const expected = `<ul><!--$s--><li>one</li><!--$s--><li>two</li><!--/$s--><li>three</li><!--/$s--></ul>`;
+
+		suspendedOne.resolve();
+		suspendedTwo.resolve();
+
+		const rendered = await promise;
+
+		expect(rendered).to.equal(expected);
+	});
+
+	it('should render JSX with nested suspense boundaries containing multiple suspending components', async () => {
+		const {
+			Suspender: SuspenderOne,
+			suspended: suspendedOne
+		} = createSuspender();
+		const {
+			Suspender: SuspenderTwo,
+			suspended: suspendedTwo
+		} = createSuspender();
+		const {
+			Suspender: SuspenderThree,
+			suspended: suspendedThree
+		} = createSuspender('three');
+
+		const promise = renderToStringAsync(
+			<ul>
+				<Suspense fallback={null}>
+					<SuspenderOne>
+						<li>one</li>
+						<Suspense fallback={null}>
+							<SuspenderTwo>
+								<li>two</li>
+							</SuspenderTwo>
+							<SuspenderThree>
+								<li>three</li>
+							</SuspenderThree>
+						</Suspense>
+						<li>four</li>
+					</SuspenderOne>
+				</Suspense>
+			</ul>
+		);
+
+		const expected = `<ul><!--$s--><li>one</li><!--$s--><li>two</li><!--/$s--><!--$s--><li>three</li><!--/$s--><li>four</li><!--/$s--></ul>`;
 
 		suspendedOne.resolve();
 		suspendedTwo.resolve();
+		await wait(0);
+		suspendedThree.resolve();
+
+		const rendered = await promise;
+
+		expect(rendered).to.equal(expected);
+	});
+
+	it('should render JSX with deeply nested suspense boundaries', 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>
+						<Suspense fallback={null}>
+							<SuspenderTwo>
+								<li>two</li>
+								<Suspense fallback={null}>
+									<SuspenderThree>
+										<li>three</li>
+									</SuspenderThree>
+								</Suspense>
+							</SuspenderTwo>
+						</Suspense>
+						<li>four</li>
+					</SuspenderOne>
+				</Suspense>
+			</ul>
+		);
+
+		const expected = `<ul><!--$s--><li>one</li><!--$s--><li>two</li><!--$s--><li>three</li><!--/$s--><!--/$s--><li>four</li><!--/$s--></ul>`;
+
+		suspendedOne.resolve();
+		suspendedTwo.resolve();
+		await wait(0);
+		suspendedThree.resolve();
 
 		const rendered = await promise;
 
@@ -127,7 +243,7 @@ describe('Async renderToString', () => {
 			</ul>
 		);
 
-		const expected = `<ul><li>one</li><li>two</li><li>three</li></ul>`;
+		const expected = `<ul><!--$s--><li>one</li><!--/$s--><!--$s--><li>two</li><!--/$s--><!--$s--><li>three</li><!--/$s--></ul>`;
 
 		suspendedOne.resolve();
 		suspendedTwo.resolve();
@@ -187,7 +303,7 @@ describe('Async renderToString', () => {
 
 		suspended.resolve();
 		const rendered = await promise;
-		expect(rendered).to.equal('<p>ok</p>');
+		expect(rendered).to.equal('<!--$s--><p>ok</p><!--/$s-->');
 	});
 
 	it('should work with an in-render suspension', async () => {
@@ -224,7 +340,10 @@ describe('Async renderToString', () => {
 			</Context.Provider>
 		);
 
-		expect(rendered).to.equal(`<div>2</div>`);
+		// Before we get to the actual DOM this suspends twice
+		expect(rendered).to.equal(
+			`<!--$s--><!--$s--><div>2</div><!--/$s--><!--/$s-->`
+		);
 	});
 
 	describe('dangerouslySetInnerHTML', () => {