Prevent removal of nested slots within islands (#7093)

* Prevent removal of nested slots within islands

* Fix build errors

.changeset/unlucky-lamps-remember.md

@@ -0,0 +1,24 @@
+---
+'@astrojs/preact': minor
+'@astrojs/svelte': minor
+'@astrojs/react': minor
+'@astrojs/solid-js': minor
+'@astrojs/vue': minor
+'astro': minor
+---
+
+Prevent removal of nested slots within islands
+
+This change introduces a new flag that renderers can add called `supportsAstroStaticSlot`. What this does is let Astro know that the render is sending `<astro-static-slot>` as placeholder values for static (non-hydrated) slots which Astro will then remove.
+
+This change is completely backwards compatible, but fixes bugs caused by combining ssr-only and client-side framework components like so:
+
+```astro
+<Component>
+  <div>
+    <Component client:load>
+      <span>Nested</span>
+    </Component>
+  </div>
+</Component>
+```

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

@@ -94,6 +94,7 @@ export interface AstroComponentMetadata {
 	hydrateArgs?: any;
 	componentUrl?: string;
 	componentExport?: { value: string; namespace?: boolean };
+	astroStaticSlot: true;
 }
 
 /** The flags supported by the Astro CLI */
@@ -1718,6 +1719,7 @@ export interface SSRLoadedRenderer extends AstroRenderer {
 			html: string;
 			attrs?: Record<string, string>;
 		}>;
+		supportsAstroStaticSlot?: boolean;
 	};
 }
 

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

@@ -54,6 +54,13 @@ function isHTMLComponent(Component: unknown) {
 	return Component && typeof Component === 'object' && (Component as any)['astro:html'];
 }
 
+const ASTRO_SLOT_EXP = /\<\/?astro-slot\b[^>]*>/g;
+const ASTRO_STATIC_SLOT_EXP = /\<\/?astro-static-slot\b[^>]*>/g;
+function removeStaticAstroSlot(html: string, supportsAstroStaticSlot: boolean) {
+	const exp = supportsAstroStaticSlot ? ASTRO_STATIC_SLOT_EXP : ASTRO_SLOT_EXP;
+	return html.replace(exp, '');
+}
+
 async function renderFrameworkComponent(
 	result: SSRResult,
 	displayName: string,
@@ -68,7 +75,10 @@ async function renderFrameworkComponent(
 	}
 
 	const { renderers, clientDirectives } = result._metadata;
-	const metadata: AstroComponentMetadata = { displayName };
+	const metadata: AstroComponentMetadata = {
+		astroStaticSlot: true,
+		displayName
+	};
 
 	const { hydration, isPage, props } = extractDirectives(_props, clientDirectives);
 	let html = '';
@@ -263,7 +273,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 			if (isPage || renderer?.name === 'astro:jsx') {
 				yield html;
 			} else if (html && html.length > 0) {
-				yield markHTMLString(html.replace(/\<\/?astro-slot\b[^>]*>/g, ''));
+				yield markHTMLString(removeStaticAstroSlot(html, renderer?.ssr?.supportsAstroStaticSlot ?? false));
 			} else {
 				yield '';
 			}
@@ -288,7 +298,11 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	if (html) {
 		if (Object.keys(children).length > 0) {
 			for (const key of Object.keys(children)) {
-				if (!html.includes(key === 'default' ? `<astro-slot>` : `<astro-slot name="${key}">`)) {
+				let tagName = renderer?.ssr?.supportsAstroStaticSlot ?
+					!!metadata.hydrate ? 'astro-slot' : 'astro-static-slot'
+					: 'astro-slot';
+				let expectedHTML = key === 'default' ? `<${tagName}>` : `<${tagName} name="${key}">`;
+				if (!html.includes(expectedHTML)) {
 					unrenderedSlots.push(key);
 				}
 			}

packages/astro/src/vite-plugin-jsx/index.ts

@@ -202,7 +202,7 @@ export default function jsx({ settings, logging }: AstroPluginJSXOptions): Plugi
 Unable to resolve a renderer that handles this file! With more than one renderer enabled, you should include an import or use a pragma comment.
 Add ${colors.cyan(
 						IMPORT_STATEMENTS[defaultRendererName] || `import '${defaultRendererName}';`
-					)} or ${colors.cyan(`/* jsxImportSource: ${defaultRendererName} */`)} to this file.
+					)} or ${colors.cyan(`/** @jsxImportSource: ${defaultRendererName} */`)} to this file.
 `
 				);
 				return null;

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

@@ -3,6 +3,7 @@ import * as cheerio from 'cheerio';
 import { loadFixture } from './test-utils.js';
 
 describe('Nested Slots', () => {
+	/** @type {import('./test-utils').Fixture} */
 	let fixture;
 
 	before(async () => {
@@ -23,4 +24,38 @@ describe('Nested Slots', () => {
 		const $ = cheerio.load(html);
 		expect($('script')).to.have.a.lengthOf(1, 'script rendered');
 	});
+
+	describe('Client components nested inside server-only framework components', () => {
+		/** @type {cheerio.CheerioAPI} */
+		let $;
+		before(async () => {
+			const html = await fixture.readFile('/server-component-nested/index.html');
+			$ = cheerio.load(html);
+		});
+
+		it('react', () => {
+			expect($('#react astro-slot')).to.have.a.lengthOf(1);
+			expect($('#react astro-static-slot')).to.have.a.lengthOf(0);
+		});
+
+		it('vue', () => {
+			expect($('#vue astro-slot')).to.have.a.lengthOf(1);
+			expect($('#vue astro-static-slot')).to.have.a.lengthOf(0);
+		});
+
+		it('preact', () => {
+			expect($('#preact astro-slot')).to.have.a.lengthOf(1);
+			expect($('#preact astro-static-slot')).to.have.a.lengthOf(0);
+		});
+
+		it('solid', () => {
+			expect($('#solid astro-slot')).to.have.a.lengthOf(1);
+			expect($('#solid astro-static-slot')).to.have.a.lengthOf(0);
+		});
+
+		it('svelte', () => {
+			expect($('#svelte astro-slot')).to.have.a.lengthOf(1);
+			expect($('#svelte astro-static-slot')).to.have.a.lengthOf(0);
+		});
+	});
 });

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

@@ -1,6 +1,16 @@
 import { defineConfig } from 'astro/config';
 import react from '@astrojs/react';
+import preact from '@astrojs/preact';
+import solid from '@astrojs/solid-js';
+import svelte from '@astrojs/svelte';
+import vue from '@astrojs/vue';
 
 export default defineConfig({
-	integrations: [react()]
+	integrations: [
+		react(),
+		preact(),
+		solid(),
+		svelte(),
+		vue()
+	]
 });

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

@@ -3,9 +3,17 @@
   "version": "0.0.0",
   "private": true,
   "dependencies": {
+    "@astrojs/preact": "workspace:*",
     "@astrojs/react": "workspace:*",
+    "@astrojs/vue": "workspace:*",
+    "@astrojs/solid-js": "workspace:*",
+    "@astrojs/svelte": "workspace:*",
     "astro": "workspace:*",
     "react": "^18.2.0",
-    "react-dom": "^18.2.0"
+    "react-dom": "^18.2.0",
+    "solid-js": "^1.7.4",
+    "svelte": "^3.58.0",
+    "vue": "^3.2.47",
+    "preact": "^10.13.2"
   }
 }

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

@@ -1,3 +1,5 @@
+import React from 'react';
+
 export default function Inner() {
   return <span>Inner</span>;
 }

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

@@ -0,0 +1,5 @@
+import React, { Fragment } from 'react';
+
+export default function PassesChildren({ children }) {
+	return <Fragment>{ children }</Fragment>;
+}

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

@@ -0,0 +1,5 @@
+import { h, Fragment } from 'preact';
+
+export default function PassesChildren({ children }) {
+	return <Fragment>{ children }</Fragment>;
+}

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

@@ -0,0 +1,5 @@
+/** @jsxImportSource solid-js */
+
+export default function PassesChildren({ children }) {
+	return <>{ children }</>;
+}

packages/astro/test/fixtures/astro-slots-nested/src/components/PassesChildrenSv.svelte

@@ -0,0 +1,3 @@
+<div class="svelte-children">
+	<slot></slot>
+</div>

packages/astro/test/fixtures/astro-slots-nested/src/components/PassesChildrenV.vue

@@ -0,0 +1,5 @@
+<template>
+	<div class="vue-wrapper">
+    <slot></slot>
+  </div>
+</template>

packages/astro/test/fixtures/astro-slots-nested/src/pages/server-component-nested.astro

@@ -0,0 +1,62 @@
+---
+import PassesChildren from '../components/PassesChildren.jsx';
+import PassesChildrenP from '../components/PassesChildrenP.jsx';
+import PassesChildrenS from '../components/PassesChildrenS.jsx';
+import PassesChildrenSv from '../components/PassesChildrenSv.svelte';
+import PassesChildrenV from '../components/PassesChildrenV.vue';
+---
+
+<html lang="en">
+	<head>
+		<title>Testing</title>
+	</head>
+	<body>
+		<main>
+			<div id="react">
+				<PassesChildren>
+					<div>
+						<PassesChildren client:load>
+							<span>Inner children</span>
+						</PassesChildren>
+					</div>
+				</PassesChildren>
+			</div>
+			<div id="preact">
+				<PassesChildrenP>
+					<div>
+						<PassesChildrenP client:load>
+							<span>Inner children</span>
+						</PassesChildrenP>
+					</div>
+				</PassesChildrenP>
+			</div>
+			<div id="solid">
+				<PassesChildrenS>
+					<div>
+						<PassesChildrenS client:load>
+							<span>Inner children</span>
+						</PassesChildrenS>
+					</div>
+				</PassesChildrenS>
+			</div>
+			<div id="svelte">
+				<PassesChildrenSv>
+					<div>
+						<PassesChildrenSv client:load>
+							<span>Inner children</span>
+						</PassesChildrenSv>
+					</div>
+				</PassesChildrenSv>
+			</div>
+			<div id="vue">
+				<PassesChildrenV>
+					<div>
+						<PassesChildrenV client:load>
+							<span>Inner children</span>
+						</PassesChildrenV>
+					</div>
+				</PassesChildrenV>
+			</div>
+		</main>
+	</body>
+</html>

packages/integrations/preact/src/server.ts

@@ -1,3 +1,4 @@
+import type { AstroComponentMetadata } from 'astro';
 import { Component as BaseComponent, h } from 'preact';
 import render from 'preact-render-to-string';
 import { getContext } from './context.js';
@@ -10,7 +11,7 @@ const slotName = (str: string) => str.trim().replace(/[-_]([a-z])/g, (_, w) => w
 let originalConsoleError: typeof console.error;
 let consoleFilterRefs = 0;
 
-function check(this: RendererContext, Component: any, props: Record<string, any>, children: any) {
+function check(this: RendererContext, Component: any, props: Record<string, any>, children: any, ) {
 	if (typeof Component !== 'function') return false;
 
 	if (Component.prototype != null && typeof Component.prototype.render === 'function') {
@@ -21,7 +22,7 @@ function check(this: RendererContext, Component: any, props: Record<string, any>
 
 	try {
 		try {
-			const { html } = renderToStaticMarkup.call(this, Component, props, children);
+			const { html } = renderToStaticMarkup.call(this, Component, props, children, undefined);
 			if (typeof html !== 'string') {
 				return false;
 			}
@@ -38,18 +39,28 @@ function check(this: RendererContext, Component: any, props: Record<string, any>
 	}
 }
 
+function shouldHydrate(metadata: AstroComponentMetadata | undefined) {
+	// Adjust how this is hydrated only when the version of Astro supports `astroStaticSlot`
+	return metadata?.astroStaticSlot ? !!metadata.hydrate : true;
+}
+
 function renderToStaticMarkup(
 	this: RendererContext,
 	Component: any,
 	props: Record<string, any>,
-	{ default: children, ...slotted }: Record<string, any>
+	{ default: children, ...slotted }: Record<string, any>,
+	metadata: AstroComponentMetadata | undefined,
 ) {
 	const ctx = getContext(this.result);
 
 	const slots: Record<string, ReturnType<typeof h>> = {};
 	for (const [key, value] of Object.entries(slotted)) {
 		const name = slotName(key);
-		slots[name] = h(StaticHtml, { value, name });
+		slots[name] = h(StaticHtml, {
+			hydrate: shouldHydrate(metadata),
+			value,
+			name
+		});
 	}
 
 	// Restore signals back onto props so that they will be passed as-is to components
@@ -61,7 +72,9 @@ function renderToStaticMarkup(
 	serializeSignals(ctx, props, attrs, propsMap);
 
 	const html = render(
-		h(Component, newProps, children != null ? h(StaticHtml, { value: children }) : children)
+		h(Component, newProps, children != null ? h(StaticHtml, {
+			hydrate: shouldHydrate(metadata),
+			value: children}) : children)
 	);
 	return {
 		attrs,
@@ -127,4 +140,5 @@ function filteredConsoleError(msg: string, ...rest: any[]) {
 export default {
 	check,
 	renderToStaticMarkup,
+	supportsAstroStaticSlot: true,
 };

packages/integrations/preact/src/static-html.ts

@@ -1,5 +1,11 @@
 import { h } from 'preact';
 
+type Props = {
+	value: string;
+	name?: string;
+	hydrate?: boolean;
+}
+
 /**
  * Astro passes `children` as a string of HTML, so we need
  * a wrapper `div` to render that content as VNodes.
@@ -7,9 +13,10 @@ import { h } from 'preact';
  * As a bonus, we can signal to Preact that this subtree is
  * entirely static and will never change via `shouldComponentUpdate`.
  */
-const StaticHtml = ({ value, name }: { value: string; name?: string }) => {
+const StaticHtml = ({ value, name, hydrate }: Props) => {
 	if (!value) return null;
-	return h('astro-slot', { name, dangerouslySetInnerHTML: { __html: value } });
+	const tagName = hydrate === false ? 'astro-static-slot' : 'astro-slot';
+	return h(tagName, { name, dangerouslySetInnerHTML: { __html: value } });
 };
 
 /**

packages/integrations/react/server-v17.js

@@ -65,7 +65,11 @@ function renderToStaticMarkup(Component, props, { default: children, ...slotted
 	};
 	const newChildren = children ?? props.children;
 	if (newChildren != null) {
-		newProps.children = React.createElement(StaticHtml, { value: newChildren });
+		newProps.children = React.createElement(StaticHtml, {
+			// Adjust how this is hydrated only when the version of Astro supports `astroStaticSlot`
+			hydrate: metadata.astroStaticSlot ? !!metadata.hydrate : true,
+			value: newChildren
+		});
 	}
 	const vnode = React.createElement(Component, newProps);
 	let html;
@@ -80,4 +84,5 @@ function renderToStaticMarkup(Component, props, { default: children, ...slotted
 export default {
 	check,
 	renderToStaticMarkup,
+	supportsAstroStaticSlot: true,
 };

packages/integrations/react/server.js

@@ -58,6 +58,11 @@ async function getNodeWritable() {
 	return Writable;
 }
 
+function needsHydration(metadata) {
+	// Adjust how this is hydrated only when the version of Astro supports `astroStaticSlot`
+	return metadata.astroStaticSlot ? !!metadata.hydrate : true;
+}
+
 async function renderToStaticMarkup(Component, props, { default: children, ...slotted }, metadata) {
 	let prefix;
 	if (this && this.result) {
@@ -69,7 +74,11 @@ async function renderToStaticMarkup(Component, props, { default: children, ...sl
 	const slots = {};
 	for (const [key, value] of Object.entries(slotted)) {
 		const name = slotName(key);
-		slots[name] = React.createElement(StaticHtml, { value, name });
+		slots[name] = React.createElement(StaticHtml, {
+			hydrate: needsHydration(metadata),
+			value,
+			name
+		});
 	}
 	// Note: create newProps to avoid mutating `props` before they are serialized
 	const newProps = {
@@ -78,7 +87,10 @@ async function renderToStaticMarkup(Component, props, { default: children, ...sl
 	};
 	const newChildren = children ?? props.children;
 	if (newChildren != null) {
-		newProps.children = React.createElement(StaticHtml, { value: newChildren });
+		newProps.children = React.createElement(StaticHtml, {
+			hydrate: needsHydration(metadata),
+			value: newChildren
+		});
 	}
 	const vnode = React.createElement(Component, newProps);
 	const renderOptions = {
@@ -182,4 +194,5 @@ async function renderToReadableStreamAsync(vnode, options) {
 export default {
 	check,
 	renderToStaticMarkup,
+	supportsAstroStaticSlot: true,
 };

packages/integrations/react/static-html.js

@@ -7,9 +7,10 @@ import { createElement as h } from 'react';
  * As a bonus, we can signal to React that this subtree is
  * entirely static and will never change via `shouldComponentUpdate`.
  */
-const StaticHtml = ({ value, name }) => {
+const StaticHtml = ({ value, name, hydrate }) => {
 	if (!value) return null;
-	return h('astro-slot', {
+	const tagName = hydrate ? 'astro-slot' : 'astro-static-slot';
+	return h(tagName, {
 		name,
 		suppressHydrationWarning: true,
 		dangerouslySetInnerHTML: { __html: value },

packages/integrations/solid/src/server.ts

@@ -18,20 +18,22 @@ function renderToStaticMarkup(
 	metadata?: undefined | Record<string, any>
 ) {
 	const renderId = metadata?.hydrate ? incrementId(getContext(this.result)) : '';
+	const needsHydrate = metadata?.astroStaticSlot ? !!metadata.hydrate : true;
+	const tagName = needsHydrate ? 'astro-slot' : 'astro-static-slot';
 
 	const html = renderToString(
 		() => {
 			const slots: Record<string, any> = {};
 			for (const [key, value] of Object.entries(slotted)) {
 				const name = slotName(key);
-				slots[name] = ssr(`<astro-slot name="${name}">${value}</astro-slot>`);
+				slots[name] = ssr(`<${tagName} name="${name}">${value}</${tagName}>`);
 			}
 			// Note: create newProps to avoid mutating `props` before they are serialized
 			const newProps = {
 				...props,
 				...slots,
 				// In Solid SSR mode, `ssr` creates the expected structure for `children`.
-				children: children != null ? ssr(`<astro-slot>${children}</astro-slot>`) : children,
+				children: children != null ? ssr(`<${tagName}>${children}</${tagName}>`) : children,
 			};
 
 			return createComponent(Component, newProps);
@@ -51,4 +53,5 @@ function renderToStaticMarkup(
 export default {
 	check,
 	renderToStaticMarkup,
+	supportsAstroStaticSlot: true,
 };

packages/integrations/svelte/server.js

@@ -2,11 +2,17 @@ function check(Component) {
 	return Component['render'] && Component['$$render'];
 }
 
-async function renderToStaticMarkup(Component, props, slotted) {
+function needsHydration(metadata) {
+	// Adjust how this is hydrated only when the version of Astro supports `astroStaticSlot`
+	return metadata.astroStaticSlot ? !!metadata.hydrate : true;
+}
+
+async function renderToStaticMarkup(Component, props, slotted, metadata) {
+	const tagName = needsHydration(metadata) ? 'astro-slot' : 'astro-static-slot';
 	const slots = {};
 	for (const [key, value] of Object.entries(slotted)) {
 		slots[key] = () =>
-			`<astro-slot${key === 'default' ? '' : ` name="${key}"`}>${value}</astro-slot>`;
+			`<${tagName}${key === 'default' ? '' : ` name="${key}"`}>${value}</${tagName}>`;
 	}
 	const { html } = Component.render(props, { $$slots: slots });
 	return { html };
@@ -15,4 +21,5 @@ async function renderToStaticMarkup(Component, props, slotted) {
 export default {
 	check,
 	renderToStaticMarkup,
+	supportsAstroStaticSlot: true,
 };

packages/integrations/vue/server.js

@@ -7,10 +7,15 @@ function check(Component) {
 	return !!Component['ssrRender'] || !!Component['__ssrInlineRender'];
 }
 
-async function renderToStaticMarkup(Component, props, slotted) {
+async function renderToStaticMarkup(Component, props, slotted, metadata) {
 	const slots = {};
 	for (const [key, value] of Object.entries(slotted)) {
-		slots[key] = () => h(StaticHtml, { value, name: key === 'default' ? undefined : key });
+		slots[key] = () => h(StaticHtml, {
+			value,
+			name: key === 'default' ? undefined : key,
+			// Adjust how this is hydrated only when the version of Astro supports `astroStaticSlot`
+			hydrate: metadata.astroStaticSlot ? !!metadata.hydrate : true,
+		});
 	}
 	const app = createSSRApp({ render: () => h(Component, props, slots) });
 	await setup(app);
@@ -21,4 +26,5 @@ async function renderToStaticMarkup(Component, props, slotted) {
 export default {
 	check,
 	renderToStaticMarkup,
+	supportsAstroStaticSlot: true,
 };

packages/integrations/vue/static-html.js

@@ -10,10 +10,12 @@ const StaticHtml = defineComponent({
 	props: {
 		value: String,
 		name: String,
+		hydrate: Boolean,
 	},
-	setup({ name, value }) {
+	setup({ name, value, hydrate }) {
 		if (!value) return () => null;
-		return () => h('astro-slot', { name, innerHTML: value });
+		let tagName = hydrate ? 'astro-slot' : 'astro-static-slot';
+		return () => h(tagName, { name, innerHTML: value });
 	},
 });
 

pnpm-lock.yaml

@@ -2194,18 +2194,42 @@ importers:
 
   packages/astro/test/fixtures/astro-slots-nested:
     dependencies:
+      '@astrojs/preact':
+        specifier: workspace:*
+        version: link:../../../../integrations/preact
       '@astrojs/react':
         specifier: workspace:*
         version: link:../../../../integrations/react
+      '@astrojs/solid-js':
+        specifier: workspace:*
+        version: link:../../../../integrations/solid
+      '@astrojs/svelte':
+        specifier: workspace:*
+        version: link:../../../../integrations/svelte
+      '@astrojs/vue':
+        specifier: workspace:*
+        version: link:../../../../integrations/vue
       astro:
         specifier: workspace:*
         version: link:../../..
+      preact:
+        specifier: ^10.13.2
+        version: 10.13.2
       react:
         specifier: ^18.2.0
         version: 18.2.0
       react-dom:
         specifier: ^18.2.0
         version: 18.2.0(react@18.2.0)
+      solid-js:
+        specifier: ^1.7.4
+        version: 1.7.4
+      svelte:
+        specifier: ^3.58.0
+        version: 3.58.0
+      vue:
+        specifier: ^3.2.47
+        version: 3.2.47
 
   packages/astro/test/fixtures/before-hydration:
     dependencies: