dojo.clone does not recognize cycles in object graphs

[valeri.felberg@…]

Following code causes an infinite recursion and crashes the browser (tested in Firefox 2 and IE 6):

var x = {}
var y = {};
    
x.y = y;
y.x = x;
    
var z = dojo.clone(x, true);

If you omit the deep parameter or set it to false, the browser does not crash but if you call dojo.debugDeep(z), Firefox shows an apparently infinite chain of objects (broken after some limit) and IE crashes.

[dylan]

Please test this with clone and see if it is still an issue.

[dante]

still an issue. too much recursion in lang.js line 216

[bill]

I assume that we don't want to fix this? I feel like we've had this discussion before.

[Eugene Lazutkin]

Do you realize that this ticket is for 0.4.1? We did have this discussion back in 0.9, and decided:

• If you want a shallow copy of an object y = dojo.mixin({}, x);
• If you want a shallow copy of an array: y = dojo.map(x, "return value;");
• The rest will be covered by the deep copy: y = dojo.clone(x);

That's why we don't support deep argument anymore --- dojo.clone() is always deep. Cyclic (e.g., circular or DAG) cases are explicitly not supported (see the inlined documents) due to speed and space concerns. Such advanced version is seldom needed, and should go to DojoX or Core, but clearly it should be out of the Base (where dojo.clone() is).

I propose to close this ticket as obsolete/wontfix, and open a new enhancement ticket for the advanced version.

If somebody wants to do the advanced version, please contact me --- I have some additional requirements for it.

[dante]

I added this explanation to ​http://docs.dojocampus.org/dojo/clone (which didn't exist). feel free to add further examples in the docs of its usage. +1 on wontfix

[bill]

Marking as wont-fix (+1 from me too. I assume we don't need an official foundation vote :-) )