-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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.
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
How to use updateConfig #3886
Comments
The final api ended up folding config updates into regular updates. So you can apply changes to |
Ahhhh, okay, brilliant, thanks @etimberg! |
I couldn't acheive this using myChart.config.data = d; // d is my new dataset
myChart.update(); |
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 |
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 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 |
@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:
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 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 #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 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/ |
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 text was updated successfully, but these errors were encountered: