diff --git a/src/isEqual.ts b/src/isEqual.ts index a4a29b79..7eb831c8 100644 --- a/src/isEqual.ts +++ b/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(); 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); diff --git a/src/test/isEqual.test.ts b/src/test/isEqual.test.ts index 6d9c012c..3cdcd3cd 100644 --- a/src/test/isEqual.test.ts +++ b/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); + }); });