inspect-js / is-equal

Are these two values conceptually equal?
MIT License
60 stars 7 forks source link

Comparing objects with cycles #21

Open kownacki opened 8 years ago

kownacki commented 8 years ago
var isEqual = require("is-equal");

var a = {};
var b = {};
a.x = {y: a};
b.x = {y: b};

console.log(isEqual(a, b));

I get RangeError: Maximum call stack size exceeded

ljharb commented 8 years ago

Preventing against this in all cases involves maintaining an array (or a Set in newer browsers, but we'd have to have the array fallback) of all seen objects. Currently, we support one-level recursion, but your example here is 3-4 levels. Although your example's cycle includes the root object, you can easily see how you'd construct an example where the cycle did not include the root object.

I'll leave this open because I'm interested in solving the problem, but I don't want to prohibitively slow down the common use case to support an edge case, if I can avoid it.

kownacki commented 8 years ago

You can avoid slowing down if you provide a flag indicating if given object may contain cycles longer than one. For example isEqual(a, b, true) will mean that a and b may contain cycles and a slower and heavier algorithm should be used. Your choice is is which case will be default.

ljharb commented 8 years ago

That's not a bad idea - enabling deep cycle detection via a flag. I'll think on that.

kownacki commented 8 years ago

It's a common pattern in many utility libraries. For example http://underscorejs.org/#uniq you can say that your array is sorted which will allow faster algorithm.

ljharb commented 6 years ago

I've played with this a bit; I'm not actually sure how to implement this in a generic fashion. a PR is welcome.