Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate configMerge and scaleMerge helpers #6022

Merged
merged 5 commits into from Jan 29, 2019
Merged

Deprecate configMerge and scaleMerge helpers #6022

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b0dc2f
Deprecate configMerge and scaleMerge helpers
simonbrunel Jan 28, 2019
dac95fd
Add deprecation tests (and fix lint error)
simonbrunel Jan 28, 2019
1d4b41a
Prevent explicit first argument deep copy
simonbrunel Jan 29, 2019
445cf19
[].slice instead of Array.prototype.slice
simonbrunel Jan 29, 2019
104991b
Update comment per code review
simonbrunel Jan 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 75 additions & 3 deletions src/core/core.controller.js
View file
Open in desktop
Expand Up @@ -35,16 +35,70 @@ defaults._set('global', {
responsiveAnimationDuration: 0
});

function mergeScaleConfig(/* 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');
simonbrunel marked this conversation as resolved.
Show resolved Hide resolved

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);
}
}
});
}

function mergeConfig(/* 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] = 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 +113,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 +1035,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
Open in desktop
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
Open in desktop
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
114 changes: 114 additions & 0 deletions test/specs/core.controller.tests.js
View file
Open in desktop
Expand Up @@ -159,6 +159,120 @@ 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'
}));
});
});

describe('config.options.responsive: false', function() {
it('should not inject the resizer element', function() {
var chart = acquireChart({
Expand Down