From 627e47d67af4846cea2acf26a96b4124001b26fc Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Thu, 21 Mar 2024 23:25:09 +0800 Subject: [PATCH] Improve Astro JSX rendering (#10473) --- .changeset/twelve-rules-collect.md | 5 + packages/astro/src/runtime/server/jsx.ts | 131 +++--------------- .../src/runtime/server/render/component.ts | 6 +- 3 files changed, 25 insertions(+), 117 deletions(-) create mode 100644 .changeset/twelve-rules-collect.md diff --git a/.changeset/twelve-rules-collect.md b/.changeset/twelve-rules-collect.md new file mode 100644 index 000000000000..e4e2ad954c15 --- /dev/null +++ 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 index 8304597ee992..b2aca1783264 100644 --- 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 { // eslint-disable-next-line @typescript-eslint/switch-exhaustiveness-check @@ -56,22 +41,10 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise { ); } - // 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 { +async function renderJSXVNode(result: SSRResult, vnode: AstroVNode): Promise { 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 index 280290b13164..8e32dd6cd895 100644 --- 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 && !/' : '\n'; str += doctype + head; }