Fixes #2: False-positive circular check

src/diff/diffMaps.ts

@@ -64,6 +64,9 @@ function diffMaps({
     );
   }
 
+  seen.delete(lhs);
+  seen.delete(rhs);
+
   return buildResult(
     rhs,
     result,

src/diff/diffObjects.ts

@@ -72,6 +72,9 @@ function diffObjects({
     );
   }
 
+  seen.delete(lhs);
+  seen.delete(rhs);
+
   return buildResult(
     rhs,
     result,

src/diff/diffSets.ts

@@ -7,6 +7,7 @@ import {
   includeDiffType,
   lastPathValue,
   maxKeysSecurityCheck,
+  stringify,
   timeoutSecurityCheck,
 } from './shared';
 
@@ -72,13 +73,16 @@ function diffSets({
     if (includeDiffType(type, config)) {
       result.push({
         type,
-        str: `${JSON.stringify(value)},`,
+        str: `${stringify(value, config)},`,
         depth,
         path: [...updatedPath, lastPathValue(type, value)],
       });
     }
   }
 
+  seen.delete(lhs);
+  seen.delete(rhs);
+
   return buildResult(
     rhs,
     result,

src/diff/index.ts

@@ -6,7 +6,6 @@ import diffConstructors from './diffConstructors';
 import { ChangeType } from '../utils/constants';
 import { areObjects, isNullOrUndefined, isPrimitive } from '../utils/fns';
 import {
-  createReplacer,
   includeDiffType,
   lastPathValue,
   timeoutSecurityCheck,
@@ -74,8 +73,8 @@ function recursiveDiff({
     }
 
     const parentDepth = depth - 1;
-    const lhsValue = JSON.stringify(lhs, createReplacer(config));
-    const rhsValue = JSON.stringify(rhs, createReplacer(config));
+    const lhsValue = stringify(lhs, config);
+    const rhsValue = stringify(rhs, config);
 
     const result = [];
 

src/diff/shared.ts

@@ -1,6 +1,6 @@
 import type { DiffConfig, DiffResult, PathHints } from '../types';
 import { ChangeType, REDACTED } from '../utils/constants';
-import { getRawValue, getWrapper } from '../utils/fns';
+import { getRawValue, getWrapper, isPrimitive } from '../utils/fns';
 
 export function lastPathValue(changeType: ChangeType, value: any) {
   return { deleted: changeType === ChangeType.REMOVE, value };
@@ -16,29 +16,55 @@ export function shouldRedactValue(key: any, config: DiffConfig) {
   return config.redactKeys?.includes?.(rawKey);
 }
 
-export function createReplacer(config: DiffConfig) {
+export function createReplacer(config: DiffConfig, obj: any) {
   const seen = new WeakSet();
 
   return function replacer(k: any, v: any) {
+    // Redact the entire value if the key matches
+    if (shouldRedactValue(k, config)) return REDACTED;
+
+    // Returns circular if iterating over the same object from the replacer
+    if (k !== '' && v === obj) return '[Circular]';
+
     if (v && typeof v === 'object') {
       if (seen.has(v)) return '[Circular]';
 
       seen.add(v);
     }
 
     if (v instanceof Set) {
-      return `Set ${JSON.stringify([...v.values()], replacer)}`;
+      const stringified = JSON.stringify([...v.values()], replacer);
+
+      seen.delete(v);
+      return `Set ${stringified}`;
     }
 
     if (v instanceof Map) {
       const entries = [...v.entries()];
       const stringified = entries
-        .map(
-          ([key, value]) =>
-            `${JSON.stringify(key, replacer)}: ${JSON.stringify(shouldRedactValue(key, config) ? REDACTED : value, replacer)}`
-        )
+        .map(([key, value]) => {
+          if (seen.has(key)) {
+            return `"[Circular]": ${JSON.stringify(value, replacer)}`;
+          }
+
+          if (!isPrimitive(key)) {
+            seen.add(key);
+          }
+
+          const serializedValue = shouldRedactValue(key, config)
+            ? REDACTED
+            : JSON.stringify(value, replacer);
+          const serialized = `${JSON.stringify(key, replacer)}: ${serializedValue}`;
+
+          if (!isPrimitive) {
+            seen.delete(key);
+          }
+
+          return serialized;
+        })
         .join(', ');
 
+      seen.delete(v);
       return `Map (${entries.length}) { ${stringified} }`;
     }
 
@@ -54,12 +80,13 @@ export function createReplacer(config: DiffConfig) {
       return `BigInt(${v.toString()})`;
     }
 
-    return shouldRedactValue(k, config) ? REDACTED : v;
+    seen.delete(v);
+    return v;
   };
 }
 
 export function stringify(obj: any, config: DiffConfig) {
-  return JSON.stringify(obj, createReplacer(config));
+  return JSON.stringify(obj, createReplacer(config, obj));
 }
 
 export function getObjectChangeResult(
@@ -92,13 +119,13 @@ export function getObjectChangeResult(
   const redactValue = shouldRedactValue(key, config);
   const rawValueInLhs = getRawValue(valueInLhs);
   const rawValueInRhs = getRawValue(valueInRhs);
-  const formattedValueInLhs = JSON.stringify(
+  const formattedValueInLhs = stringify(
     redactValue ? REDACTED : rawValueInLhs,
-    createReplacer(config)
+    config
   );
-  const formattedValueInRhs = JSON.stringify(
+  const formattedValueInRhs = stringify(
     redactValue ? REDACTED : rawValueInRhs,
-    createReplacer(config)
+    config
   );
 
   let type = ChangeType.NOOP;
@@ -126,18 +153,20 @@ export function getObjectChangeResult(
 
   // If the type of change should be included in the results
   if (includeDiffType(type, config)) {
+    const stringifiedParsedKey = stringify(parsedKey, config);
+
     if (type === ChangeType.UPDATE && !config.showUpdatedOnly) {
       result.push({
         type: ChangeType.REMOVE,
-        str: `${JSON.stringify(parsedKey, createReplacer(config))}: ${formattedValueInLhs},`,
+        str: `${stringifiedParsedKey}: ${formattedValueInLhs},`,
         depth,
         path: [...path, lastPathValue(ChangeType.REMOVE, valueInLhs)],
       });
     }
 
     result.push({
       type,
-      str: `${JSON.stringify(parsedKey, createReplacer(config))}: ${formattedValue},`,
+      str: `${stringifiedParsedKey}: ${formattedValue},`,
       depth,
       path: [...path, lastPathValue(type, pathValue)],
     });

src/test/arrays.spec.js

@@ -245,4 +245,28 @@ describe('Diff Arrays', () => {
       { type: ChangeType.NOOP, str: ']', depth: 0, path: [] },
     ]);
   });
+
+  it('handles the same reference multiple times', () => {
+    const sameRef = {};
+    const a = [sameRef, sameRef];
+
+    expect(diff(undefined, a)).toEqual([
+      { type: ChangeType.ADD, str: '[', depth: 0, path: [] },
+      {
+        type: ChangeType.ADD,
+        str: '0: {',
+        depth: 1,
+        path: [0],
+      },
+      { type: ChangeType.ADD, str: '},', depth: 1, path: [0] },
+      {
+        type: ChangeType.ADD,
+        str: '1: {',
+        depth: 1,
+        path: [1],
+      },
+      { type: ChangeType.ADD, str: '},', depth: 1, path: [1] },
+      { type: ChangeType.ADD, str: ']', depth: 0, path: [] },
+    ]);
+  });
 });

src/test/maps.spec.js

@@ -326,21 +326,101 @@ describe('Diff Maps', () => {
     ]);
   });
 
-  it('handles circular references as keys', () => {
-    const a = new Map();
-    const b = new Map();
+  it('handles circular references in keys', () => {
+    const objKey = {};
+    objKey.self = objKey;
 
-    b.set(b, 'foo');
+    const setKey = new Set();
+    setKey.add(setKey);
+
+    const mapKey = new Map();
+    mapKey.set(mapKey, 'foo');
+
+    // Should not be detected as circular keys
+    const sameRef = {};
+    const objKeyNotCircular = { foo: sameRef, bar: sameRef };
+    const setKeyNotCircular = new Set([{ foo: sameRef }, { bar: sameRef }]);
+    const mapKeyNotCircular = new Map([
+      ['foo', sameRef],
+      ['bar', sameRef],
+    ]);
+
+    const a = new Map();
+    const b = new Map([
+      [objKey, 'bar'],
+      [setKey, 'bar'],
+      [mapKey, 'bar'],
+      [objKeyNotCircular, 'bar'],
+      [setKeyNotCircular, 'bar'],
+      [mapKeyNotCircular, 'bar'],
+    ]);
 
     expect(diff(a, b)).toEqual([
-      { type: ChangeType.NOOP, str: 'Map (1) {', depth: 0, path: [] },
+      { type: ChangeType.NOOP, str: 'Map (6) {', depth: 0, path: [] },
       {
         type: ChangeType.ADD,
-        str: '"Map (1) { \\"[Circular]\\": \\"foo\\" }": "foo",',
+        str: '{"self":"[Circular]"}: "bar",',
         depth: 1,
-        path: ['__MAP__', b, { deleted: false, value: 'foo' }],
+        path: ['__MAP__', objKey, { deleted: false, value: 'bar' }],
+      },
+      {
+        type: ChangeType.ADD,
+        str: '"Set [\\"[Circular]\\"]": "bar",',
+        depth: 1,
+        path: ['__MAP__', setKey, { deleted: false, value: 'bar' }],
+      },
+      {
+        type: ChangeType.ADD,
+        str: '"Map (1) { \\"[Circular]\\": \\"foo\\" }": "bar",',
+        depth: 1,
+        path: ['__MAP__', mapKey, { deleted: false, value: 'bar' }],
+      },
+      {
+        type: ChangeType.ADD,
+        str: '{"foo":{},"bar":{}}: "bar",',
+        depth: 1,
+        path: ['__MAP__', objKeyNotCircular, { deleted: false, value: 'bar' }],
+      },
+      {
+        type: ChangeType.ADD,
+        str: '"Set [{\\"foo\\":{}},{\\"bar\\":{}}]": "bar",',
+        depth: 1,
+        path: ['__MAP__', setKeyNotCircular, { deleted: false, value: 'bar' }],
+      },
+      {
+        type: ChangeType.ADD,
+        str: '"Map (2) { \\"foo\\": {}, \\"bar\\": {} }": "bar",',
+        depth: 1,
+        path: ['__MAP__', mapKeyNotCircular, { deleted: false, value: 'bar' }],
       },
       { type: ChangeType.NOOP, str: '}', depth: 0, path: [] },
     ]);
   });
+
+  it('handles the same reference multiple times', () => {
+    const sameRef = {};
+    const a = new Map([
+      ['foo', sameRef],
+      ['bar', sameRef],
+    ]);
+
+    expect(diff(undefined, a)).toEqual([
+      { type: ChangeType.ADD, str: 'Map (2) {', depth: 0, path: [] },
+      {
+        type: ChangeType.ADD,
+        str: '"foo": {',
+        depth: 1,
+        path: ['__MAP__', 'foo'],
+      },
+      { type: ChangeType.ADD, str: '},', depth: 1, path: ['__MAP__', 'foo'] },
+      {
+        type: ChangeType.ADD,
+        str: '"bar": {',
+        depth: 1,
+        path: ['__MAP__', 'bar'],
+      },
+      { type: ChangeType.ADD, str: '},', depth: 1, path: ['__MAP__', 'bar'] },
+      { type: ChangeType.ADD, str: '}', depth: 0, path: [] },
+    ]);
+  });
 });

src/test/objects.spec.js

@@ -292,6 +292,50 @@ describe('Diff Objects', () => {
     ]);
   });
 
+  it('handles the same reference multiple times', () => {
+    const sameRef = {};
+    const a = {
+      foo: sameRef,
+      bar: {
+        baz: sameRef,
+      },
+    };
+
+    a.self = a;
+
+    expect(diff(undefined, a)).toEqual([
+      { type: ChangeType.ADD, str: '{', depth: 0, path: [] },
+      {
+        type: ChangeType.ADD,
+        str: '"foo": {',
+        depth: 1,
+        path: ['foo'],
+      },
+      { type: ChangeType.ADD, str: '},', depth: 1, path: ['foo'] },
+      {
+        type: ChangeType.ADD,
+        str: '"bar": {',
+        depth: 1,
+        path: ['bar'],
+      },
+      {
+        type: ChangeType.ADD,
+        str: '"baz": {',
+        depth: 2,
+        path: ['bar', 'baz'],
+      },
+      { type: ChangeType.ADD, str: '},', depth: 2, path: ['bar', 'baz'] },
+      { type: ChangeType.ADD, str: '},', depth: 1, path: ['bar'] },
+      {
+        type: ChangeType.ADD,
+        str: '"self": [Circular],',
+        depth: 1,
+        path: ['self'],
+      },
+      { type: ChangeType.ADD, str: '}', depth: 0, path: [] },
+    ]);
+  });
+
   it('traverses the same object', () => {
     const a = { foo: 'bar' };