Skip to content

Commit

Permalink
Deprecate configMerge and scaleMerge helpers (chartjs#6022)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@simonbrunel
simonbrunel authored Jan 29, 2019
1 parent 69bc5f0 commit f31445f
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 225 deletions.
88 changes: 85 additions & 3 deletions src/core/core.controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {});
Expand All @@ -59,7 +123,7 @@ function updateConfig(chart) {
layouts.removeBox(chart, scale);
});

newOptions = helpers.configMerge(
newOptions = mergeConfig(
defaults.global,
defaults[chart.config.type],
newOptions);
Expand Down Expand Up @@ -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;
55 changes: 0 additions & 55 deletions src/core/core.helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/helpers/helpers.core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
144 changes: 144 additions & 0 deletions test/specs/core.controller.tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Loading

0 comments on commit f31445f

Please sign in to comment.