Fix cleaning up metasets (#9656)

* Fix cleaning up metasets

I believe it's a mistake to only delete the metaset if it has a valid controller; see f191f2f5 for where this behavior was introduced.

This is a minimal fix for #9653; as discussed there, it may also be worth updating `updateHoverStyle`.

As of #7030, `this._metasets` should always be defined, so checking whether it's undefined is no longer necessary.

* Add a test covering metaset behavior

* Add a regression test for #9653; fix `toHaveSize` usage

* Fix test failure

diff --git a/src/core/core.controller.js b/src/core/core.controller.js
index 8e288578d6ac411933eda35f657319f9ec4f11a3..ce6fbc7a87650babf27199d581f84714ce5c7c2c 100644 (file)
--- a/src/core/core.controller.js
+++ b/src/core/core.controller.js
@@ -799,12 +799,11 @@ class Chart {
         * @private
         */
   _destroyDatasetMeta(datasetIndex) {
-    const meta = this._metasets && this._metasets[datasetIndex];
-
+    const meta = this._metasets[datasetIndex];
     if (meta && meta.controller) {
       meta.controller._destroy();
-      delete this._metasets[datasetIndex];
     }
+    delete this._metasets[datasetIndex];
   }
 
   _stop() {
@@ -916,7 +915,7 @@ class Chart {
 
       _remove('resize', listener);
 
-      // Stop animating and remove metasets, so when re-attached, the animations start from begining.
+      // Stop animating and remove metasets, so when re-attached, the animations start from beginning.
       this._stop();
       this._resize(0, 0);
 

diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js
index cb4eba08c071a2b4dd0951d62790450f096b4777..d9e7418f00bc6f5b218b1505360c9c0b5aa2754e 100644 (file)
--- a/test/specs/core.controller.tests.js
+++ b/test/specs/core.controller.tests.js
@@ -1121,7 +1121,7 @@ describe('Chart', function() {
       });
     });
 
-    it('should resize the canvas if attached to the DOM after construction with mutiple parents', function(done) {
+    it('should resize the canvas if attached to the DOM after construction with multiple parents', function(done) {
       var canvas = document.createElement('canvas');
       var wrapper = document.createElement('div');
       var wrapper2 = document.createElement('div');
@@ -1848,7 +1848,7 @@ describe('Chart', function() {
       expect(metasets[2].order).toEqual(3);
       expect(metasets[3].order).toEqual(4);
     });
-    it('should be moved when datasets are removed from begining', function() {
+    it('should be moved when datasets are removed from beginning', function() {
       this.chart.data.datasets.splice(0, 2);
       this.chart.update();
       const metasets = this.chart._metasets;
@@ -1910,6 +1910,26 @@ describe('Chart', function() {
     });
   });
 
+  describe('_destroyDatasetMeta', function() {
+    beforeEach(function() {
+      this.chart = acquireChart({
+        type: 'line',
+        data: {
+          datasets: [
+            {label: '1', order: 2},
+            {label: '2', order: 1},
+            {label: '3', order: 4},
+            {label: '4', order: 3},
+          ]
+        }
+      });
+    });
+    it('cleans up metasets when the chart is destroyed', function() {
+      this.chart.destroy();
+      expect(this.chart._metasets).toEqual([undefined, undefined, undefined, undefined]);
+    });
+  });
+
   describe('data visibility', function() {
     it('should hide a dataset', function() {
       var chart = acquireChart({

diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js
index 9cf619c5efa4ef2ba4420f75646f1b474997cf62..ef0945d9537620a1b683d976e33058efddb2f850 100644 (file)
--- a/test/specs/plugin.tooltip.tests.js
+++ b/test/specs/plugin.tooltip.tests.js
@@ -1555,6 +1555,48 @@ describe('Plugin.Tooltip', function() {
       chart.tooltip.setActiveElements([{datasetIndex: 0, index: 0}], {x: 0, y: 0});
       expect(chart.tooltip.getActiveElements()[0].element).toBe(meta.data[0]);
     });
+
+    it('should update active elements when datasets are removed and added', async function() {
+      var dataset = {
+        label: 'Dataset 1',
+        data: [10, 20, 30],
+        pointHoverBorderColor: 'rgb(255, 0, 0)',
+        pointHoverBackgroundColor: 'rgb(0, 255, 0)'
+      };
+      var chart = window.acquireChart({
+        type: 'line',
+        data: {
+          datasets: [dataset],
+          labels: ['Point 1', 'Point 2', 'Point 3']
+        },
+        options: {
+          plugins: {
+            tooltip: {
+              mode: 'nearest',
+              intersect: true
+            }
+          }
+        }
+      });
+
+      var meta = chart.getDatasetMeta(0);
+      var point = meta.data[1];
+      var expectedPoint = jasmine.objectContaining({datasetIndex: 0, index: 1});
+
+      await jasmine.triggerMouseEvent(chart, 'mousemove', point);
+
+      expect(chart.tooltip.getActiveElements()).toEqual([expectedPoint]);
+
+      chart.data.datasets = [];
+      chart.update();
+
+      expect(chart.tooltip.getActiveElements()).toEqual([]);
+
+      chart.data.datasets = [dataset];
+      chart.update();
+
+      expect(chart.tooltip.getActiveElements()).toEqual([expectedPoint]);
+    });
   });
 
   describe('events', function() {