Skip to content

Commit

Permalink
fix(Angular.copy): preserve prototype chain when copying objects
Browse files Browse the repository at this point in the history
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 angular#5063
Closes angular#3767
Closes angular#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`.
  • Loading branch information
gentooboontoo authored and Cameron Knight committed Jul 16, 2014
1 parent 714c42c commit 43ebaca
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
15 changes: 9 additions & 6 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down
10 changes: 10 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 43ebaca

Please sign in to comment.