Hoist hydration script out of slot templates (#4891)

* Hoist hydration script out of slot templates

* Add changeset

* Fix HTML components

* Mark as html string

.changeset/smart-elephants-check.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Hoist hydration scripts out of slot templates

packages/astro/src/runtime/server/render/any.ts

@@ -1,10 +1,15 @@
 import { escapeHTML, HTMLString, markHTMLString } from '../escape.js';
 import { AstroComponent, renderAstroComponent } from './astro.js';
-import { stringifyChunk } from './common.js';
+import { SlotString } from './slot.js';
 
 export async function* renderChild(child: any): AsyncIterable<any> {
 	child = await child;
-	if (child instanceof HTMLString) {
+	if(child instanceof SlotString) {
+		if(child.instructions) {
+			yield * child.instructions;
+		}
+		yield child;
+	} else if (child instanceof HTMLString) {
 		yield child;
 	} else if (Array.isArray(child)) {
 		for (const value of child) {
@@ -38,19 +43,3 @@ export async function* renderChild(child: any): AsyncIterable<any> {
 		yield child;
 	}
 }
-
-export async function renderSlot(result: any, slotted: string, fallback?: any): Promise<string> {
-	if (slotted) {
-		let iterator = renderChild(slotted);
-		let content = '';
-		for await (const chunk of iterator) {
-			if ((chunk as any).type === 'directive') {
-				content += stringifyChunk(result, chunk);
-			} else {
-				content += chunk;
-			}
-		}
-		return markHTMLString(content);
-	}
-	return fallback;
-}

packages/astro/src/runtime/server/render/component.ts

@@ -5,14 +5,14 @@ import { HTMLBytes, markHTMLString } from '../escape.js';
 import { extractDirectives, generateHydrateScript } from '../hydration.js';
 import { serializeProps } from '../serialize.js';
 import { shorthash } from '../shorthash.js';
-import { renderSlot } from './any.js';
+import { renderSlot, renderSlots } from './slot.js';
 import {
 	isAstroComponentFactory,
 	renderAstroComponent,
 	renderTemplate,
 	renderToIterable,
 } from './astro.js';
-import { Fragment, Renderer } from './common.js';
+import { Fragment, Renderer, stringifyChunk } from './common.js';
 import { componentIsHTMLElement, renderHTMLElement } from './dom.js';
 import { formatList, internalSpreadAttributes, renderElement, voidElementNames } from './util.js';
 
@@ -68,18 +68,10 @@ export async function renderComponent(
 
 		// .html components
 		case 'html': {
-			const children: Record<string, string> = {};
-			if (slots) {
-				await Promise.all(
-					Object.entries(slots).map(([key, value]) =>
-						renderSlot(result, value as string).then((output) => {
-							children[key] = output;
-						})
-					)
-				);
-			}
+			const { slotInstructions, children } = await renderSlots(result, slots);
 			const html = (Component as any).render({ slots: children });
-			return markHTMLString(html);
+			const hydrationHtml = slotInstructions ? slotInstructions.map(instr => stringifyChunk(result, instr)).join('') : '';
+			return markHTMLString(hydrationHtml + html);
 		}
 
 		case 'astro-factory': {
@@ -130,16 +122,7 @@ Did you mean to add ${formatList(probableRendererNames.map((r) => '`' + r + '`')
 		throw new Error(message);
 	}
 
-	const children: Record<string, string> = {};
-	if (slots) {
-		await Promise.all(
-			Object.entries(slots).map(([key, value]) =>
-				renderSlot(result, value as string).then((output) => {
-					children[key] = output;
-				})
-			)
-		);
-	}
+	const { children, slotInstructions } = await renderSlots(result, slots);
 
 	// Call the renderers `check` hook to see if any claim this component.
 	let renderer: SSRLoadedRenderer | undefined;
@@ -345,6 +328,9 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	}
 
 	async function* renderAll() {
+		if(slotInstructions) {
+			yield * slotInstructions;
+		}
 		yield { type: 'directive', hydration, result };
 		yield markHTMLString(renderElement('astro-island', island, false));
 	}

packages/astro/src/runtime/server/render/dom.ts

@@ -1,7 +1,7 @@
 import type { SSRResult } from '../../../@types/astro';
 
 import { markHTMLString } from '../escape.js';
-import { renderSlot } from './any.js';
+import { renderSlot } from './slot.js';
 import { toAttributeString } from './util.js';
 
 export function componentIsHTMLElement(Component: unknown) {

packages/astro/src/runtime/server/render/index.ts

@@ -1,6 +1,6 @@
 import { renderTemplate } from './astro.js';
 
-export { renderSlot } from './any.js';
+export { renderSlot } from './slot.js';
 export { renderAstroComponent, renderTemplate, renderToString } from './astro.js';
 export { Fragment, Renderer, stringifyChunk } from './common.js';
 export { renderComponent } from './component.js';

packages/astro/src/runtime/server/render/slot.ts

@@ -0,0 +1,59 @@
+import type { RenderInstruction } from './types.js';
+import type { SSRResult } from '../../../@types/astro.js';
+
+import { renderChild } from './any.js';
+import { HTMLString, markHTMLString } from '../escape.js';
+
+export class SlotString extends HTMLString {
+	public instructions: null | RenderInstruction[];
+	constructor(content: string, instructions: null | RenderInstruction[]) {
+		super(content);
+		this.instructions = instructions;
+	}
+}
+
+export async function renderSlot(_result: any, slotted: string, fallback?: any): Promise<string> {
+	if (slotted) {
+		let iterator = renderChild(slotted);
+		let content = '';
+		let instructions: null | RenderInstruction[] = null;
+		for await (const chunk of iterator) {
+			if ((chunk as any).type === 'directive') {
+				if(instructions === null) {
+					instructions = [];
+				}
+				instructions.push(chunk);
+			} else {
+				content += chunk;
+			}
+		}
+		return markHTMLString(new SlotString(content, instructions));
+	}
+	return fallback;
+}
+
+interface RenderSlotsResult {
+	slotInstructions: null | RenderInstruction[],
+	children: Record<string, string>;
+}
+
+export async function renderSlots(result: SSRResult, slots: any = {}): Promise<RenderSlotsResult> {
+	let slotInstructions: RenderSlotsResult['slotInstructions'] = null;
+	let children: RenderSlotsResult['children'] = {};
+	if (slots) {
+		await Promise.all(
+			Object.entries(slots).map(([key, value]) =>
+				renderSlot(result, value as string).then((output: any) => {
+					if(output.instructions) {
+						if(slotInstructions === null) {
+							slotInstructions = [];
+						}
+						slotInstructions.push(...output.instructions);
+					}
+					children[key] = output;
+				})
+			)
+		);
+	}
+	return { slotInstructions, children };
+}

packages/astro/test/astro-slots-nested.test.js

@@ -0,0 +1,20 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('Nested Slots', () => {
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({ root: './fixtures/astro-slots-nested/' });
+		await fixture.build();
+	});
+
+	it('Hidden nested slots see their hydration scripts hoisted', async () => {
+		const html = await fixture.readFile('/hidden-nested/index.html');
+		const $ = cheerio.load(html);
+		expect($('script')).to.have.a.lengthOf(1, 'script rendered');
+		const scriptInTemplate = $($('template')[0].children[0]).find('script');
+		expect(scriptInTemplate).to.have.a.lengthOf(0, 'script defined outside of the inner template');
+	});
+});

packages/astro/test/fixtures/astro-slots-nested/astro.config.mjs

@@ -0,0 +1,6 @@
+import { defineConfig } from 'astro/config';
+import react from '@astrojs/react';
+
+export default defineConfig({
+	integrations: [react()]
+});

packages/astro/test/fixtures/astro-slots-nested/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@test/astro-slots-nested",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/react": "workspace:*"
+  }
+}

packages/astro/test/fixtures/astro-slots-nested/src/components/Inner.tsx

@@ -0,0 +1,3 @@
+export default function Inner() {
+  return <span>Inner</span>;
+}

packages/astro/test/fixtures/astro-slots-nested/src/components/Parent.jsx

@@ -0,0 +1,19 @@
+import { useState, useEffect } from 'react';
+
+export default function Parent({ children }) {
+  const [show, setShow] = useState(false);
+
+  useEffect(() => {
+    console.log('mount');
+  }, []);
+
+  return (
+    <>
+      <p>
+        <input type="checkbox" value={show} onChange={() => setShow(!show)} />
+        Toggle show (true should show "Inner")
+      </p>
+      {show ? children : 'Nothing'}
+    </>
+  );
+}

packages/astro/test/fixtures/astro-slots-nested/src/pages/hidden-nested.astro

@@ -0,0 +1,18 @@
+---
+import Parent from '../components/Parent'
+import Inner from '../components/Inner'
+---
+
+<html lang="en">
+	<head>
+		<title>Testing</title>
+	</head>
+	<body>
+		<main>
+			<!-- Try to remove client:load to see it work -->
+			<Parent client:load>
+				<Inner client:load />
+			</Parent>
+		</main>
+	</body>
+</html>

pnpm-lock.yaml

@@ -1397,6 +1397,14 @@ importers:
     dependencies:
       astro: link:../../..
 
+  packages/astro/test/fixtures/astro-slots-nested:
+    specifiers:
+      '@astrojs/react': workspace:*
+      astro: workspace:*
+    dependencies:
+      '@astrojs/react': link:../../../../integrations/react
+      astro: link:../../..
+
   packages/astro/test/fixtures/before-hydration:
     specifiers:
       '@astrojs/preact': workspace:*