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

isEqual does not works if one object has circular references #114

Open
zsilbi opened this issue Feb 20, 2025 · 3 comments · May be fixed by #119
Open

isEqual does not works if one object has circular references #114

zsilbi opened this issue Feb 20, 2025 · 3 comments · May be fixed by #119
Labels
bug Something isn't working

Comments

@zsilbi
Copy link

zsilbi commented Feb 20, 2025

Environment

ohash v2.0.2
Node v23.8.0

Reproduction

Added failing tests for serialize(): https://github.com/zsilbi/ohash/tree/test-serialize-consistency

Describe the bug

I switched to isEqual() from an older library in one of my projects, but it does not behave as I expected. Objects with the same structure are indexed based on their reference, leading to differences in their serialized form.

import { serialize } from "ohash";
import { isEqual } from "ohash/utils";

const a = { value: 1 };

const vvv = {
  value: { value: 1 },
  values: [{ value: 1 }, { value: 1 }],
};

const aaa = {
  value: a,
  values: [a, a],
};

isEqual(vvv, aaa); // false
serialize(vvv); // "{value:{value:1},values:[{value:1},{value:1},]}"
serialize(aaa); // "{value:{value:1},values:[#1,#1,]}"

This indexing feature helps handle circular references but makes isEqual() unreliable for my use case.

Would it be possible to improve this behavior, or should the documentation be updated to clarify the exact mechanics of this function?

@zsilbi zsilbi added the bug Something isn't working label Feb 20, 2025
@pi0
Copy link
Member

pi0 commented Feb 20, 2025

Thanks for the explanations.

It seems a bug to me, I think we can add an option to disable circular counters and use it by default for isEqual

@pi0
Copy link
Member

pi0 commented Feb 20, 2025

Testing more, while your case above can be handled by disabling feature (it is duplicate reference), there seems no easy way to distinguish from an actual circular reference like const obj = {}; obj.foo = obj;

@pi0 pi0 changed the title serialize() inconsistency: isEqual() returns false for structurally identical objects isEqual does not works if one object has circular references Feb 20, 2025
@zsilbi
Copy link
Author

zsilbi commented Feb 20, 2025

Testing more, while your case above can be handled by disabling feature (it is duplicate reference), there seems no easy way to distinguish from an actual circular reference like const obj = {}; obj.foo = obj;

My first idea would be to capture the output of non-circular objects from the serialization and insert them back if they are available: zsilbi@689f5e0

The tests pass (other than increased the bundle size) but I think there can be more edge cases where this can fail.

@zsilbi zsilbi linked a pull request Feb 20, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants