Fix Astro components parent-child render order (#8187)

.changeset/mean-snakes-play.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix Astro components parent-child render order

packages/astro/src/runtime/server/render/any.ts

@@ -2,6 +2,7 @@ import { escapeHTML, isHTMLString, markHTMLString } from '../escape.js';
 import { isAstroComponentInstance, isRenderTemplateResult } from './astro/index.js';
 import { isRenderInstance, type RenderDestination } from './common.js';
 import { SlotString } from './slot.js';
+import { renderToBufferDestination } from './util.js';
 
 export async function renderChild(destination: RenderDestination, child: any) {
 	child = await child;
@@ -10,8 +11,14 @@ export async function renderChild(destination: RenderDestination, child: any) {
 	} else if (isHTMLString(child)) {
 		destination.write(child);
 	} else if (Array.isArray(child)) {
-		for (const c of child) {
-			await renderChild(destination, c);
+		// Render all children eagerly and in parallel
+		const childRenders = child.map((c) => {
+			return renderToBufferDestination((bufferDestination) => {
+				return renderChild(bufferDestination, c);
+			});
+		});
+		for (const childRender of childRenders) {
+			await childRender.renderToFinalDestination(destination);
 		}
 	} else if (typeof child === 'function') {
 		// Special: If a child is a function, call it automatically.

packages/astro/src/runtime/server/render/astro/render-template.ts

@@ -2,6 +2,7 @@ import { markHTMLString } from '../../escape.js';
 import { isPromise } from '../../util.js';
 import { renderChild } from '../any.js';
 import type { RenderDestination } from '../common.js';
+import { renderToBufferDestination } from '../util.js';
 
 const renderTemplateResultSym = Symbol.for('astro.renderTemplateResult');
 
@@ -32,14 +33,23 @@ export class RenderTemplateResult {
 	}
 
 	async render(destination: RenderDestination) {
+		// Render all expressions eagerly and in parallel
+		const expRenders = this.expressions.map((exp) => {
+			return renderToBufferDestination((bufferDestination) => {
+				// Skip render if falsy, except the number 0
+				if (exp || exp === 0) {
+					return renderChild(bufferDestination, exp);
+				}
+			});
+		});
+
 		for (let i = 0; i < this.htmlParts.length; i++) {
 			const html = this.htmlParts[i];
-			const exp = this.expressions[i];
+			const expRender = expRenders[i];
 
 			destination.write(markHTMLString(html));
-			// Skip render if falsy, except the number 0
-			if (exp || exp === 0) {
-				await renderChild(destination, exp);
+			if (expRender) {
+				await expRender.renderToFinalDestination(destination);
 			}
 		}
 	}

packages/astro/src/runtime/server/render/common.ts

@@ -37,9 +37,11 @@ export interface RenderDestination {
 }
 
 export interface RenderInstance {
-	render(destination: RenderDestination): Promise<void> | void;
+	render: RenderFunction;
 }
 
+export type RenderFunction = (destination: RenderDestination) => Promise<void> | void;
+
 export const Fragment = Symbol.for('astro:fragment');
 export const Renderer = Symbol.for('astro:renderer');
 

packages/astro/src/runtime/server/render/component.ts

@@ -23,7 +23,6 @@ import {
 	Renderer,
 	chunkToString,
 	type RenderDestination,
-	type RenderDestinationChunk,
 	type RenderInstance,
 } from './common.js';
 import { componentIsHTMLElement, renderHTMLElement } from './dom.js';
@@ -421,27 +420,12 @@ function renderAstroComponent(
 	slots: any = {}
 ): RenderInstance {
 	const instance = createAstroComponentInstance(result, displayName, Component, props, slots);
-
-	// Eagerly render the component so they are rendered in parallel.
-	// Render to buffer for now until our returned render function is called.
-	const bufferChunks: RenderDestinationChunk[] = [];
-	const bufferDestination: RenderDestination = {
-		write: (chunk) => bufferChunks.push(chunk),
-	};
-	// Don't await for the render to finish to not block streaming
-	const renderPromise = instance.render(bufferDestination);
-
 	return {
 		async render(destination) {
-			// Write the buffered chunks to the real destination
-			for (const chunk of bufferChunks) {
-				destination.write(chunk);
-			}
-			// Free memory
-			bufferChunks.length = 0;
-			// Re-assign the real destination so `instance.render` will continue and write to the new destination
-			bufferDestination.write = (chunk) => destination.write(chunk);
-			await renderPromise;
+			// NOTE: This render call can't be pre-invoked outside of this function as it'll also initialize the slots
+			// recursively, which causes each Astro components in the tree to be called bottom-up, and is incorrect.
+			// The slots are initialized eagerly for head propagation.
+			await instance.render(destination);
 		},
 	};
 }

packages/astro/src/runtime/server/render/util.ts

@@ -1,4 +1,5 @@
 import type { SSRElement } from '../../../@types/astro';
+import type { RenderDestination, RenderDestinationChunk, RenderFunction } from './common.js';
 
 import { HTMLString, markHTMLString } from '../escape.js';
 import { serializeListValue } from '../util.js';
@@ -145,3 +146,56 @@ export function renderElement(
 	}
 	return `<${name}${internalSpreadAttributes(props, shouldEscape)}>${children}</${name}>`;
 }
+
+/**
+ * Executes the `bufferRenderFunction` to prerender it into a buffer destination, and return a promise
+ * with an object containing the `renderToFinalDestination` function to flush the buffer to the final
+ * destination.
+ *
+ * @example
+ * ```ts
+ * // Render components in parallel ahead of time
+ * const finalRenders = [ComponentA, ComponentB].map((comp) => {
+ *   return renderToBufferDestination(async (bufferDestination) => {
+ *     await renderComponentToDestination(bufferDestination);
+ *   });
+ * });
+ * // Render array of components serially
+ * for (const finalRender of finalRenders) {
+ *   await finalRender.renderToFinalDestination(finalDestination);
+ * }
+ * ```
+ */
+export function renderToBufferDestination(bufferRenderFunction: RenderFunction): {
+	renderToFinalDestination: RenderFunction;
+} {
+	// Keep chunks in memory
+	const bufferChunks: RenderDestinationChunk[] = [];
+	const bufferDestination: RenderDestination = {
+		write: (chunk) => bufferChunks.push(chunk),
+	};
+
+	// Don't await for the render to finish to not block streaming
+	const renderPromise = bufferRenderFunction(bufferDestination);
+
+	// Return a closure that writes the buffered chunk
+	return {
+		async renderToFinalDestination(destination) {
+			// Write the buffered chunks to the real destination
+			for (const chunk of bufferChunks) {
+				destination.write(chunk);
+			}
+
+			// NOTE: We don't empty `bufferChunks` after it's written as benchmarks show
+			// that it causes poorer performance, likely due to forced memory re-allocation,
+			// instead of letting the garbage collector handle it automatically.
+			// (Unsure how this affects on limited memory machines)
+
+			// Re-assign the real destination so `instance.render` will continue and write to the new destination
+			bufferDestination.write = (chunk) => destination.write(chunk);
+
+			// Wait for render to finish entirely
+			await renderPromise;
+		},
+	};
+}

packages/astro/test/astro-basic.test.js

@@ -102,6 +102,12 @@ describe('Astro basics', () => {
 			// will have already erred by now, but add test anyway
 			expect(await fixture.readFile('/special-“characters” -in-file/index.html')).to.be.ok;
 		});
+
+		it('renders the components top-down', async () => {
+			const html = await fixture.readFile('/order/index.html');
+			const $ = cheerio.load(html);
+			expect($('#rendered-order').text()).to.eq('Rendered order: A, B')
+		})
 	});
 
 	it('Supports void elements whose name is a string (#2062)', async () => {

packages/astro/test/fixtures/astro-basic/src/components/OrderA.astro

@@ -0,0 +1,7 @@
+---
+globalThis.__ASTRO_TEST_ORDER__ ??= []
+globalThis.__ASTRO_TEST_ORDER__.push('A')
+---
+
+<p>A</p>
+<slot />

packages/astro/test/fixtures/astro-basic/src/components/OrderB.astro

@@ -0,0 +1,7 @@
+---
+globalThis.__ASTRO_TEST_ORDER__ ??= []
+globalThis.__ASTRO_TEST_ORDER__.push('B')
+---
+
+<p>B</p>
+<slot />

packages/astro/test/fixtures/astro-basic/src/components/OrderLast.astro

@@ -0,0 +1 @@
+<p id="rendered-order">Rendered order: {() => (globalThis.__ASTRO_TEST_ORDER__ ?? []).join(', ')}</p>

packages/astro/test/fixtures/astro-basic/src/pages/order.astro

@@ -0,0 +1,13 @@
+---
+import OrderA from "../components/OrderA.astro";
+import OrderB from "../components/OrderB.astro";
+import OrderLast from "../components/OrderLast.astro";
+
+globalThis.__ASTRO_TEST_ORDER__ = [];
+---
+
+<OrderA>
+  <OrderB>
+    <OrderLast />
+  </OrderB>
+</OrderA>