Improve Astro JSX rendering (#10473)

withastro · Mar 21, 2024 · 627e47d · 627e47d
1 parent cf8f2ca
commit 627e47d
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 117 deletions.
diff --git a/.changeset/twelve-rules-collect.md b/.changeset/twelve-rules-collect.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes and improves performance when rendering Astro JSX
diff --git a/packages/astro/src/runtime/server/jsx.ts b/packages/astro/src/runtime/server/jsx.ts
@@ -1,4 +1,3 @@
-/* eslint-disable no-console */
 import type { SSRResult } from '../../@types/astro.js';
 import { AstroJSX, type AstroVNode, isVNode } from '../../jsx-runtime/index.js';
 import {
@@ -13,28 +12,14 @@ import { renderComponentToString } from './render/component.js';
 
 const ClientOnlyPlaceholder = 'astro-client-only';
 
-class Skip {
-	count: number;
-	constructor(public vnode: AstroVNode) {
-		this.count = 0;
-	}
-
-	increment() {
-		this.count++;
-	}
-
-	haveNoTried() {
-		return this.count === 0;
-	}
-
-	isCompleted() {
-		return this.count > 2;
-	}
-	static symbol = Symbol('astro:jsx:skip');
-}
-
-let originalConsoleError: any;
-let consoleFilterRefs = 0;
+// If the `vnode.type` is a function, we could render it as JSX or as framework components.
+// Inside `renderJSXNode`, we first try to render as framework components, and if `renderJSXNode`
+// is called again while rendering the component, it's likely that the `astro:jsx` is invoking
+// `renderJSXNode` again (loop). In this case, we try to render as JSX instead.
+//
+// This Symbol is assigned to `vnode.props` to track if it had tried to render as framework components.
+// It mutates `vnode.props` to be able to scope to the current render call.
+const hasTriedRenderComponentSymbol = Symbol('hasTriedRenderComponent');
 
 export async function renderJSX(result: SSRResult, vnode: any): Promise<any> {
 	// eslint-disable-next-line @typescript-eslint/switch-exhaustiveness-check
@@ -56,22 +41,10 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise<any> {
 			);
 	}
 
-	// Extract the skip from the props, if we've already attempted a previous render
-	let skip: Skip;
-	if (vnode.props) {
-		if (vnode.props[Skip.symbol]) {
-			skip = vnode.props[Skip.symbol];
-		} else {
-			skip = new Skip(vnode);
-		}
-	} else {
-		skip = new Skip(vnode);
-	}
-
-	return renderJSXVNode(result, vnode, skip);
+	return renderJSXVNode(result, vnode);
 }
 
-async function renderJSXVNode(result: SSRResult, vnode: AstroVNode, skip: Skip): Promise<any> {
+async function renderJSXVNode(result: SSRResult, vnode: AstroVNode): Promise<any> {
 	if (isVNode(vnode)) {
 		// eslint-disable-next-line @typescript-eslint/switch-exhaustiveness-check
 		switch (true) {
@@ -105,36 +78,20 @@ Did you forget to import the component or is it possible there is a typo?`);
 		}
 
 		if (vnode.type) {
-			if (typeof vnode.type === 'function' && (vnode.type as any)['astro:renderer']) {
-				skip.increment();
-			}
 			if (typeof vnode.type === 'function' && vnode.props['server:root']) {
 				const output = await vnode.type(vnode.props ?? {});
 				return await renderJSX(result, output);
 			}
 			if (typeof vnode.type === 'function') {
-				if (skip.haveNoTried() || skip.isCompleted()) {
-					useConsoleFilter();
-					try {
-						const output = await vnode.type(vnode.props ?? {});
-						let renderResult: any;
-						if (output?.[AstroJSX]) {
-							renderResult = await renderJSXVNode(result, output, skip);
-							return renderResult;
-						} else if (!output) {
-							renderResult = await renderJSXVNode(result, output, skip);
-							return renderResult;
-						}
-					} catch (e: unknown) {
-						if (skip.isCompleted()) {
-							throw e;
-						}
-						skip.increment();
-					} finally {
-						finishUsingConsoleFilter();
+				if (vnode.props[hasTriedRenderComponentSymbol]) {
+					const output = await vnode.type(vnode.props ?? {});
+					if (output?.[AstroJSX] || !output) {
+						return await renderJSXVNode(result, output);
+					} else {
+						return;
 					}
 				} else {
-					skip.increment();
+					vnode.props[hasTriedRenderComponentSymbol] = true;
 				}
 			}
 
@@ -176,7 +133,6 @@ Did you forget to import the component or is it possible there is a typo?`);
 			}
 			await Promise.all(slotPromises);
 
-			props[Skip.symbol] = skip;
 			let output: string;
 			if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) {
 				output = await renderComponentToString(
@@ -231,56 +187,3 @@ function prerenderElementChildren(tag: string, children: any) {
 		return children;
 	}
 }
-
-/**
- * Reduces console noise by filtering known non-problematic errors.
- *
- * Performs reference counting to allow parallel usage from async code.
- *
- * To stop filtering, please ensure that there always is a matching call
- * to `finishUsingConsoleFilter` afterwards.
- */
-function useConsoleFilter() {
-	consoleFilterRefs++;
-
-	if (!originalConsoleError) {
-		originalConsoleError = console.error;
-
-		try {
-			console.error = filteredConsoleError;
-		} catch (error) {
-			// If we're unable to hook `console.error`, just accept it
-		}
-	}
-}
-
-/**
- * Indicates that the filter installed by `useConsoleFilter`
- * is no longer needed by the calling code.
- */
-function finishUsingConsoleFilter() {
-	consoleFilterRefs--;
-
-	// Note: Instead of reverting `console.error` back to the original
-	// when the reference counter reaches 0, we leave our hook installed
-	// to prevent potential race conditions once `check` is made async
-}
-
-/**
- * Hook/wrapper function for the global `console.error` function.
- *
- * Ignores known non-problematic errors while any code is using the console filter.
- * Otherwise, simply forwards all arguments to the original function.
- */
-function filteredConsoleError(msg: any, ...rest: any[]) {
-	if (consoleFilterRefs > 0 && typeof msg === 'string') {
-		// In `check`, we attempt to render JSX components through Preact.
-		// When attempting this on a React component, React may output
-		// the following error, which we can safely filter out:
-		const isKnownReactHookError =
-			msg.includes('Warning: Invalid hook call.') &&
-			msg.includes('https://reactjs.org/link/invalid-hook-call');
-		if (isKnownReactHookError) return;
-	}
-	originalConsoleError(msg, ...rest);
-}
diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
@@ -515,7 +515,7 @@ export async function renderComponentToString(
 	// Handle head injection if required. Note that this needs to run early so
 	// we can ensure getting a value for `head`.
 	let head = '';
-	if (nonAstroPageNeedsHeadInjection(Component)) {
+	if (isPage && !result.partial && nonAstroPageNeedsHeadInjection(Component)) {
 		for (const headChunk of maybeRenderHead()) {
 			head += chunkToString(result, headChunk);
 		}
@@ -525,9 +525,9 @@ export async function renderComponentToString(
 		const destination: RenderDestination = {
 			write(chunk) {
 				// Automatic doctype and head insertion for pages
-				if (isPage && !renderedFirstPageChunk) {
+				if (isPage && !result.partial && !renderedFirstPageChunk) {
 					renderedFirstPageChunk = true;
-					if (!result.partial && !/<!doctype html/i.test(String(chunk))) {
+					if (!/<!doctype html/i.test(String(chunk))) {
 						const doctype = result.compressHTML ? '<!DOCTYPE html>' : '<!DOCTYPE html>\n';
 						str += doctype + head;
 					}