Skip to content

Commit a2c1070

Browse files
simonbruneletimberg
authored andcommitted
Fix updating plugin options (chartjs#5144)
Cached plugin descriptors hold a reference on the plugin options, which break if the plugin options object is replaced. That case happens when the user updates the plugin options with a new object, but also since the new config update logic (chartjs#4198) that now always clones the plugin options. The fix consists in explicitly invalidating that cache before updating the chart.
1 parent 9e2f65c commit a2c1070

File tree

3 files changed

+48
-1
lines changed

3 files changed

+48
-1
lines changed

src/core/core.controller.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,10 @@ module.exports = function(Chart) {
376376

377377
updateConfig(me);
378378

379+
// plugins options references might have change, let's invalidate the cache
380+
// https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
381+
plugins._invalidate(me);
382+
379383
if (plugins.notify(me, 'beforeUpdate') === false) {
380384
return;
381385
}

src/core/core.plugins.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ module.exports = {
121121
* @private
122122
*/
123123
descriptors: function(chart) {
124-
var cache = chart._plugins || (chart._plugins = {});
124+
var cache = chart.$plugins || (chart.$plugins = {});
125125
if (cache.id === this._cacheId) {
126126
return cache.descriptors;
127127
}
@@ -157,6 +157,16 @@ module.exports = {
157157
cache.descriptors = descriptors;
158158
cache.id = this._cacheId;
159159
return descriptors;
160+
},
161+
162+
/**
163+
* Invalidates cache for the given chart: descriptors hold a reference on plugin option,
164+
* but in some cases, this reference can be changed by the user when updating options.
165+
* https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
166+
* @private
167+
*/
168+
_invalidate: function(chart) {
169+
delete chart.$plugins;
160170
}
161171
};
162172

test/specs/core.plugin.tests.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,39 @@ describe('Chart.plugins', function() {
339339

340340
expect(plugin.hook).toHaveBeenCalled();
341341
expect(plugin.hook.calls.first().args[1]).toEqual({a: 'foobar'});
342+
343+
delete Chart.defaults.global.plugins.a;
344+
});
345+
346+
// https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
347+
it('should invalidate cache when update plugin options', function() {
348+
var plugin = {id: 'a', hook: function() {}};
349+
var chart = window.acquireChart({
350+
plugins: [plugin],
351+
options: {
352+
plugins: {
353+
a: {
354+
foo: 'foo'
355+
}
356+
}
357+
},
358+
});
359+
360+
spyOn(plugin, 'hook');
361+
362+
Chart.plugins.notify(chart, 'hook');
363+
364+
expect(plugin.hook).toHaveBeenCalled();
365+
expect(plugin.hook.calls.first().args[1]).toEqual({foo: 'foo'});
366+
367+
chart.options.plugins.a = {bar: 'bar'};
368+
chart.update();
369+
370+
plugin.hook.calls.reset();
371+
Chart.plugins.notify(chart, 'hook');
372+
373+
expect(plugin.hook).toHaveBeenCalled();
374+
expect(plugin.hook.calls.first().args[1]).toEqual({bar: 'bar'});
342375
});
343376
});
344377
});

0 commit comments

Comments
 (0)