From f5550104155e9229d26cf39f3b5f8f2337d4f11a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Cruz?= Date: Wed, 26 Mar 2025 01:09:24 -0600 Subject: [PATCH 1/4] Add unit tests to check for same reference --- src/test/arrays.spec.js | 24 ++++++++++++++++++++++++ src/test/maps.spec.js | 27 +++++++++++++++++++++++++++ src/test/objects.spec.js | 27 +++++++++++++++++++++++++++ src/test/sets.spec.js | 24 ++++++++++++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/src/test/arrays.spec.js b/src/test/arrays.spec.js index 9ae388a..0c33d47 100644 --- a/src/test/arrays.spec.js +++ b/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: [] }, + ]); + }); }); diff --git a/src/test/maps.spec.js b/src/test/maps.spec.js index df5bf97..4819cbe 100644 --- a/src/test/maps.spec.js +++ b/src/test/maps.spec.js @@ -343,4 +343,31 @@ describe('Diff Maps', () => { { 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: [] }, + ]); + }); }); diff --git a/src/test/objects.spec.js b/src/test/objects.spec.js index 7cbfa9c..b2d5cb8 100644 --- a/src/test/objects.spec.js +++ b/src/test/objects.spec.js @@ -292,6 +292,33 @@ describe('Diff Objects', () => { ]); }); + it('handles the same reference multiple times', () => { + const sameRef = {}; + const a = { + foo: sameRef, + bar: sameRef, + }; + + 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: '},', depth: 1, path: ['bar'] }, + { type: ChangeType.ADD, str: '}', depth: 0, path: [] }, + ]); + }); + it('traverses the same object', () => { const a = { foo: 'bar' }; diff --git a/src/test/sets.spec.js b/src/test/sets.spec.js index 6709aa2..19b542c 100644 --- a/src/test/sets.spec.js +++ b/src/test/sets.spec.js @@ -295,4 +295,28 @@ describe('Diff Sets', () => { { type: ChangeType.NOOP, str: ']', depth: 0, path: [] }, ]); }); + + it('handles the same reference multiple times', () => { + const sameRef = {}; + const a = new Set([sameRef, sameRef]); + + expect(diff(undefined, a)).toEqual([ + { type: ChangeType.ADD, str: 'Set [', depth: 0, path: [] }, + { + type: ChangeType.ADD, + str: '"ref": {', + depth: 1, + path: ['__SET__'], + }, + { type: ChangeType.ADD, str: '},', depth: 1, path: ['__SET__'] }, + { + type: ChangeType.ADD, + str: '"ref": {', + depth: 1, + path: ['__SET__'], + }, + { type: ChangeType.ADD, str: '},', depth: 1, path: ['__SET__'] }, + { type: ChangeType.ADD, str: ']', depth: 0, path: [] }, + ]); + }); }); From 1196e7e305eaf7cbbaf1aef14abc3e964e02df00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Cruz?= Date: Wed, 26 Mar 2025 08:50:51 -0600 Subject: [PATCH 2/4] Add unit test to check for same reference in keys --- src/test/maps.spec.js | 67 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/src/test/maps.spec.js b/src/test/maps.spec.js index 4819cbe..f6ef7ec 100644 --- a/src/test/maps.spec.js +++ b/src/test/maps.spec.js @@ -326,19 +326,72 @@ 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; + + 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], + ]); - b.set(b, 'foo'); + 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: '{"self":"[Circular]"}: "bar",', + depth: 1, + 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 (1) { \\"[Circular]\\": \\"foo\\" }": "foo",', + str: '"Map (2) { \\"foo\\": {}, \\"bar\\": {} }": "bar",', depth: 1, - path: ['__MAP__', b, { deleted: false, value: 'foo' }], + path: ['__MAP__', mapKeyNotCircular, { deleted: false, value: 'bar' }], }, { type: ChangeType.NOOP, str: '}', depth: 0, path: [] }, ]); From a0cad5db245631a64730ca948bf587b8f0523ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Cruz?= Date: Thu, 27 Mar 2025 02:21:28 -0600 Subject: [PATCH 3/4] Fix #2: false-positive circular check --- src/diff/diffMaps.ts | 3 ++ src/diff/diffObjects.ts | 3 ++ src/diff/diffSets.ts | 3 ++ src/diff/shared.ts | 59 ++++++++++++++++++++++++++++++---------- src/test/objects.spec.js | 19 ++++++++++++- src/test/sets.spec.js | 16 ++++++++++- 6 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/diff/diffMaps.ts b/src/diff/diffMaps.ts index 71ae0d3..46a689f 100644 --- a/src/diff/diffMaps.ts +++ b/src/diff/diffMaps.ts @@ -64,6 +64,9 @@ function diffMaps({ ); } + seen.delete(lhs); + seen.delete(rhs); + return buildResult( rhs, result, diff --git a/src/diff/diffObjects.ts b/src/diff/diffObjects.ts index 032e390..ad01e42 100644 --- a/src/diff/diffObjects.ts +++ b/src/diff/diffObjects.ts @@ -72,6 +72,9 @@ function diffObjects({ ); } + seen.delete(lhs); + seen.delete(rhs); + return buildResult( rhs, result, diff --git a/src/diff/diffSets.ts b/src/diff/diffSets.ts index adcc99c..92d808f 100644 --- a/src/diff/diffSets.ts +++ b/src/diff/diffSets.ts @@ -79,6 +79,9 @@ function diffSets({ } } + seen.delete(lhs); + seen.delete(rhs); + return buildResult( rhs, result, diff --git a/src/diff/shared.ts b/src/diff/shared.ts index 281cf6b..d1301d1 100644 --- a/src/diff/shared.ts +++ b/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,10 +16,16 @@ 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]'; @@ -27,18 +33,38 @@ export function createReplacer(config: DiffConfig) { } 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,10 +153,12 @@ 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)], }); @@ -137,7 +166,7 @@ export function getObjectChangeResult( result.push({ type, - str: `${JSON.stringify(parsedKey, createReplacer(config))}: ${formattedValue},`, + str: `${stringifiedParsedKey}: ${formattedValue},`, depth, path: [...path, lastPathValue(type, pathValue)], }); diff --git a/src/test/objects.spec.js b/src/test/objects.spec.js index b2d5cb8..e5d06de 100644 --- a/src/test/objects.spec.js +++ b/src/test/objects.spec.js @@ -296,9 +296,13 @@ describe('Diff Objects', () => { const sameRef = {}; const a = { foo: sameRef, - bar: sameRef, + bar: { + baz: sameRef, + }, }; + a.self = a; + expect(diff(undefined, a)).toEqual([ { type: ChangeType.ADD, str: '{', depth: 0, path: [] }, { @@ -314,7 +318,20 @@ describe('Diff Objects', () => { 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: [] }, ]); }); diff --git a/src/test/sets.spec.js b/src/test/sets.spec.js index 19b542c..8a542a9 100644 --- a/src/test/sets.spec.js +++ b/src/test/sets.spec.js @@ -298,7 +298,7 @@ describe('Diff Sets', () => { it('handles the same reference multiple times', () => { const sameRef = {}; - const a = new Set([sameRef, sameRef]); + const a = new Set([{ foo: sameRef }, { bar: sameRef }]); expect(diff(undefined, a)).toEqual([ { type: ChangeType.ADD, str: 'Set [', depth: 0, path: [] }, @@ -308,6 +308,13 @@ describe('Diff Sets', () => { depth: 1, path: ['__SET__'], }, + { + type: ChangeType.ADD, + str: '"foo": {', + depth: 2, + path: ['__SET__', 'foo'], + }, + { type: ChangeType.ADD, str: '},', depth: 2, path: ['__SET__', 'foo'] }, { type: ChangeType.ADD, str: '},', depth: 1, path: ['__SET__'] }, { type: ChangeType.ADD, @@ -315,6 +322,13 @@ describe('Diff Sets', () => { depth: 1, path: ['__SET__'], }, + { + type: ChangeType.ADD, + str: '"bar": {', + depth: 2, + path: ['__SET__', 'bar'], + }, + { type: ChangeType.ADD, str: '},', depth: 2, path: ['__SET__', 'bar'] }, { type: ChangeType.ADD, str: '},', depth: 1, path: ['__SET__'] }, { type: ChangeType.ADD, str: ']', depth: 0, path: [] }, ]); From 0ebdb8bb9d5d48bd090acf9d61b414b8f9066fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Cruz?= Date: Thu, 27 Mar 2025 02:27:02 -0600 Subject: [PATCH 4/4] Use util.stringify in favor of json.stringify with replacer --- src/diff/diffSets.ts | 3 ++- src/diff/index.ts | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/diff/diffSets.ts b/src/diff/diffSets.ts index 92d808f..dff31f0 100644 --- a/src/diff/diffSets.ts +++ b/src/diff/diffSets.ts @@ -7,6 +7,7 @@ import { includeDiffType, lastPathValue, maxKeysSecurityCheck, + stringify, timeoutSecurityCheck, } from './shared'; @@ -72,7 +73,7 @@ function diffSets({ if (includeDiffType(type, config)) { result.push({ type, - str: `${JSON.stringify(value)},`, + str: `${stringify(value, config)},`, depth, path: [...updatedPath, lastPathValue(type, value)], }); diff --git a/src/diff/index.ts b/src/diff/index.ts index 972673e..2494385 100644 --- a/src/diff/index.ts +++ b/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 = [];