Yield out potentional slot instructions when rendering dynamic tags (#…

…4981) * Yield out potentional slot instructions when rendering dynamic tags * Adding a changeset * yield instead of return * Handle the fact that renderComponent returns an iterable * Only yield out html once
withastro · Oct 5, 2022 · 1f890b3 · 1f890b3
1 parent 8f9791d
commit 1f890b3
Show file tree

Hide file tree

Showing 11 changed files with 132 additions and 32 deletions.
diff --git a/.changeset/four-doors-exercise.md b/.changeset/four-doors-exercise.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Ensure dynamic tags have their slot instructions yielded
diff --git a/packages/astro/src/runtime/server/jsx.ts b/packages/astro/src/runtime/server/jsx.ts
@@ -1,4 +1,5 @@
 /* eslint-disable no-console */
+import type { ComponentIterable } from './render/component';
 import { SSRResult } from '../../@types/astro.js';
 import { AstroJSX, isVNode } from '../../jsx-runtime/index.js';
 import {
@@ -129,7 +130,7 @@ Did you forget to import the component or is it possible there is a typo?`);
 			}
 			await Promise.all(slotPromises);
 
-			let output: string | AsyncIterable<string | HTMLBytes | RenderInstruction>;
+			let output: ComponentIterable;
 			if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) {
 				output = await renderComponent(
 					result,

diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts
@@ -34,6 +34,7 @@ function guessRenderers(componentUrl?: string): string[] {
 }
 
 type ComponentType = 'fragment' | 'html' | 'astro-factory' | 'unknown';
+export type ComponentIterable = AsyncIterable<string | HTMLBytes | RenderInstruction>;
 
 function getComponentType(Component: unknown): ComponentType {
 	if (Component === Fragment) {
@@ -54,7 +55,7 @@ export async function renderComponent(
 	Component: unknown,
 	_props: Record<string | number, any>,
 	slots: any = {}
-): Promise<string | AsyncIterable<string | HTMLBytes | RenderInstruction>> {
+): Promise<ComponentIterable> {
 	Component = await Component;
 
 	switch (getComponentType(Component)) {
@@ -279,10 +280,17 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	}
 
 	if (!hydration) {
-		if (isPage || renderer?.name === 'astro:jsx') {
-			return html;
-		}
-		return markHTMLString(html.replace(/\<\/?astro-slot\>/g, ''));
+		return (async function *() {
+			if (slotInstructions) {
+				yield* slotInstructions;
+			}
+
+			if (isPage || renderer?.name === 'astro:jsx') {
+				yield html;
+			} else {	
+				yield markHTMLString(html.replace(/\<\/?astro-slot\>/g, ''));
+			}
+		})();
 	}
 
 	// Include componentExport name, componentUrl, and props in hash to dedupe identical islands

diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts
@@ -1,5 +1,6 @@
 import type { SSRResult } from '../../../@types/astro';
 import type { AstroComponentFactory } from './index';
+import type { ComponentIterable } from './component';
 
 import { isHTMLString } from '../escape.js';
 import { createResponse } from '../response.js';
@@ -19,6 +20,29 @@ function nonAstroPageNeedsHeadInjection(pageComponent: NonAstroPageComponent): b
 	return needsHeadRenderingSymbol in pageComponent && !!pageComponent[needsHeadRenderingSymbol];
 }
 
+async function iterableToHTMLBytes(
+	result: SSRResult,
+	iterable: ComponentIterable,
+	onDocTypeInjection?: (parts: HTMLParts) => Promise<void>
+): Promise<Uint8Array> {
+	const parts = new HTMLParts();
+	let i = 0;
+	for await (const chunk of iterable) {
+		if (isHTMLString(chunk)) {
+			if (i === 0) {
+				if (!/<!doctype html/i.test(String(chunk))) {
+					parts.append('<!DOCTYPE html>\n', result);
+					if(onDocTypeInjection) {
+						await onDocTypeInjection(parts);
+					}
+				}
+			}
+		}
+		parts.append(chunk, result);
+	}
+	return parts.toArrayBuffer();
+}
+
 export async function renderPage(
 	result: SSRResult,
 	componentFactory: AstroComponentFactory | NonAstroPageComponent,
@@ -35,21 +59,16 @@ export async function renderPage(
 			pageProps,
 			null
 		);
-		let html = output.toString();
-		if (!/<!doctype html/i.test(html)) {
-			let rest = html;
-			html = `<!DOCTYPE html>`;
-			// This symbol currently exists for md components, but is something that could
-			// be added for any page-level component that's not an Astro component.
-			// to signal that head rendering is needed.
+
+		// Accumulate the HTML string and append the head if necessary.
+		const bytes = await iterableToHTMLBytes(result, output, async (parts) => {
 			if (nonAstroPageNeedsHeadInjection(componentFactory)) {
 				for await (let chunk of maybeRenderHead(result)) {
-					html += chunk;
+					parts.append(chunk, result);
 				}
 			}
-			html += rest;
-		}
-		const bytes = encoder.encode(html);
+		});
+
 		return new Response(bytes, {
 			headers: new Headers([
 				['Content-Type', 'text/html; charset=utf-8'],
@@ -80,7 +99,7 @@ export async function renderPage(
 									}
 								}
 
-								let bytes = chunkToByteArray(result, chunk);
+								const bytes = chunkToByteArray(result, chunk);
 								controller.enqueue(bytes);
 								i++;
 							}
@@ -93,20 +112,7 @@ export async function renderPage(
 				},
 			});
 		} else {
-			let parts = new HTMLParts();
-			let i = 0;
-			for await (const chunk of iterable) {
-				if (isHTMLString(chunk)) {
-					if (i === 0) {
-						if (!/<!doctype html/i.test(String(chunk))) {
-							parts.append('<!DOCTYPE html>\n', result);
-						}
-					}
-				}
-				parts.append(chunk, result);
-				i++;
-			}
-			body = parts.toArrayBuffer();
+			body = await iterableToHTMLBytes(result, iterable);
 			headers.set('Content-Length', body.byteLength.toString());
 		}
 

diff --git a/packages/astro/test/astro-slot-with-client.test.js b/packages/astro/test/astro-slot-with-client.test.js
@@ -0,0 +1,19 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('Slots with client: directives', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({ root: './fixtures/astro-slot-with-client/' });
+		await fixture.build();
+	});
+
+	it('Tags of dynamic tags works', async () => {
+		const html = await fixture.readFile('/index.html');
+		const $ = cheerio.load(html);
+		expect($('script')).to.have.a.lengthOf(1);
+	});
+});
diff --git a/packages/astro/test/fixtures/astro-slot-with-client/astro.config.mjs b/packages/astro/test/fixtures/astro-slot-with-client/astro.config.mjs
@@ -0,0 +1,8 @@
+import { defineConfig } from 'astro/config';
+import preact from '@astrojs/preact';
+
+export default defineConfig({
+	integrations: [
+		preact()
+	]
+});
diff --git a/packages/astro/test/fixtures/astro-slot-with-client/package.json b/packages/astro/test/fixtures/astro-slot-with-client/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "@test/astro-slot-with-client",
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/preact": "workspace:*",
+    "preact": "^10.11.0"
+  }
+}
diff --git a/packages/astro/test/fixtures/astro-slot-with-client/src/components/Slotted.astro b/packages/astro/test/fixtures/astro-slot-with-client/src/components/Slotted.astro
@@ -0,0 +1,3 @@
+<div id="default">
+  <slot />
+</div>
diff --git a/packages/astro/test/fixtures/astro-slot-with-client/src/components/Thing.jsx b/packages/astro/test/fixtures/astro-slot-with-client/src/components/Thing.jsx
@@ -0,0 +1,8 @@
+
+export default function(props) {
+	return (
+		<div class="thing">
+			{ props.c }
+		</div>
+	)
+}
diff --git a/packages/astro/test/fixtures/astro-slot-with-client/src/pages/index.astro b/packages/astro/test/fixtures/astro-slot-with-client/src/pages/index.astro
@@ -0,0 +1,24 @@
+---
+import Slotted from '../components/Slotted.astro';
+import Thing from '../components/Thing.jsx';
+
+const Tag = 'section';
+---
+
+<html>
+  <head>
+    <!-- Head Stuff -->
+  </head>
+  <body>
+    <Slotted>
+      <Tag>
+				<span>More</span>
+				<Thing client:load>
+					<div slot="c">
+						inner content
+					</div>	
+				</Thing>
+			</Tag>
+    </Slotted>
+  </body>
+</html>
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml