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 all 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
88 changes: 85 additions & 3 deletions src/core/core.controller.js
View file
Open in desktop
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
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
144 changes: 144 additions & 0 deletions test/specs/core.controller.tests.js
View file
Open in desktop
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