angular.copy should respect hasOwnProperty and not copy entire prototype chain

[jpsimons]

It's just a change on https://github.com/angular/angular.js/blob/master/src/Angular.js#L556 like:

  - for ( var key in source) {
  -   destination[key] = copy(source[key]);
  - }

  + for ( var key in source) {
  +   if (source.hasOwnProperty(key)) {
  +     destination[key] = copy(source[key]);
  +   }
  + }

Can't think of any reason people would want the prototype chain treated as data...

We hit this issue in a couple of places, one we have "extended" ngResource objects with model-methods on them, and those get copied on $save. Not a problem until something references the DOM and it tries to copy the entire document.

Also when we open an edit dialog, we snapshot the current state of the data so that cancel would revert it. We do that using angular.copy and likewise, all the prototype methods get copied which breaks if the DOM is referenced somehow, with a stack overflow.

[jpsimons]

Or even better, it would be nice to provide custom copy behavior, like how objects with a forEach property use that instead of the default forEach. Since we're using Angular with Closure, we might have private fields (ending with underscore) and we could skip those for copy().

[jpsimons]

Thinking about this more, maybe it's fine as it is. We can still enrich the model objects, we could just make sure we only add functions and not objects to it. That way JSON.stringify skips the methods and everything just works. If we need objects (not data objects, but objects that the model methods need), we could just capture them in closure and expose getter methods.

[pkozlowski-opensource]

@jpsimons angular.copy is documented as "creates a deep copy of source": http://docs.angularjs.org/api/angular.copy so it works as designed.

Closing for now as changing angular.copy would be a really breaking change. Please feel free to open another issue providing more info about your use-case.

[kaharlichenko]

@pkozlowski-opensource, sorry for necroposting, but that's definitely a bug. Why would anybody want to follow prototype chain while copying? As for the use cases that are affected by this bug I suppose that @jpsimons provided a couple of reasonable ones. Actually I also have hit the same issue with exactly the same two use cases, so I think that they aren't that rare. Do you need a jsfiddle to reproduce it? Or how else can we help fixing it?

[r7lemieux]

Agree, I need a deep copy function, this angular copy does not work: The prototype methods are copied on the object. This is not a copy. Maybe change the name of the method and provide a real copy.

[diegovilar]

This is not a copy. Maybe change the name of the method and provide a real copy.

I believe it IS a copy. I believe angular.copy() works that way by design. It's not an object cloning utility in the sense that it does not promise to give you a new object of the same type of the original one. It is a copying utility. It will give you an object/array with the same properties that original one has, but the new object is not guaranteed to be of the same type (unless the original is either a POJO or an Array). It's gonna be a POJO or an Array. Since it's not gonna inherit the prototype chain, it needs to copy everything in the prototype chain to the new object for you to have a new compatible object as far as properties go.

Now, a new angular.clone() utility would be nice to have. It could try its best to really clone the original object, maybe even check for an available interface implementation on the object that it could delegate the operation to. BUT, that seems out of the framework scope. Angular has utilities it needs to have to achieve what it proposes, and angular.copy() suffices. A cloning function, it seems, belongs to an utility library.