Circular references throw stack overflows

[rtfeldman]

If you call Immutable(obj) on an obj that contains a circular reference, you get a stack overflow as it tries to clone it.

I don't think supporting objects containing circular references is a good idea, but you should at least get a nice error message.

A heuristic could be useful here for performance, e.g. start by just keep count of how big the stack is getting; if it gets over (let's say) 1,000 frames, start over with circular reference detection.

[alanhogan]

Is it undesirable or impossible for immutable data structures to handle cycles?

[alanhogan]

A better question is: why do you think it is unwise to support circular references? I am merely curious, of course

[rtfeldman]

Just that it's time-consuming both from an implementation and from a performance perspective. It would entail building up an intermediate data structure just to track where things need to end up, then converting that back into a cloned object with references appropriately reconnected...

This came up because I accidentally called Immutable on a React component (which contained a circular reference), and it just threw a stack overflow, which made it harder to figure out what I'd done wrong. As soon as I realized the root cause, it was easy to fix, so having a check that results in an exception is all I need to avoid that particular issue in the future. 😄

[rtfeldman]

That said, if it's something someone actually needs, I'm not aware of any reason it couldn't be done.

[alanhogan]

Yep, makes total sense. seems smart to me

[crudh]

Now that there is a dev/prod mode maybe it would make sense to check this only in development mode? That way most cases can be found without sacrificing any performance.

A really simple way would be to do a test before making it immutable, like:
JSON.stringify(object);

If it contains a circular reference it will throw a TypeError with the message "Converting circular structure to JSON".

[rtfeldman]

Great point! It could also then console.error(object) so you could actually see the problematic object in question.

[vegetabill]

FYI I was creating an immutable copy of a Backbone object that was using the backbone-associations add-on and ran into this. immutablejs handles them fine but I liked the sound of this library for all the reasons mentioned in your blog: http://noredinktech.tumblr.com/post/107617838018/switching-from-immutable-js-to-seamless-immutable

[rtfeldman]

Resolved by #119