From 919df13b91eb561ae939e9be51e5a76ca97d8512 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Thu, 8 Sep 2022 17:02:29 -0500 Subject: [PATCH] Improve cyclic reference detection, now ignores non-cyclic shared references (#4684) * fix: improve cyclic reference detection, now ignores references that are not parent/child * fix: only track cyclic parents Co-authored-by: Nate Moore --- .changeset/lovely-clocks-tease.md | 5 ++ .../astro/src/runtime/server/serialize.ts | 50 ++++++++------ packages/astro/test/serialize.test.js | 67 +++++++++++++++++++ 3 files changed, 103 insertions(+), 19 deletions(-) create mode 100644 .changeset/lovely-clocks-tease.md create mode 100644 packages/astro/test/serialize.test.js diff --git a/.changeset/lovely-clocks-tease.md b/.changeset/lovely-clocks-tease.md new file mode 100644 index 000000000..d6000cf17 --- /dev/null +++ b/.changeset/lovely-clocks-tease.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes regression introduced in [#4646](https://github.com/withastro/astro/pull/4646) with better cyclic reference detection diff --git a/packages/astro/src/runtime/server/serialize.ts b/packages/astro/src/runtime/server/serialize.ts index 10812ab75..024a689b0 100644 --- a/packages/astro/src/runtime/server/serialize.ts +++ b/packages/astro/src/runtime/server/serialize.ts @@ -13,30 +13,44 @@ const PROP_TYPE = { URL: 7, }; -function serializeArray(value: any[], metadata: AstroComponentMetadata): any[] { - return value.map((v) => convertToSerializedForm(v, metadata)); -} - -function serializeObject( - value: Record, - metadata: AstroComponentMetadata -): Record { - if (cyclicRefs.has(value)) { +function serializeArray(value: any[], metadata: AstroComponentMetadata | Record = {}, parents = new WeakSet()): any[] { + if (parents.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( + parents.add(value); + const serialized = value.map((v) => { + return convertToSerializedForm(v, metadata, parents) + }); + parents.delete(value); + return serialized; +} + +function serializeObject( + value: Record, + metadata: AstroComponentMetadata | Record = {}, + parents = new WeakSet() +): Record { + if (parents.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.`); + } + parents.add(value); + const serialized = Object.fromEntries( Object.entries(value).map(([k, v]) => { - return [k, convertToSerializedForm(v, metadata)]; + return [k, convertToSerializedForm(v, metadata, parents)]; }) ); + parents.delete(value); + return serialized; } function convertToSerializedForm( value: any, - metadata: AstroComponentMetadata + metadata: AstroComponentMetadata | Record = {}, + parents = new WeakSet() ): [ValueOf, any] { const tag = Object.prototype.toString.call(value); switch (tag) { @@ -49,13 +63,13 @@ function convertToSerializedForm( case '[object Map]': { return [ PROP_TYPE.Map, - JSON.stringify(serializeArray(Array.from(value as Map), metadata)), + JSON.stringify(serializeArray(Array.from(value as Map), metadata, parents)), ]; } case '[object Set]': { return [ PROP_TYPE.Set, - JSON.stringify(serializeArray(Array.from(value as Set), metadata)), + JSON.stringify(serializeArray(Array.from(value as Set), metadata, parents)), ]; } case '[object BigInt]': { @@ -65,11 +79,11 @@ function convertToSerializedForm( return [PROP_TYPE.URL, (value as URL).toString()]; } case '[object Array]': { - return [PROP_TYPE.JSON, JSON.stringify(serializeArray(value, metadata))]; + return [PROP_TYPE.JSON, JSON.stringify(serializeArray(value, metadata, parents))]; } default: { if (value !== null && typeof value === 'object') { - return [PROP_TYPE.Value, serializeObject(value, metadata)]; + return [PROP_TYPE.Value, serializeObject(value, metadata, parents)]; } else { return [PROP_TYPE.Value, value]; } @@ -77,9 +91,7 @@ function convertToSerializedForm( } } -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/test/serialize.test.js b/packages/astro/test/serialize.test.js new file mode 100644 index 000000000..ad28638b2 --- /dev/null +++ b/packages/astro/test/serialize.test.js @@ -0,0 +1,67 @@ +import { expect } from 'chai'; +import { serializeProps } from '../dist/runtime/server/serialize.js'; + +describe('serialize', () => { + it('serializes a plain value', () => { + const input = { a: 1 }; + const output = `{"a":[0,1]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes an array', () => { + const input = { a: [0] }; + const output = `{"a":[1,"[[0,0]]"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes a regular expression', () => { + const input = { a: /b/ }; + const output = `{"a":[2,"b"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes a Date', () => { + const input = { a: new Date(0) }; + const output = `{"a":[3,"1970-01-01T00:00:00.000Z"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes a Map', () => { + const input = { a: new Map([[0, 1]]) }; + const output = `{"a":[4,"[[1,\\"[[0,0],[0,1]]\\"]]"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes a Set', () => { + const input = { a: new Set([0, 1, 2, 3]) }; + const output = `{"a":[5,"[[0,0],[0,1],[0,2],[0,3]]"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes a BigInt', () => { + const input = { a: BigInt('1') }; + const output = `{"a":[6,"1"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('serializes a URL', () => { + const input = { a: new URL('https://example.com/') }; + const output = `{"a":[7,"https://example.com/"]}`; + expect(serializeProps(input)).to.equal(output); + }) + it('cannot serialize a cyclic reference', () => { + const a = {} + a.b = a; + const input = { a }; + expect(() => serializeProps(input)).to.throw(/cyclic/); + }) + it('cannot serialize a cyclic array', () => { + const input = { foo: ['bar'] } + input.foo.push(input) + expect(() => serializeProps(input)).to.throw(/cyclic/); + }) + it('cannot serialize a deep cyclic reference', () => { + const a = { b: {}}; + a.b.c = a; + const input = { a }; + expect(() => serializeProps(input)).to.throw(/cyclic/); + }) + it('can serialize shared references that are not cyclic', () => { + const b = {} + const input = { a: { b, b }, b }; + expect(() => serializeProps(input)).not.to.throw(); + }) +})