diff --git a/.changeset/giant-beds-beam.md b/.changeset/giant-beds-beam.md new file mode 100644 index 0000000000..0aaae5db6f --- /dev/null +++ b/.changeset/giant-beds-beam.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Add cyclic ref detection when serializing props diff --git a/packages/astro/e2e/error-cyclic.test.js b/packages/astro/e2e/error-cyclic.test.js new file mode 100644 index 0000000000..78c3bd1ead --- /dev/null +++ b/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'); + }); +}); diff --git a/packages/astro/e2e/fixtures/error-cyclic/astro.config.mjs b/packages/astro/e2e/fixtures/error-cyclic/astro.config.mjs new file mode 100644 index 0000000000..08916b1fea --- /dev/null +++ b/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()], +}); diff --git a/packages/astro/e2e/fixtures/error-cyclic/package.json b/packages/astro/e2e/fixtures/error-cyclic/package.json new file mode 100644 index 0000000000..46cce7a02f --- /dev/null +++ b/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:*" + } +} diff --git a/packages/astro/e2e/fixtures/error-cyclic/src/components/PreactCounter.tsx b/packages/astro/e2e/fixtures/error-cyclic/src/components/PreactCounter.tsx new file mode 100644 index 0000000000..b0570046c4 --- /dev/null +++ b/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 ( +
+ +
{count}
+ +
{children}
+
+ ); +} diff --git a/packages/astro/e2e/fixtures/error-cyclic/src/pages/index.astro b/packages/astro/e2e/fixtures/error-cyclic/src/pages/index.astro new file mode 100644 index 0000000000..32d68994bf --- /dev/null +++ b/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) +--- + + diff --git a/packages/astro/src/runtime/server/hydration.ts b/packages/astro/src/runtime/server/hydration.ts index fea251b792..30688237ac 100644 --- a/packages/astro/src/runtime/server/hydration.ts +++ b/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'] = ''; diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 066a863539..389fdecf4f 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/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( - `\n${html}\n${serializeProps( - props - )}` + `\n${html}\n${serializeProps(props, metadata)}` ); const island = await generateHydrateScript( diff --git a/packages/astro/src/runtime/server/serialize.ts b/packages/astro/src/runtime/server/serialize.ts index 21996a9320..4de21c08da 100644 --- a/packages/astro/src/runtime/server/serialize.ts +++ b/packages/astro/src/runtime/server/serialize.ts @@ -1,3 +1,7 @@ +import type { + AstroComponentMetadata, +} from '../../@types/astro'; + type ValueOf = T[keyof T]; const PROP_TYPE = { @@ -11,19 +15,25 @@ const PROP_TYPE = { URL: 7, }; -function serializeArray(value: any[]): any[] { - return value.map((v) => convertToSerializedForm(v)); +function serializeArray(value: any[], metadata: AstroComponentMetadata): any[] { + return value.map((v) => convertToSerializedForm(v, metadata)); } -function serializeObject(value: Record): Record { +function serializeObject(value: Record, metadata: AstroComponentMetadata): Record { + 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, any] { +function convertToSerializedForm(value: any, metadata: AstroComponentMetadata): [ValueOf, any] { const tag = Object.prototype.toString.call(value); switch (tag) { case '[object Date]': { @@ -33,10 +43,10 @@ function convertToSerializedForm(value: any): [ValueOf, any] { return [PROP_TYPE.RegExp, (value as RegExp).source]; } case '[object Map]': { - return [PROP_TYPE.Map, JSON.stringify(serializeArray(Array.from(value as Map)))]; + return [PROP_TYPE.Map, JSON.stringify(serializeArray(Array.from(value as Map), metadata))]; } case '[object Set]': { - return [PROP_TYPE.Set, JSON.stringify(serializeArray(Array.from(value as Set)))]; + return [PROP_TYPE.Set, JSON.stringify(serializeArray(Array.from(value as Set), metadata))]; } case '[object BigInt]': { return [PROP_TYPE.BigInt, (value as bigint).toString()]; @@ -45,11 +55,11 @@ function convertToSerializedForm(value: any): [ValueOf, 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, any] { } } -export function serializeProps(props: any) { - return JSON.stringify(serializeObject(props)); +let cyclicRefs = new WeakSet(); +export function serializeProps(props: any, metadata: AstroComponentMetadata) { + const serialized = JSON.stringify(serializeObject(props, metadata)); + cyclicRefs = new WeakSet(); + return serialized; } diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts index 525ec0a86b..883e3c91d1 100644 --- a/packages/astro/src/vite-plugin-astro-server/index.ts +++ b/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(``) res.end(); } else { writeHtmlResponse( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9e7446c688..8417f6568b 100644 --- a/pnpm-lock.yaml +++ b/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