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

Commit 845d17e

Browse files
committed
fix(copy): do not copy the same object twice
1 parent 071be60 commit 845d17e

File tree

2 files changed

+81
-28
lines changed

2 files changed

+81
-28
lines changed

src/Angular.js

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -795,19 +795,29 @@ function copy(source, destination, stackSource, stackDest) {
795795

796796
if (!destination) {
797797
destination = source;
798-
if (source) {
798+
if (isObject(source)) {
799+
var index;
800+
if (stackSource && (index = stackSource.indexOf(source)) !== -1) {
801+
return stackDest[index];
802+
}
803+
799804
if (isArray(source)) {
800-
destination = copy(source, [], stackSource, stackDest);
805+
return copy(source, [], stackSource, stackDest);
801806
} else if (isTypedArray(source)) {
802807
destination = new source.constructor(source);
803808
} else if (isDate(source)) {
804809
destination = new Date(source.getTime());
805810
} else if (isRegExp(source)) {
806811
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
807812
destination.lastIndex = source.lastIndex;
808-
} else if (isObject(source)) {
813+
} else {
809814
var emptyObject = Object.create(getPrototypeOf(source));
810-
destination = copy(source, emptyObject, stackSource, stackDest);
815+
return copy(source, emptyObject, stackSource, stackDest);
816+
}
817+
818+
if (stackDest) {
819+
stackSource.push(source);
820+
stackDest.push(destination);
811821
}
812822
}
813823
} else {
@@ -818,9 +828,6 @@ function copy(source, destination, stackSource, stackDest) {
818828
stackDest = stackDest || [];
819829

820830
if (isObject(source)) {
821-
var index = stackSource.indexOf(source);
822-
if (index !== -1) return stackDest[index];
823-
824831
stackSource.push(source);
825832
stackDest.push(destination);
826833
}
@@ -829,12 +836,7 @@ function copy(source, destination, stackSource, stackDest) {
829836
if (isArray(source)) {
830837
destination.length = 0;
831838
for (var i = 0; i < source.length; i++) {
832-
result = copy(source[i], null, stackSource, stackDest);
833-
if (isObject(source[i])) {
834-
stackSource.push(source[i]);
835-
stackDest.push(result);
836-
}
837-
destination.push(result);
839+
destination.push(copy(source[i], null, stackSource, stackDest));
838840
}
839841
} else {
840842
var h = destination.$$hashKey;
@@ -848,37 +850,27 @@ function copy(source, destination, stackSource, stackDest) {
848850
if (isBlankObject(source)) {
849851
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
850852
for (key in source) {
851-
putValue(key, source[key], destination, stackSource, stackDest);
853+
destination[key] = copy(source[key], null, stackSource, stackDest);
852854
}
853855
} else if (source && typeof source.hasOwnProperty === 'function') {
854856
// Slow path, which must rely on hasOwnProperty
855857
for (key in source) {
856858
if (source.hasOwnProperty(key)) {
857-
putValue(key, source[key], destination, stackSource, stackDest);
859+
destination[key] = copy(source[key], null, stackSource, stackDest);
858860
}
859861
}
860862
} else {
861863
// Slowest path --- hasOwnProperty can't be called as a method
862864
for (key in source) {
863865
if (hasOwnProperty.call(source, key)) {
864-
putValue(key, source[key], destination, stackSource, stackDest);
866+
destination[key] = copy(source[key], null, stackSource, stackDest);
865867
}
866868
}
867869
}
868870
setHashKey(destination,h);
869871
}
870872
}
871873
return destination;
872-
873-
function putValue(key, val, destination, stackSource, stackDest) {
874-
// No context allocation, trivial outer scope, easily inlined
875-
var result = copy(val, null, stackSource, stackDest);
876-
if (isObject(val)) {
877-
stackSource.push(val);
878-
stackDest.push(result);
879-
}
880-
destination[key] = result;
881-
}
882874
}
883875

884876
/**

test/AngularSpec.js

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ describe('angular', function() {
336336
expect(hashKey(dst)).not.toEqual(hashKey(src));
337337
});
338338

339-
it('should retain the previous $$hashKey', function() {
339+
it('should retain the previous $$hashKey when copying object with hashKey', function() {
340340
var src,dst,h;
341341
src = {};
342342
dst = {};
@@ -351,7 +351,21 @@ describe('angular', function() {
351351
expect(hashKey(dst)).toEqual(h);
352352
});
353353

354-
it('should handle circular references when circularRefs is turned on', function() {
354+
it('should retain the previous $$hashKey when copying non-object', function() {
355+
var dst = {};
356+
var h = hashKey(dst);
357+
358+
copy(null, dst);
359+
expect(hashKey(dst)).toEqual(h);
360+
361+
copy(42, dst);
362+
expect(hashKey(dst)).toEqual(h);
363+
364+
copy(new Date(), dst);
365+
expect(hashKey(dst)).toEqual(h);
366+
});
367+
368+
it('should handle circular references', function() {
355369
var a = {b: {a: null}, self: null, selfs: [null, null, [null]]};
356370
a.b.a = a;
357371
a.self = a;
@@ -362,13 +376,60 @@ describe('angular', function() {
362376

363377
expect(aCopy).not.toBe(a);
364378
expect(aCopy).toBe(aCopy.self);
379+
expect(aCopy).toBe(aCopy.selfs[2][0]);
365380
expect(aCopy.selfs[2]).not.toBe(a.selfs[2]);
381+
382+
var copyTo = [];
383+
aCopy = copy(a, copyTo);
384+
expect(aCopy).toBe(copyTo);
385+
expect(aCopy).not.toBe(a);
386+
expect(aCopy).toBe(aCopy.self);
387+
});
388+
389+
it('should handle objects with multiple references', function() {
390+
var b = {};
391+
var a = [b, -1, b];
392+
393+
var aCopy = copy(a);
394+
expect(aCopy[0]).not.toBe(a[0]);
395+
expect(aCopy[0]).toBe(aCopy[2]);
396+
397+
var copyTo = [];
398+
aCopy = copy(a, copyTo);
399+
expect(aCopy).toBe(copyTo);
400+
expect(aCopy[0]).not.toBe(a[0]);
401+
expect(aCopy[0]).toBe(aCopy[2]);
402+
});
403+
404+
it('should handle date/regex objects with multiple references', function() {
405+
var re = /foo/;
406+
var d = new Date();
407+
var o = {re: re, re2: re, d: d, d2: d};
408+
409+
var oCopy = copy(o);
410+
expect(oCopy.re).toBe(oCopy.re2);
411+
expect(oCopy.d).toBe(oCopy.d2);
412+
413+
oCopy = copy(o, {});
414+
expect(oCopy.re).toBe(oCopy.re2);
415+
expect(oCopy.d).toBe(oCopy.d2);
366416
});
367417

368418
it('should clear destination arrays correctly when source is non-array', function() {
369419
expect(copy(null, [1,2,3])).toEqual([]);
370420
expect(copy(undefined, [1,2,3])).toEqual([]);
371421
expect(copy({0: 1, 1: 2}, [1,2,3])).toEqual([1,2]);
422+
expect(copy(new Date(), [1,2,3])).toEqual([]);
423+
expect(copy(/a/, [1,2,3])).toEqual([]);
424+
expect(copy(true, [1,2,3])).toEqual([]);
425+
});
426+
427+
it('should clear destination objects correctly when source is non-array', function() {
428+
expect(copy(null, {0:1,1:2,2:3})).toEqual({});
429+
expect(copy(undefined, {0:1,1:2,2:3})).toEqual({});
430+
expect(copy(new Date(), {0:1,1:2,2:3})).toEqual({});
431+
expect(copy(/a/, {0:1,1:2,2:3})).toEqual({});
432+
expect(copy(true, {0:1,1:2,2:3})).toEqual({});
372433
});
373434

374435
it('should copy objects with no prototype parent', function() {

0 commit comments

Comments
 (0)