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.

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;
 			}

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;
 	}
 };
 

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'});
 		});
 	});
 });