fix(isEqual): track ancestors, not every value the walk has seen

src/isEqual.ts

@@ -9,6 +9,9 @@ import warning from './warning';
  */
 function isEqual(obj1: any, obj2: any, shallow = false): boolean {
   // https://github.com/mapbox/mapbox-gl-js/pull/5979/files#diff-fde7145050c47cc3a306856efd5f9c3016e86e859de9afbd02c879be5067e58f
+  // The ANCESTORS of the value being compared, not every value seen so far: a cycle is a value
+  // reachable from itself, while the same reference met again in a sibling branch is a repeat and
+  // has to compare normally. Entries are released as the walk unwinds, below.
   const refSet = new Set<any>();
   function deepEqual(a: any, b: any, level = 1): boolean {
     const circular = refSet.has(a);
@@ -24,26 +27,31 @@ function isEqual(obj1: any, obj2: any, shallow = false): boolean {
     }
     refSet.add(a);
     const newLevel = level + 1;
-    if (Array.isArray(a)) {
-      if (!Array.isArray(b) || a.length !== b.length) {
-        return false;
-      }
-      for (let i = 0; i < a.length; i++) {
-        if (!deepEqual(a[i], b[i], newLevel)) {
+    try {
+      if (Array.isArray(a)) {
+        if (!Array.isArray(b) || a.length !== b.length) {
           return false;
         }
+        for (let i = 0; i < a.length; i++) {
+          if (!deepEqual(a[i], b[i], newLevel)) {
+            return false;
+          }
+        }
+        return true;
       }
-      return true;
-    }
-    if (a && b && typeof a === 'object' && typeof b === 'object') {
-      const keys = Object.keys(a);
-      if (keys.length !== Object.keys(b).length) {
-        return false;
+      if (a && b && typeof a === 'object' && typeof b === 'object') {
+        const keys = Object.keys(a);
+        if (keys.length !== Object.keys(b).length) {
+          return false;
+        }
+        return keys.every(key => deepEqual(a[key], b[key], newLevel));
       }
-      return keys.every(key => deepEqual(a[key], b[key], newLevel));
+      // other
+      return false;
+    } finally {
+      // The walk has left this branch, so `a` is no longer an ancestor of what comes next.
+      refSet.delete(a);
     }
-    // other
-    return false;
   }
 
   return deepEqual(obj1, obj2);

src/test/isEqual.test.ts

@@ -1,5 +1,5 @@
 import isEqual from '../isEqual';
-import warning from '../warning';
+import warning, { resetWarned } from '../warning';
 
 describe('isEqual', () => {
   let errorSpy: jest.SpyInstance;
@@ -8,6 +8,12 @@ describe('isEqual', () => {
     errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
   });
 
+  beforeEach(() => {
+    // `warning` is warningOnce: without this, a message emitted by an earlier test is suppressed
+    // here, and asserting that nothing warned would prove nothing.
+    resetWarned();
+  });
+
   afterEach(() => {
     errorSpy.mockReset();
   });
@@ -103,4 +109,50 @@ describe('isEqual', () => {
     const valueIsEqual = isEqual(obj, obj2);
     expect(valueIsEqual).toBe(false);
   });
+
+  const circularWarnings = () =>
+    errorSpy.mock.calls.filter(([message]) =>
+      String(message).includes('circular references'),
+    );
+
+  it('should equal when one side reuses a reference across keys', () => {
+    const shared: any[] = [];
+
+    const valueIsEqual = isEqual(
+      { errors: shared, warnings: shared },
+      { errors: [], warnings: [] },
+    );
+    expect(valueIsEqual).toBe(true);
+    expect(circularWarnings()).toHaveLength(0);
+  });
+
+  it('should equal when one side reuses an object across keys', () => {
+    const point = { x: 1 };
+
+    const valueIsEqual = isEqual(
+      { a: point, b: point },
+      { a: { x: 1 }, b: { x: 1 } },
+    );
+    expect(valueIsEqual).toBe(true);
+    expect(circularWarnings()).toHaveLength(0);
+  });
+
+  it('should equal when one side reuses a reference inside an array', () => {
+    const point = { x: 1 };
+
+    const valueIsEqual = isEqual([point, point], [{ x: 1 }, { x: 1 }]);
+    expect(valueIsEqual).toBe(true);
+    expect(circularWarnings()).toHaveLength(0);
+  });
+
+  it('should still detect a cycle reached through an array', () => {
+    const a: any = { list: [] };
+    a.list.push(a);
+    const b: any = { list: [] };
+    b.list.push(b);
+
+    const valueIsEqual = isEqual(a, b);
+    expect(valueIsEqual).toBe(false);
+    expect(circularWarnings().length).toBeGreaterThan(0);
+  });
 });