Hi,
Really liking v2.5.0 (especially being able to update options now). The only problem i'm having is how to call updateConfig. I've tried a few different things including:
(for the sake of these examples, the chart instance will be called chartInst)
Chart.updateConfig(chartInst)chartInst.updateConfig(newOptions)chartInst.updateConfig()updateConfig(chartInst)But none of these work and there's nothing in the documentation to explain how to use this new feature. Is anyone able to give me some insight into how it's used?
The final api ended up folding config updates into regular updates.
So you can apply changes to chartInst.options and then calling chartInst.update
Ahhhh, okay, brilliant, thanks @etimberg!
I couldn't acheive this using .options
Here is the solution that works for me https://github.com/chartjs/Chart.js/issues/593#issuecomment-215649465
myChart.config.data = d; // d is my new dataset
myChart.update();
@etimberg
This really simple attempt at updating options fails horribly:
chart.options = newOptions;
chart.update();
This would be much less of a headache if the options object (and data, while we're at it) wasn't mutated internally…
@vdh data is now fully supported starting v2.6 (chart.data = newData), so we can do the same for options (would be a bit different since we need to handle defaults).
One complication with changing the entire options object is that you may end up needing to effectively destroy and recreate the entire graph. Obviously new defaults have to be merged, but axes could change and that leads to a lot of complications
@etimberg This isn't a very approachable design when trying to do functional programming. I shouldn't have to worry about hidden minefields like that when trying to update a few options.
Take for example, the scale options, which auto-generate ids if none are defined. There was no indication anywhere that this would break when trying to update options, because suddenly that id is gone when that section of the options object is replaced.
A better approach would have been to store that important id inside something internal, or even better, require those ids to begin with so you would have a unique identifier when writing code to update those options. Ideally, both store it internally and require ids (at least for people expecting to use .update).
Not everyone is going to have a picturesque scenario where they know exactly where they need to drill down into a data structure to make their updates. I'm using React and Immutable.js, so I basically had to cobble together something vaguely safe using lodash's mergeWith. But I can't be sure if my hack will actually be safe because that options object is an undocumented bundle of mystery.
@vdh I agree that this is not great behaviour, especially from a functional perspective.
I think we can definitely improve things on the axis side. Some options I see are:
Both options would modify: https://github.com/chartjs/Chart.js/blob/master/src/core/core.controller.js#L47-L53
I think a potential (untested) work-around would be to do the following. In essence, it merges the new config on top of the old config and sets that. That way mutated or merged properties are not lost
// assumes newConfig is already known here
newConfig = Chart.helpers.extend(chart.config, newConfig);
chart.config = newConfig;
chart.update();
@etimberg That's exactly what I've had to do to update the options:
import mergeWith from 'lodash/mergeWith';
const chartJsCustomizer = (existingValue, newValue) => {
if (Array.isArray(existingValue)) {
if (
Array.isArray(newValue) &&
existingValue.length === newValue.length &&
existingValue.length > 0 &&
typeof existingValue[0] === 'object'
) {
// Two arrays of same length with objects, mutate each item instead
existingValue.forEach((item, index) => {
mergeWith(item, newValue[index], chartJsCustomizer);
});
return existingValue;
}
// Replace old array with new value
return newValue;
}
// Default to lodash merge logic
return undefined;
};
// later…
// Update the options
mergeWith(chart.options, newOptions, chartJsCustomizer);
But the caveats are:
null-ed, due to the merge strategy.This is the kind of rabbit hole that happens when objects are mutated instead of copied 😢
One of the things we did think about is trying to get rid of the array xAxes and yAxes properties. This would at least make the merging cleaner since one option was to use the ID of the axis as the key of some nested config. This would end up making things more straightforward, but I'm not yet convinced we can maintain backwards compatibility.
As much as I would love to remove the config merge with the global config, it does keep the library smaller because we don't have to check for defaults in a lot of different places. Maybe we need to keep the original config around somewhere so that we know what the unmerged state looked like.
@etimberg I still seem to be having trouble with updating the options. It seems to be specifically be coming down to scale id issues. codepen example , maybe it's something I'm doing
Let's close this as a duplicate of https://github.com/chartjs/Chart.js/issues/4197. That one has a pending PR. Both issues should be fixed when the PR is committed
I tested this in v3.0.0-alpha. We've replaced the xAxes and yAxes arrays with a scale configuration that each axis' config mapped by ID.
I built https://jsfiddle.net/zse3qmfb/1/ from the 'toggle scale type' sample but updated it to replace the options object on change. It seems to work fine.
Closing per my previous comment. We're up to v3.0.0-beta.3 now and this is still working. Updated fiddle: https://jsfiddle.net/30c45yLd/
Most helpful comment
@etimberg
This really simple attempt at updating options fails horribly:
This would be much less of a headache if the options object (and data, while we're at it) wasn't mutated internally…