fix(type-coercion): use Data Pipes #259
Conversation
| @@ -1143,7 +1140,7 @@ class ItemSet extends Component { | |||
| const me = this; | |||
|
|
|||
| ids.forEach(id => { | |||
| const itemData = me.itemsData.get(id, me.itemOptions); | |||
There was a problem hiding this comment.
Why was me.itemOptions removed?
There was a problem hiding this comment.
I found no place where it would be altered and it just set the same type coercion as was used everywhere anyway.
| * @returns A Data Set like object that saves data into the raw Data Set and | ||
| * retrieves them from the coerced Data Set. | ||
| */ | ||
| export function typeCoerceDataSet( |
There was a problem hiding this comment.
Can you explain better what's the idea again of this function? I still haven;t grasp the idea of 'type coerced' data sets...
There was a problem hiding this comment.
I added some more info about the inner workings. It basically just imitates how it worked with the type constructor parameter but without forcing this functionality into the data set itself.
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.
Are you sure there is no need for |
|
🎉 This PR is included in version 6.5.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Dependencies are only things marked as external in Rollup and unless I overlooked something there's no such thing. Moment is simply copy pasted into |
|
So you say we can empty out all dependencies in "@egjs/hammerjs": "2.0.17",
"emitter-component": "1.1.1",
"keycharm": "^0.3.0",
"moment": "2.24.0",
"propagating-hammerjs": "1.4.7",
"vis-data": "6.3.5",
"vis-util": "2.1.0"? |
|
They are necessary when Rollup is running (as dev dependencies) but Rollup will put them into the bundled file. When the bundled file is running it already has all of them included and will never use those from node_modules. |
|
So you mean we can move the dependencies to the |
|
Yeah exactly. |
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.PS: I'm not going to change this here as it's off topic but this library has no runtime dependencies. Everything's bundled by Rollup so all the dependencies listed in package.json are completely unnecessary to use this.