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

Commit

Permalink
fix(copy): do not copy the same object twice
Browse files Browse the repository at this point in the history
  • Loading branch information
@jbedard @lgalfaso
jbedard authored and lgalfaso committed Jun 8, 2015
1 parent 071be60 commit 0e622f7
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 28 deletions.
48 changes: 22 additions & 26 deletions src/Angular.js
View file
Expand Up @@ -795,19 +795,33 @@ function copy(source, destination, stackSource, stackDest) {

if (!destination) {
destination = source;
if (source) {
if (isObject(source)) {
var index;
if (stackSource && (index = stackSource.indexOf(source)) !== -1) {
return stackDest[index];
}

// TypedArray, Date and RegExp have specific copy functionality and must be
// pushed onto the stack before returning.
// Array and other objects create the base object and recurse to copy child
// objects. The array/object will be pushed onto the stack when recursed.
if (isArray(source)) {
destination = copy(source, [], stackSource, stackDest);
return copy(source, [], stackSource, stackDest);
} else if (isTypedArray(source)) {
destination = new source.constructor(source);
} else if (isDate(source)) {
destination = new Date(source.getTime());
} else if (isRegExp(source)) {
destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
destination.lastIndex = source.lastIndex;
} else if (isObject(source)) {
} else {
var emptyObject = Object.create(getPrototypeOf(source));
destination = copy(source, emptyObject, stackSource, stackDest);
return copy(source, emptyObject, stackSource, stackDest);
}

if (stackDest) {
stackSource.push(source);
stackDest.push(destination);
}
}
} else {
Expand All @@ -818,9 +832,6 @@ function copy(source, destination, stackSource, stackDest) {
stackDest = stackDest || [];

if (isObject(source)) {
var index = stackSource.indexOf(source);
if (index !== -1) return stackDest[index];

stackSource.push(source);
stackDest.push(destination);
}
Expand All @@ -829,12 +840,7 @@ function copy(source, destination, stackSource, stackDest) {
if (isArray(source)) {
destination.length = 0;
for (var i = 0; i < source.length; i++) {
result = copy(source[i], null, stackSource, stackDest);
if (isObject(source[i])) {
stackSource.push(source[i]);
stackDest.push(result);
}
destination.push(result);
destination.push(copy(source[i], null, stackSource, stackDest));
}
} else {
var h = destination.$$hashKey;
Expand All @@ -848,37 +854,27 @@ function copy(source, destination, stackSource, stackDest) {
if (isBlankObject(source)) {
// createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
for (key in source) {
putValue(key, source[key], destination, stackSource, stackDest);
destination[key] = copy(source[key], null, stackSource, stackDest);
}
} else if (source && typeof source.hasOwnProperty === 'function') {
// Slow path, which must rely on hasOwnProperty
for (key in source) {
if (source.hasOwnProperty(key)) {
putValue(key, source[key], destination, stackSource, stackDest);
destination[key] = copy(source[key], null, stackSource, stackDest);
}
}
} else {
// Slowest path --- hasOwnProperty can't be called as a method
for (key in source) {
if (hasOwnProperty.call(source, key)) {
putValue(key, source[key], destination, stackSource, stackDest);
destination[key] = copy(source[key], null, stackSource, stackDest);
}
}
}
setHashKey(destination,h);
}
}
return destination;

function putValue(key, val, destination, stackSource, stackDest) {
// No context allocation, trivial outer scope, easily inlined
var result = copy(val, null, stackSource, stackDest);
if (isObject(val)) {
stackSource.push(val);
stackDest.push(result);
}
destination[key] = result;
}
}

/**
Expand Down
65 changes: 63 additions & 2 deletions test/AngularSpec.js
View file
Expand Up @@ -336,7 +336,7 @@ describe('angular', function() {
expect(hashKey(dst)).not.toEqual(hashKey(src));
});

it('should retain the previous $$hashKey', function() {
it('should retain the previous $$hashKey when copying object with hashKey', function() {
var src,dst,h;
src = {};
dst = {};
Expand All @@ -351,7 +351,21 @@ describe('angular', function() {
expect(hashKey(dst)).toEqual(h);
});

it('should handle circular references when circularRefs is turned on', function() {
it('should retain the previous $$hashKey when copying non-object', function() {
var dst = {};
var h = hashKey(dst);

copy(null, dst);
expect(hashKey(dst)).toEqual(h);

copy(42, dst);
expect(hashKey(dst)).toEqual(h);

copy(new Date(), dst);
expect(hashKey(dst)).toEqual(h);
});

it('should handle circular references', function() {
var a = {b: {a: null}, self: null, selfs: [null, null, [null]]};
a.b.a = a;
a.self = a;
Expand All @@ -362,13 +376,60 @@ describe('angular', function() {

expect(aCopy).not.toBe(a);
expect(aCopy).toBe(aCopy.self);
expect(aCopy).toBe(aCopy.selfs[2][0]);
expect(aCopy.selfs[2]).not.toBe(a.selfs[2]);

var copyTo = [];
aCopy = copy(a, copyTo);
expect(aCopy).toBe(copyTo);
expect(aCopy).not.toBe(a);
expect(aCopy).toBe(aCopy.self);
});

it('should handle objects with multiple references', function() {
var b = {};
var a = [b, -1, b];

var aCopy = copy(a);
expect(aCopy[0]).not.toBe(a[0]);
expect(aCopy[0]).toBe(aCopy[2]);

var copyTo = [];
aCopy = copy(a, copyTo);
expect(aCopy).toBe(copyTo);
expect(aCopy[0]).not.toBe(a[0]);
expect(aCopy[0]).toBe(aCopy[2]);
});

it('should handle date/regex objects with multiple references', function() {
var re = /foo/;
var d = new Date();
var o = {re: re, re2: re, d: d, d2: d};

var oCopy = copy(o);
expect(oCopy.re).toBe(oCopy.re2);
expect(oCopy.d).toBe(oCopy.d2);

oCopy = copy(o, {});
expect(oCopy.re).toBe(oCopy.re2);
expect(oCopy.d).toBe(oCopy.d2);
});

it('should clear destination arrays correctly when source is non-array', function() {
expect(copy(null, [1,2,3])).toEqual([]);
expect(copy(undefined, [1,2,3])).toEqual([]);
expect(copy({0: 1, 1: 2}, [1,2,3])).toEqual([1,2]);
expect(copy(new Date(), [1,2,3])).toEqual([]);
expect(copy(/a/, [1,2,3])).toEqual([]);
expect(copy(true, [1,2,3])).toEqual([]);
});

it('should clear destination objects correctly when source is non-array', function() {
expect(copy(null, {0:1,1:2,2:3})).toEqual({});
expect(copy(undefined, {0:1,1:2,2:3})).toEqual({});
expect(copy(new Date(), {0:1,1:2,2:3})).toEqual({});
expect(copy(/a/, {0:1,1:2,2:3})).toEqual({});
expect(copy(true, {0:1,1:2,2:3})).toEqual({});
});

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

0 comments on commit 0e622f7

Please sign in to comment.