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

fix(angular.copy): support circular references #7618

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 43 additions & 13 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ function isLeafNode (node) {
</file>
</example>
*/
function copy(source, destination) {
function copy(source, destination, stackSource, stackDest) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so sure about having the stack* parameters public.
Maybe a private version of .copy should be created and used internally.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it really hurts to expose them, we could separate it but all it would do is add an extra function call and make the code a bit bigger --- probably not worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to what cait said. just not document them.

if (isWindow(source) || isScope(source)) {
throw ngMinErr('cpws',
"Can't copy! Making copies of Window or Scope instances is not supported.");
Expand All @@ -767,52 +767,82 @@ function copy(source, destination) {
destination = source;
if (source) {
if (isArray(source)) {
destination = copy(source, []);
destination = copy(source, [], stackSource, stackDest);
} else if (isDate(source)) {
destination = new Date(source.getTime());
} else if (isRegExp(source)) {
destination = new RegExp(source.source);
} else if (isObject(source)) {
destination = copy(source, {});
destination = copy(source, {}, stackSource, stackDest);
}
}
} else {
if (source === destination) throw ngMinErr('cpi',
"Can't copy! Source and destination are identical.");

stackSource = stackSource || [];
stackDest = stackDest || [];

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

stackSource.push(source);
stackDest.push(destination);
}

var result;
if (isArray(source)) {
destination.length = 0;
for ( var i = 0; i < source.length; i++) {
destination.push(copy(source[i]));
result = copy(source[i], null, stackSource, stackDest);
if (isObject(source[i])) {
stackSource.push(source[i]);
stackDest.push(result);
}
destination.push(result);
}
} else {
var h = destination.$$hashKey;
forEach(destination, function(value, key) {
delete destination[key];
});
for ( var key in source) {
destination[key] = copy(source[key]);
result = copy(source[key], null, stackSource, stackDest);
if (isObject(source[key])) {
stackSource.push(source[key]);
stackDest.push(result);
}
destination[key] = result;
}
setHashKey(destination,h);
}

}
return destination;
}

/**
* Create a shallow copy of an object
* Creates a shallow copy of an object, an array or a primitive
*/
function shallowCopy(src, dst) {
dst = dst || {};
if (isArray(src)) {
dst = dst || [];

for ( var i = 0; i < src.length; i++) {
dst[i] = src[i];
}
} else if (isObject(src)) {
dst = dst || {};

for(var key in src) {
// shallowCopy is only ever called by $compile nodeLinkFn, which has control over src
// so we don't need to worry about using our custom hasOwnProperty here
if (src.hasOwnProperty(key) && !(key.charAt(0) === '$' && key.charAt(1) === '$')) {
dst[key] = src[key];
for (var key in src) {
if (hasOwnProperty.call(src, key) && !(key.charAt(0) === '$' && key.charAt(1) === '$')) {
dst[key] = src[key];
}
}
}

return dst;
return dst || src;
}


Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/ngClass.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function classDirective(name, selector) {
updateClasses(oldClasses, newClasses);
}
}
oldVal = copy(newVal);
oldVal = shallowCopy(newVal);
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
// we need to work of an array, so we need to see if anything was inserted/removed
scope.$watch(function selectMultipleWatch() {
if (!equals(lastView, ctrl.$viewValue)) {
lastView = copy(ctrl.$viewValue);
lastView = shallowCopy(ctrl.$viewValue);
ctrl.$render();
}
});
Expand Down
8 changes: 4 additions & 4 deletions src/ng/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ function $HttpProvider() {
common: {
'Accept': 'application/json, text/plain, */*'
},
post: copy(CONTENT_TYPE_APPLICATION_JSON),
put: copy(CONTENT_TYPE_APPLICATION_JSON),
patch: copy(CONTENT_TYPE_APPLICATION_JSON)
post: shallowCopy(CONTENT_TYPE_APPLICATION_JSON),
put: shallowCopy(CONTENT_TYPE_APPLICATION_JSON),
patch: shallowCopy(CONTENT_TYPE_APPLICATION_JSON)
},

xsrfCookieName: 'XSRF-TOKEN',
Expand Down Expand Up @@ -874,7 +874,7 @@ function $HttpProvider() {
} else {
// serving from cache
if (isArray(cachedResp)) {
resolvePromise(cachedResp[1], cachedResp[0], copy(cachedResp[2]), cachedResp[3]);
resolvePromise(cachedResp[1], cachedResp[0], shallowCopy(cachedResp[2]), cachedResp[3]);
} else {
resolvePromise(cachedResp, 200, {}, 'OK');
}
Expand Down
2 changes: 1 addition & 1 deletion src/ng/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ function $ParseProvider() {
self.$$postDigestQueue.push(function () {
// create a copy if the value is defined and it is not a $sce value
if ((stable = isDefined(lastValue)) && !lastValue.$$unwrapTrustedValue) {
lastValue = copy(lastValue);
lastValue = copy(lastValue, null);
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ function $RootScopeProvider(){
&& isNaN(value) && isNaN(last)))) {
dirty = true;
lastDirtyWatch = watch;
watch.last = watch.eq ? copy(value) : value;
watch.last = watch.eq ? copy(value, null) : value;
watch.fn(value, ((last === initWatchVal) ? value : last), current);
if (ttl < 5) {
logIdx = 4 - ttl;
Expand Down
2 changes: 1 addition & 1 deletion src/ng/sce.js
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ function $SceProvider() {
'document. See http://docs.angularjs.org/api/ng.$sce for more information.');
}

var sce = copy(SCE_CONTEXTS);
var sce = shallowCopy(SCE_CONTEXTS);

/**
* @ngdoc method
Expand Down
33 changes: 33 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,20 @@ describe('angular', function() {
// make sure we retain the old key
expect(hashKey(dst)).toEqual(h);
});

it('should handle circular references when circularRefs is turned on', function () {
var a = {b: {a: null}, self: null, selfs: [null, null, [null]]};
a.b.a = a;
a.self = a;
a.selfs = [a, a.b, [a]];

var aCopy = copy(a, null);
expect(aCopy).toEqual(a);

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

describe("extend", function() {
Expand Down Expand Up @@ -218,6 +232,25 @@ describe('angular', function() {
expect(clone.hello).toBeUndefined();
expect(clone.goodbye).toBe("world");
});

it('should handle arrays', function() {
var original = [{}, 1],
clone = [];

var aCopy = shallowCopy(original);
expect(aCopy).not.toBe(original);
expect(aCopy).toEqual(original);
expect(aCopy[0]).toBe(original[0]);

expect(shallowCopy(original, clone)).toBe(clone);
expect(clone).toEqual(original);
});

it('should handle primitives', function() {
expect(shallowCopy('test')).toBe('test');
expect(shallowCopy(3)).toBe(3);
expect(shallowCopy(true)).toBe(true);
});
});

describe('elementHTML', function() {
Expand Down