Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Astro components parent-child render order #8187

Merged
merged 4 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mean-snakes-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix Astro components parent-child render order
11 changes: 9 additions & 2 deletions packages/astro/src/runtime/server/render/any.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down
18 changes: 14 additions & 4 deletions packages/astro/src/runtime/server/render/astro/render-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/runtime/server/render/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
24 changes: 4 additions & 20 deletions packages/astro/src/runtime/server/render/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
Renderer,
chunkToString,
type RenderDestination,
type RenderDestinationChunk,
type RenderInstance,
} from './common.js';
import { componentIsHTMLElement, renderHTMLElement } from './dom.js';
Expand Down Expand Up @@ -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);
},
};
}
Expand Down
54 changes: 54 additions & 0 deletions packages/astro/src/runtime/server/render/util.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
},
};
}
6 changes: 6 additions & 0 deletions packages/astro/test/astro-basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
globalThis.__ASTRO_TEST_ORDER__ ??= []
globalThis.__ASTRO_TEST_ORDER__.push('A')
---

<p>A</p>
<slot />
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
globalThis.__ASTRO_TEST_ORDER__ ??= []
globalThis.__ASTRO_TEST_ORDER__.push('B')
---

<p>B</p>
<slot />
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p id="rendered-order">Rendered order: {() => (globalThis.__ASTRO_TEST_ORDER__ ?? []).join(', ')}</p>
13 changes: 13 additions & 0 deletions packages/astro/test/fixtures/astro-basic/src/pages/order.astro
Original file line number Diff line number Diff line change
@@ -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>