From fe82744c2bbc6ddeb03f4906e525fd58bf41d537 Mon Sep 17 00:00:00 2001 From: Matthew Lymer Date: Mon, 10 Feb 2025 15:53:45 -0500 Subject: [PATCH] Address code review comments --- .../server/render/astro/render-template.ts | 2 +- .../src/runtime/server/render/component.ts | 26 +++---------------- .../astro/src/runtime/server/render/util.ts | 25 ++++++++++++++---- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/packages/astro/src/runtime/server/render/astro/render-template.ts b/packages/astro/src/runtime/server/render/astro/render-template.ts index e9ba2607a6c1..330368a6ab1b 100644 --- a/packages/astro/src/runtime/server/render/astro/render-template.ts +++ b/packages/astro/src/runtime/server/render/astro/render-template.ts @@ -1,7 +1,7 @@ import { markHTMLString } from '../../escape.js'; import { isPromise } from '../../util.js'; import { renderChild } from '../any.js'; -import type { RenderDestination, } from '../common.js'; +import type { RenderDestination } from '../common.js'; import { createBufferedRenderer } from '../util.js'; const renderTemplateResultSym = Symbol.for('astro.renderTemplateResult'); diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 2c6d4b840b24..f00234a7b756 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -448,7 +448,7 @@ function renderAstroComponent( const instance = createAstroComponentInstance(result, displayName, Component, props, slots); return { - render(destination) { + render(destination: RenderDestination): Promise | void { // 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. @@ -465,29 +465,11 @@ export function renderComponent( slots: ComponentSlots = {}, ): RenderInstance | Promise { if (isPromise(Component)) { - return Component.catch(handleCancellation).then((x) => - renderComponentImplFast(result, displayName, x, props, slots), - ); - } - - return renderComponentImplFast(result, displayName, Component, props, slots); - - function handleCancellation(e: unknown) { - if (result.cancelled) - return { - render() {}, - }; - throw e; + return Component.catch(handleCancellation).then((x) => { + return renderComponent(result, displayName, x, props, slots); + }); } -} -function renderComponentImplFast( - result: SSRResult, - displayName: string, - Component: unknown, - props: Record, - slots: ComponentSlots = {}, -): RenderInstance | Promise { if (isFragmentComponent(Component)) { return renderFragmentComponent(result, slots).catch(handleCancellation); } diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index f4bc6a1c9b89..1d1b1e17bfa2 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -153,13 +153,18 @@ export function renderElement( const noop = () => {}; /** - * Renders into a buffer until `renderToFinalDestination` is called (which + * Renders into a buffer until `flush` is called (which * flushes the buffer) */ class BufferedRenderer implements RenderDestination, RendererFlusher { private chunks: RenderDestinationChunk[] = []; private renderPromise: Promise | void; private destination: RenderDestination; + + /** + * Determines whether buffer has been flushed + * to the final destination. + */ private flushed = false; public constructor(destination: RenderDestination, renderFunction: RenderFunction) { @@ -174,6 +179,11 @@ class BufferedRenderer implements RenderDestination, RendererFlusher { } public write(chunk: RenderDestinationChunk): void { + // Before the buffer has been flushed, we want to + // append to the buffer, afterwards we'll write + // to the underlying destination if subsequent + // writes arrive. + if (this.flushed) { this.destination.write(chunk); } else { @@ -183,7 +193,7 @@ class BufferedRenderer implements RenderDestination, RendererFlusher { public flush(): void | Promise { if (this.flushed) { - throw new Error('Already been flushed.'); + throw new Error('The render buffer has already been flushed.'); } this.flushed = true; @@ -204,20 +214,20 @@ class BufferedRenderer implements RenderDestination, RendererFlusher { /** * 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 + * with an object containing the `flush` 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) => { + * return createBufferedRenderer(finalDestination, async (bufferDestination) => { * await renderComponentToDestination(bufferDestination); * }); * }); * // Render array of components serially * for (const finalRender of finalRenders) { - * await finalRender.renderToFinalDestination(finalDestination); + * await finalRender.flush(); * } * ``` */ @@ -229,6 +239,11 @@ export function createBufferedRenderer( } export interface RendererFlusher { + /** + * Flushes the current renderer to the underlying renderer. + * + * See example of `createBufferedRenderer` for usage. + */ flush(): void | Promise; }