From 5e927eeda4a4eca181d2afaec9131f74c13efd9c Mon Sep 17 00:00:00 2001 From: Chris O Date: Wed, 17 Dec 2014 09:57:54 -0800 Subject: [PATCH 1/5] feat(extend): optionally deep-extend when last parameter is `true` --- src/Angular.js | 16 +++++++++++++--- test/AngularSpec.js | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 80a9eadb9795..cd95e8bf2ec0 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -333,7 +333,7 @@ function setHashKey(obj, h) { * Extends the destination object `dst` by copying own enumerable properties from the `src` object(s) * to `dst`. You can specify multiple `src` objects. If you want to preserve original objects, you can do so * by passing an empty object as the target: `var object = angular.extend({}, object1, object2)`. - * Note: Keep in mind that `angular.extend` does not support recursive merge (deep copy). + * Note: Pass `true` in as the last argument to perform a recursive merge (deep copy). * * @param {Object} dst Destination object. * @param {...Object} src Source object(s). @@ -341,14 +341,24 @@ function setHashKey(obj, h) { */ function extend(dst) { var h = dst.$$hashKey; + var argsLength = arguments.length; + var isDeep = (argsLength >= 3) && (arguments[argsLength - 1] === true); - for (var i = 1, ii = arguments.length; i < ii; i++) { + if (isDeep) --argsLength; + + for (var i = 1; i < argsLength; i++) { var obj = arguments[i]; if (obj) { var keys = Object.keys(obj); for (var j = 0, jj = keys.length; j < jj; j++) { var key = keys[j]; - dst[key] = obj[key]; + var src = obj[key]; + + if (isDeep && isObject(dst[key])) { + src = extend(dst[key], src, true); + } + + dst[key] = src; } } } diff --git a/test/AngularSpec.js b/test/AngularSpec.js index e63789100947..9cdf751d6efc 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -223,6 +223,21 @@ describe('angular', function() { // make sure we retain the old key expect(hashKey(dst)).toEqual(h); }); + + it('should perform deep extend when last argument is true', function() { + var src = { foo: { bar: 'foobar', age: 10, nothing: null, undef: void 0 }}, + dst = { foo: { bazz: 'foobazz' }}; + extend(dst, src, true); + expect(dst).toEqual({ + foo: { + bar: 'foobar', + bazz: 'foobazz', + age: 10, + nothing: null, + undef: void 0 + } + }); + }); }); describe('shallow copy', function() { From f68306103583ac2baa7324292990d808e3617eb8 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Thu, 18 Dec 2014 16:46:30 -0500 Subject: [PATCH 2/5] refactor(extend): apply caitp's review comments for good measure :) Derpity doot Closes #10507 --- src/Angular.js | 30 ++++++++++++++---------- test/AngularSpec.js | 56 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index cd95e8bf2ec0..c396fc7e13b9 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -333,37 +333,43 @@ function setHashKey(obj, h) { * Extends the destination object `dst` by copying own enumerable properties from the `src` object(s) * to `dst`. You can specify multiple `src` objects. If you want to preserve original objects, you can do so * by passing an empty object as the target: `var object = angular.extend({}, object1, object2)`. - * Note: Pass `true` in as the last argument to perform a recursive merge (deep copy). * * @param {Object} dst Destination object. * @param {...Object} src Source object(s). + * @param {boolean=} deep if the last parameter is set to `true`, objects are recursively merged + * (deep copy). Defaults to `false`. * @returns {Object} Reference to `dst`. */ function extend(dst) { var h = dst.$$hashKey; var argsLength = arguments.length; - var isDeep = (argsLength >= 3) && (arguments[argsLength - 1] === true); + var isDeep = false; + if (argsLength >= 3) { + var maybeIsDeep = arguments[argsLength - 1]; + // Secret code to use deep extend without adding hash keys to destination object properties! + if (maybeIsDeep === true || maybeIsDeep === 0xFACECAFE) isDeep = maybeIsDeep; + } if (isDeep) --argsLength; for (var i = 1; i < argsLength; i++) { var obj = arguments[i]; - if (obj) { - var keys = Object.keys(obj); - for (var j = 0, jj = keys.length; j < jj; j++) { - var key = keys[j]; - var src = obj[key]; - - if (isDeep && isObject(dst[key])) { - src = extend(dst[key], src, true); - } + if (!isObject(obj) && !isFunction(obj)) continue; + var keys = Object.keys(obj); + for (var j = 0, jj = keys.length; j < jj; j++) { + var key = keys[j]; + var src = obj[key]; + if (isDeep && isObject(src)) { + if (!isObject(dst[key])) dst[key] = isArray(src) ? [] : {}; + extend(dst[key], src, 0xFACECAFE); + } else { dst[key] = src; } } } - setHashKey(dst, h); + if (isDeep !== 0xFACECAFE) setHashKey(dst, h); return dst; } diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 9cdf751d6efc..855307743bae 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -225,18 +225,66 @@ describe('angular', function() { }); it('should perform deep extend when last argument is true', function() { - var src = { foo: { bar: 'foobar', age: 10, nothing: null, undef: void 0 }}, + var src = { foo: { bar: 'foobar' }}, dst = { foo: { bazz: 'foobazz' }}; extend(dst, src, true); expect(dst).toEqual({ foo: { bar: 'foobar', bazz: 'foobazz', - age: 10, - nothing: null, - undef: void 0 } }); + expect(dst.foo.$$hashKey).toBeUndefined(); + }); + + + it('should replace primitives with objects when deep extending', function() { + var dst = { foo: "bloop" }; + var src = { foo: { bar: { baz: "bloop" }}}; + extend(dst, src, true); + expect(dst).toEqual({ + foo: { + bar: { + baz: "bloop" + } + } + }); + }); + + + it('should replace null values with objects when deep extending', function() { + var dst = { foo: null }; + var src = { foo: { bar: { baz: "bloop" }}}; + extend(dst, src, true); + expect(dst).toEqual({ + foo: { + bar: { + baz: "bloop" + } + } + }); + }); + + + it('should not deep extend function-valued sources when deep extending', function() { + function fn() {} + var dst = { foo: 1 }; + var src = { foo: fn }; + extend(dst, src, true); + expect(dst).toEqual({ + foo: fn + }); + }); + + + it('should create a new array if destination property is a non-object and source property is an array when deep-extending', function() { + var dst = { foo: NaN }; + var src = { foo: [1,2,3] }; + extend(dst, src, true); + expect(dst).toEqual({ + foo: [1,2,3] + }); + expect(dst.foo).not.toBe(src.foo); }); }); From 550f4ac1bff39bfcc5811b94fa05f08d6ce25958 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Thu, 18 Dec 2014 16:50:18 -0500 Subject: [PATCH 3/5] style(Angular.js): make jscs happy --- src/Angular.js | 4 ++-- test/AngularSpec.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index c396fc7e13b9..7770d465a080 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -359,8 +359,8 @@ function extend(dst) { for (var j = 0, jj = keys.length; j < jj; j++) { var key = keys[j]; var src = obj[key]; - - if (isDeep && isObject(src)) { + + if (isDeep && isObject(src)) { if (!isObject(dst[key])) dst[key] = isArray(src) ? [] : {}; extend(dst[key], src, 0xFACECAFE); } else { diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 855307743bae..b1a52567c413 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -231,7 +231,7 @@ describe('angular', function() { expect(dst).toEqual({ foo: { bar: 'foobar', - bazz: 'foobazz', + bazz: 'foobazz' } }); expect(dst.foo.$$hashKey).toBeUndefined(); From b3f74d01c685c902b46bcefc93f03dd43e0888cd Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Fri, 19 Dec 2014 22:26:30 -0500 Subject: [PATCH 4/5] feat(Angular.js): implement angular.merge() helper angular.merge() is like angular.extend(), but will "deeply" copy object properties from source objects. --- src/.jshintrc | 1 + src/Angular.js | 78 +++++++++++++++++++++++++++----------------- src/AngularPublic.js | 1 + test/.jshintrc | 1 + test/AngularSpec.js | 33 +++++++++++-------- 5 files changed, 70 insertions(+), 44 deletions(-) diff --git a/src/.jshintrc b/src/.jshintrc index 55414b5b3fb8..fdb544d3fc9b 100644 --- a/src/.jshintrc +++ b/src/.jshintrc @@ -34,6 +34,7 @@ "nextUid": false, "setHashKey": false, "extend": false, + "merge": false, "int": false, "inherit": false, "noop": false, diff --git a/src/Angular.js b/src/Angular.js index 7770d465a080..8647a31d37a3 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -323,6 +323,32 @@ function setHashKey(obj, h) { } } + +function baseExtend(dst, objs, deep) { + var h = dst.$$hashKey; + + for (var i = 0, ii = objs.length; i < ii; ++i) { + var obj = objs[i]; + if (!isObject(obj) && !isFunction(obj)) continue; + var keys = Object.keys(obj); + for (var j = 0, jj = keys.length; j < jj; j++) { + var key = keys[j]; + var src = obj[key]; + + if (deep && isObject(src)) { + if (!isObject(dst[key])) dst[key] = isArray(src) ? [] : {}; + baseExtend(dst[key], [src], true); + } else { + dst[key] = src; + } + } + } + + setHashKey(dst, h); + return dst; +} + + /** * @ngdoc function * @name angular.extend @@ -336,41 +362,33 @@ function setHashKey(obj, h) { * * @param {Object} dst Destination object. * @param {...Object} src Source object(s). - * @param {boolean=} deep if the last parameter is set to `true`, objects are recursively merged - * (deep copy). Defaults to `false`. * @returns {Object} Reference to `dst`. */ function extend(dst) { - var h = dst.$$hashKey; - var argsLength = arguments.length; - var isDeep = false; - if (argsLength >= 3) { - var maybeIsDeep = arguments[argsLength - 1]; - // Secret code to use deep extend without adding hash keys to destination object properties! - if (maybeIsDeep === true || maybeIsDeep === 0xFACECAFE) isDeep = maybeIsDeep; - } - - if (isDeep) --argsLength; + return baseExtend(dst, slice.call(arguments, 1), false); +} - for (var i = 1; i < argsLength; i++) { - var obj = arguments[i]; - if (!isObject(obj) && !isFunction(obj)) continue; - var keys = Object.keys(obj); - for (var j = 0, jj = keys.length; j < jj; j++) { - var key = keys[j]; - var src = obj[key]; - if (isDeep && isObject(src)) { - if (!isObject(dst[key])) dst[key] = isArray(src) ? [] : {}; - extend(dst[key], src, 0xFACECAFE); - } else { - dst[key] = src; - } - } - } - - if (isDeep !== 0xFACECAFE) setHashKey(dst, h); - return dst; +/** +* @ngdoc function +* @name angular.merge +* @module ng +* @kind function +* +* @description +* Deeply extends the destination object `dst` by copying own enumerable properties from the `src` object(s) +* to `dst`. You can specify multiple `src` objects. If you want to preserve original objects, you can do so +* by passing an empty object as the target: `var object = angular.merge({}, object1, object2)`. +* +* Unlike {@link angular.extend extend()}, `merge()` recursively descends into object properties of source +* objects, performing a deep copy. +* +* @param {Object} dst Destination object. +* @param {...Object} src Source object(s). +* @returns {Object} Reference to `dst`. +*/ +function merge(dst) { + return baseExtend(dst, slice.call(arguments, 1), true); } function int(str) { diff --git a/src/AngularPublic.js b/src/AngularPublic.js index b81257b9fff7..db9e2be0141c 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -116,6 +116,7 @@ function publishExternalAPI(angular) { 'bootstrap': bootstrap, 'copy': copy, 'extend': extend, + 'merge': merge, 'equals': equals, 'element': jqLite, 'forEach': forEach, diff --git a/test/.jshintrc b/test/.jshintrc index 6bca8382f2d9..87d775ae7f03 100644 --- a/test/.jshintrc +++ b/test/.jshintrc @@ -31,6 +31,7 @@ "nextUid": false, "setHashKey": false, "extend": false, + "merge": false, "int": false, "inherit": false, "noop": false, diff --git a/test/AngularSpec.js b/test/AngularSpec.js index b1a52567c413..3800b4c01c30 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -223,25 +223,29 @@ describe('angular', function() { // make sure we retain the old key expect(hashKey(dst)).toEqual(h); }); + }); + - it('should perform deep extend when last argument is true', function() { - var src = { foo: { bar: 'foobar' }}, - dst = { foo: { bazz: 'foobazz' }}; - extend(dst, src, true); + describe('merge', function() { + it('should recursively copy objects into dst from left to right', function() { + var dst = { foo: { bar: 'foobar' }}; + var src1 = { foo: { bazz: 'foobazz' }}; + var src2 = { foo: { bozz: 'foobozz' }}; + merge(dst, src1, src2); expect(dst).toEqual({ foo: { bar: 'foobar', - bazz: 'foobazz' + bazz: 'foobazz', + bozz: 'foobozz' } }); - expect(dst.foo.$$hashKey).toBeUndefined(); }); - it('should replace primitives with objects when deep extending', function() { + it('should replace primitives with objects', function() { var dst = { foo: "bloop" }; var src = { foo: { bar: { baz: "bloop" }}}; - extend(dst, src, true); + merge(dst, src); expect(dst).toEqual({ foo: { bar: { @@ -252,10 +256,10 @@ describe('angular', function() { }); - it('should replace null values with objects when deep extending', function() { + it('should replace null values in destination with objects', function() { var dst = { foo: null }; var src = { foo: { bar: { baz: "bloop" }}}; - extend(dst, src, true); + merge(dst, src, true); expect(dst).toEqual({ foo: { bar: { @@ -266,21 +270,21 @@ describe('angular', function() { }); - it('should not deep extend function-valued sources when deep extending', function() { + it('should copy references to functions by value rather than merging', function() { function fn() {} var dst = { foo: 1 }; var src = { foo: fn }; - extend(dst, src, true); + merge(dst, src); expect(dst).toEqual({ foo: fn }); }); - it('should create a new array if destination property is a non-object and source property is an array when deep-extending', function() { + it('should create a new array if destination property is a non-object and source property is an array', function() { var dst = { foo: NaN }; var src = { foo: [1,2,3] }; - extend(dst, src, true); + merge(dst, src, true); expect(dst).toEqual({ foo: [1,2,3] }); @@ -288,6 +292,7 @@ describe('angular', function() { }); }); + describe('shallow copy', function() { it('should make a copy', function() { var original = {key:{}}; From 090e9b1b40d221bdf13c84026271340c355f473e Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Fri, 19 Dec 2014 22:30:42 -0500 Subject: [PATCH 5/5] perf(extend/merge): explicitly ignore hashKey property Previously, hashKeys were copied from source objects and subsequently deleted or replaced with the original. Now, there is no need to delete properties (which can be expensive), and no need to track the initial hashKey value. --- src/Angular.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 8647a31d37a3..5d217c8f25d2 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -325,14 +325,13 @@ function setHashKey(obj, h) { function baseExtend(dst, objs, deep) { - var h = dst.$$hashKey; - for (var i = 0, ii = objs.length; i < ii; ++i) { var obj = objs[i]; if (!isObject(obj) && !isFunction(obj)) continue; var keys = Object.keys(obj); for (var j = 0, jj = keys.length; j < jj; j++) { var key = keys[j]; + if (key === '$$hashKey') continue; var src = obj[key]; if (deep && isObject(src)) { @@ -344,7 +343,6 @@ function baseExtend(dst, objs, deep) { } } - setHashKey(dst, h); return dst; }