Rewrite the clone and merge helpers (chartjs#4422)

The `clone` method now accepts any type of input but also recursively perform a deep copy of the array items. Rewrite the `configMerge` and `scaleMerge` helpers which now rely on a new generic and customizable `merge` method, that one accepts a target object in which multiple sources are deep copied. Note that the target (first argument) is not cloned and will be modified after calling `merge(target, sources)`. Add a `mergeIf` helper which merge the source properties only if they do not exist in the target object.

src/core/core.helpers.js

@@ -8,19 +8,7 @@ module.exports = function(Chart) {
 	var helpers = Chart.helpers;
 
 	// -- Basic js utility methods
-	helpers.clone = function(obj) {
-		var objClone = {};
-		helpers.each(obj, function(value, key) {
-			if (helpers.isArray(value)) {
-				objClone[key] = value.slice(0);
-			} else if (typeof value === 'object' && value !== null) {
-				objClone[key] = helpers.clone(value);
-			} else {
-				objClone[key] = value;
-			}
-		});
-		return objClone;
-	};
+
 	helpers.extend = function(base) {
 		var setFn = function(value, key) {
 			base[key] = value;
@@ -30,75 +18,60 @@ module.exports = function(Chart) {
 		}
 		return base;
 	};
-	// Need a special merge function to chart configs since they are now grouped
-	helpers.configMerge = function(_base) {
-		var base = helpers.clone(_base);
-		helpers.each(Array.prototype.slice.call(arguments, 1), function(extension) {
-			helpers.each(extension, function(value, key) {
-				var baseHasProperty = base.hasOwnProperty(key);
-				var baseVal = baseHasProperty ? base[key] : {};
+
+	helpers.configMerge = function(/* objects ... */) {
+		return helpers.merge(helpers.clone(arguments[0]), [].slice.call(arguments, 1), {
+			merger: function(key, target, source, options) {
+				var tval = target[key] || {};
+				var sval = source[key];
 
 				if (key === 'scales') {
-					// Scale config merging is complex. Add our own function here for that
-					base[key] = helpers.scaleMerge(baseVal, value);
+					// scale config merging is complex. Add our own function here for that
+					target[key] = helpers.scaleMerge(tval, sval);
 				} else if (key === 'scale') {
-					// Used in polar area & radar charts since there is only one scale
-					base[key] = helpers.configMerge(baseVal, Chart.scaleService.getScaleDefaults(value.type), value);
-				} else if (baseHasProperty
-						&& typeof baseVal === 'object'
-						&& !helpers.isArray(baseVal)
-						&& baseVal !== null
-						&& typeof value === 'object'
-						&& !helpers.isArray(value)) {
-					// If we are overwriting an object with an object, do a merge of the properties.
-					base[key] = helpers.configMerge(baseVal, value);
+					// used in polar area & radar charts since there is only one scale
+					target[key] = helpers.merge(tval, [Chart.scaleService.getScaleDefaults(sval.type), sval]);
 				} else {
-					// can just overwrite the value in this case
-					base[key] = value;
+					helpers._merger(key, target, source, options);
 				}
-			});
+			}
 		});
-
-		return base;
 	};
-	helpers.scaleMerge = function(_base, extension) {
-		var base = helpers.clone(_base);
-
-		helpers.each(extension, function(value, key) {
-			if (key === 'xAxes' || key === 'yAxes') {
-				// These properties are arrays of items
-				if (base.hasOwnProperty(key)) {
-					helpers.each(value, function(valueObj, index) {
-						var axisType = helpers.valueOrDefault(valueObj.type, key === 'xAxes' ? 'category' : 'linear');
-						var axisDefaults = Chart.scaleService.getScaleDefaults(axisType);
-						if (index >= base[key].length || !base[key][index].type) {
-							base[key].push(helpers.configMerge(axisDefaults, valueObj));
-						} else if (valueObj.type && valueObj.type !== base[key][index].type) {
-							// Type changed. Bring in the new defaults before we bring in valueObj so that valueObj can override the correct scale defaults
-							base[key][index] = helpers.configMerge(base[key][index], axisDefaults, valueObj);
+
+	helpers.scaleMerge = function(/* objects ... */) {
+		return helpers.merge(helpers.clone(arguments[0]), [].slice.call(arguments, 1), {
+			merger: function(key, target, source, options) {
+				if (key === 'xAxes' || key === 'yAxes') {
+					var slen = source[key].length;
+					var i, type, scale, defaults;
+
+					if (!target[key]) {
+						target[key] = [];
+					}
+
+					for (i = 0; i < slen; ++i) {
+						scale = source[key][i];
+						type = helpers.valueOrDefault(scale.type, key === 'xAxes'? 'category' : 'linear');
+						defaults = Chart.scaleService.getScaleDefaults(type);
+
+						if (i >= target[key].length) {
+							target[key].push({});
+						}
+
+						if (!target[key][i].type || (scale.type && scale.type !== target[key][i].type)) {
+							// new/untyped scale or type changed: let's apply the new defaults
+							// then merge source scale to correctly overwrite the defaults.
+							helpers.merge(target[key][i], [defaults, scale]);
 						} else {
-							// Type is the same
-							base[key][index] = helpers.configMerge(base[key][index], valueObj);
+							// scales type are the same
+							helpers.merge(target[key][i], scale);
 						}
-					});
+					}
 				} else {
-					base[key] = [];
-					helpers.each(value, function(valueObj) {
-						var axisType = helpers.valueOrDefault(valueObj.type, key === 'xAxes' ? 'category' : 'linear');
-						base[key].push(helpers.configMerge(Chart.scaleService.getScaleDefaults(axisType), valueObj));
-					});
+					helpers._merger(key, target, source, options);
 				}
-			} else if (base.hasOwnProperty(key) && typeof base[key] === 'object' && base[key] !== null && typeof value === 'object') {
-				// If we are overwriting an object with an object, do a merge of the properties.
-				base[key] = helpers.configMerge(base[key], value);
-
-			} else {
-				// can just overwrite the value in this case
-				base[key] = value;
 			}
 		});
-
-		return base;
 	};
 
 	helpers.where = function(collection, filterCallback) {

src/core/core.scaleService.js

@@ -22,7 +22,7 @@ module.exports = function(Chart) {
 		},
 		getScaleDefaults: function(type) {
 			// Return the scale defaults merged with the global settings so that we always use the latest ones
-			return this.defaults.hasOwnProperty(type) ? helpers.scaleMerge(Chart.defaults.scale, this.defaults[type]) : {};
+			return this.defaults.hasOwnProperty(type) ? helpers.merge({}, [Chart.defaults.scale, this.defaults[type]]) : {};
 		},
 		updateScaleDefaults: function(type, additions) {
 			var defaults = this.defaults;

src/helpers/helpers.core.js

@@ -147,6 +147,110 @@ module.exports = function(Chart) {
 			}
 
 			return true;
+		},
+
+		/**
+		 * Returns a deep copy of `source` without keeping references on objects and arrays.
+		 * @param {*} source - The value to clone.
+		 * @returns {*}
+		 */
+		clone: function(source) {
+			if (helpers.isArray(source)) {
+				return source.map(helpers.clone);
+			}
+
+			if (helpers.isObject(source)) {
+				var target = {};
+				var keys = Object.keys(source);
+				var klen = keys.length;
+				var k = 0;
+
+				for (; k<klen; ++k) {
+					target[keys[k]] = helpers.clone(source[keys[k]]);
+				}
+
+				return target;
+			}
+
+			return source;
+		},
+
+		/**
+		 * The default merger when Chart.helpers.merge is called without merger option.
+		 * Note(SB): this method is also used by configMerge and scaleMerge as fallback.
+		 * @private
+		 */
+		_merger: function(key, target, source, options) {
+			var tval = target[key];
+			var sval = source[key];
+
+			if (helpers.isObject(tval) && helpers.isObject(sval)) {
+				helpers.merge(tval, sval, options);
+			} else {
+				target[key] = helpers.clone(sval);
+			}
+		},
+
+		/**
+		 * Merges source[key] in target[key] only if target[key] is undefined.
+		 * @private
+		 */
+		_mergerIf: function(key, target, source) {
+			var tval = target[key];
+			var sval = source[key];
+
+			if (helpers.isObject(tval) && helpers.isObject(sval)) {
+				helpers.mergeIf(tval, sval);
+			} else if (!target.hasOwnProperty(key)) {
+				target[key] = helpers.clone(sval);
+			}
+		},
+
+		/**
+		 * Recursively deep copies `source` properties into `target` with the given `options`.
+		 * IMPORTANT: `target` is not cloned and will be updated with `source` properties.
+		 * @param {Object} target - The target object in which all sources are merged into.
+		 * @param {Object|Array(Object)} source - Object(s) to merge into `target`.
+		 * @param {Object} [options] - Merging options:
+		 * @param {Function} [options.merger] - The merge method (key, target, source, options)
+		 * @returns {Object} The `target` object.
+		 */
+		merge: function(target, source, options) {
+			var sources = helpers.isArray(source)? source : [source];
+			var ilen = sources.length;
+			var merge, i, keys, klen, k;
+
+			if (!helpers.isObject(target)) {
+				return target;
+			}
+
+			options = options || {};
+			merge = options.merger || helpers._merger;
+
+			for (i=0; i<ilen; ++i) {
+				source = sources[i];
+				if (!helpers.isObject(source)) {
+					continue;
+				}
+
+				keys = Object.keys(source);
+				for (k=0, klen = keys.length; k<klen; ++k) {
+					merge(keys[k], target, source, options);
+				}
+			}
+
+			return target;
+		},
+
+		/**
+		 * Recursively deep copies `source` properties into `target` *only* if not defined in target.
+		 * IMPORTANT: `target` is not cloned and will be updated with `source` properties.
+		 * @param {Object} target - The target object in which all sources are merged into.
+		 * @param {Object|Array(Object)} source - Object(s) to merge into `target`.
+		 * @returns {Object} The `target` object.
+		 */
+		mergeIf: function(target, source) {
+			return helpers.merge(target, source, {merger: helpers._mergerIf});
 		}
 	};
 

src/plugins/plugin.legend.js

@@ -524,7 +524,7 @@ module.exports = function(Chart) {
 			var legend = chart.legend;
 
 			if (legendOpts) {
-				legendOpts = helpers.configMerge(Chart.defaults.global.legend, legendOpts);
+				helpers.mergeIf(legendOpts, Chart.defaults.global.legend);
 
 				if (legend) {
 					layout.configure(chart, legend, legendOpts);

src/plugins/plugin.title.js

@@ -224,7 +224,7 @@ module.exports = function(Chart) {
 			var titleBlock = chart.titleBlock;
 
 			if (titleOpts) {
-				titleOpts = helpers.configMerge(Chart.defaults.global.title, titleOpts);
+				helpers.mergeIf(titleOpts, Chart.defaults.global.title);
 
 				if (titleBlock) {
 					layout.configure(chart, titleBlock, titleOpts);

test/specs/core.helpers.tests.js

@@ -6,25 +6,6 @@ describe('Core helper tests', function() {
 		helpers = window.Chart.helpers;
 	});
 
-	it('should clone an object', function() {
-		var testData = {
-			myProp1: 'abc',
-			myProp2: ['a', 'b'],
-			myProp3: {
-				myProp4: 5,
-				myProp5: [1, 2]
-			}
-		};
-
-		var clone = helpers.clone(testData);
-		expect(clone).toEqual(testData);
-		expect(clone).not.toBe(testData);
-
-		expect(clone.myProp2).not.toBe(testData.myProp2);
-		expect(clone.myProp3).not.toBe(testData.myProp3);
-		expect(clone.myProp3.myProp5).not.toBe(testData.myProp3.myProp5);
-	});
-
 	it('should extend an object', function() {
 		var original = {
 			myProp1: 'abc',