Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(equals): handle objects with circular references #11653

Closed
wants to merge 1 commit into from
Closed

fix(equals): handle objects with circular references #11653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/Angular.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,12 @@ function shallowCopy(src, dst) {

return dst || src;
}

function equalsCacheContains(cache, o1, o2) {
for (var i = 0, ii = cache.length; i < ii; i += 2) {
if (cache[i] === o1 && cache[i + 1] === o2) return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be faster (not tested):

var index = 0, length = cache.length;
while (index < length) {
  index = cache.indexOf(o1, index);
  if (index == -1) break;
  if (cache[index + 1] === o2) return true;
}
return false;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not safe as is (e.g. index has to be an even number).

}
return false;
}

/**
* @ngdoc function
Expand Down Expand Up @@ -890,24 +895,27 @@ function shallowCopy(src, dst) {
* @param {*} o2 Object or value to compare.
* @returns {boolean} True if arguments are equal.
*/
function equals(o1, o2) {
function equals(o1, o2, cache) {
if (o1 === o2) return true;
if (o1 === null || o2 === null) return false;
if (o1 !== o1 && o2 !== o2) return true; // NaN === NaN
cache = cache || [];
if (equalsCacheContains(cache, o1, o2)) return true;
cache.push(o1, o2);
var t1 = typeof o1, t2 = typeof o2, length, key, keySet;
if (t1 == t2) {
if (t1 == 'object') {
if (isArray(o1)) {
if (!isArray(o2)) return false;
if ((length = o1.length) == o2.length) {
for (key = 0; key < length; key++) {
if (!equals(o1[key], o2[key])) return false;
if (!equals(o1[key], o2[key], cache)) return false;
}
return true;
}
} else if (isDate(o1)) {
if (!isDate(o2)) return false;
return equals(o1.getTime(), o2.getTime());
return equals(o1.getTime(), o2.getTime(), cache);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to this PR, but since getTime() returns a number, couldn't we just return o1.getTime() === o2.getTime() ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is to support dates where getTime() returns NaN...

} else if (isRegExp(o1)) {
return isRegExp(o2) ? o1.toString() == o2.toString() : false;
} else {
Expand All @@ -916,7 +924,7 @@ function equals(o1, o2) {
keySet = {};
for (key in o1) {
if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
if (!equals(o1[key], o2[key])) return false;
if (!equals(o1[key], o2[key], cache)) return false;
keySet[key] = true;
}
for (key in o2) {
Expand Down
9 changes: 9 additions & 0 deletions test/AngularSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,15 @@ describe('angular', function() {
it('should return false when comparing an object and a Date', function() {
expect(equals({}, new Date())).toBe(false);
});

it('should handle objects with circular references', function() {
var elem1, elem2;
elem1 = {};
elem1.ref = elem1;
elem2 = {};
elem2.ref = elem2;
expect(equals(elem1, elem2)).toBe(true);
});
});


Expand Down