Merge pull request #457 from lemonmade/fix/async-suspense-array-rendering

fix: renderToStringAsync produces commas for suspended components with complex children

Author: JoviDeCroock

.changeset/witty-goats-repair.md

@@ -0,0 +1,5 @@
+---
+"preact-render-to-string": patch
+---
+
+fix: renderToStringAsync produces commas for suspended components with complex children

src/index.js

@@ -34,6 +34,25 @@ const EMPTY_STR = '';
 const BEGIN_SUSPENSE_DENOMINATOR = '<!--$s-->';
 const END_SUSPENSE_DENOMINATOR = '<!--/$s-->';
 
+/**
+ * Wraps a render result with suspense boundary markers, handling all possible
+ * return types from _renderToString: string, Array, or Promise.
+ * @param {string | Array | Promise} result
+ * @returns {string | Array | Promise}
+ */
+function wrapWithSuspenseMarkers(result) {
+	if (typeof result === 'string') {
+		return BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR;
+	} else if (isArray(result)) {
+		result.unshift(BEGIN_SUSPENSE_DENOMINATOR);
+		result.push(END_SUSPENSE_DENOMINATOR);
+		return result;
+	} else if (result && typeof result.then === 'function') {
+		return result.then(wrapWithSuspenseMarkers);
+	}
+	return BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR;
+}
+
 // Global state for the current render pass
 let beforeDiff, afterDiff, renderHook, ummountHook;
 
@@ -498,18 +517,7 @@ 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 wrapWithSuspenseMarkers(str);
 			}
 
 			return str;
@@ -556,9 +564,7 @@ function _renderToString(
 						asyncMode,
 						renderer
 					);
-					return vnode._suspended
-						? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR
-						: result;
+					return vnode._suspended ? wrapWithSuspenseMarkers(result) : result;
 				} catch (e) {
 					if (!e || typeof e.then != 'function') throw e;
 

test/compat/async.test.jsx

@@ -360,6 +360,82 @@ describe('Async renderToString', () => {
 		);
 	});
 
+	it('should not produce commas or [object Promise] when cascading suspensions produce array results', async () => {
+		// Regression: when component A suspends then re-renders with children
+		// that also suspend at multiple levels (B→C→D), the .then() handler
+		// wrapping suspended output receives an Array instead of a string.
+		// String concatenation with an Array calls Array.toString(), injecting
+		// commas into the HTML.
+		let aResolved = false;
+		const aPromise = new Promise((r) =>
+			setTimeout(() => {
+				aResolved = true;
+				r();
+			}, 5)
+		);
+
+		let cResolved = false;
+		const cPromise = new Promise((r) =>
+			setTimeout(() => {
+				cResolved = true;
+				r();
+			}, 15)
+		);
+
+		let dResolved = false;
+		const dPromise = new Promise((r) =>
+			setTimeout(() => {
+				dResolved = true;
+				r();
+			}, 25)
+		);
+
+		function A() {
+			if (!aResolved) throw aPromise;
+			return <B />;
+		}
+
+		function B() {
+			return (
+				<div>
+					<p>b-content</p>
+					<C />
+					<p>b-footer</p>
+				</div>
+			);
+		}
+
+		function C() {
+			if (!cResolved) throw cPromise;
+			return (
+				<Fragment>
+					<span>c-before</span>
+					<D />
+					<span>c-after</span>
+				</Fragment>
+			);
+		}
+
+		function D() {
+			if (!dResolved) throw dPromise;
+			return <em>d-content</em>;
+		}
+
+		const rendered = await renderToStringAsync(
+			<Suspense fallback={null}>
+				<A />
+			</Suspense>
+		);
+
+		expect(rendered).not.to.contain(',');
+		expect(rendered).not.to.contain('[object Promise]');
+		expect(rendered).to.contain('<p>b-content</p>');
+		expect(rendered).to.contain('<span>c-before</span>');
+		expect(rendered).to.contain('<em>d-content</em>');
+		expect(rendered).to.contain('<span>c-after</span>');
+		expect(rendered).to.contain('<p>b-footer</p>');
+	});
+
 	describe('dangerouslySetInnerHTML', () => {
 		it('should support dangerouslySetInnerHTML', async () => {
 			// some invalid HTML to make sure we're being flakey: