made into an ES6 module, included rollup for umd build

[tomvantilburg]

As per issue #4 , inspired by the way d3 has been rolled up. I'm not experienced with eslint, but the current setup seems to work. Also tape runs the tests.

Notable changes:

• added rollup module
• set eslint to es6
• added dist
• moved code to src/delaunator.js
• made index.js a wrapper for exporting module
• added npm run build for rolling up

[tomvantilburg]

I just committed the changes. Did I understand the prepublish script correctly?

[skyrpex]

May I step in? I don't want to change your point of view, just ask a few questions :+1:

1. I'd like to keep the full source in /index.js.

Why is that? Rollup will bundle your code into one file.

3. The built file should not be committed to the repo. Instead, it should be added to .gitignore, and a build step should be added to prepublish script.

I've had problems in the past with this. I had either to include the built files into the repo or moving my tooling dependencies from devDependencies to dependencies (which is far from ideal for modules). Maybe this has changed recently?

[oxygen]

It would be pretty to have one class per file (even if staying with prototypal ES5 inheritance), directories as namespaces (src/Delauney.js pretty much for this project) and other stuff, as well as optionally publishing a browser (not node) build right in the repo.

It could make life easier for future contributors.

Not exported functions could be static functions on the class (or whathaveyou ES5 constructor). That way we would be able to extend the Delauney class in other projects, which right now isn't really possible, not even with monkey patching. Which would mean having to copy paste, which would immediately negate the usefulness of npm modules.

I'd be happy to send a pull request with the above refactors, waiting for the author to respond. Without compromising on performance, no need to be touching anything related to it.

Having a browser build should be a separate debate, as usually other projects will just compile this together with their source to reduce the number of files. So having a browser build into a single file (even if not included in repo) is not really useful to most people.

[mourner]

I'd like to keep the full source in /index.js. Why is that? Rollup will bundle your code into one file.

That's my personal preference for maintaining small projects like this. I split the source code into multiple files when it helps me organize the code, but this is not the case here.

I've had problems in the past with this. I had either to include the built files into the repo or moving my tooling dependencies from devDependencies to dependencies (which is far from ideal for modules). Maybe this has changed recently?

This depends on how you configure the NPM package. Either using .npmignore or files property of package.json along with building on prepublish allows you to make sure the built file is published in the package contents.

That way we would be able to extend the Delauney class in other projects, which right now isn't really possible, not even with monkey patching.

@oxygen can you explain your point here? I don't understand why wouldn't you be able to extend the exported Delaunay class. Instead of monkey patching, the better pattern is to "subclass" like this:

function MyDelaunay() {
    Delaunay.apply(this, arguments);
}
MyDelaunay.prototype = Object.create(Delaunay.prototype);
MyDelaunay.foo = function () { ... }

[mourner]

Thanks for the input to everyone in this thread and in the ticket #4. I've decided to put the ES6 migration on hold for now, mainly to avoid increasing my burden as a maintainer. I may reconsider in future, but for now I'd like to keep things simple and recommend people to use tools like rollup-plugin-commonjs. Can't wait to see ES6 modules landing in a stable Node version.

[oxygen]

Like I said there are some functions in there which are not part of Delauney. In a browser they'd be globally scoped, but not in node or when using a node build system.

They could be made (static) a part of Delauney like this: Delauney.removeNode = function(){}

Also, that would help with namespacing in a browser environment, if including index.js directly (the only possible reason I see for preferring ES5 over ES6).

[mourner]

@oxygen if some functions in the source are not exported, they are not meant to be used externally. This is intended.

For the browser build, you can use a service like wzrd.in which automatically browserifies any module and builds it into a UMD bundle: https://wzrd.in/standalone/delaunator@latest

[DenisCarriere]

Wow! https://wzrd.in is truly black magic! This is really amazing (never have to bundle those small packages ever)

[mourner]

Hey everyone, I've reconsidered. Delaunator v2.0 is a ES module now: https://github.com/mapbox/delaunator/releases/tag/v2.0.0