Closed tomvantilburg closed 7 years ago
Yes, this makes sense. I'll look into it. Love Rollup.
I've made a PR, tested in chrome, node and plv8
Just checking: is anything still needed from my side in this PR?
Hey, sorry — have too much on my plate, — I'll get to it next week. Want to poke with it a bit before I decide to merge.
Sure, and I'll be happy to refactor if you have issues with it. For me the modules are doing just what I needed for now.
I'm a big fan of @mourner's pure ES5 libraries, having zero overhead is a big relief.
Also there's almost zero benefit to using Rollup in this library since there's absolutely no dependencies, the only file that is used is the index.js
which is only 11KB.
Sounds like this is forcing the use of Rollup and not very practical for this library (Rollup is useful, however it doesn't seem very useful here).
@tomvantilburg Are you aware of the CommonJS Rollup Plugin? This might help to integrate this library with your build.
Putting this on hold for now — see more info in https://github.com/mapbox/delaunator/pull/5#issuecomment-319637166
Would be great to turn this into an ES6 module:
export default function Delaunator(points, getX, getY) {
Takes some overhead with roll-up and forces you to make a dist as well for now but makes it future proof (and for me easier to integrate ;-)I can make a PR if interested.