Fix updating plugin options (#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 (#4198) that now always clones the plugin options. The fix consists in explicitly invalidating that cache before updating the chart.

diff --git a/src/core/core.controller.js b/src/core/core.controller.js
index e32255edcea62385be475998f17f3c21fcdbaaf0..e29a5b0769c6c0a4af4618e4edc75549390ad6bd 100644 (file)
--- a/src/core/core.controller.js
+++ b/src/core/core.controller.js
@@ -376,6 +376,10 @@ module.exports = function(Chart) {
 
                        updateConfig(me);
 
+                       // plugins options references might have change, let's invalidate the cache
+                       // https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
+                       plugins._invalidate(me);
+
                        if (plugins.notify(me, 'beforeUpdate') === false) {
                                return;
                        }

diff --git a/src/core/core.plugins.js b/src/core/core.plugins.js
index f5e8d10d8ac766daab466a2307f3f43ed1250796..f2fbcadec31f0cf6ef55d115c312a30aaba80749 100644 (file)
--- a/src/core/core.plugins.js
+++ b/src/core/core.plugins.js
@@ -121,7 +121,7 @@ module.exports = {
         * @private
         */
        descriptors: function(chart) {
-               var cache = chart._plugins || (chart._plugins = {});
+               var cache = chart.$plugins || (chart.$plugins = {});
                if (cache.id === this._cacheId) {
                        return cache.descriptors;
                }
@@ -157,6 +157,16 @@ module.exports = {
                cache.descriptors = descriptors;
                cache.id = this._cacheId;
                return descriptors;
+       },
+
+       /**
+        * Invalidates cache for the given chart: descriptors hold a reference on plugin option,
+        * but in some cases, this reference can be changed by the user when updating options.
+        * https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
+        * @private
+        */
+       _invalidate: function(chart) {
+               delete chart.$plugins;
        }
 };
 

diff --git a/test/specs/core.plugin.tests.js b/test/specs/core.plugin.tests.js
index 387d78808c72069cc942aaae38f3b111389124c2..3a9e908a38d1955782c6ab6edc82890fc4aa071b 100644 (file)
--- a/test/specs/core.plugin.tests.js
+++ b/test/specs/core.plugin.tests.js
@@ -339,6 +339,39 @@ describe('Chart.plugins', function() {
 
                        expect(plugin.hook).toHaveBeenCalled();
                        expect(plugin.hook.calls.first().args[1]).toEqual({a: 'foobar'});
+
+                       delete Chart.defaults.global.plugins.a;
+               });
+
+               // https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
+               it('should invalidate cache when update plugin options', function() {
+                       var plugin = {id: 'a', hook: function() {}};
+                       var chart = window.acquireChart({
+                               plugins: [plugin],
+                               options: {
+                                       plugins: {
+                                               a: {
+                                                       foo: 'foo'
+                                               }
+                                       }
+                               },
+                       });
+
+                       spyOn(plugin, 'hook');
+
+                       Chart.plugins.notify(chart, 'hook');
+
+                       expect(plugin.hook).toHaveBeenCalled();
+                       expect(plugin.hook.calls.first().args[1]).toEqual({foo: 'foo'});
+
+                       chart.options.plugins.a = {bar: 'bar'};
+                       chart.update();
+
+                       plugin.hook.calls.reset();
+                       Chart.plugins.notify(chart, 'hook');
+
+                       expect(plugin.hook).toHaveBeenCalled();
+                       expect(plugin.hook.calls.first().args[1]).toEqual({bar: 'bar'});
                });
        });
 });