Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit ada25a9

Browse files
jbedardpetebacondarwin
authored andcommitted
perf(copy): only validate/clear user specified destination
Closes #12068
1 parent 0f956b2 commit ada25a9

File tree

2 files changed

+103
-79
lines changed

2 files changed

+103
-79
lines changed

src/Angular.js

+90-79
Original file line numberDiff line numberDiff line change
@@ -731,100 +731,111 @@ function arrayRemove(array, value) {
731731
</file>
732732
</example>
733733
*/
734-
function copy(source, destination, stackSource, stackDest) {
735-
if (isWindow(source) || isScope(source)) {
736-
throw ngMinErr('cpws',
737-
"Can't copy! Making copies of Window or Scope instances is not supported.");
738-
}
739-
if (isTypedArray(destination)) {
740-
throw ngMinErr('cpta',
741-
"Can't copy! TypedArray destination cannot be mutated.");
742-
}
743-
744-
if (!destination) {
745-
destination = source;
746-
if (isObject(source)) {
747-
var index;
748-
if (stackSource && (index = stackSource.indexOf(source)) !== -1) {
749-
return stackDest[index];
750-
}
734+
function copy(source, destination) {
735+
var stackSource = [];
736+
var stackDest = [];
751737

752-
// TypedArray, Date and RegExp have specific copy functionality and must be
753-
// pushed onto the stack before returning.
754-
// Array and other objects create the base object and recurse to copy child
755-
// objects. The array/object will be pushed onto the stack when recursed.
756-
if (isArray(source)) {
757-
return copy(source, [], stackSource, stackDest);
758-
} else if (isTypedArray(source)) {
759-
destination = new source.constructor(source);
760-
} else if (isDate(source)) {
761-
destination = new Date(source.getTime());
762-
} else if (isRegExp(source)) {
763-
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
764-
destination.lastIndex = source.lastIndex;
765-
} else if (isFunction(source.cloneNode)) {
766-
destination = source.cloneNode(true);
767-
} else {
768-
var emptyObject = Object.create(getPrototypeOf(source));
769-
return copy(source, emptyObject, stackSource, stackDest);
770-
}
738+
if (destination) {
739+
if (isTypedArray(destination)) {
740+
throw ngMinErr('cpta', "Can't copy! TypedArray destination cannot be mutated.");
741+
}
742+
if (source === destination) {
743+
throw ngMinErr('cpi', "Can't copy! Source and destination are identical.");
744+
}
771745

772-
if (stackDest) {
773-
stackSource.push(source);
774-
stackDest.push(destination);
775-
}
746+
// Empty the destination object
747+
if (isArray(destination)) {
748+
destination.length = 0;
749+
} else {
750+
forEach(destination, function(value, key) {
751+
if (key !== '$$hashKey') {
752+
delete destination[key];
753+
}
754+
});
776755
}
777-
} else {
778-
if (source === destination) throw ngMinErr('cpi',
779-
"Can't copy! Source and destination are identical.");
780756

781-
stackSource = stackSource || [];
782-
stackDest = stackDest || [];
757+
stackSource.push(source);
758+
stackDest.push(destination);
759+
return copyRecurse(source, destination);
760+
}
783761

784-
if (isObject(source)) {
785-
stackSource.push(source);
786-
stackDest.push(destination);
787-
}
762+
return copyElement(source);
788763

764+
function copyRecurse(source, destination) {
765+
var h = destination.$$hashKey;
789766
var result, key;
790767
if (isArray(source)) {
791-
destination.length = 0;
792-
for (var i = 0; i < source.length; i++) {
793-
destination.push(copy(source[i], null, stackSource, stackDest));
768+
for (var i = 0, ii = source.length; i < ii; i++) {
769+
destination.push(copyElement(source[i]));
794770
}
795-
} else {
796-
var h = destination.$$hashKey;
797-
if (isArray(destination)) {
798-
destination.length = 0;
799-
} else {
800-
forEach(destination, function(value, key) {
801-
delete destination[key];
802-
});
771+
} else if (isBlankObject(source)) {
772+
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
773+
for (key in source) {
774+
destination[key] = copyElement(source[key]);
803775
}
804-
if (isBlankObject(source)) {
805-
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
806-
for (key in source) {
807-
destination[key] = copy(source[key], null, stackSource, stackDest);
808-
}
809-
} else if (source && typeof source.hasOwnProperty === 'function') {
810-
// Slow path, which must rely on hasOwnProperty
811-
for (key in source) {
812-
if (source.hasOwnProperty(key)) {
813-
destination[key] = copy(source[key], null, stackSource, stackDest);
814-
}
776+
} else if (source && typeof source.hasOwnProperty === 'function') {
777+
// Slow path, which must rely on hasOwnProperty
778+
for (key in source) {
779+
if (source.hasOwnProperty(key)) {
780+
destination[key] = copyElement(source[key]);
815781
}
816-
} else {
817-
// Slowest path --- hasOwnProperty can't be called as a method
818-
for (key in source) {
819-
if (hasOwnProperty.call(source, key)) {
820-
destination[key] = copy(source[key], null, stackSource, stackDest);
821-
}
782+
}
783+
} else {
784+
// Slowest path --- hasOwnProperty can't be called as a method
785+
for (key in source) {
786+
if (hasOwnProperty.call(source, key)) {
787+
destination[key] = copyElement(source[key]);
822788
}
823789
}
824-
setHashKey(destination,h);
825790
}
791+
setHashKey(destination, h);
792+
return destination;
793+
}
794+
795+
function copyElement(source) {
796+
// Simple values
797+
if (!isObject(source)) {
798+
return source;
799+
}
800+
801+
// Already copied values
802+
var index = stackSource.indexOf(source);
803+
if (index !== -1) {
804+
return stackDest[index];
805+
}
806+
807+
if (isWindow(source) || isScope(source)) {
808+
throw ngMinErr('cpws',
809+
"Can't copy! Making copies of Window or Scope instances is not supported.");
810+
}
811+
812+
var needsRecurse = false;
813+
var destination;
814+
815+
if (isArray(source)) {
816+
destination = [];
817+
needsRecurse = true;
818+
} else if (isTypedArray(source)) {
819+
destination = new source.constructor(source);
820+
} else if (isDate(source)) {
821+
destination = new Date(source.getTime());
822+
} else if (isRegExp(source)) {
823+
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
824+
destination.lastIndex = source.lastIndex;
825+
} else if (isFunction(source.cloneNode)) {
826+
destination = source.cloneNode(true);
827+
} else {
828+
destination = Object.create(getPrototypeOf(source));
829+
needsRecurse = true;
830+
}
831+
832+
stackSource.push(source);
833+
stackDest.push(destination);
834+
835+
return needsRecurse
836+
? copyRecurse(source, destination)
837+
: destination;
826838
}
827-
return destination;
828839
}
829840

830841
/**

test/AngularSpec.js

+13
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,19 @@ describe('angular', function() {
313313
it('should throw an exception if a Scope is being copied', inject(function($rootScope) {
314314
expect(function() { copy($rootScope.$new()); }).
315315
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
316+
expect(function() { copy({child: $rootScope.$new()}, {}); }).
317+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
318+
expect(function() { copy([$rootScope.$new()]); }).
319+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
316320
}));
317321

318322
it('should throw an exception if a Window is being copied', function() {
319323
expect(function() { copy(window); }).
320324
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
325+
expect(function() { copy({child: window}); }).
326+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
327+
expect(function() { copy([window], []); }).
328+
toThrowMinErr("ng", "cpws", "Can't copy! Making copies of Window or Scope instances is not supported.");
321329
});
322330

323331
it('should throw an exception when source and destination are equivalent', function() {
@@ -334,6 +342,11 @@ describe('angular', function() {
334342
hashKey(src);
335343
dst = copy(src);
336344
expect(hashKey(dst)).not.toEqual(hashKey(src));
345+
346+
src = {foo: {}};
347+
hashKey(src.foo);
348+
dst = copy(src);
349+
expect(hashKey(src.foo)).not.toEqual(hashKey(dst.foo));
337350
});
338351

339352
it('should retain the previous $$hashKey when copying object with hashKey', function() {

0 commit comments

Comments
 (0)