From: Simon Brunel Date: Tue, 29 Jan 2019 16:52:21 +0000 (+0100) Subject: Deprecate configMerge and scaleMerge helpers (#6022) X-Git-Tag: v2.8.0-rc.1~43 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0697d0de9073c456565c9deeb20cb36d85e858c9;p=thirdparty%2FChart.js.git Deprecate configMerge and scaleMerge helpers (#6022) These methods shouldn't have been public since they are specific to the chart controller internal logic. Note that this scale custom merging will be removed in v3. --- diff --git a/src/core/core.controller.js b/src/core/core.controller.js index a552bb58a..1d85ff1ba 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -35,16 +35,80 @@ defaults._set('global', { responsiveAnimationDuration: 0 }); +/** + * Recursively merge the given config objects representing the `scales` option + * by incorporating scale defaults in `xAxes` and `yAxes` array items, then + * returns a deep copy of the result, thus doesn't alter inputs. + */ +function mergeScaleConfig(/* config objects ... */) { + return helpers.merge({}, [].slice.call(arguments), { + merger: function(key, target, source, options) { + if (key === 'xAxes' || key === 'yAxes') { + var slen = source[key].length; + var i, type, scale; + + if (!target[key]) { + target[key] = []; + } + + for (i = 0; i < slen; ++i) { + scale = source[key][i]; + type = valueOrDefault(scale.type, key === 'xAxes' ? 'category' : 'linear'); + + 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], [scaleService.getScaleDefaults(type), scale]); + } else { + // scales type are the same + helpers.merge(target[key][i], scale); + } + } + } else { + helpers._merger(key, target, source, options); + } + } + }); +} + +/** + * Recursively merge the given config objects as the root options by handling + * default scale options for the `scales` and `scale` properties, then returns + * a deep copy of the result, thus doesn't alter inputs. + */ +function mergeConfig(/* config objects ... */) { + return helpers.merge({}, [].slice.call(arguments), { + 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 + target[key] = mergeScaleConfig(tval, sval); + } else if (key === 'scale') { + // used in polar area & radar charts since there is only one scale + target[key] = helpers.merge(tval, [scaleService.getScaleDefaults(sval.type), sval]); + } else { + helpers._merger(key, target, source, options); + } + } + }); +} + function initConfig(config) { config = config || {}; - // Do NOT use configMerge() for the data object because this method merges arrays + // Do NOT use mergeConfig for the data object because this method merges arrays // and so would change references to labels and datasets, preventing data updates. var data = config.data = config.data || {}; data.datasets = data.datasets || []; data.labels = data.labels || []; - config.options = helpers.configMerge( + config.options = mergeConfig( defaults.global, defaults[config.type], config.options || {}); @@ -59,7 +123,7 @@ function updateConfig(chart) { layouts.removeBox(chart, scale); }); - newOptions = helpers.configMerge( + newOptions = mergeConfig( defaults.global, defaults[chart.config.type], newOptions); @@ -981,3 +1045,21 @@ Chart.Controller = Chart; * @private */ Chart.types = {}; + +/** + * Provided for backward compatibility, not available anymore. + * @namespace Chart.helpers.configMerge + * @deprecated since version 2.8.0 + * @todo remove at version 3 + * @private + */ +helpers.configMerge = mergeConfig; + +/** + * Provided for backward compatibility, not available anymore. + * @namespace Chart.helpers.scaleMerge + * @deprecated since version 2.8.0 + * @todo remove at version 3 + * @private + */ +helpers.scaleMerge = mergeScaleConfig; diff --git a/src/core/core.helpers.js b/src/core/core.helpers.js index 82f2c9811..a30b3723b 100644 --- a/src/core/core.helpers.js +++ b/src/core/core.helpers.js @@ -5,66 +5,11 @@ var color = require('chartjs-color'); var defaults = require('./core.defaults'); var helpers = require('../helpers/index'); -var scaleService = require('../core/core.scaleService'); module.exports = function() { // -- Basic js utility methods - 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 - target[key] = helpers.scaleMerge(tval, sval); - } else if (key === 'scale') { - // used in polar area & radar charts since there is only one scale - target[key] = helpers.merge(tval, [scaleService.getScaleDefaults(sval.type), sval]); - } else { - helpers._merger(key, target, source, options); - } - } - }); - }; - - 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; - - if (!target[key]) { - target[key] = []; - } - - for (i = 0; i < slen; ++i) { - scale = source[key][i]; - type = helpers.valueOrDefault(scale.type, key === 'xAxes' ? 'category' : 'linear'); - - 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], [scaleService.getScaleDefaults(type), scale]); - } else { - // scales type are the same - helpers.merge(target[key][i], scale); - } - } - } else { - helpers._merger(key, target, source, options); - } - } - }); - }; - helpers.where = function(collection, filterCallback) { if (helpers.isArray(collection) && Array.prototype.filter) { return collection.filter(filterCallback); diff --git a/src/helpers/helpers.core.js b/src/helpers/helpers.core.js index 4464eb7ba..1b798c925 100644 --- a/src/helpers/helpers.core.js +++ b/src/helpers/helpers.core.js @@ -192,7 +192,7 @@ var helpers = { /** * 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. + * Note(SB): also used by mergeConfig and mergeScaleConfig as fallback. * @private */ _merger: function(key, target, source, options) { diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index 9bcb45d1f..819074393 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -159,6 +159,150 @@ describe('Chart', function() { }); }); + describe('when merging scale options', function() { + beforeEach(function() { + Chart.helpers.merge(Chart.defaults.scale, { + _jasmineCheckA: 'a0', + _jasmineCheckB: 'b0', + _jasmineCheckC: 'c0' + }); + + Chart.helpers.merge(Chart.scaleService.defaults.logarithmic, { + _jasmineCheckB: 'b1', + _jasmineCheckC: 'c1', + }); + }); + + afterEach(function() { + delete Chart.defaults.scale._jasmineCheckA; + delete Chart.defaults.scale._jasmineCheckB; + delete Chart.defaults.scale._jasmineCheckC; + delete Chart.scaleService.defaults.logarithmic._jasmineCheckB; + delete Chart.scaleService.defaults.logarithmic._jasmineCheckC; + }); + + it('should default to "category" for x scales and "linear" for y scales', function() { + var chart = acquireChart({ + type: 'line', + options: { + scales: { + xAxes: [ + {id: 'foo0'}, + {id: 'foo1'} + ], + yAxes: [ + {id: 'bar0'}, + {id: 'bar1'} + ] + } + } + }); + + expect(chart.scales.foo0.type).toBe('category'); + expect(chart.scales.foo1.type).toBe('category'); + expect(chart.scales.bar0.type).toBe('linear'); + expect(chart.scales.bar1.type).toBe('linear'); + }); + + it('should correctly apply defaults on central scale', function() { + var chart = acquireChart({ + type: 'line', + options: { + scale: { + id: 'foo', + type: 'logarithmic', + _jasmineCheckC: 'c2', + _jasmineCheckD: 'd2' + } + } + }); + + // let's check a few values from the user options and defaults + + expect(chart.scales.foo.type).toBe('logarithmic'); + expect(chart.scales.foo.options).toBe(chart.options.scale); + expect(chart.scales.foo.options).toEqual( + jasmine.objectContaining({ + _jasmineCheckA: 'a0', + _jasmineCheckB: 'b1', + _jasmineCheckC: 'c2', + _jasmineCheckD: 'd2' + })); + }); + + it('should correctly apply defaults on xy scales', function() { + var chart = acquireChart({ + type: 'line', + options: { + scales: { + xAxes: [{ + id: 'foo', + type: 'logarithmic', + _jasmineCheckC: 'c2', + _jasmineCheckD: 'd2' + }], + yAxes: [{ + id: 'bar', + type: 'time', + _jasmineCheckC: 'c2', + _jasmineCheckE: 'e2' + }] + } + } + }); + + expect(chart.scales.foo.type).toBe('logarithmic'); + expect(chart.scales.foo.options).toBe(chart.options.scales.xAxes[0]); + expect(chart.scales.foo.options).toEqual( + jasmine.objectContaining({ + _jasmineCheckA: 'a0', + _jasmineCheckB: 'b1', + _jasmineCheckC: 'c2', + _jasmineCheckD: 'd2' + })); + + expect(chart.scales.bar.type).toBe('time'); + expect(chart.scales.bar.options).toBe(chart.options.scales.yAxes[0]); + expect(chart.scales.bar.options).toEqual( + jasmine.objectContaining({ + _jasmineCheckA: 'a0', + _jasmineCheckB: 'b0', + _jasmineCheckC: 'c2', + _jasmineCheckE: 'e2' + })); + }); + + it('should not alter defaults when merging config', function() { + var chart = acquireChart({ + type: 'line', + options: { + _jasmineCheck: 42, + scales: { + xAxes: [{ + id: 'foo', + type: 'linear', + _jasmineCheck: 42, + }], + yAxes: [{ + id: 'bar', + type: 'category', + _jasmineCheck: 42, + }] + } + } + }); + + expect(chart.options._jasmineCheck).toBeDefined(); + expect(chart.scales.foo.options._jasmineCheck).toBeDefined(); + expect(chart.scales.bar.options._jasmineCheck).toBeDefined(); + + expect(Chart.defaults.line._jasmineCheck).not.toBeDefined(); + expect(Chart.defaults.global._jasmineCheck).not.toBeDefined(); + expect(Chart.scaleService.defaults.linear._jasmineCheck).not.toBeDefined(); + expect(Chart.scaleService.defaults.category._jasmineCheck).not.toBeDefined(); + }); + }); + describe('config.options.responsive: false', function() { it('should not inject the resizer element', function() { var chart = acquireChart({ diff --git a/test/specs/core.helpers.tests.js b/test/specs/core.helpers.tests.js index 796148aaf..b0c7431c8 100644 --- a/test/specs/core.helpers.tests.js +++ b/test/specs/core.helpers.tests.js @@ -6,172 +6,6 @@ describe('Core helper tests', function() { helpers = window.Chart.helpers; }); - it('should merge a normal config without scales', function() { - var baseConfig = { - valueProp: 5, - arrayProp: [1, 2, 3, 4, 5, 6], - objectProp: { - prop1: 'abc', - prop2: 56 - } - }; - - var toMerge = { - valueProp2: null, - arrayProp: ['a', 'c'], - objectProp: { - prop1: 'c', - prop3: 'prop3' - } - }; - - var merged = helpers.configMerge(baseConfig, toMerge); - expect(merged).toEqual({ - valueProp: 5, - valueProp2: null, - arrayProp: ['a', 'c'], - objectProp: { - prop1: 'c', - prop2: 56, - prop3: 'prop3' - } - }); - }); - - it('should merge scale configs', function() { - var baseConfig = { - scales: { - prop1: { - abc: 123, - def: '456' - }, - prop2: 777, - yAxes: [{ - type: 'linear', - }, { - type: 'log' - }] - } - }; - - var toMerge = { - scales: { - prop1: { - def: 'bbb', - ghi: 78 - }, - prop2: null, - yAxes: [{ - type: 'linear', - axisProp: 456 - }, { - // pulls in linear default config since axis type changes - type: 'linear', - position: 'right' - }, { - // Pulls in linear default config since axis not in base - type: 'linear' - }] - } - }; - - var merged = helpers.configMerge(baseConfig, toMerge); - expect(merged).toEqual({ - scales: { - prop1: { - abc: 123, - def: 'bbb', - ghi: 78 - }, - prop2: null, - yAxes: [{ - type: 'linear', - axisProp: 456 - }, { - display: true, - - gridLines: { - color: 'rgba(0, 0, 0, 0.1)', - drawBorder: true, - drawOnChartArea: true, - drawTicks: true, // draw ticks extending towards the label - tickMarkLength: 10, - lineWidth: 1, - offsetGridLines: false, - display: true, - zeroLineColor: 'rgba(0,0,0,0.25)', - zeroLineWidth: 1, - zeroLineBorderDash: [], - zeroLineBorderDashOffset: 0.0, - borderDash: [], - borderDashOffset: 0.0 - }, - position: 'right', - offset: false, - scaleLabel: Chart.defaults.scale.scaleLabel, - ticks: { - beginAtZero: false, - minRotation: 0, - maxRotation: 50, - mirror: false, - padding: 0, - reverse: false, - display: true, - callback: merged.scales.yAxes[1].ticks.callback, // make it nicer, then check explicitly below - autoSkip: true, - autoSkipPadding: 0, - labelOffset: 0, - minor: {}, - major: {}, - }, - type: 'linear' - }, { - display: true, - - gridLines: { - color: 'rgba(0, 0, 0, 0.1)', - drawBorder: true, - drawOnChartArea: true, - drawTicks: true, // draw ticks extending towards the label, - tickMarkLength: 10, - lineWidth: 1, - offsetGridLines: false, - display: true, - zeroLineColor: 'rgba(0,0,0,0.25)', - zeroLineWidth: 1, - zeroLineBorderDash: [], - zeroLineBorderDashOffset: 0.0, - borderDash: [], - borderDashOffset: 0.0 - }, - position: 'left', - offset: false, - scaleLabel: Chart.defaults.scale.scaleLabel, - ticks: { - beginAtZero: false, - minRotation: 0, - maxRotation: 50, - mirror: false, - padding: 0, - reverse: false, - display: true, - callback: merged.scales.yAxes[2].ticks.callback, // make it nicer, then check explicitly below - autoSkip: true, - autoSkipPadding: 0, - labelOffset: 0, - minor: {}, - major: {}, - }, - type: 'linear' - }] - } - }); - - // Are these actually functions - expect(merged.scales.yAxes[1].ticks.callback).toEqual(jasmine.any(Function)); - expect(merged.scales.yAxes[2].ticks.callback).toEqual(jasmine.any(Function)); - }); - it('should filter an array', function() { var data = [-10, 0, 6, 0, 7]; var callback = function(item) { diff --git a/test/specs/global.deprecations.tests.js b/test/specs/global.deprecations.tests.js index d358742a5..d9e705a1c 100644 --- a/test/specs/global.deprecations.tests.js +++ b/test/specs/global.deprecations.tests.js @@ -42,6 +42,18 @@ describe('Deprecations', function() { expect(Chart.types).toEqual({}); }); }); + + describe('Chart.helpers.configMerge', function() { + it('should be defined as a function', function() { + expect(typeof Chart.helpers.configMerge).toBe('function'); + }); + }); + + describe('Chart.helpers.scaleMerge', function() { + it('should be defined as a function', function() { + expect(typeof Chart.helpers.scaleMerge).toBe('function'); + }); + }); }); describe('Version 2.7.3', function() {