]> git.ipfire.org Git - thirdparty/Chart.js.git/commitdiff
Remove data checks and always re-parse data (#7458)
authorJukka Kurkela <jukka.kurkela@gmail.com>
Mon, 8 Jun 2020 21:49:55 +0000 (00:49 +0300)
committerGitHub <noreply@github.com>
Mon, 8 Jun 2020 21:49:55 +0000 (17:49 -0400)
* Remove data checks and always re-parse data
* Fix removed helper namespace

docs/docs/getting-started/v3-migration.md
src/core/core.datasetController.js
src/helpers/helpers.core.js
test/specs/core.datasetController.tests.js
test/specs/helpers.core.tests.js

index 8405fa72836473aafc6352016d5cdd85a63b51aa..8476b49480b80153a181e2d5832b4ec7e14b33c7 100644 (file)
@@ -250,6 +250,7 @@ The following properties and methods were removed:
 
 * `helpers.addEvent`
 * `helpers.aliasPixel`
+* `helpers.arrayEquals`
 * `helpers.configMerge`
 * `helpers.findIndex`
 * `helpers.findNextWhere`
index a5626b899a8274fd885417531b197c242005c659..48b2c79684f5ed2abc67fe93c9283ddf8a845d31 100644 (file)
@@ -1,5 +1,5 @@
 import Animations from './core.animations';
-import {isObject, merge, _merger, isArray, valueOrDefault, mergeIf, arrayEquals} from '../helpers/helpers.core';
+import {isObject, merge, _merger, isArray, valueOrDefault, mergeIf} from '../helpers/helpers.core';
 import {resolve} from '../helpers/helpers.options';
 import {getHoverColor} from '../helpers/helpers.color';
 import {sign} from '../helpers/helpers.math';
@@ -233,10 +233,7 @@ export default class DatasetController {
                this._config = undefined;
                this._parsing = false;
                this._data = undefined;
-               this._dataCopy = undefined;
-               this._dataModified = false;
                this._objectData = undefined;
-               this._labels = undefined;
                this._scaleStacked = {};
 
                this.initialize();
@@ -339,7 +336,6 @@ export default class DatasetController {
        }
 
        /**
-        * @return {boolean} whether the data was modified
         * @private
         */
        _dataCheck() {
@@ -352,50 +348,17 @@ export default class DatasetController {
                // the internal meta data accordingly.
 
                if (isObject(data)) {
-                       // Object data is currently monitored for replacement only
-                       if (me._objectData === data) {
-                               return false;
-                       }
                        me._data = convertObjectDataToArray(data);
-                       me._objectData = data;
-               } else {
-                       if (me._data === data && !me._dataModified && arrayEquals(data, me._dataCopy)) {
-                               return false;
-                       }
-
+               } else if (me._data !== data) {
                        if (me._data) {
                                // This case happens when the user replaced the data array instance.
                                unlistenArrayEvents(me._data, me);
                        }
-
-                       // Store a copy to detect direct modifications.
-                       // Note: This is suboptimal, but better than always parsing the data
-                       me._dataCopy = data.slice(0);
-
-                       me._dataModified = false;
-
                        if (data && Object.isExtensible(data)) {
                                listenArrayEvents(data, me);
                        }
                        me._data = data;
                }
-               return true;
-       }
-
-       /**
-        * @private
-        */
-       _labelCheck() {
-               const me = this;
-               const iScale = me._cachedMeta.iScale;
-               const labels = iScale ? iScale.getLabels() : me.chart.data.labels;
-
-               if (me._labels === labels) {
-                       return false;
-               }
-
-               me._labels = labels;
-               return true;
        }
 
        addElements() {
@@ -418,13 +381,12 @@ export default class DatasetController {
 
        buildOrUpdateElements() {
                const me = this;
-               const dataChanged = me._dataCheck();
-               const labelsChanged = me._labelCheck();
-               const scaleChanged = me._scaleCheck();
                const meta = me._cachedMeta;
                const dataset = me.getDataset();
                let stackChanged = false;
 
+               me._dataCheck();
+
                // make sure cached _stacked status is current
                meta._stacked = isStacked(meta.vScale, meta);
 
@@ -440,7 +402,7 @@ export default class DatasetController {
 
                // Re-sync meta data in case the user replaced the data array or if we missed
                // any updates and so make sure that we handle number of datapoints changing.
-               me._resyncElements(dataChanged || labelsChanged || scaleChanged || stackChanged);
+               me._resyncElements();
 
                // if stack changed, update stack values for the whole dataset
                if (stackChanged) {
@@ -708,22 +670,6 @@ export default class DatasetController {
                }
        }
 
-       /**
-        * @private
-        */
-       _scaleCheck() {
-               const me = this;
-               const meta = me._cachedMeta;
-               const iScale = meta.iScale;
-               const vScale = meta.vScale;
-               const cache = me._scaleStacked;
-               return !cache ||
-                       !iScale ||
-                       !vScale ||
-                       cache[iScale.id] !== iScale.options.stacked ||
-                       cache[vScale.id] !== vScale.options.stacked;
-       }
-
        /**
         * @return {number|boolean}
         * @protected
@@ -1049,7 +995,7 @@ export default class DatasetController {
        /**
         * @private
         */
-       _resyncElements(changed) {
+       _resyncElements() {
                const me = this;
                const meta = me._cachedMeta;
                const numMeta = meta.data.length;
@@ -1057,17 +1003,12 @@ export default class DatasetController {
 
                if (numData > numMeta) {
                        me._insertElements(numMeta, numData - numMeta);
-                       if (changed && numMeta) {
-                               // _insertElements parses the new elements. The old ones might need parsing too.
-                               me.parse(0, numMeta);
-                       }
                } else if (numData < numMeta) {
                        meta.data.splice(numData, numMeta - numData);
                        meta._parsed.splice(numData, numMeta - numData);
-                       me.parse(0, numData);
-               } else if (changed) {
-                       me.parse(0, numData);
                }
+               // Re-parse the old elements (new elements are parsed in _insertElements)
+               me.parse(0, Math.min(numData, numMeta));
        }
 
        /**
@@ -1113,7 +1054,6 @@ export default class DatasetController {
        _onDataPush() {
                const count = arguments.length;
                this._insertElements(this.getDataset().data.length - count, count);
-               this._dataModified = true;
        }
 
        /**
@@ -1121,7 +1061,6 @@ export default class DatasetController {
         */
        _onDataPop() {
                this._removeElements(this._cachedMeta.data.length - 1, 1);
-               this._dataModified = true;
        }
 
        /**
@@ -1129,7 +1068,6 @@ export default class DatasetController {
         */
        _onDataShift() {
                this._removeElements(0, 1);
-               this._dataModified = true;
        }
 
        /**
@@ -1138,7 +1076,6 @@ export default class DatasetController {
        _onDataSplice(start, count) {
                this._removeElements(start, count);
                this._insertElements(start, arguments.length - 2);
-               this._dataModified = true;
        }
 
        /**
@@ -1146,7 +1083,6 @@ export default class DatasetController {
         */
        _onDataUnshift() {
                this._insertElements(0, arguments.length);
-               this._dataModified = true;
        }
 }
 
index 6a5f52581553ac73ba9c2042c8e7ca0347a78986..01fa51f349dc2feba31205f0b7ce52621a049a10 100644 (file)
@@ -131,37 +131,6 @@ export function each(loopable, fn, thisArg, reverse) {
        }
 }
 
-/**
- * Returns true if the `a0` and `a1` arrays have the same content, else returns false.
- * @see https://stackoverflow.com/a/14853974
- * @param {Array} a0 - The array to compare
- * @param {Array} a1 - The array to compare
- * @returns {boolean}
- */
-export function arrayEquals(a0, a1) {
-       let i, ilen, v0, v1;
-
-       if (!a0 || !a1 || a0.length !== a1.length) {
-               return false;
-       }
-
-       for (i = 0, ilen = a0.length; i < ilen; ++i) {
-               v0 = a0[i];
-               v1 = a1[i];
-
-               if (v0 instanceof Array && v1 instanceof Array) {
-                       if (!arrayEquals(v0, v1)) {
-                               return false;
-                       }
-               } else if (v0 !== v1) {
-                       // NOTE: two different object instances will never be equal: {x:20} != {x:20}
-                       return false;
-               }
-       }
-
-       return true;
-}
-
 /**
  * Returns true if the `a0` and `a1` arrays have the same content, else returns false.
  * @param {Array} a0 - The array to compare
index 0e2d83a5f70fb4fe719b90a09a7f75a386ab5b05..c78b96a05679b6bf2ad3b1054b6f23cf4eb56270 100644 (file)
@@ -369,6 +369,30 @@ describe('Chart.DatasetController', function() {
                expect(meta.data[0].x).toEqual(firstX);
        });
 
+       // https://github.com/chartjs/Chart.js/issues/7445
+       it('should re-synchronize metadata when data is objects and directly altered', function() {
+               var data = [{x: 'a', y: 1}, {x: 'b', y: 2}, {x: 'c', y: 3}];
+               var chart = acquireChart({
+                       type: 'line',
+                       data: {
+                               labels: ['a', 'b', 'c'],
+                               datasets: [{
+                                       data,
+                                       fill: true
+                               }]
+                       }
+               });
+
+               var meta = chart.getDatasetMeta(0);
+
+               expect(meta.data.length).toBe(3);
+               const y3 = meta.data[2].y;
+
+               data[0].y = 3;
+               chart.update();
+               expect(meta.data[0].y).toEqual(y3);
+       });
+
        it('should re-synchronize metadata when scaleID changes', function() {
                var chart = acquireChart({
                        type: 'line',
index aa687784ed59d9947487b62bb87bc2d17d17a2af..9b613d142322acee8b6e0150d89dc4d9c4c63731 100644 (file)
@@ -241,29 +241,6 @@ describe('Chart.helpers.core', function() {
                });
        });
 
-       describe('arrayEquals', function() {
-               it('should return false if arrays are not the same', function() {
-                       expect(helpers.arrayEquals([], [42])).toBeFalsy();
-                       expect(helpers.arrayEquals([42], ['42'])).toBeFalsy();
-                       expect(helpers.arrayEquals([1, 2, 3], [1, 2, 3, 4])).toBeFalsy();
-                       expect(helpers.arrayEquals(['foo', 'bar'], ['bar', 'foo'])).toBeFalsy();
-                       expect(helpers.arrayEquals([1, 2, 3], [1, 2, 'foo'])).toBeFalsy();
-                       expect(helpers.arrayEquals([1, 2, [3, 4]], [1, 2, [3, 'foo']])).toBeFalsy();
-                       expect(helpers.arrayEquals([{a: 42}], [{a: 42}])).toBeFalsy();
-               });
-               it('should return false if arrays are not the same', function() {
-                       var o0 = {};
-                       var o1 = {};
-                       var o2 = {};
-
-                       expect(helpers.arrayEquals([], [])).toBeTruthy();
-                       expect(helpers.arrayEquals([1, 2, 3], [1, 2, 3])).toBeTruthy();
-                       expect(helpers.arrayEquals(['foo', 'bar'], ['foo', 'bar'])).toBeTruthy();
-                       expect(helpers.arrayEquals([true, false, true], [true, false, true])).toBeTruthy();
-                       expect(helpers.arrayEquals([o0, o1, o2], [o0, o1, o2])).toBeTruthy();
-               });
-       });
-
        describe('_elementsEqual', function() {
                it('should return true if arrays are the same', function() {
                        expect(helpers._elementsEqual(