Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparing objects with cycles #21

Open
kownacki opened this issue May 2, 2016 · 5 comments
Open

Comparing objects with cycles #21

kownacki opened this issue May 2, 2016 · 5 comments

Comments

@kownacki
Copy link

kownacki commented May 2, 2016

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
Copy link
Member

ljharb commented May 2, 2016

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
Copy link
Author

kownacki commented May 2, 2016

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
Copy link
Member

ljharb commented May 2, 2016

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

@kownacki
Copy link
Author

kownacki commented May 2, 2016

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
Copy link
Member

ljharb commented Jun 8, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants