Skip to content

Commit 0baff95

Browse files
vmcruzvmcruz
authored
Merge pull request #3 from vmcruz/fix/issue-2/circular-check
Fixes #2: False-positive circular check
2 parents 29c0af1 + 0ebdb8b commit 0baff95

File tree

9 files changed

+250
-26
lines changed

9 files changed

+250
-26
lines changed

‎src/diff/diffMaps.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ function diffMaps({
6464
);
6565
}
6666

67+
seen.delete(lhs);
68+
seen.delete(rhs);
69+
6770
return buildResult(
6871
rhs,
6972
result,

‎src/diff/diffObjects.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ function diffObjects({
7272
);
7373
}
7474

75+
seen.delete(lhs);
76+
seen.delete(rhs);
77+
7578
return buildResult(
7679
rhs,
7780
result,

‎src/diff/diffSets.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
includeDiffType,
88
lastPathValue,
99
maxKeysSecurityCheck,
10+
stringify,
1011
timeoutSecurityCheck,
1112
} from './shared';
1213

@@ -72,13 +73,16 @@ function diffSets({
7273
if (includeDiffType(type, config)) {
7374
result.push({
7475
type,
75-
str: `${JSON.stringify(value)},`,
76+
str: `${stringify(value, config)},`,
7677
depth,
7778
path: [...updatedPath, lastPathValue(type, value)],
7879
});
7980
}
8081
}
8182

83+
seen.delete(lhs);
84+
seen.delete(rhs);
85+
8286
return buildResult(
8387
rhs,
8488
result,

‎src/diff/index.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import diffConstructors from './diffConstructors';
66
import { ChangeType } from '../utils/constants';
77
import { areObjects, isNullOrUndefined, isPrimitive } from '../utils/fns';
88
import {
9-
createReplacer,
109
includeDiffType,
1110
lastPathValue,
1211
timeoutSecurityCheck,
@@ -74,8 +73,8 @@ function recursiveDiff({
7473
}
7574

7675
const parentDepth = depth - 1;
77-
const lhsValue = JSON.stringify(lhs, createReplacer(config));
78-
const rhsValue = JSON.stringify(rhs, createReplacer(config));
76+
const lhsValue = stringify(lhs, config);
77+
const rhsValue = stringify(rhs, config);
7978

8079
const result = [];
8180

‎src/diff/shared.ts‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { DiffConfig, DiffResult, PathHints } from '../types';
22
import { ChangeType, REDACTED } from '../utils/constants';
3-
import { getRawValue, getWrapper } from '../utils/fns';
3+
import { getRawValue, getWrapper, isPrimitive } from '../utils/fns';
44

55
export function lastPathValue(changeType: ChangeType, value: any) {
66
return { deleted: changeType === ChangeType.REMOVE, value };
@@ -16,29 +16,55 @@ export function shouldRedactValue(key: any, config: DiffConfig) {
1616
return config.redactKeys?.includes?.(rawKey);
1717
}
1818

19-
export function createReplacer(config: DiffConfig) {
19+
export function createReplacer(config: DiffConfig, obj: any) {
2020
const seen = new WeakSet();
2121

2222
return function replacer(k: any, v: any) {
23+
// Redact the entire value if the key matches
24+
if (shouldRedactValue(k, config)) return REDACTED;
25+
26+
// Returns circular if iterating over the same object from the replacer
27+
if (k !== '' && v === obj) return '[Circular]';
28+
2329
if (v && typeof v === 'object') {
2430
if (seen.has(v)) return '[Circular]';
2531

2632
seen.add(v);
2733
}
2834

2935
if (v instanceof Set) {
30-
return `Set ${JSON.stringify([...v.values()], replacer)}`;
36+
const stringified = JSON.stringify([...v.values()], replacer);
37+
38+
seen.delete(v);
39+
return `Set ${stringified}`;
3140
}
3241

3342
if (v instanceof Map) {
3443
const entries = [...v.entries()];
3544
const stringified = entries
36-
.map(
37-
([key, value]) =>
38-
`${JSON.stringify(key, replacer)}: ${JSON.stringify(shouldRedactValue(key, config) ? REDACTED : value, replacer)}`
39-
)
45+
.map(([key, value]) => {
46+
if (seen.has(key)) {
47+
return `"[Circular]": ${JSON.stringify(value, replacer)}`;
48+
}
49+
50+
if (!isPrimitive(key)) {
51+
seen.add(key);
52+
}
53+
54+
const serializedValue = shouldRedactValue(key, config)
55+
? REDACTED
56+
: JSON.stringify(value, replacer);
57+
const serialized = `${JSON.stringify(key, replacer)}: ${serializedValue}`;
58+
59+
if (!isPrimitive) {
60+
seen.delete(key);
61+
}
62+
63+
return serialized;
64+
})
4065
.join(', ');
4166

67+
seen.delete(v);
4268
return `Map (${entries.length}) { ${stringified} }`;
4369
}
4470

@@ -54,12 +80,13 @@ export function createReplacer(config: DiffConfig) {
5480
return `BigInt(${v.toString()})`;
5581
}
5682

57-
return shouldRedactValue(k, config) ? REDACTED : v;
83+
seen.delete(v);
84+
return v;
5885
};
5986
}
6087

6188
export function stringify(obj: any, config: DiffConfig) {
62-
return JSON.stringify(obj, createReplacer(config));
89+
return JSON.stringify(obj, createReplacer(config, obj));
6390
}
6491

6592
export function getObjectChangeResult(
@@ -92,13 +119,13 @@ export function getObjectChangeResult(
92119
const redactValue = shouldRedactValue(key, config);
93120
const rawValueInLhs = getRawValue(valueInLhs);
94121
const rawValueInRhs = getRawValue(valueInRhs);
95-
const formattedValueInLhs = JSON.stringify(
122+
const formattedValueInLhs = stringify(
96123
redactValue ? REDACTED : rawValueInLhs,
97-
createReplacer(config)
124+
config
98125
);
99-
const formattedValueInRhs = JSON.stringify(
126+
const formattedValueInRhs = stringify(
100127
redactValue ? REDACTED : rawValueInRhs,
101-
createReplacer(config)
128+
config
102129
);
103130

104131
let type = ChangeType.NOOP;
@@ -126,18 +153,20 @@ export function getObjectChangeResult(
126153

127154
// If the type of change should be included in the results
128155
if (includeDiffType(type, config)) {
156+
const stringifiedParsedKey = stringify(parsedKey, config);
157+
129158
if (type === ChangeType.UPDATE && !config.showUpdatedOnly) {
130159
result.push({
131160
type: ChangeType.REMOVE,
132-
str: `${JSON.stringify(parsedKey, createReplacer(config))}: ${formattedValueInLhs},`,
161+
str: `${stringifiedParsedKey}: ${formattedValueInLhs},`,
133162
depth,
134163
path: [...path, lastPathValue(ChangeType.REMOVE, valueInLhs)],
135164
});
136165
}
137166

138167
result.push({
139168
type,
140-
str: `${JSON.stringify(parsedKey, createReplacer(config))}: ${formattedValue},`,
169+
str: `${stringifiedParsedKey}: ${formattedValue},`,
141170
depth,
142171
path: [...path, lastPathValue(type, pathValue)],
143172
});

‎src/test/arrays.spec.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,4 +245,28 @@ describe('Diff Arrays', () => {
245245
{ type: ChangeType.NOOP, str: ']', depth: 0, path: [] },
246246
]);
247247
});
248+
249+
it('handles the same reference multiple times', () => {
250+
const sameRef = {};
251+
const a = [sameRef, sameRef];
252+
253+
expect(diff(undefined, a)).toEqual([
254+
{ type: ChangeType.ADD, str: '[', depth: 0, path: [] },
255+
{
256+
type: ChangeType.ADD,
257+
str: '0: {',
258+
depth: 1,
259+
path: [0],
260+
},
261+
{ type: ChangeType.ADD, str: '},', depth: 1, path: [0] },
262+
{
263+
type: ChangeType.ADD,
264+
str: '1: {',
265+
depth: 1,
266+
path: [1],
267+
},
268+
{ type: ChangeType.ADD, str: '},', depth: 1, path: [1] },
269+
{ type: ChangeType.ADD, str: ']', depth: 0, path: [] },
270+
]);
271+
});
248272
});

‎src/test/maps.spec.js‎

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -326,21 +326,101 @@ describe('Diff Maps', () => {
326326
]);
327327
});
328328

329-
it('handles circular references as keys', () => {
330-
const a = new Map();
331-
const b = new Map();
329+
it('handles circular references in keys', () => {
330+
const objKey = {};
331+
objKey.self = objKey;
332332

333-
b.set(b, 'foo');
333+
const setKey = new Set();
334+
setKey.add(setKey);
335+
336+
const mapKey = new Map();
337+
mapKey.set(mapKey, 'foo');
338+
339+
// Should not be detected as circular keys
340+
const sameRef = {};
341+
const objKeyNotCircular = { foo: sameRef, bar: sameRef };
342+
const setKeyNotCircular = new Set([{ foo: sameRef }, { bar: sameRef }]);
343+
const mapKeyNotCircular = new Map([
344+
['foo', sameRef],
345+
['bar', sameRef],
346+
]);
347+
348+
const a = new Map();
349+
const b = new Map([
350+
[objKey, 'bar'],
351+
[setKey, 'bar'],
352+
[mapKey, 'bar'],
353+
[objKeyNotCircular, 'bar'],
354+
[setKeyNotCircular, 'bar'],
355+
[mapKeyNotCircular, 'bar'],
356+
]);
334357

335358
expect(diff(a, b)).toEqual([
336-
{ type: ChangeType.NOOP, str: 'Map (1) {', depth: 0, path: [] },
359+
{ type: ChangeType.NOOP, str: 'Map (6) {', depth: 0, path: [] },
337360
{
338361
type: ChangeType.ADD,
339-
str: '"Map (1) { \\"[Circular]\\": \\"foo\\" }": "foo",',
362+
str: '{"self":"[Circular]"}: "bar",',
340363
depth: 1,
341-
path: ['__MAP__', b, { deleted: false, value: 'foo' }],
364+
path: ['__MAP__', objKey, { deleted: false, value: 'bar' }],
365+
},
366+
{
367+
type: ChangeType.ADD,
368+
str: '"Set [\\"[Circular]\\"]": "bar",',
369+
depth: 1,
370+
path: ['__MAP__', setKey, { deleted: false, value: 'bar' }],
371+
},
372+
{
373+
type: ChangeType.ADD,
374+
str: '"Map (1) { \\"[Circular]\\": \\"foo\\" }": "bar",',
375+
depth: 1,
376+
path: ['__MAP__', mapKey, { deleted: false, value: 'bar' }],
377+
},
378+
{
379+
type: ChangeType.ADD,
380+
str: '{"foo":{},"bar":{}}: "bar",',
381+
depth: 1,
382+
path: ['__MAP__', objKeyNotCircular, { deleted: false, value: 'bar' }],
383+
},
384+
{
385+
type: ChangeType.ADD,
386+
str: '"Set [{\\"foo\\":{}},{\\"bar\\":{}}]": "bar",',
387+
depth: 1,
388+
path: ['__MAP__', setKeyNotCircular, { deleted: false, value: 'bar' }],
389+
},
390+
{
391+
type: ChangeType.ADD,
392+
str: '"Map (2) { \\"foo\\": {}, \\"bar\\": {} }": "bar",',
393+
depth: 1,
394+
path: ['__MAP__', mapKeyNotCircular, { deleted: false, value: 'bar' }],
342395
},
343396
{ type: ChangeType.NOOP, str: '}', depth: 0, path: [] },
344397
]);
345398
});
399+
400+
it('handles the same reference multiple times', () => {
401+
const sameRef = {};
402+
const a = new Map([
403+
['foo', sameRef],
404+
['bar', sameRef],
405+
]);
406+
407+
expect(diff(undefined, a)).toEqual([
408+
{ type: ChangeType.ADD, str: 'Map (2) {', depth: 0, path: [] },
409+
{
410+
type: ChangeType.ADD,
411+
str: '"foo": {',
412+
depth: 1,
413+
path: ['__MAP__', 'foo'],
414+
},
415+
{ type: ChangeType.ADD, str: '},', depth: 1, path: ['__MAP__', 'foo'] },
416+
{
417+
type: ChangeType.ADD,
418+
str: '"bar": {',
419+
depth: 1,
420+
path: ['__MAP__', 'bar'],
421+
},
422+
{ type: ChangeType.ADD, str: '},', depth: 1, path: ['__MAP__', 'bar'] },
423+
{ type: ChangeType.ADD, str: '}', depth: 0, path: [] },
424+
]);
425+
});
346426
});

‎src/test/objects.spec.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,50 @@ describe('Diff Objects', () => {
292292
]);
293293
});
294294

295+
it('handles the same reference multiple times', () => {
296+
const sameRef = {};
297+
const a = {
298+
foo: sameRef,
299+
bar: {
300+
baz: sameRef,
301+
},
302+
};
303+
304+
a.self = a;
305+
306+
expect(diff(undefined, a)).toEqual([
307+
{ type: ChangeType.ADD, str: '{', depth: 0, path: [] },
308+
{
309+
type: ChangeType.ADD,
310+
str: '"foo": {',
311+
depth: 1,
312+
path: ['foo'],
313+
},
314+
{ type: ChangeType.ADD, str: '},', depth: 1, path: ['foo'] },
315+
{
316+
type: ChangeType.ADD,
317+
str: '"bar": {',
318+
depth: 1,
319+
path: ['bar'],
320+
},
321+
{
322+
type: ChangeType.ADD,
323+
str: '"baz": {',
324+
depth: 2,
325+
path: ['bar', 'baz'],
326+
},
327+
{ type: ChangeType.ADD, str: '},', depth: 2, path: ['bar', 'baz'] },
328+
{ type: ChangeType.ADD, str: '},', depth: 1, path: ['bar'] },
329+
{
330+
type: ChangeType.ADD,
331+
str: '"self": [Circular],',
332+
depth: 1,
333+
path: ['self'],
334+
},
335+
{ type: ChangeType.ADD, str: '}', depth: 0, path: [] },
336+
]);
337+
});
338+
295339
it('traverses the same object', () => {
296340
const a = { foo: 'bar' };
297341

0 commit comments

Comments
 (0)