From 43ebaca58cdfa52f436ce3805f7687d7ae9bf4b7 Mon Sep 17 00:00:00 2001 From: Julien Sanchez Date: Thu, 21 Nov 2013 09:20:54 +0100 Subject: [PATCH] fix(Angular.copy): preserve prototype chain when copying objects So far, angular.copy was copying all properties including those from prototype chain and was losing the whole prototype chain (except for Date, Regexp, and Array). Deep copy should exclude properties from the prototype chain because it is useless to do so. When modified, properties from prototype chain are overwritten on the object itself and will be deeply copied then. Moreover, preserving prototype chain allows instanceof operator to be consistent between the source object and the copy. Before this change, var Foo = function() {}; var foo = new Foo(); var fooCopy = angular.copy(foo); foo instanceof Foo; // => true fooCopy instanceof Foo; // => false Now, foo instanceof Foo; // => true fooCopy instanceof Foo; // => true The new behaviour is useful when using $http transformResponse. When receiving JSON data, we could transform it and instantiate real object "types" from it. The transformed response is always copied by Angular. The old behaviour was losing the whole prototype chain and broke all "types" from third-party libraries depending on instanceof. Closes #5063 Closes #3767 Closes #4996 BREAKING CHANGE: This changes `angular.copy` so that it applies the prototype of the original object to the copied object. Previously, `angular.copy` would copy properties of the original object's prototype chain directly onto the copied object. This means that if you iterate over only the copied object's `hasOwnProperty` properties, it will no longer contain the properties from the prototype. This is actually much more reasonable behaviour and it is unlikely that applications are actually relying on this. If this behaviour is relied upon, in an app, then one should simply iterate over all the properties on the object (and its inherited properties) and not filter them with `hasOwnProperty`. **Be aware that this change also uses a feature that is not compatible with IE8.** If you need this to work on IE8 then you would need to provide a polyfill for `Object.create` and `Object.getPrototypeOf`. --- src/Angular.js | 15 +++++++++------ test/AngularSpec.js | 10 ++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index c135a7b0916b..d216469c7f00 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -772,7 +772,8 @@ function copy(source, destination, stackSource, stackDest) { } else if (isRegExp(source)) { destination = new RegExp(source.source); } else if (isObject(source)) { - destination = copy(source, {}, stackSource, stackDest); + var emptyObject = Object.create(Object.getPrototypeOf(source)); + destination = copy(source, emptyObject, stackSource, stackDest); } } } else { @@ -807,12 +808,14 @@ function copy(source, destination, stackSource, stackDest) { delete destination[key]; }); for ( var key in source) { - result = copy(source[key], null, stackSource, stackDest); - if (isObject(source[key])) { - stackSource.push(source[key]); - stackDest.push(result); + if(source.hasOwnProperty(key)) { + result = copy(source[key], null, stackSource, stackDest); + if (isObject(source[key])) { + stackSource.push(source[key]); + stackDest.push(result); + } + destination[key] = result; } - destination[key] = result; } setHashKey(destination,h); } diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 5d75eb1dca41..fea74f81212a 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -24,6 +24,16 @@ describe('angular', function() { expect(copy([], arr)).toBe(arr); }); + it("should preserve prototype chaining", function() { + var GrandParentProto = {}; + var ParentProto = Object.create(GrandParentProto); + var obj = Object.create(ParentProto); + expect(ParentProto.isPrototypeOf(copy(obj))).toBe(true); + expect(GrandParentProto.isPrototypeOf(copy(obj))).toBe(true); + var Foo = function() {}; + expect(copy(new Foo()) instanceof Foo).toBe(true); + }); + it("should copy Date", function() { var date = new Date(123); expect(copy(date) instanceof Date).toBeTruthy();