diff options
11 files changed, 123 insertions, 27 deletions
diff --git a/.changeset/mean-snakes-play.md b/.changeset/mean-snakes-play.md new file mode 100644 index 000000000..ae68ba958 --- /dev/null +++ b/.changeset/mean-snakes-play.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix Astro components parent-child render order diff --git a/packages/astro/src/runtime/server/render/any.ts b/packages/astro/src/runtime/server/render/any.ts index 7c181fecb..89c927576 100644 --- a/packages/astro/src/runtime/server/render/any.ts +++ b/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. 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 1d5af33fc..f12344fe8 100644 --- a/packages/astro/src/runtime/server/render/astro/render-template.ts +++ b/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); } } } diff --git a/packages/astro/src/runtime/server/render/common.ts b/packages/astro/src/runtime/server/render/common.ts index d4c7a1242..7beaed2eb 100644 --- a/packages/astro/src/runtime/server/render/common.ts +++ b/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'); diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 5f61e68b5..1d58706a6 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/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); }, }; } diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index e007fe6f1..156e21167 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/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; + }, + }; +} diff --git a/packages/astro/test/astro-basic.test.js b/packages/astro/test/astro-basic.test.js index 92b6af410..1a92b3f18 100644 --- a/packages/astro/test/astro-basic.test.js +++ b/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 () => { diff --git a/packages/astro/test/fixtures/astro-basic/src/components/OrderA.astro b/packages/astro/test/fixtures/astro-basic/src/components/OrderA.astro new file mode 100644 index 000000000..7caa0d868 --- /dev/null +++ b/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 /> diff --git a/packages/astro/test/fixtures/astro-basic/src/components/OrderB.astro b/packages/astro/test/fixtures/astro-basic/src/components/OrderB.astro new file mode 100644 index 000000000..b2984be5c --- /dev/null +++ b/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 /> diff --git a/packages/astro/test/fixtures/astro-basic/src/components/OrderLast.astro b/packages/astro/test/fixtures/astro-basic/src/components/OrderLast.astro new file mode 100644 index 000000000..35cd932dc --- /dev/null +++ b/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> diff --git a/packages/astro/test/fixtures/astro-basic/src/pages/order.astro b/packages/astro/test/fixtures/astro-basic/src/pages/order.astro new file mode 100644 index 000000000..ca5b2fdd5 --- /dev/null +++ b/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> |