Skip to content

Commit

Permalink
Throw helpful errors when attempting to serialize cyclic references (#…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
natemoo-re and natemoo-re committed Sep 7, 2022
1 parent b5a91d4 commit 98f242c
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/giant-beds-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Add cyclic ref detection when serializing props
24 changes: 24 additions & 0 deletions packages/astro/e2e/error-cyclic.test.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
7 changes: 7 additions & 0 deletions packages/astro/e2e/fixtures/error-cyclic/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from 'astro/config';
import preact from '@astrojs/preact';

// https://astro.build/config
export default defineConfig({
integrations: [preact()],
});
9 changes: 9 additions & 0 deletions packages/astro/e2e/fixtures/error-cyclic/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@e2e/error-cyclic",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*",
"@astrojs/preact": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -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>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
import { PreactCounter } from '../components/PreactCounter'
const cycle: any = { foo: ['bar'] }
cycle.foo.push(cycle)
---

<PreactCounter client:load cycle={cycle} />
2 changes: 1 addition & 1 deletion packages/astro/src/runtime/server/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'] = '';
Expand Down
4 changes: 1 addition & 3 deletions packages/astro/src/runtime/server/render/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
props
)}`
`<!--${metadata.componentExport!.value}:${metadata.componentUrl}-->\n${html}\n${serializeProps(props, metadata)}`
);

const island = await generateHydrateScript(
Expand Down
35 changes: 24 additions & 11 deletions packages/astro/src/runtime/server/serialize.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import type {
AstroComponentMetadata,
} from '../../@types/astro';

type ValueOf<T> = T[keyof T];

const PROP_TYPE = {
Expand All @@ -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<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]': {
Expand All @@ -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()];
Expand All @@ -45,18 +55,21 @@ 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];
}
}
}
}

export function serializeProps(props: any) {
return JSON.stringify(serializeObject(props));
let cyclicRefs = new WeakSet<any>();
export function serializeProps(props: any, metadata: AstroComponentMetadata) {
const serialized = JSON.stringify(serializeObject(props, metadata));
cyclicRefs = new WeakSet<any>();
return serialized;
}
1 change: 1 addition & 0 deletions packages/astro/src/vite-plugin-astro-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 98f242c

Please sign in to comment.