False circular reference positive for repeated reference
Closed this issue · 4 comments
Ran into a surprising circular reference error when using Redux DevTools Extension. This happened when my state included a repeated reference, under a similar key name. There were other circular references in the object which appeared to be correctly identified.
Here's a minimal reproduction example, the problem is that the repeated ref in "myDataAgain" is identified as circular, but there is no circular reference here:
import jsan from "jsan";
const myCircularRef = {};
myCircularRef.myself = myCircularRef;
const data = [{}];
const state = {
myCircularRef, // A real error is necessary to reproduce the issue
myData: data, // This is fine
myDataAgain: data, // This appears as "[CIRCULAR]"
aDifferentRefToData: data // This is fine
};
const result = jsan.stringify(state, null, 4, { circular: "[CIRCULAR]" });
console.log(result);
Which produces this output:
{
"myCircularRef": {
"myself": "[CIRCULAR]"
},
"myData": [
{}
],
"myDataAgain": "[CIRCULAR]",
"aDifferentRefToData": {
"$jsan": "$.myData"
}
}
The following conditions seem necessary:
- A real error of some sort in the object being serialized (there's an intentional circular reference here)
- A repeated reference, where the key associated with the second reference contains the key of the first reference ("myDataAgain" contains "myData")
If you change any of these things, the repeated ref is no longer identified as being circular, and serializes as "aDifferentRefToData" does.
Interesting, it looks like
Line 114 in e7a4c68
should change to be
if (path.indexOf === foundPath) {
I'm not sure if or how that would break the existing behavior. You can create a PR for this if you'd like (including a test case), or I can create a fix for this when I get a chance.
Thanks for reporting and good catch!
I think I have a viable PR here - all I've done is ignore the final component of the current path, since that seems like that's what tripping it up here. All existing tests pass and I've added a TDD test to confirm this fixes the issue as observed.
Yup, thanks!