Throw helpful errors when attempting to serialize cyclic references (#4646)

* fix(#4332): add helpful error on cyclic references

* chore: add changeset

* test(e2e): add cyclic reference test

* test(e2e): add preact integration

* chore: update lockfile

* fix: ensure vite client is loaded for 500 responses

Co-authored-by: Nate Moore <nate@astro.build>

.changeset/giant-beds-beam.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Add cyclic ref detection when serializing props

packages/astro/e2e/error-cyclic.test.js

@@ -0,0 +1,24 @@
+import { expect } from '@playwright/test';
+import { testFactory, getErrorOverlayMessage } from './test-utils.js';
+
+const test = testFactory({ root: './fixtures/error-cyclic/' });
+
+let devServer;
+
+test.beforeEach(async ({ astro }) => {
+	devServer = await astro.startDevServer();
+});
+
+test.afterEach(async ({ astro }) => {
+	await devServer.stop();
+	astro.resetAllFiles();
+});
+
+test.describe('Error: Cyclic Reference', () => {
+	test('overlay', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/'));
+
+		const message = await getErrorOverlayMessage(page);
+		expect(message).toMatch('Cyclic reference');
+	});
+});

packages/astro/e2e/fixtures/error-cyclic/astro.config.mjs

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

packages/astro/e2e/fixtures/error-cyclic/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@e2e/error-cyclic",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/preact": "workspace:*"
+  }
+}

packages/astro/e2e/fixtures/error-cyclic/src/components/PreactCounter.tsx

@@ -0,0 +1,17 @@
+import { useState } from 'preact/hooks';
+
+/** a counter written in Preact */
+export function PreactCounter({ children, id }) {
+	const [count, setCount] = useState(0);
+	const add = () => setCount((i) => i + 1);
+	const subtract = () => setCount((i) => i - 1);
+
+	return (
+		<div id={id} class="counter">
+			<button class="decrement" onClick={subtract}>-</button>
+			<pre>{count}</pre>
+			<button class="increment" onClick={add}>+</button>
+			<div class="children">{children}</div>
+		</div>
+	);
+}

packages/astro/e2e/fixtures/error-cyclic/src/pages/index.astro

@@ -0,0 +1,8 @@
+---
+import { PreactCounter } from '../components/PreactCounter'
+
+const cycle: any = { foo: ['bar'] }
+cycle.foo.push(cycle)
+---
+
+<PreactCounter client:load cycle={cycle} />

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

@@ -146,7 +146,7 @@ export async function generateHydrateScript(
 	if (renderer.clientEntrypoint) {
 		island.props['component-export'] = componentExport.value;
 		island.props['renderer-url'] = await result.resolve(decodeURI(renderer.clientEntrypoint));
-		island.props['props'] = escapeHTML(serializeProps(props));
+		island.props['props'] = escapeHTML(serializeProps(props, metadata));
 	}
 
 	island.props['ssr'] = '';

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

@@ -302,9 +302,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 
 	// Include componentExport name, componentUrl, and props in hash to dedupe identical islands
 	const astroId = shorthash(
-		`<!--${metadata.componentExport!.value}:${metadata.componentUrl}-->\n${html}\n${serializeProps(
+		`<!--${metadata.componentExport!.value}:${metadata.componentUrl}-->\n${html}\n${serializeProps(props, metadata)}`
-			props
-		)}`
 	);
 
 	const island = await generateHydrateScript(

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

@@ -1,3 +1,7 @@
+import type {
+	AstroComponentMetadata,
+} from '../../@types/astro';
+
 type ValueOf<T> = T[keyof T];
 
 const PROP_TYPE = {
@@ -11,19 +15,25 @@ const PROP_TYPE = {
 	URL: 7,
 };
 
-function serializeArray(value: any[]): any[] {
+function serializeArray(value: any[], metadata: AstroComponentMetadata): any[] {
-	return value.map((v) => convertToSerializedForm(v));
+	return value.map((v) => convertToSerializedForm(v, metadata));
 }
 
-function serializeObject(value: Record<any, any>): Record<any, any> {
+function serializeObject(value: Record<any, any>, metadata: AstroComponentMetadata): Record<any, any> {
+	if (cyclicRefs.has(value)) {
+		throw new Error(`Cyclic reference detected while serializing props for <${metadata.displayName} client:${metadata.hydrate}>!
+
+Cyclic references cannot be safely serialized for client-side usage. Please remove the cyclic reference.`)
+	}
+	cyclicRefs.add(value);
 	return Object.fromEntries(
 		Object.entries(value).map(([k, v]) => {
-			return [k, convertToSerializedForm(v)];
+			return [k, convertToSerializedForm(v, metadata)];
 		})
 	);
 }
 
-function convertToSerializedForm(value: any): [ValueOf<typeof PROP_TYPE>, any] {
+function convertToSerializedForm(value: any, metadata: AstroComponentMetadata): [ValueOf<typeof PROP_TYPE>, any] {
 	const tag = Object.prototype.toString.call(value);
 	switch (tag) {
 		case '[object Date]': {
@@ -33,10 +43,10 @@ function convertToSerializedForm(value: any): [ValueOf<typeof PROP_TYPE>, any] {
 			return [PROP_TYPE.RegExp, (value as RegExp).source];
 		}
 		case '[object Map]': {
-			return [PROP_TYPE.Map, JSON.stringify(serializeArray(Array.from(value as Map<any, any>)))];
+			return [PROP_TYPE.Map, JSON.stringify(serializeArray(Array.from(value as Map<any, any>), metadata))];
 		}
 		case '[object Set]': {
-			return [PROP_TYPE.Set, JSON.stringify(serializeArray(Array.from(value as Set<any>)))];
+			return [PROP_TYPE.Set, JSON.stringify(serializeArray(Array.from(value as Set<any>), metadata))];
 		}
 		case '[object BigInt]': {
 			return [PROP_TYPE.BigInt, (value as bigint).toString()];
@@ -45,11 +55,11 @@ function convertToSerializedForm(value: any): [ValueOf<typeof PROP_TYPE>, any] {
 			return [PROP_TYPE.URL, (value as URL).toString()];
 		}
 		case '[object Array]': {
-			return [PROP_TYPE.JSON, JSON.stringify(serializeArray(value))];
+			return [PROP_TYPE.JSON, JSON.stringify(serializeArray(value, metadata))];
 		}
 		default: {
 			if (value !== null && typeof value === 'object') {
-				return [PROP_TYPE.Value, serializeObject(value)];
+				return [PROP_TYPE.Value, serializeObject(value, metadata)];
 			} else {
 				return [PROP_TYPE.Value, value];
 			}
@@ -57,6 +67,9 @@ function convertToSerializedForm(value: any): [ValueOf<typeof PROP_TYPE>, any] {
 	}
 }
 
-export function serializeProps(props: any) {
+let cyclicRefs = new WeakSet<any>();
-	return JSON.stringify(serializeObject(props));
+export function serializeProps(props: any, metadata: AstroComponentMetadata) {
+	const serialized = JSON.stringify(serializeObject(props, metadata));
+	cyclicRefs = new WeakSet<any>();
+	return serialized;
 }

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

@@ -120,6 +120,7 @@ async function handle500Response(
 ) {
 	res.on('close', () => setTimeout(() => viteServer.ws.send(getViteErrorPayload(err)), 200));
 	if (res.headersSent) {
+		res.write(`<script type="module" src="/@vite/client"></script>`)
 		res.end();
 	} else {
 		writeHtmlResponse(

pnpm-lock.yaml

@@ -585,6 +585,14 @@ importers:
       '@astrojs/vue': link:../../../../integrations/vue
       astro: link:../../..
 
+  packages/astro/e2e/fixtures/error-cyclic:
+    specifiers:
+      '@astrojs/preact': workspace:*
+      astro: workspace:*
+    dependencies:
+      '@astrojs/preact': link:../../../../integrations/preact
+      astro: link:../../..
+
   packages/astro/e2e/fixtures/error-react-spectrum:
     specifiers:
       '@adobe/react-spectrum': ^3.18.0