Clone cached options if enableOptionSharing!=true (#7837)

Clone cached options if enableOptionSharing!=true

diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js
index 3d8bfdf7e596af16ea529819f68f246f48276209..fde4f110ff04e66e95e92a4c2b19064b19d27047 100644 (file)
--- a/src/core/core.datasetController.js
+++ b/src/core/core.datasetController.js
@@ -147,17 +147,11 @@ function getFirstScaleId(chart, axis) {
        return Object.keys(scales).filter(key => scales[key].axis === axis).shift();
 }
 
-function optionKeys(optionNames) {
-       return isArray(optionNames) ? optionNames : Object.keys(optionNames);
-}
-
-function optionKey(key, active) {
-       return active ? 'hover' + _capitalize(key) : key;
-}
-
-function isDirectUpdateMode(mode) {
-       return mode === 'reset' || mode === 'none';
-}
+const optionKeys = (optionNames) => isArray(optionNames) ? optionNames : Object.keys(optionNames);
+const optionKey = (key, active) => active ? 'hover' + _capitalize(key) : key;
+const isDirectUpdateMode = (mode) => mode === 'reset' || mode === 'none';
+const cloneIfNotShared = (cached, shared) => shared ? cached : Object.assign({}, cached);
+const freezeIfShared = (values, shared) => shared ? Object.freeze(values) : values;
 
 export default class DatasetController {
 
@@ -732,10 +726,11 @@ export default class DatasetController {
                mode = mode || 'default';
                const me = this;
                const active = mode === 'active';
-               const cached = me._cachedDataOpts;
+               const cache = me._cachedDataOpts;
+               const cached = cache[mode];
                const sharing = me.enableOptionSharing;
-               if (cached[mode]) {
-                       return cached[mode];
+               if (cached) {
+                       return cloneIfNotShared(cached, sharing);
                }
                const info = {cacheable: !active};
 
@@ -754,7 +749,7 @@ export default class DatasetController {
                        // We cache options by `mode`, which can be 'active' for example. This enables us
                        // to have the 'active' element options and 'default' options to switch between
                        // when interacting.
-                       cached[mode] = sharing ? Object.freeze(values) : values;
+                       cache[mode] = freezeIfShared(values, sharing);
                }
 
                return values;

diff --git a/test/specs/core.datasetController.tests.js b/test/specs/core.datasetController.tests.js
index 4e375df3a1bfaac08645d1f9f1c47305ca3604b5..4445324f9aa9405ac2b920b7486651fc9d33b071 100644 (file)
--- a/test/specs/core.datasetController.tests.js
+++ b/test/specs/core.datasetController.tests.js
@@ -664,6 +664,73 @@ describe('Chart.DatasetController', function() {
                });
        });
 
+       describe('resolveDataElementOptions', function() {
+               it('should cache options when possible', function() {
+                       const chart = acquireChart({
+                               type: 'line',
+                               data: {
+                                       datasets: [{
+                                               data: [1, 2, 3],
+                                       }]
+                               },
+                       });
+
+                       const controller = chart.getDatasetMeta(0).controller;
+
+                       expect(controller.enableOptionSharing).toBeTrue();
+
+                       const opts0 = controller.resolveDataElementOptions(0);
+                       const opts1 = controller.resolveDataElementOptions(1);
+
+                       expect(opts0 === opts1).toBeTrue();
+                       expect(opts0.$shared).toBeTrue();
+                       expect(Object.isFrozen(opts0)).toBeTrue();
+               });
+
+               it('should not cache options when option sharing is disabled', function() {
+                       const chart = acquireChart({
+                               type: 'radar',
+                               data: {
+                                       datasets: [{
+                                               data: [1, 2, 3],
+                                       }]
+                               },
+                       });
+
+                       const controller = chart.getDatasetMeta(0).controller;
+
+                       expect(controller.enableOptionSharing).toBeFalse();
+
+                       const opts0 = controller.resolveDataElementOptions(0);
+                       const opts1 = controller.resolveDataElementOptions(1);
+
+                       expect(opts0 === opts1).toBeFalse();
+                       expect(opts0.$shared).not.toBeTrue();
+                       expect(Object.isFrozen(opts0)).toBeFalse();
+               });
+
+               it('should not cache options when functions are used', function() {
+                       const chart = acquireChart({
+                               type: 'line',
+                               data: {
+                                       datasets: [{
+                                               data: [1, 2, 3],
+                                               backgroundColor: () => 'red'
+                                       }]
+                               },
+                       });
+
+                       const controller = chart.getDatasetMeta(0).controller;
+
+                       const opts0 = controller.resolveDataElementOptions(0);
+                       const opts1 = controller.resolveDataElementOptions(1);
+
+                       expect(opts0 === opts1).toBeFalse();
+                       expect(opts0.$shared).not.toBeTrue();
+                       expect(Object.isFrozen(opts0)).toBeFalse();
+               });
+       });
+
        describe('_resolveAnimations', function() {
                it('should resolve to empty Animations when globally disabled', function() {
                        const chart = acquireChart({