From 8f6f41a55e4d172b9c42c9186f9368bd18513e66 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Jun 2022 08:21:08 +0000 Subject: [PATCH 1/3] Add tests to identify faulty cases --- packages/utils/test/normalize.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/utils/test/normalize.test.ts b/packages/utils/test/normalize.test.ts index e5d01de4e962..467dfda93964 100644 --- a/packages/utils/test/normalize.test.ts +++ b/packages/utils/test/normalize.test.ts @@ -285,6 +285,30 @@ describe('normalize()', () => { // @ts-ignore target lacks a construct signature expect(normalize([{ a }, { b: new B() }, c])).toEqual([{ a: 1 }, { b: 2 }, 3]); }); + + test('should return return normalized object even if toJSON throws', () => { + const subject = { a: 1, foo: 'bar' } as any; + subject.toJSON = () => { + throw new Error("I'm faulty!"); + }; + expect(normalize(subject)).toEqual({ a: 1, foo: 'bar', toJSON: '[Function: ]' }); + }); + + test('should return return object without circular references when toJSON returns an object with circular references', () => { + const subject: any = {}; + subject.toJSON = () => { + const egg: any = {}; + egg.chicken = egg; + return egg; + }; + expect(normalize(subject)).toEqual({ chicken: '[Circular ~]' }); + }); + + test('should detect circular reference when toJSON returns the original object', () => { + const subject: any = {}; + subject.toJSON = () => subject; + expect(normalize(subject)).toEqual('[Circular ~]'); + }); }); describe('changes unserializeable/global values/classes to its string representation', () => { From 92a2c94d142e8c34a4fcb3da612c2d3db02a71c1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Jun 2022 08:46:32 +0000 Subject: [PATCH 2/3] fix(utils): Handle `toJSON` methods that return circular references --- packages/utils/src/normalize.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/utils/src/normalize.ts b/packages/utils/src/normalize.ts index c88ab00b510f..f40304374137 100644 --- a/packages/utils/src/normalize.ts +++ b/packages/utils/src/normalize.ts @@ -77,16 +77,6 @@ function visit( ): Primitive | ObjOrArray { const [memoize, unmemoize] = memo; - // If the value has a `toJSON` method, see if we can bail and let it do the work - const valueWithToJSON = value as unknown & { toJSON?: () => Primitive | ObjOrArray }; - if (valueWithToJSON && typeof valueWithToJSON.toJSON === 'function') { - try { - return valueWithToJSON.toJSON(); - } catch (err) { - // pass (The built-in `toJSON` failed, but we can still try to do it ourselves) - } - } - // Get the simple cases out of the way first if (value === null || (['number', 'boolean', 'string'].includes(typeof value) && !isNaN(value))) { return value as Primitive; @@ -120,6 +110,18 @@ function visit( return '[Circular ~]'; } + // If the value has a `toJSON` method, we call it to extract more information + const valueWithToJSON = value as unknown & { toJSON?: () => unknown }; + if (valueWithToJSON && typeof valueWithToJSON.toJSON === 'function') { + try { + const jsonValue = valueWithToJSON.toJSON(); + // We need to normalize the return value of `.toJSON()` in case it has circular references + return visit('', jsonValue, depth - 1, maxProperties, memo); + } catch (err) { + // pass (The built-in `toJSON` failed, but we can still try to do it ourselves) + } + } + // At this point we know we either have an object or an array, we haven't seen it before, and we're going to recurse // because we haven't yet reached the max depth. Create an accumulator to hold the results of visiting each // property/entry, and keep track of the number of items we add to it. From 79a2e6829abde57467348e3380acee9be8af0724 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 28 Jun 2022 15:24:18 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Lukas Stracke Co-authored-by: Lukas Stracke --- packages/utils/test/normalize.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/utils/test/normalize.test.ts b/packages/utils/test/normalize.test.ts index 467dfda93964..f9d909798d37 100644 --- a/packages/utils/test/normalize.test.ts +++ b/packages/utils/test/normalize.test.ts @@ -286,7 +286,7 @@ describe('normalize()', () => { expect(normalize([{ a }, { b: new B() }, c])).toEqual([{ a: 1 }, { b: 2 }, 3]); }); - test('should return return normalized object even if toJSON throws', () => { + test('should return a normalized object even if toJSON throws', () => { const subject = { a: 1, foo: 'bar' } as any; subject.toJSON = () => { throw new Error("I'm faulty!"); @@ -294,7 +294,7 @@ describe('normalize()', () => { expect(normalize(subject)).toEqual({ a: 1, foo: 'bar', toJSON: '[Function: ]' }); }); - test('should return return object without circular references when toJSON returns an object with circular references', () => { + test('should return an object without circular references when toJSON returns an object with circular references', () => { const subject: any = {}; subject.toJSON = () => { const egg: any = {};