From 15a6555d21e0a74bfcff057eea006f541f239b47 Mon Sep 17 00:00:00 2001 From: Lucas Fernandes da Costa Date: Sat, 13 Jul 2019 18:09:00 +0100 Subject: [PATCH] fix: handle circular references correctly in objects (closes #8663) --- CHANGELOG.md | 1 + .../__snapshots__/matchers.test.js.snap | 86 ++++++++++++++ .../expect/src/__tests__/matchers.test.js | 110 ++++++++++++++---- packages/expect/src/__tests__/utils.test.js | 54 +++++++++ packages/expect/src/utils.ts | 40 +++++-- 5 files changed, 260 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f9aa69ebc30..24eb8a965277 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - `[jest-core]` Fix incorrect `passWithNoTests` warning ([#8595](https://github.com/facebook/jest/pull/8595)) - `[jest-snapshots]` Fix test retries that contain snapshots ([#8629](https://github.com/facebook/jest/pull/8629)) - `[jest-mock]` Fix incorrect assignments when restoring mocks in instances where they originally didn't exist ([#8631](https://github.com/facebook/jest/pull/8631)) +- `[expect]` Fix stack overflow when matching objects with circular references ([#8687](https://github.com/facebook/jest/pull/8687)) ### Chore & Maintenance diff --git a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap index 38ef42eebc21..ab1a435ca4c2 100644 --- a/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap @@ -4092,6 +4092,92 @@ Expected: not Set {2, 1} Received: Set {1, 2}" `; +exports[`toMatchObject() circular references simple circular references {pass: false} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "world", "ref": [Circular]}) 1`] = `"Maximum call stack size exceeded"`; + +exports[`toMatchObject() circular references simple circular references {pass: false} expect({"ref": "not a ref"}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = ` +"expect(received).toMatchObject(expected) + +- Expected ++ Received + + Object { +- \\"a\\": \\"hello\\", +- \\"ref\\": [Circular], ++ \\"ref\\": \\"not a ref\\", + }" +`; + +exports[`toMatchObject() circular references simple circular references {pass: false} expect({}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = ` +"expect(received).toMatchObject(expected) + +- Expected ++ Received + +- Object { +- \\"a\\": \\"hello\\", +- \\"ref\\": [Circular], +- } ++ Object {}" +`; + +exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = ` +"expect(received).not.toMatchObject(expected) + +Expected: not {\\"a\\": \\"hello\\", \\"ref\\": [Circular]}" +`; + +exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({}) 1`] = ` +"expect(received).not.toMatchObject(expected) + +Expected: not {} +Received: {\\"a\\": \\"hello\\", \\"ref\\": [Circular]}" +`; + +exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"a": "world", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `"Maximum call stack size exceeded"`; + +exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"nestedObj": {"parentObj": "not the parent ref"}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = ` +"expect(received).toMatchObject(expected) + +- Expected ++ Received + + Object { +- \\"a\\": \\"hello\\", + \\"nestedObj\\": Object { +- \\"parentObj\\": [Circular], ++ \\"parentObj\\": \\"not the parent ref\\", + }, + }" +`; + +exports[`toMatchObject() circular references transitive circular references {pass: false} expect({}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = ` +"expect(received).toMatchObject(expected) + +- Expected ++ Received + +- Object { +- \\"a\\": \\"hello\\", +- \\"nestedObj\\": Object { +- \\"parentObj\\": [Circular], +- }, +- } ++ Object {}" +`; + +exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = ` +"expect(received).not.toMatchObject(expected) + +Expected: not {\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}" +`; + +exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({}) 1`] = ` +"expect(received).not.toMatchObject(expected) + +Expected: not {} +Received: {\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}" +`; + exports[`toMatchObject() throws expect("44").toMatchObject({}) 1`] = ` "expect(received).toMatchObject(expected) diff --git a/packages/expect/src/__tests__/matchers.test.js b/packages/expect/src/__tests__/matchers.test.js index 9b5661339c0f..7b2dba4ff358 100644 --- a/packages/expect/src/__tests__/matchers.test.js +++ b/packages/expect/src/__tests__/matchers.test.js @@ -1537,7 +1537,91 @@ describe('toMatchObject()', () => { } } - [ + const testNotToMatchSnapshots = tuples => { + tuples.forEach(([n1, n2]) => { + it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify( + n2, + )})`, () => { + jestExpect(n1).toMatchObject(n2); + expect(() => + jestExpect(n1).not.toMatchObject(n2), + ).toThrowErrorMatchingSnapshot(); + }); + }); + }; + + const testToMatchSnapshots = tuples => { + tuples.forEach(([n1, n2]) => { + it(`{pass: false} expect(${stringify(n1)}).toMatchObject(${stringify( + n2, + )})`, () => { + jestExpect(n1).not.toMatchObject(n2); + expect(() => + jestExpect(n1).toMatchObject(n2), + ).toThrowErrorMatchingSnapshot(); + }); + }); + }; + + describe('circular references', () => { + describe('simple circular references', () => { + const circularObj = {a: 'hello'}; + circularObj.ref = circularObj; + + const differentCircularObj = {a: 'world'}; + differentCircularObj.ref = differentCircularObj; + + const otherCircularObj = {a: 'hello'}; + otherCircularObj.ref = otherCircularObj; + + const primitiveInsteadOfRef = {}; + primitiveInsteadOfRef.ref = 'not a ref'; + + testNotToMatchSnapshots([ + [circularObj, {}], + [otherCircularObj, circularObj], + ]); + + testToMatchSnapshots([ + [{}, circularObj], + [circularObj, differentCircularObj], + [primitiveInsteadOfRef, circularObj], + ]); + }); + + describe('transitive circular references', () => { + const transitiveCircularObj = {a: 'hello'}; + transitiveCircularObj.nestedObj = {parentObj: transitiveCircularObj}; + + const otherTransitiveCircularObj = {a: 'hello'}; + otherTransitiveCircularObj.nestedObj = { + parentObj: otherTransitiveCircularObj, + }; + + const differentTransitiveCircularObj = {a: 'world'}; + differentTransitiveCircularObj.nestedObj = { + parentObj: differentTransitiveCircularObj, + }; + + const primitiveInsteadOfRef = {}; + primitiveInsteadOfRef.nestedObj = { + parentObj: 'not the parent ref', + }; + + testNotToMatchSnapshots([ + [transitiveCircularObj, {}], + [otherTransitiveCircularObj, transitiveCircularObj], + ]); + + testToMatchSnapshots([ + [{}, transitiveCircularObj], + [differentTransitiveCircularObj, transitiveCircularObj], + [primitiveInsteadOfRef, transitiveCircularObj], + ]); + }); + }); + + testNotToMatchSnapshots([ [{a: 'b', c: 'd'}, {a: 'b'}], [{a: 'b', c: 'd'}, {a: 'b', c: 'd'}], [{a: 'b', t: {x: {r: 'r'}, z: 'z'}}, {a: 'b', t: {z: 'z'}}], @@ -1560,18 +1644,9 @@ describe('toMatchObject()', () => { [new Error('bar'), {message: 'bar'}], [new Foo(), {a: undefined, b: 'b'}], [Object.assign(Object.create(null), {a: 'b'}), {a: 'b'}], - ].forEach(([n1, n2]) => { - it(`{pass: true} expect(${stringify(n1)}).toMatchObject(${stringify( - n2, - )})`, () => { - jestExpect(n1).toMatchObject(n2); - expect(() => - jestExpect(n1).not.toMatchObject(n2), - ).toThrowErrorMatchingSnapshot(); - }); - }); + ]); - [ + testToMatchSnapshots([ [{a: 'b', c: 'd'}, {e: 'b'}], [{a: 'b', c: 'd'}, {a: 'b!', c: 'd'}], [{a: 'a', c: 'd'}, {a: jestExpect.any(Number)}], @@ -1597,16 +1672,7 @@ describe('toMatchObject()', () => { [[1, 2, 3], [1, 2, 2]], [new Error('foo'), new Error('bar')], [Object.assign(Object.create(null), {a: 'b'}), {c: 'd'}], - ].forEach(([n1, n2]) => { - it(`{pass: false} expect(${stringify(n1)}).toMatchObject(${stringify( - n2, - )})`, () => { - jestExpect(n1).not.toMatchObject(n2); - expect(() => - jestExpect(n1).toMatchObject(n2), - ).toThrowErrorMatchingSnapshot(); - }); - }); + ]); [ [null, {}], diff --git a/packages/expect/src/__tests__/utils.test.js b/packages/expect/src/__tests__/utils.test.js index 0850053b120a..99d115d30265 100644 --- a/packages/expect/src/__tests__/utils.test.js +++ b/packages/expect/src/__tests__/utils.test.js @@ -202,6 +202,60 @@ describe('subsetEquality()', () => { test('undefined does not return errors', () => { expect(subsetEquality(undefined, {foo: 'bar'})).not.toBeTruthy(); }); + + describe('matching subsets with circular references', () => { + test('simple circular references', () => { + const circularObj = {a: 'hello'}; + circularObj.ref = circularObj; + + const otherCircularObj = {a: 'hello'}; + otherCircularObj.ref = otherCircularObj; + + const differentCircularObj = {a: 'world'}; + differentCircularObj.ref = differentCircularObj; + + const primitiveInsteadOfRef = {}; + primitiveInsteadOfRef.ref = 'not a ref'; + + expect(subsetEquality(circularObj, {})).toBe(true); + expect(subsetEquality({}, circularObj)).toBe(false); + expect(subsetEquality(otherCircularObj, circularObj)).toBe(true); + expect(subsetEquality(differentCircularObj, circularObj)).toBe(false); + expect(subsetEquality(primitiveInsteadOfRef, circularObj)).toBe(false); + }); + + test('transitive circular references', () => { + const transitiveCircularObj = {a: 'hello'}; + transitiveCircularObj.nestedObj = {parentObj: transitiveCircularObj}; + + const otherTransitiveCircularObj = {a: 'hello'}; + otherTransitiveCircularObj.nestedObj = { + parentObj: otherTransitiveCircularObj, + }; + + const differentTransitiveCircularObj = {a: 'world'}; + differentTransitiveCircularObj.nestedObj = { + parentObj: differentTransitiveCircularObj, + }; + + const primitiveInsteadOfRef = {}; + primitiveInsteadOfRef.nestedObj = { + parentObj: 'not the parent ref', + }; + + expect(subsetEquality(transitiveCircularObj, {})).toBe(true); + expect(subsetEquality({}, transitiveCircularObj)).toBe(false); + expect( + subsetEquality(otherTransitiveCircularObj, transitiveCircularObj), + ).toBe(true); + expect( + subsetEquality(differentTransitiveCircularObj, transitiveCircularObj), + ).toBe(false); + expect(subsetEquality(primitiveInsteadOfRef, transitiveCircularObj)).toBe( + false, + ); + }); + }); }); describe('iterableEquality', () => { diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index f8aa8aec01d3..7580c91d913f 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -268,16 +268,38 @@ export const subsetEquality = ( object: any, subset: any, ): undefined | boolean => { - if (!isObjectWithKeys(subset)) { - return undefined; - } + const seenReferences = new WeakMap(); + + // subsetEquality needs to keep track of the references + // it has already visited to avoid infinite loops in case + // there are circular references in the subset passed to it. + const subsetEqualityWithContext = ( + seenReferences: WeakMap, + ) => (object: any, subset: any): undefined | boolean => { + if (!isObjectWithKeys(subset)) { + return undefined; + } - return Object.keys(subset).every( - key => - object != null && - hasOwnProperty(object, key) && - equals(object[key], subset[key], [iterableEquality, subsetEquality]), - ); + return Object.keys(subset).every(key => { + if (isObjectWithKeys(subset[key])) { + if (seenReferences.get(subset[key])) { + return equals(object[key], subset[key], [iterableEquality]); + } + seenReferences.set(subset[key], true); + } + + return ( + object != null && + hasOwnProperty(object, key) && + equals(object[key], subset[key], [ + iterableEquality, + subsetEqualityWithContext(seenReferences), + ]) + ); + }); + }; + + return subsetEqualityWithContext(seenReferences)(object, subset); }; export const typeEquality = (a: any, b: any) => {