fix(type-coercion): use Data Pipes

Switch from the deprecated type property of Data Set to Data Pipes. This solves some or maybe all issues from #254. PS: There is a bug in Vis Data that prevents this from working see visjs/vis-data/pull/78.
visjs · Jan 19, 2020 · f4b59d7 · fastlorenzo · Jan 22, 2020 · fastlorenzo
1 parent 4a79838
commit f4b59d7
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 63 deletions.
diff --git a/lib/timeline/Graph2d.js b/lib/timeline/Graph2d.js
@@ -1,5 +1,5 @@
 import moment from '../module/moment';
-import util from '../util';
+import util, { typeCoerceDataSet } from '../util';
 import { DataSet }  from 'vis-data';
 import { DataView } from 'vis-data';
 import Range from './Range';
@@ -193,21 +193,20 @@ Graph2d.prototype.setItems = function(items) {
     newDataSet = null;
   }
   else if (items instanceof DataSet || items instanceof DataView) {
-    newDataSet = items;
+    newDataSet = typeCoerceDataSet(items);
   }
   else {
     // turn an array into a dataset
-    newDataSet = new DataSet(items, {
-      type: {
-        start: 'Date',
-        end: 'Date'
-      }
-    });
+    newDataSet = typeCoerceDataSet(new DataSet(items));
   }
 
   // set items
+  if (this.itemsData) {
+    // stop maintaining a coerced version of the old data set
+    this.itemsData.dispose()
+  }
   this.itemsData = newDataSet;
-  this.linegraph && this.linegraph.setItems(newDataSet);
+  this.linegraph && this.linegraph.setItems(newDataSet.rawDS);
 
   if (initialLoad) {
     if (this.options.start != undefined || this.options.end != undefined) {

diff --git a/lib/timeline/Timeline.js b/lib/timeline/Timeline.js
@@ -1,5 +1,5 @@
 import moment from '../module/moment';
-import util from '../util';
+import util, { typeCoerceDataSet } from '../util';
 import { DataSet }  from 'vis-data';
 import { DataView } from 'vis-data';
 import Range from './Range';
@@ -288,9 +288,9 @@ export default class Timeline extends Core {
         const itemsData = this.itemsData;
         if (itemsData) {
           const selection = this.getSelection();
-          this.setItems(null);          // remove all
-          this.setItems(itemsData);     // add all
-          this.setSelection(selection); // restore selection
+          this.setItems(null);            // remove all
+          this.setItems(itemsData.rawDS); // add all
+          this.setSelection(selection);   // restore selection
         }
       }
     }
@@ -309,21 +309,20 @@ export default class Timeline extends Core {
       newDataSet = null;
     }
     else if (items instanceof DataSet || items instanceof DataView) {
-      newDataSet = items;
+      newDataSet = typeCoerceDataSet(items);
     }
     else {
       // turn an array into a dataset
-      newDataSet = new DataSet(items, {
-        type: {
-          start: 'Date',
-          end: 'Date'
-        }
-      });
+      newDataSet = typeCoerceDataSet(new DataSet(items));
     }
 
     // set items
+    if (this.itemsData) {
+      // stop maintaining a coerced version of the old data set
+      this.itemsData.dispose();
+    }
     this.itemsData = newDataSet;
-    this.itemSet && this.itemSet.setItems(newDataSet);
+    this.itemSet && this.itemSet.setItems(newDataSet.rawDS);
   }
 
   /**
@@ -419,12 +418,7 @@ export default class Timeline extends Core {
     const ids = Array.isArray(id) ? id : [id];
 
     // get the specified item(s)
-    const itemsData = this.itemsData.getDataSet().get(ids, {
-      type: {
-        start: 'Date',
-        end: 'Date'
-      }
-    });
+    const itemsData = this.itemsData.get(ids);
 
     // calculate minimum start and maximum end of specified items
     let start = null;
@@ -533,8 +527,7 @@ export default class Timeline extends Core {
     const animation = (options && options.animation !== undefined) ? options.animation : true;
     let range;
 
-    const dataset = this.itemsData && this.itemsData.getDataSet();
-    if (dataset.length === 1 && dataset.get()[0].end === undefined) {
+    if (this.itemsData.length === 1 && this.itemsData.get()[0].end === undefined) {
       // a single item -> don't fit, just show a range around the item from -4 to +3 days
       range = this.getDataRange();
       this.moveTo(range.min.valueOf(), {animation}, callback);
@@ -645,9 +638,8 @@ export default class Timeline extends Core {
     let min = null;
     let max = null;
 
-    const dataset = this.itemsData && this.itemsData.getDataSet();
-    if (dataset) {
-      dataset.forEach(item => {
+    if (this.itemsData) {
+      this.itemsData.forEach(item => {
         const start = util.convert(item.start, 'Date').valueOf();
         const end   = util.convert(item.end != undefined ? item.end : item.start, 'Date').valueOf();
         if (min === null || start < min) {

diff --git a/lib/timeline/component/ItemSet.js b/lib/timeline/component/ItemSet.js
@@ -1,5 +1,5 @@
 import Hammer from '../../module/hammer';
-import util from '../../util';
+import util, { typeCoerceDataSet } from '../../util';
 import { DataSet }  from 'vis-data';
 import { DataView } from 'vis-data';
 import TimeStep from '../TimeStep';
@@ -129,12 +129,7 @@ class ItemSet extends Component {
     this.options = util.extend({}, this.defaultOptions);
     this.options.rtl = options.rtl;
     this.options.onTimeout = options.onTimeout;
-
-    // options for getting items from the DataSet with the correct type
-    this.itemOptions = {
-      type: {start: 'Date', end: 'Date'}
-    };
-
+
     this.conversion = {
       toScreen: body.util.toScreen,
       toTime: body.util.toTime
@@ -960,7 +955,7 @@ class ItemSet extends Component {
       this.itemsData = null;
     }
     else if (items instanceof DataSet || items instanceof DataView) {
-      this.itemsData = items;
+      this.itemsData = typeCoerceDataSet(items);
     }
     else {
       throw new TypeError('Data must be an instance of DataSet or DataView');
@@ -972,6 +967,9 @@ class ItemSet extends Component {
         oldItemsData.off(event, callback);
       });
 
+      // stop maintaining a coerced version of the old data set
+      oldItemsData.dispose()
+
       // remove all drawn items
       ids = oldItemsData.getIds();
       this._onRemove(ids);
@@ -1000,7 +998,7 @@ class ItemSet extends Component {
    * @returns {vis.DataSet | null}
    */
   getItems() {
-    return this.itemsData;
+    return this.itemsData != null ? this.itemsData.rawDS : null;
   }
 
   /**
@@ -1094,15 +1092,14 @@ class ItemSet extends Component {
    */
   removeItem(id) {
     const item = this.itemsData.get(id);
-    const dataset = this.itemsData.getDataSet();
 
     if (item) {
       // confirm deletion
       this.options.onRemove(item, item => {
         if (item) {
           // remove by id here, it is possible that an item has no id defined
           // itself, so better not delete by the item itself
-          dataset.remove(id);
+          this.itemsData.remove(id);
         }
       });
     }
@@ -1143,7 +1140,7 @@ class ItemSet extends Component {
     const me = this;
 
     ids.forEach(id => {
-      const itemData = me.itemsData.get(id, me.itemOptions);
+      const itemData = me.itemsData.get(id);
       let item = me.items[id];
       const type = itemData ? me._getType(itemData) : null;
 
@@ -1607,7 +1604,7 @@ class ItemSet extends Component {
     };
 
     const id = util.randomUUID();
-    itemData[this.itemsData._idProp] = id;
+    itemData[this.itemsData.idProp] = id;
 
     const group = this.groupFromTarget(event);
     if (group) {
@@ -1820,20 +1817,19 @@ class ItemSet extends Component {
       event.stopPropagation();
 
       const me = this;
-      const dataset = this.itemsData.getDataSet();
       const itemProps = this.touchParams.itemProps;
       this.touchParams.itemProps = null;
 
       itemProps.forEach(props => {
         const id = props.item.id;
-        const exists = me.itemsData.get(id, me.itemOptions) != null;
+        const exists = me.itemsData.get(id) != null;
 
         if (!exists) {
           // add a new item
           me.options.onAdd(props.item.data, itemData => {
             me._removeItem(props.item); // remove temporary item
             if (itemData) {
-              me.itemsData.getDataSet().add(itemData);
+              me.itemsData.add(itemData);
             }
 
             // force re-stacking of all items next redraw
@@ -1846,8 +1842,8 @@ class ItemSet extends Component {
           me.options.onMove(itemData, itemData => {
             if (itemData) {
               // apply changes
-              itemData[dataset._idProp] = id; // ensure the item contains its id (can be undefined)
-              dataset.update(itemData);
+              itemData[this.itemsData.idProp] = id; // ensure the item contains its id (can be undefined)
+              this.itemsData.update(itemData);
             }
             else {
               // restore original values
@@ -2303,7 +2299,7 @@ class ItemSet extends Component {
       const itemData = me.itemsData.get(item.id); // get a clone of the data from the dataset
       this.options.onUpdate(itemData, itemData => {
         if (itemData) {
-          me.itemsData.getDataSet().update(itemData);
+          me.itemsData.update(itemData);
         }
       });
     }
@@ -2347,7 +2343,7 @@ class ItemSet extends Component {
       newItemData.content = newItemData.content ? newItemData.content : 'new item';
       newItemData.start = newItemData.start ? newItemData.start : (snap ? snap(start, scale, step) : start);
       newItemData.type = newItemData.type || 'box';
-      newItemData[this.itemsData._idProp] = newItemData.id || util.randomUUID();
+      newItemData[this.itemsData.idProp] = newItemData.id || util.randomUUID();
 
       if (newItemData.type == 'range' && !newItemData.end) {
         end = this.body.util.toTime(x + this.props.width / 5);
@@ -2358,7 +2354,7 @@ class ItemSet extends Component {
         start: snap ? snap(start, scale, step) : start,
         content: 'new item'
       };
-      newItemData[this.itemsData._idProp] = util.randomUUID();
+      newItemData[this.itemsData.idProp] = util.randomUUID();
 
       // when default type is a range, add a default end date to the new item
       if (this.options.type === 'range') {
@@ -2376,7 +2372,7 @@ class ItemSet extends Component {
     newItemData = this._cloneItemData(newItemData);     // convert start and end to the correct type
     this.options.onAdd(newItemData, item => {
       if (item) {
-        me.itemsData.getDataSet().add(item);
+        me.itemsData.add(item);
         if (event.type == 'drop') {
           me.setSelection([item.id]);
         }
@@ -2420,7 +2416,7 @@ class ItemSet extends Component {
         if (!this.options.multiselectPerGroup || lastSelectedGroup == undefined || lastSelectedGroup == itemGroup) {
           selection.push(item.id);
         }
-        const range = ItemSet._getItemRange(this.itemsData.get(selection, this.itemOptions));
+        const range = ItemSet._getItemRange(this.itemsData.get(selection));
 
         if (!this.options.multiselectPerGroup || lastSelectedGroup == itemGroup) {
           // select all items within the selection range
@@ -2608,7 +2604,7 @@ class ItemSet extends Component {
 
     if (!type) {
       // convert start and end date to the type (Date, Moment, ...) configured in the DataSet
-      type = this.itemsData.getDataSet()._options.type;
+      type = this.itemsData.type;
     }
 
     if (clone.start != undefined) {

diff --git a/lib/timeline/component/LineGraph.js b/lib/timeline/component/LineGraph.js
@@ -1,4 +1,4 @@
-import util from '../../util';
+import util, { typeCoerceDataSet } from '../../util';
 import * as DOMutil from '../../DOMutil';
 import { DataSet }  from 'vis-data';
 import { DataView } from 'vis-data';
@@ -254,7 +254,7 @@ LineGraph.prototype.setItems = function (items) {
     this.itemsData = null;
   }
   else if (items instanceof DataSet || items instanceof DataView) {
-    this.itemsData = items;
+    this.itemsData = typeCoerceDataSet(items);
   }
   else {
     throw new TypeError('Data must be an instance of DataSet or DataView');
@@ -266,6 +266,9 @@ LineGraph.prototype.setItems = function (items) {
       oldItemsData.off(event, callback);
     });
 
+    // stop maintaining a coerced version of the old data set
+    oldItemsData.dispose()
+
     // remove all drawn items
     ids = oldItemsData.getIds();
     this._onRemove(ids);
@@ -433,7 +436,7 @@ LineGraph.prototype._updateAllGroupData = function (ids, groupIds) {
   if (this.itemsData != null) {
     var groupsContent = {};
     var items = this.itemsData.get();
-    var fieldId = this.itemsData._idProp;
+    var fieldId = this.itemsData.idProp;
     var idMap = {};
     if (ids){
       ids.map(function (id) {