Skip to content

Commit 0d5865d

Browse files
committed
Fix areEqual treating distinct Dates/RegExps as equal
areEqual fell through to its generic object branch for Date and RegExp instances. There, Object.keys() returns an empty array for both values, so the key-count and per-key checks pass vacuously and any two Dates (or any two RegExps) were reported as equal regardless of their values. This means change detection misses updates when an observable holds a Date or RegExp: replacing new Date(0) with new Date(1000) would not be seen as a change. Compare Dates by getTime() and RegExps by source+flags before the object fallback, and add tests covering both.
1 parent a072775 commit 0d5865d

2 files changed

Lines changed: 76 additions & 11 deletions

File tree

src/utils/__tests__/areEqual.test.ts

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,12 @@ describe('areEqual function', () => {
171171
describe('Type mismatch comparisons', () => {
172172
test('Map vs non-Map returns false', () => {
173173
const map = new Map([['key', 'value']]);
174-
expect(areEqual(map, { key: 'value' }, ComparisonMethod.ShallowEqual)).toBe(
175-
false
176-
);
177-
expect(areEqual({ key: 'value' }, map, ComparisonMethod.ShallowEqual)).toBe(
178-
false
179-
);
174+
expect(
175+
areEqual(map, { key: 'value' }, ComparisonMethod.ShallowEqual)
176+
).toBe(false);
177+
expect(
178+
areEqual({ key: 'value' }, map, ComparisonMethod.ShallowEqual)
179+
).toBe(false);
180180
expect(areEqual(map, { key: 'value' }, ComparisonMethod.DeepEquals)).toBe(
181181
false
182182
);
@@ -187,8 +187,12 @@ describe('areEqual function', () => {
187187

188188
test('Set vs non-Set returns false', () => {
189189
const set = new Set([1, 2, 3]);
190-
expect(areEqual(set, [1, 2, 3], ComparisonMethod.ShallowEqual)).toBe(false);
191-
expect(areEqual([1, 2, 3], set, ComparisonMethod.ShallowEqual)).toBe(false);
190+
expect(areEqual(set, [1, 2, 3], ComparisonMethod.ShallowEqual)).toBe(
191+
false
192+
);
193+
expect(areEqual([1, 2, 3], set, ComparisonMethod.ShallowEqual)).toBe(
194+
false
195+
);
192196
expect(areEqual(set, [1, 2, 3], ComparisonMethod.DeepEquals)).toBe(false);
193197
expect(areEqual([1, 2, 3], set, ComparisonMethod.DeepEquals)).toBe(false);
194198
});
@@ -231,21 +235,27 @@ describe('areEqual function', () => {
231235

232236
test('Map vs primitive returns false', () => {
233237
const map = new Map();
234-
expect(areEqual(map, 'string', ComparisonMethod.ShallowEqual)).toBe(false);
238+
expect(areEqual(map, 'string', ComparisonMethod.ShallowEqual)).toBe(
239+
false
240+
);
235241
expect(areEqual(map, 42, ComparisonMethod.ShallowEqual)).toBe(false);
236242
expect(areEqual(map, true, ComparisonMethod.DeepEquals)).toBe(false);
237243
});
238244

239245
test('Set vs primitive returns false', () => {
240246
const set = new Set();
241-
expect(areEqual(set, 'string', ComparisonMethod.ShallowEqual)).toBe(false);
247+
expect(areEqual(set, 'string', ComparisonMethod.ShallowEqual)).toBe(
248+
false
249+
);
242250
expect(areEqual(set, 42, ComparisonMethod.ShallowEqual)).toBe(false);
243251
expect(areEqual(set, true, ComparisonMethod.DeepEquals)).toBe(false);
244252
});
245253

246254
test('Array vs primitive returns false', () => {
247255
const arr: unknown[] = [];
248-
expect(areEqual(arr, 'string', ComparisonMethod.ShallowEqual)).toBe(false);
256+
expect(areEqual(arr, 'string', ComparisonMethod.ShallowEqual)).toBe(
257+
false
258+
);
249259
expect(areEqual(arr, 42, ComparisonMethod.ShallowEqual)).toBe(false);
250260
expect(areEqual(arr, true, ComparisonMethod.DeepEquals)).toBe(false);
251261
});
@@ -369,4 +379,41 @@ describe('areEqual function', () => {
369379
expect(areEqual(obj3, obj4, ComparisonMethod.ShallowEqual)).toBe(false);
370380
});
371381
});
382+
383+
describe('Date and RegExp comparison', () => {
384+
test('equal Dates with different references are equal', () => {
385+
expect(
386+
areEqual(new Date(0), new Date(0), ComparisonMethod.ShallowEqual)
387+
).toBe(true);
388+
expect(
389+
areEqual(new Date(0), new Date(0), ComparisonMethod.DeepEquals)
390+
).toBe(true);
391+
});
392+
393+
test('Dates with different times are not equal', () => {
394+
expect(
395+
areEqual(new Date(0), new Date(1000), ComparisonMethod.ShallowEqual)
396+
).toBe(false);
397+
expect(
398+
areEqual(new Date(0), new Date(1000), ComparisonMethod.DeepEquals)
399+
).toBe(false);
400+
});
401+
402+
test('a Date is not equal to a plain object', () => {
403+
expect(areEqual(new Date(0), {}, ComparisonMethod.DeepEquals)).toBe(
404+
false
405+
);
406+
});
407+
408+
test('equal RegExps with different references are equal', () => {
409+
expect(areEqual(/abc/gi, /abc/gi, ComparisonMethod.ShallowEqual)).toBe(
410+
true
411+
);
412+
});
413+
414+
test('RegExps with different patterns or flags are not equal', () => {
415+
expect(areEqual(/abc/g, /abd/g, ComparisonMethod.DeepEquals)).toBe(false);
416+
expect(areEqual(/abc/g, /abc/i, ComparisonMethod.DeepEquals)).toBe(false);
417+
});
418+
});
372419
});

src/utils/areEqual.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ export function areEqual(
5656
return false;
5757
}
5858

59+
// Compare Dates by their time value. Without this, two Date instances fall
60+
// through to the object branch below, where Object.keys() is empty for both,
61+
// so any two Dates would incorrectly be considered equal.
62+
if (a instanceof Date !== b instanceof Date) {
63+
return false;
64+
}
65+
if (a instanceof Date && b instanceof Date) {
66+
return a.getTime() === b.getTime();
67+
}
68+
69+
// Compare RegExps by their source and flags, for the same reason as Dates.
70+
if (a instanceof RegExp !== b instanceof RegExp) {
71+
return false;
72+
}
73+
if (a instanceof RegExp && b instanceof RegExp) {
74+
return a.source === b.source && a.flags === b.flags;
75+
}
76+
5977
// Compare Maps
6078
if (a instanceof Map !== b instanceof Map) {
6179
return false;

0 commit comments

Comments
 (0)