mapbox / delaunator

An incredibly fast JavaScript library for Delaunay triangulation of 2D points
https://mapbox.github.io/delaunator/
ISC License
2.3k stars 141 forks source link

Adopt UMD with default export. #15

Closed mbostock closed 6 years ago

mbostock commented 6 years ago

Adopting UMD enables this library to be loaded in a browser without needing to define a global window.modules = {} first (and without leaking globals like dist, area etc.).

<script src="https://unpkg.com/delaunator"></script>
<script>

console.log(Delaunator); // Hooray!

</script>

It also allows the library to be loaded by an AMD loader such as RequireJS and d3-require.

<script src="https://unpkg.com/d3-require"></script>
<script>

d3.require("delaunator").then(function(Delaunator) {
  console.log(Delaunator); // Hooray!
});

</script>

And of course it retains compatibility with CommonJS.

var Delaunator = require("delaunator");

The specific UMD pattern here is copied from Rollup. If you prefer, I’d be willing to expand this PR to rewrite this library as ES modules, and then use Rollup to generate the UMD (and any other bundle format) automatically. That would make this library easier to consume in environments that support ES modules.

Note that this doesn’t pass the tests: for readability purposes, I didn’t indent the code inside the surrounding factory function. The ESLint configuration here prevents you from using the UMD pattern, since the "use strict" goes inside the factory function. I figured I would send this along for review so you can suggest how you’d like this fixed.

Thanks for the great library!

mbostock commented 6 years ago

FWIW, I’ve started using @std/esm in all my Node projects, which makes using ES modules seamless (node -r @std/esm). And unlike node --experimental-modules, it doesn’t require you to adopt the .mjs extension, and offers interop compatibility with CommonJS (e.g., import {readFile} from "fs";).

But I’m not here to tell you how to write code in your modules—I just want a bundle published to npm that I can use in a browser without jumping through hoops. Unfortunately wzrd.in isn’t reliable enough to be used in production. If you publish a UMD or AMD to npm, we’ll be able to use a robust, npm-backed CDN such as CDNJS or unpkg.

So while my personal preference here would be to use @std/esm (for tests) and Rollup to bundle, especially because that means I can also consume this library as ES modules, I’m happy to pick whatever method you prefer to publish a UMD bundle. If you want to run browserify in a prepare script that would be 💯. Just let me know!

mourner commented 6 years ago

Let's do the browserify prepare script route for now, but I'll consider adopting ES6 + Rollup in future. I tried using @std/esm a few days after its official announcement, but experienced terrible bugs that made it unusable for me, stack traces were fully broken in particular. Perhaps it got more stable since then?

mbostock commented 6 years ago

Yep, jdalton’s been putting a lot of work into it since release. I highly recommend it.

Will followup with a new PR adopting Browserify. Thanks!

mourner commented 6 years ago

@mbostock btw, can you point me to a good example repo that takes advantage of ES6 modules and @std/esm?

mbostock commented 6 years ago

@mourner Sorry, I don’t have any public ones (yet!). But I have been thinking of writing a little tutorial on it…