Fix handling circular references correctly in objects (closes #8663)

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
 

packages/expect/src/__tests__/__snapshots__/matchers.test.js.snap

@@ -4092,6 +4092,128 @@ Expected: not <green>Set {2, 1}</>
 Received:     <red>Set {1, 2}</>"
 `;
 
+exports[`toMatchObject() circular references simple circular references {pass: false} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "world", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<green>-   \\"a\\": \\"world\\",</>
+<red>+   \\"a\\": \\"hello\\",</>
+<dim>    \\"ref\\": [Circular],</>
+<dim>  }</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: false} expect({"ref": "not a ref"}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<green>-   \\"ref\\": [Circular],</>
+<red>+   \\"ref\\": \\"not a ref\\",</>
+<dim>  }</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: false} expect({}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<green>- Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<green>-   \\"ref\\": [Circular],</>
+<green>- }</>
+<red>+ Object {}</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({"a": "hello", "ref": [Circular]}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{\\"a\\": \\"hello\\", \\"ref\\": [Circular]}</>"
+`;
+
+exports[`toMatchObject() circular references simple circular references {pass: true} expect({"a": "hello", "ref": [Circular]}).toMatchObject({}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{}</>
+Received:     <red>{\\"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`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<red>+   \\"a\\": \\"world\\",</>
+<dim>    \\"nestedObj\\": Object {</>
+<dim>      \\"parentObj\\": [Circular],</>
+<dim>    },</>
+<dim>  }</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: false} expect({"nestedObj": {"parentObj": "not the parent ref"}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<dim>    \\"nestedObj\\": Object {</>
+<green>-     \\"parentObj\\": [Circular],</>
+<red>+     \\"parentObj\\": \\"not the parent ref\\",</>
+<dim>    },</>
+<dim>  }</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: false} expect({}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<green>- Object {</>
+<green>-   \\"a\\": \\"hello\\",</>
+<green>-   \\"nestedObj\\": Object {</>
+<green>-     \\"parentObj\\": [Circular],</>
+<green>-   },</>
+<green>- }</>
+<red>+ Object {}</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({"a": "hello", "nestedObj": {"parentObj": [Circular]}}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}</>"
+`;
+
+exports[`toMatchObject() circular references transitive circular references {pass: true} expect({"a": "hello", "nestedObj": {"parentObj": [Circular]}}).toMatchObject({}) 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+Expected: not <green>{}</>
+Received:     <red>{\\"a\\": \\"hello\\", \\"nestedObj\\": {\\"parentObj\\": [Circular]}}</>"
+`;
+
+exports[`toMatchObject() does not match properties up in the prototype chain 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
+
+<green>- Expected</>
+<red>+ Received</>
+
+<dim>  Object {</>
+<dim>    \\"other\\": \\"child\\",</>
+<green>-   \\"ref\\": [Circular],</>
+<dim>  }</>"
+`;
+
 exports[`toMatchObject() throws expect("44").toMatchObject({}) 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toMatchObject<dim>(</><green>expected</><dim>)</>
 

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 circularObjA1 = {a: 'hello'};
+      circularObjA1.ref = circularObjA1;
+
+      const circularObjB = {a: 'world'};
+      circularObjB.ref = circularObjB;
+
+      const circularObjA2 = {a: 'hello'};
+      circularObjA2.ref = circularObjA2;
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.ref = 'not a ref';
+
+      testNotToMatchSnapshots([
+        [circularObjA1, {}],
+        [circularObjA2, circularObjA1],
+      ]);
+
+      testToMatchSnapshots([
+        [{}, circularObjA1],
+        [circularObjA1, circularObjB],
+        [primitiveInsteadOfRef, circularObjA1],
+      ]);
+    });
+
+    describe('transitive circular references', () => {
+      const transitiveCircularObjA1 = {a: 'hello'};
+      transitiveCircularObjA1.nestedObj = {parentObj: transitiveCircularObjA1};
+
+      const transitiveCircularObjA2 = {a: 'hello'};
+      transitiveCircularObjA2.nestedObj = {
+        parentObj: transitiveCircularObjA2,
+      };
+
+      const transitiveCircularObjB = {a: 'world'};
+      transitiveCircularObjB.nestedObj = {
+        parentObj: transitiveCircularObjB,
+      };
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.nestedObj = {
+        parentObj: 'not the parent ref',
+      };
+
+      testNotToMatchSnapshots([
+        [transitiveCircularObjA1, {}],
+        [transitiveCircularObjA2, transitiveCircularObjA1],
+      ]);
+
+      testToMatchSnapshots([
+        [{}, transitiveCircularObjA1],
+        [transitiveCircularObjB, transitiveCircularObjA1],
+        [primitiveInsteadOfRef, transitiveCircularObjA1],
+      ]);
+    });
+  });
+
+  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, {}],
@@ -1628,4 +1694,20 @@ describe('toMatchObject()', () => {
       ).toThrowErrorMatchingSnapshot();
     });
   });
+
+  it('does not match properties up in the prototype chain', () => {
+    const a = {};
+    a.ref = a;
+
+    const b = Object.create(a);
+    b.other = 'child';
+
+    const matcher = {other: 'child'};
+    matcher.ref = matcher;
+
+    jestExpect(b).not.toMatchObject(matcher);
+    expect(() =>
+      jestExpect(b).toMatchObject(matcher),
+    ).toThrowErrorMatchingSnapshot();
+  });
 });

packages/expect/src/__tests__/utils.test.js

@@ -164,6 +164,74 @@ describe('getObjectSubset()', () => {
       },
     );
   });
+
+  describe('calculating subsets of objects with circular references', () => {
+    test('simple circular references', () => {
+      const nonCircularObj = {a: 'world', b: 'something'};
+
+      const circularObjA = {a: 'hello'};
+      circularObjA.ref = circularObjA;
+
+      const circularObjB = {a: 'world'};
+      circularObjB.ref = circularObjB;
+
+      const primitiveInsteadOfRef = {b: 'something'};
+      primitiveInsteadOfRef.ref = 'not a ref';
+
+      const nonCircularRef = {b: 'something'};
+      nonCircularRef.ref = {};
+
+      expect(getObjectSubset(circularObjA, nonCircularObj)).toEqual({
+        a: 'hello',
+      });
+      expect(getObjectSubset(nonCircularObj, circularObjA)).toEqual({
+        a: 'world',
+      });
+
+      expect(getObjectSubset(circularObjB, circularObjA)).toEqual(circularObjB);
+
+      expect(getObjectSubset(primitiveInsteadOfRef, circularObjA)).toEqual({
+        ref: 'not a ref',
+      });
+      expect(getObjectSubset(nonCircularRef, circularObjA)).toEqual({
+        ref: {},
+      });
+    });
+
+    test('transitive circular references', () => {
+      const nonCircularObj = {a: 'world', b: 'something'};
+
+      const transitiveCircularObjA = {a: 'hello'};
+      transitiveCircularObjA.nestedObj = {parentObj: transitiveCircularObjA};
+
+      const transitiveCircularObjB = {a: 'world'};
+      transitiveCircularObjB.nestedObj = {parentObj: transitiveCircularObjB};
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.nestedObj = {otherProp: 'not the parent ref'};
+
+      const nonCircularRef = {};
+      nonCircularRef.nestedObj = {otherProp: {}};
+
+      expect(getObjectSubset(transitiveCircularObjA, nonCircularObj)).toEqual({
+        a: 'hello',
+      });
+      expect(getObjectSubset(nonCircularObj, transitiveCircularObjA)).toEqual({
+        a: 'world',
+      });
+
+      expect(
+        getObjectSubset(transitiveCircularObjB, transitiveCircularObjA),
+      ).toEqual(transitiveCircularObjB);
+
+      expect(
+        getObjectSubset(primitiveInsteadOfRef, transitiveCircularObjA),
+      ).toEqual({nestedObj: {otherProp: 'not the parent ref'}});
+      expect(getObjectSubset(nonCircularRef, transitiveCircularObjA)).toEqual({
+        nestedObj: {otherProp: {}},
+      });
+    });
+  });
 });
 
 describe('emptyObject()', () => {
@@ -202,6 +270,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 circularObjA1 = {a: 'hello'};
+      circularObjA1.ref = circularObjA1;
+
+      const circularObjA2 = {a: 'hello'};
+      circularObjA2.ref = circularObjA2;
+
+      const circularObjB = {a: 'world'};
+      circularObjB.ref = circularObjB;
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.ref = 'not a ref';
+
+      expect(subsetEquality(circularObjA1, {})).toBe(true);
+      expect(subsetEquality({}, circularObjA1)).toBe(false);
+      expect(subsetEquality(circularObjA2, circularObjA1)).toBe(true);
+      expect(subsetEquality(circularObjB, circularObjA1)).toBe(false);
+      expect(subsetEquality(primitiveInsteadOfRef, circularObjA1)).toBe(false);
+    });
+
+    test('transitive circular references', () => {
+      const transitiveCircularObjA1 = {a: 'hello'};
+      transitiveCircularObjA1.nestedObj = {parentObj: transitiveCircularObjA1};
+
+      const transitiveCircularObjA2 = {a: 'hello'};
+      transitiveCircularObjA2.nestedObj = {
+        parentObj: transitiveCircularObjA2,
+      };
+
+      const transitiveCircularObjB = {a: 'world'};
+      transitiveCircularObjB.nestedObj = {
+        parentObj: transitiveCircularObjB,
+      };
+
+      const primitiveInsteadOfRef = {};
+      primitiveInsteadOfRef.nestedObj = {
+        parentObj: 'not the parent ref',
+      };
+
+      expect(subsetEquality(transitiveCircularObjA1, {})).toBe(true);
+      expect(subsetEquality({}, transitiveCircularObjA1)).toBe(false);
+      expect(
+        subsetEquality(transitiveCircularObjA2, transitiveCircularObjA1),
+      ).toBe(true);
+      expect(
+        subsetEquality(transitiveCircularObjB, transitiveCircularObjA1),
+      ).toBe(false);
+      expect(
+        subsetEquality(primitiveInsteadOfRef, transitiveCircularObjA1),
+      ).toBe(false);
+    });
+  });
 });
 
 describe('iterableEquality', () => {

packages/expect/src/utils.ts

@@ -104,7 +104,11 @@ export const getPath = (
 
 // Strip properties from object that are not present in the subset. Useful for
 // printing the diff for toMatchObject() without adding unrelated noise.
-export const getObjectSubset = (object: any, subset: any): any => {
+export const getObjectSubset = (
+  object: any,
+  subset: any,
+  seenReferences: WeakMap<object, boolean> = new WeakMap(),
+): any => {
   if (Array.isArray(object)) {
     if (Array.isArray(subset) && subset.length === object.length) {
       return subset.map((sub: any, i: number) =>
@@ -113,18 +117,17 @@ export const getObjectSubset = (object: any, subset: any): any => {
     }
   } else if (object instanceof Date) {
     return object;
-  } else if (
-    typeof object === 'object' &&
-    object !== null &&
-    typeof subset === 'object' &&
-    subset !== null
-  ) {
+  } else if (isObject(object) && isObject(subset)) {
     const trimmed: any = {};
-    Object.keys(subset)
-      .filter(key => hasOwnProperty(object, key))
-      .forEach(
-        key => (trimmed[key] = getObjectSubset(object[key], subset[key])),
-      );
+    seenReferences.set(object, trimmed);
+
+    Object.keys(object)
+      .filter(key => hasOwnProperty(subset, key))
+      .forEach(key => {
+        trimmed[key] = seenReferences.has(object[key])
+          ? seenReferences.get(object[key])
+          : getObjectSubset(object[key], subset[key], seenReferences);
+      });
 
     if (Object.keys(trimmed).length > 0) {
       return trimmed;
@@ -257,9 +260,10 @@ export const iterableEquality = (
   return true;
 };
 
+const isObject = (a: any) => a !== null && typeof a === 'object';
+
 const isObjectWithKeys = (a: any) =>
-  a !== null &&
-  typeof a === 'object' &&
+  isObject(a) &&
   !(a instanceof Error) &&
   !(a instanceof Array) &&
   !(a instanceof Date);
@@ -268,16 +272,36 @@ export const subsetEquality = (
   object: any,
   subset: any,
 ): undefined | boolean => {
-  if (!isObjectWithKeys(subset)) {
-    return undefined;
-  }
+  // 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, boolean> = new 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()(object, subset);
 };
 
 export const typeEquality = (a: any, b: any) => {