Circular references not detected/handled.

[LoganDark]

/home/ubuntu/workspace/jsbot/node_modules/neatjson.js:45
                        }else if (o instanceof Array){
                                    ^

RangeError: Maximum call stack size exceeded

with some huge objects:

and prettifier code:

[LoganDark]

isn't this supposed to deal with circular references? i think those are circular references, anyway

[Phrogz]

See the "To Do" section at the bottom of the readme: circular references are not currently detected.

[LoganDark]

@Phrogz here's a way you could possibly do it:

• Keep an array of values you have come across
• Insert a placeholder instead if you come across a value in that array

[Phrogz]

@LoganDark I appreciate the suggestion. I know how to detect circular references, and I can think of several ways of handling them once detected. My preference would be to throw a custom error, since there is no valid way in JSON of properly representing an object with self-references, and I would not want to produce bad data that looks like it's a proper representation.

I've chosen not to handle them so far because:

1. It already errors out (albeit not as quickly, and not with a nice message).
2. It will slow down the speed of JSON generation. The speed impact would be slight for the Ruby version, but could be significant for the JS version for large objects. I'm not keen on making the library slower just to give a nice error message when 'bad' data is fed in.

[LoganDark]

Than maybe something like a detectCircular option? [That of course, comes with a note saying the result may not be proper JSON]

[Phrogz]

I can think of many different ways this would behave. Which of the following interest you? And why are they interesting? When would you use them?

var circular = { a:{who:'a'}, b:{who:'b'} };
circular.a.b = circular.b;
circular.b.a = circular.a;

1. Maximize speed for well-behaved input, fail catastrophically when circular references are present. (This is the current behavior.)

   neatJSON(circular, {cycles:undefined} );
   // Uncaught RangeError: Maximum call stack size exceeded
   

2. Detect circular references and throw a custom error when detected. (This is what JSON.stringify does.)

   neatJSON(circular, {cycles:"error"} );
   // Uncaught CircularReferenceError
   

3. Detect circular references and remove the key/array entry with the reference. (This is sort of what Chrome's inspector does.)

   neatJSON(circular, {cycles:"remove"} );
   // {
   //   "a":{"who":"a", "b":{"who":"b"}},
   //   "b":{"who":"b", "a":{"who":"a"}}
   // } 
   

4. Detect circular references and replace with custom markup (possibly producing invalid JSON).

   neatJSON(circular, {cycles:"replace", cycleString:"/*OMG!*/"} );
   // {
   //   "a":{"who":"a", "b":{"who":"b", "a":/*OMG!*/ }},
   //   "b":{"who":"b", "a":{"who":"a", "b":/*OMG!*/ }}
   // } 
   

   This wouldn't have to produce invalid JSON. You could use cycleString:"null" or cycleString:'"(circular reference)"', for example.

5. Detect circular references and replace with some sort of (invalid JSON) reference to where the original value would be.

   neatJSON(circular, {cycles:"reference", objectName:"myObj"} );
   // {
   //   "a":{"who":"a", "b":{"who":"b", "a":myObj.a }},
   //   "b":{"who":"b", "a":{"who":"a", "b":myObj.b }}
   // } 
   

   Note that while this option is not hard to code, it is more work than the others. Further, if desirable, it would be possible to detect not just circular references, but any reference previously seen:

   neatJSON(circular, {cycles:"reference", objectName:"myObj"} );
   // {
   //   "a":{"who":"a", "b":{"who":"b", "a":myObj.a }},
   //   "b":myObj.a.b
   // } 
   

[LoganDark]

Why not implement them all? cycles option seems to work good enough in your examples. Maybe defaulting to undefined?

Of course you would have to implement it in whatever other language this is implemented in (I forgot, I think it's ruby)

[Phrogz]

cycles option seems to work good enough in your examples. Maybe defaulting to undefined?

I think you misunderstand what I wrote. I was simply suggesting what the interface might look like for each option, and what the output might be like. There is no cycles option currently in the codebase.

Why not implement them all?

Allow me to ~answer that by asking the question in a different way:

"Why not do a lot of coding, for features that possibly no one will use, which will bloat the codebase and make it harder to maintain?"

Hopefully the answer to that question is evident within the phrasing itself. :)

Writing and maintaining code is a balance between functionality and maintainability. Feature creep and code scarring are the enemies. See YAGNI and "The Best Code You Will Ever Write".

[Phrogz]

This feature request is on hold, pending more 'votes' for specific handling of circular references.