Slot bug testing

This reverts commit 1d3a0a16f3.

packages/astro/src/@types/astro.ts

@@ -1135,11 +1135,25 @@ export interface SSRElement {
 	children: string;
 }
 
+export interface HydrationMetadata {
+	directive: string;
+	value: string;
+	componentUrl: string;
+	componentExport: { value: string };
+}
+
+export interface SSRRenderInstruction {
+	type: 'directive';
+	result: SSRResult;
+	hydration: HydrationMetadata;
+}
+
 export interface SSRMetadata {
 	renderers: SSRLoadedRenderer[];
 	pathname: string;
 	hasHydrationScript: boolean;
 	hasDirectives: Set<string>;
+	pendingInstructions: Set<SSRRenderInstruction>;
 }
 
 export interface SSRResult {

packages/astro/src/core/render/result.ts

@@ -265,6 +265,7 @@ const canonicalURL = new URL(Astro.url.pathname, Astro.site);
 			pathname,
 			hasHydrationScript: false,
 			hasDirectives: new Set(),
+			pendingInstructions: new Set(),
 		},
 		response,
 	};

packages/astro/src/runtime/server/hydration.ts

@@ -1,5 +1,6 @@
 import type {
 	AstroComponentMetadata,
+	HydrationMetadata,
 	SSRElement,
 	SSRLoadedRenderer,
 	SSRResult,
@@ -12,12 +13,7 @@ const HydrationDirectivesRaw = ['load', 'idle', 'media', 'visible', 'only'];
 const HydrationDirectives = new Set(HydrationDirectivesRaw);
 export const HydrationDirectiveProps = new Set(HydrationDirectivesRaw.map((n) => `client:${n}`));
 
-export interface HydrationMetadata {
-	directive: string;
-	value: string;
-	componentUrl: string;
-	componentExport: { value: string };
-}
+export { HydrationMetadata };
 
 interface ExtractedProps {
 	isPage: boolean;

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

@@ -26,7 +26,7 @@ export {
 	stringifyChunk,
 	voidElementNames,
 } from './render/index.js';
-export type { AstroComponentFactory, RenderInstruction } from './render/index.js';
+export type { AstroComponentFactory } from './render/index.js';
 import type { AstroComponentFactory } from './render/index.js';
 
 import { markHTMLString } from './escape.js';

packages/astro/src/runtime/server/jsx.ts

@@ -1,12 +1,11 @@
 /* eslint-disable no-console */
-import { SSRResult } from '../../@types/astro.js';
+import { SSRRenderInstruction, SSRResult } from '../../@types/astro.js';
 import { AstroJSX, isVNode } from '../../jsx-runtime/index.js';
 import {
 	escapeHTML,
 	HTMLString,
 	markHTMLString,
 	renderComponent,
-	RenderInstruction,
 	renderToString,
 	spreadAttributes,
 	stringifyChunk,
@@ -122,7 +121,7 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise<any> {
 			}
 			await Promise.all(slotPromises);
 
-			let output: string | AsyncIterable<string | RenderInstruction>;
+			let output: string | AsyncIterable<string | SSRRenderInstruction>;
 			if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) {
 				output = await renderComponent(
 					result,

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

@@ -1,6 +1,6 @@
+import { SSRRenderInstruction, SSRResult } from '../../../@types/astro';
 import { escapeHTML, HTMLString, markHTMLString } from '../escape.js';
 import { AstroComponent, renderAstroComponent } from './astro.js';
-import { stringifyChunk } from './common.js';
 
 export async function* renderChild(child: any): AsyncIterable<any> {
 	child = await child;
@@ -34,13 +34,19 @@ export async function* renderChild(child: any): AsyncIterable<any> {
 	}
 }
 
-export async function renderSlot(result: any, slotted: string, fallback?: any): Promise<string> {
+export async function renderSlot(
+	result: SSRResult,
+	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);
+				// Do not render directives scripts in a slot, instead mark them as pending
+				// so that renderPage includes them ASAP.
+				result._metadata.pendingInstructions.add(chunk as SSRRenderInstruction);
 			} else {
 				content += chunk;
 			}

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

@@ -1,6 +1,5 @@
-import type { SSRResult } from '../../../@types/astro';
+import type { SSRRenderInstruction, SSRResult } from '../../../@types/astro';
 import type { AstroComponentFactory } from './index';
-import type { RenderInstruction } from './types';
 
 import { markHTMLString } from '../escape.js';
 import { HydrationDirectiveProps } from '../hydration.js';
@@ -58,7 +57,7 @@ export function isAstroComponent(obj: any): obj is AstroComponent {
 
 export async function* renderAstroComponent(
 	component: InstanceType<typeof AstroComponent>
-): AsyncIterable<string | RenderInstruction> {
+): AsyncIterable<string | SSRRenderInstruction> {
 	for await (const value of component) {
 		if (value || value === 0) {
 			for await (const chunk of renderChild(value)) {
@@ -104,7 +103,7 @@ export async function renderToIterable(
 	displayName: string,
 	props: any,
 	children: any
-): Promise<AsyncIterable<string | RenderInstruction>> {
+): Promise<AsyncIterable<string | SSRRenderInstruction>> {
 	validateComponentProps(props, displayName);
 	const Component = await componentFactory(result, props, children);
 

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

@@ -1,5 +1,4 @@
-import type { SSRResult } from '../../../@types/astro';
-import type { RenderInstruction } from './types.js';
+import type { SSRRenderInstruction, SSRResult } from '../../../@types/astro';
 
 import { markHTMLString } from '../escape.js';
 import {
@@ -15,10 +14,10 @@ export const Renderer = Symbol.for('astro:renderer');
 // Rendering produces either marked strings of HTML or instructions for hydration.
 // These directive instructions bubble all the way up to renderPage so that we
 // can ensure they are added only once, and as soon as possible.
-export function stringifyChunk(result: SSRResult, chunk: string | RenderInstruction) {
+export function stringifyChunk(result: SSRResult, chunk: string | SSRRenderInstruction) {
 	switch ((chunk as any).type) {
 		case 'directive': {
-			const { hydration } = chunk as RenderInstruction;
+			const { hydration } = chunk as SSRRenderInstruction;
 			let needsHydrationScript = hydration && determineIfNeedsHydrationScript(result);
 			let needsDirectiveScript =
 				hydration && determinesIfNeedsDirectiveScript(result, hydration.directive);

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

@@ -1,5 +1,9 @@
-import type { AstroComponentMetadata, SSRLoadedRenderer, SSRResult } from '../../../@types/astro';
-import type { RenderInstruction } from './types.js';
+import type {
+	AstroComponentMetadata,
+	SSRLoadedRenderer,
+	SSRRenderInstruction,
+	SSRResult,
+} from '../../../@types/astro';
 
 import { markHTMLString } from '../escape.js';
 import { extractDirectives, generateHydrateScript } from '../hydration.js';
@@ -49,7 +53,7 @@ export async function renderComponent(
 	Component: unknown,
 	_props: Record<string | number, any>,
 	slots: any = {}
-): Promise<string | AsyncIterable<string | RenderInstruction>> {
+): Promise<string | AsyncIterable<string | SSRRenderInstruction>> {
 	Component = await Component;
 
 	switch (getComponentType(Component)) {
@@ -79,7 +83,7 @@ export async function renderComponent(
 
 		case 'astro-factory': {
 			async function* renderAstroComponentInline(): AsyncGenerator<
-				string | RenderInstruction,
+				string | SSRRenderInstruction,
 				void,
 				undefined
 			> {

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

@@ -7,7 +7,6 @@ export { renderComponent } from './component.js';
 export { renderHTMLElement } from './dom.js';
 export { maybeRenderHead, renderHead } from './head.js';
 export { renderPage } from './page.js';
-export type { RenderInstruction } from './types';
 export { addAttribute, defineScriptVars, voidElementNames } from './util.js';
 
 // The callback passed to to $$createComponent

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

@@ -49,13 +49,28 @@ export async function renderPage(
 		let headers = new Headers(init.headers);
 		let body: BodyInit;
 
+		// Combines HTML chunks coming from the iterable with rendering instructions
+		// added to metadata. These instructions need to go out first to ensure
+		// the scripts exist before the islands that need them.
+		async function* eachChunk() {
+			for await (const chunk of iterable) {
+				if (result._metadata.pendingInstructions.size) {
+					for (const instr of result._metadata.pendingInstructions) {
+						result._metadata.pendingInstructions.delete(instr);
+						yield instr;
+					}
+				}
+				yield chunk;
+			}
+		}
+
 		if (streaming) {
 			body = new ReadableStream({
 				start(controller) {
 					async function read() {
 						let i = 0;
 						try {
-							for await (const chunk of iterable) {
+							for await (const chunk of eachChunk()) {
 								let html = stringifyChunk(result, chunk);
 
 								if (i === 0) {
@@ -77,7 +92,7 @@ export async function renderPage(
 		} else {
 			body = '';
 			let i = 0;
-			for await (const chunk of iterable) {
+			for await (const chunk of eachChunk()) {
 				let html = stringifyChunk(result, chunk);
 				if (i === 0) {
 					if (!/<!doctype html/i.test(html)) {

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

@@ -1,8 +0,0 @@
-import type { SSRResult } from '../../../@types/astro';
-import type { HydrationMetadata } from '../hydration.js';
-
-export interface RenderInstruction {
-	type: 'directive';
-	result: SSRResult;
-	hydration: HydrationMetadata;
-}

packages/astro/test/fixtures/hydration-race/src/components/Layout.astro

@@ -0,0 +1,6 @@
+---
+import Wrapper from './Wrapper.astro';
+---
+
+<Wrapper />
+<slot />

packages/astro/test/fixtures/hydration-race/src/pages/slot.astro

@@ -0,0 +1,15 @@
+---
+import Layout from '../components/Layout.astro';
+import Wrapper from '../components/Wrapper.astro';
+import One from '../components/One.jsx';
+---
+<html>
+<head>
+	<title>Testing</title>
+</head>
+<body>
+	<Layout>
+		<One name="Three" client:idle />
+	</Layout>
+</body>
+</html>

packages/astro/test/fixtures/slot-head-bubbling/astro.config.mjs

@@ -0,0 +1,7 @@
+import { defineConfig } from 'astro/config';
+import svelte from '@astrojs/svelte';
+
+// https://astro.build/config
+export default defineConfig({
+	integrations: [svelte()],
+});

packages/astro/test/fixtures/slot-head-bubbling/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@test/slot-head-bubbling",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/svelte": "workspace:*"
+  }
+}

packages/astro/test/fixtures/slot-head-bubbling/src/components/Counter.svelte

@@ -0,0 +1,12 @@
+<script>
+  let count = 1;
+
+  function handleClick() {
+    count = count + 1;
+  }
+</script>
+  
+<div>
+  <pre>{count}</pre>
+  <button on:click={handleClick}>Click</button>
+</div>

packages/astro/test/fixtures/slot-head-bubbling/src/layouts/Layout.astro

@@ -0,0 +1,22 @@
+---
+export interface Props {
+	title: string;
+}
+
+const { title } = Astro.props as Props;
+---
+
+<!DOCTYPE html>
+<html lang="en">
+	<head>
+		<meta charset="UTF-8" />
+		<meta name="viewport" content="width=device-width" />
+		<meta name="generator" content={Astro.generator} />
+		<title>{title}</title>
+	</head>
+	<body>
+		<main>
+			<slot />
+		</main>
+	</body>
+</html>

packages/astro/test/fixtures/slot-head-bubbling/src/pages/index.astro

@@ -0,0 +1,8 @@
+---
+import Layout from '../layouts/Layout.astro';
+import Counter from '../components/Counter.svelte'
+---
+
+<Layout title="Welcome to Astro.">
+	<Counter client:visible />
+</Layout>

packages/astro/test/hydration-race.test.js

@@ -26,4 +26,37 @@ describe('Hydration script ordering', async () => {
 		expect($('style')).to.have.a.lengthOf(1, 'hydration style added once');
 		expect($('script')).to.have.a.lengthOf(1, 'only one hydration script needed');
 	});
+
+	it('Hydration placed in correct location when there is slots used', async () => {
+		let html = await fixture.readFile('/slot/index.html');
+		let $ = cheerio.load(html);
+		//console.log(html);
+
+		// First, let's make sure all islands rendered (or test is bad)
+		expect($('astro-island')).to.have.a.lengthOf(3);
+
+		// Now let's make sure the hydration script is placed before the first component
+		let firstIsland = $($('astro-island').get(0));
+		let el = firstIsland.prev();
+		let foundScript = false;
+
+		// Traverse the DOM going backwards until we find a script, if there is one.
+		while (el.length) {
+			let last = el;
+			while (el.length) {
+				if (el.prop('tagName') === 'SCRIPT') {
+					foundScript = true;
+				}
+				last = el;
+				el = el.prev();
+			}
+			el = last.parent();
+		}
+
+		expect(foundScript).to.be.true;
+
+		// Sanity check that we're only rendering them once.
+		expect($('style')).to.have.a.lengthOf(1, 'hydration style added once');
+		expect($('script')).to.have.a.lengthOf(1, 'only one hydration script needed');
+	});
 });

packages/astro/test/slot-head-bubbling.test.js

@@ -0,0 +1,18 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('Slots head bubbling', () => {
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({ root: './fixtures/slot-head-bubbling/' });
+		await fixture.build();
+	});
+
+	it('Renders hydration CSS in the head', async () => {
+		const html = await fixture.readFile('/index.html');
+		console.log(html);
+		//const $ = cheerio.load(html);
+	});
+});

pnpm-lock.yaml

@@ -1769,6 +1769,14 @@ importers:
     dependencies:
       astro: link:../../..
 
+  packages/astro/test/fixtures/slot-head-bubbling:
+    specifiers:
+      '@astrojs/svelte': workspace:*
+      astro: workspace:*
+    dependencies:
+      '@astrojs/svelte': link:../../../../integrations/svelte
+      astro: link:../../..
+
   packages/astro/test/fixtures/slots-preact:
     specifiers:
       '@astrojs/mdx': workspace:*