From 2d7f0a46c3f58278f883cb829d9e8a109151559d Mon Sep 17 00:00:00 2001
From: Simon Brunel <simonbrunel@users.noreply.github.com>
Date: Sat, 13 Jan 2018 14:23:50 +0100
Subject: [PATCH] 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     |  4 ++++
 src/core/core.plugins.js        | 12 +++++++++++-
 test/specs/core.plugin.tests.js | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/core/core.controller.js b/src/core/core.controller.js
index e32255edc..e29a5b076 100644
--- 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 f5e8d10d8..f2fbcadec 100644
--- 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 387d78808..3a9e908a3 100644
--- 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'});
 		});
 	});
 });