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

Commit 33c67ce

Browse files
jbedardlgalfaso
authored andcommitted
perf(copy): only validate/clear user specified destination
Closes #12068
1 parent ad4296d commit 33c67ce

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
@@ -794,100 +794,111 @@ function arrayRemove(array, value) {
794794
</file>
795795
</example>
796796
*/
797-
function copy(source, destination, stackSource, stackDest) {
798-
if (isWindow(source) || isScope(source)) {
799-
throw ngMinErr('cpws',
800-
"Can't copy! Making copies of Window or Scope instances is not supported.");
801-
}
802-
if (isTypedArray(destination)) {
803-
throw ngMinErr('cpta',
804-
"Can't copy! TypedArray destination cannot be mutated.");
805-
}
806-
807-
if (!destination) {
808-
destination = source;
809-
if (isObject(source)) {
810-
var index;
811-
if (stackSource && (index = stackSource.indexOf(source)) !== -1) {
812-
return stackDest[index];
813-
}
797+
function copy(source, destination) {
798+
var stackSource = [];
799+
var stackDest = [];
814800

815-
// TypedArray, Date and RegExp have specific copy functionality and must be
816-
// pushed onto the stack before returning.
817-
// Array and other objects create the base object and recurse to copy child
818-
// objects. The array/object will be pushed onto the stack when recursed.
819-
if (isArray(source)) {
820-
return copy(source, [], stackSource, stackDest);
821-
} else if (isTypedArray(source)) {
822-
destination = new source.constructor(source);
823-
} else if (isDate(source)) {
824-
destination = new Date(source.getTime());
825-
} else if (isRegExp(source)) {
826-
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
827-
destination.lastIndex = source.lastIndex;
828-
} else if (isFunction(source.cloneNode)) {
829-
destination = source.cloneNode(true);
830-
} else {
831-
var emptyObject = Object.create(getPrototypeOf(source));
832-
return copy(source, emptyObject, stackSource, stackDest);
833-
}
801+
if (destination) {
802+
if (isTypedArray(destination)) {
803+
throw ngMinErr('cpta', "Can't copy! TypedArray destination cannot be mutated.");
804+
}
805+
if (source === destination) {
806+
throw ngMinErr('cpi', "Can't copy! Source and destination are identical.");
807+
}
834808

835-
if (stackDest) {
836-
stackSource.push(source);
837-
stackDest.push(destination);
838-
}
809+
// Empty the destination object
810+
if (isArray(destination)) {
811+
destination.length = 0;
812+
} else {
813+
forEach(destination, function(value, key) {
814+
if (key !== '$$hashKey') {
815+
delete destination[key];
816+
}
817+
});
839818
}
840-
} else {
841-
if (source === destination) throw ngMinErr('cpi',
842-
"Can't copy! Source and destination are identical.");
843819

844-
stackSource = stackSource || [];
845-
stackDest = stackDest || [];
820+
stackSource.push(source);
821+
stackDest.push(destination);
822+
return copyRecurse(source, destination);
823+
}
846824

847-
if (isObject(source)) {
848-
stackSource.push(source);
849-
stackDest.push(destination);
850-
}
825+
return copyElement(source);
851826

827+
function copyRecurse(source, destination) {
828+
var h = destination.$$hashKey;
852829
var result, key;
853830
if (isArray(source)) {
854-
destination.length = 0;
855-
for (var i = 0; i < source.length; i++) {
856-
destination.push(copy(source[i], null, stackSource, stackDest));
831+
for (var i = 0, ii = source.length; i < ii; i++) {
832+
destination.push(copyElement(source[i]));
857833
}
858-
} else {
859-
var h = destination.$$hashKey;
860-
if (isArray(destination)) {
861-
destination.length = 0;
862-
} else {
863-
forEach(destination, function(value, key) {
864-
delete destination[key];
865-
});
834+
} else if (isBlankObject(source)) {
835+
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
836+
for (key in source) {
837+
destination[key] = copyElement(source[key]);
866838
}
867-
if (isBlankObject(source)) {
868-
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
869-
for (key in source) {
870-
destination[key] = copy(source[key], null, stackSource, stackDest);
871-
}
872-
} else if (source && typeof source.hasOwnProperty === 'function') {
873-
// Slow path, which must rely on hasOwnProperty
874-
for (key in source) {
875-
if (source.hasOwnProperty(key)) {
876-
destination[key] = copy(source[key], null, stackSource, stackDest);
877-
}
839+
} else if (source && typeof source.hasOwnProperty === 'function') {
840+
// Slow path, which must rely on hasOwnProperty
841+
for (key in source) {
842+
if (source.hasOwnProperty(key)) {
843+
destination[key] = copyElement(source[key]);
878844
}
879-
} else {
880-
// Slowest path --- hasOwnProperty can't be called as a method
881-
for (key in source) {
882-
if (hasOwnProperty.call(source, key)) {
883-
destination[key] = copy(source[key], null, stackSource, stackDest);
884-
}
845+
}
846+
} else {
847+
// Slowest path --- hasOwnProperty can't be called as a method
848+
for (key in source) {
849+
if (hasOwnProperty.call(source, key)) {
850+
destination[key] = copyElement(source[key]);
885851
}
886852
}
887-
setHashKey(destination,h);
888853
}
854+
setHashKey(destination, h);
855+
return destination;
856+
}
857+
858+
function copyElement(source) {
859+
// Simple values
860+
if (!isObject(source)) {
861+
return source;
862+
}
863+
864+
// Already copied values
865+
var index = stackSource.indexOf(source);
866+
if (index !== -1) {
867+
return stackDest[index];
868+
}
869+
870+
if (isWindow(source) || isScope(source)) {
871+
throw ngMinErr('cpws',
872+
"Can't copy! Making copies of Window or Scope instances is not supported.");
873+
}
874+
875+
var needsRecurse = false;
876+
var destination;
877+
878+
if (isArray(source)) {
879+
destination = [];
880+
needsRecurse = true;
881+
} else if (isTypedArray(source)) {
882+
destination = new source.constructor(source);
883+
} else if (isDate(source)) {
884+
destination = new Date(source.getTime());
885+
} else if (isRegExp(source)) {
886+
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
887+
destination.lastIndex = source.lastIndex;
888+
} else if (isFunction(source.cloneNode)) {
889+
destination = source.cloneNode(true);
890+
} else {
891+
destination = Object.create(getPrototypeOf(source));
892+
needsRecurse = true;
893+
}
894+
895+
stackSource.push(source);
896+
stackDest.push(destination);
897+
898+
return needsRecurse
899+
? copyRecurse(source, destination)
900+
: destination;
889901
}
890-
return destination;
891902
}
892903

893904
/**

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)