dagrejs / dagre-d3

A D3-based renderer for Dagre
MIT License
2.86k stars 591 forks source link

d3 dependency not getting loaded via jspm #198

Open myitcv opened 9 years ago

myitcv commented 9 years ago

Related conversation: https://github.com/jspm/jspm-cli/issues/1280

The short summary from that conversation is that jspm ends up not loading d3 because nowhere in dagre-d3 do we find a require('d3') (SystemJS is lazy loading).

Indeed this change removed the require.

Can you help to explain the rationale for that change?

Would you be averse to a PR that made lib/d3.js as follows:

require('d3');
module.exports = window.d3;

PS. awesome package, thanks very much

cpettitt commented 9 years ago

The reason it was pulled was:

  1. Most often people were including D3 directly on the page. This lead to loading D3 twice (twice the cost on the wire) and also lead to weird situations where you'd add an event handler on one instance of D3 but have the context on the other instance.
  2. I wanted people to have flexibility to use a newer version of D3 than what I'd last released with.

This arrangement works with bower, but script loading is explicit there. Can you put a dummy require in to make SystemJS or otherwise force it to eagerly load?

myitcv commented 8 years ago

@cpettitt thanks very much for the quick reply, apologies for this tardy response.

Most often people were including D3 directly on the page

Ok, I understand the motivation now. However, as you can see this makes it difficult for people who want to rely on the dependency being loaded.

I wanted people to have flexibility to use a newer version of D3 than what I'd last released with

I don't think declaring (and loading) d3 as a dependency precludes this, just so long as you don't pin your dependency (in package.json) to a specific version.

Can you put a dummy require in to make SystemJS or otherwise force it to eagerly load?

For now I've done this

However I'm wondering whether we can't satisfy both camps with a bit of a hack. Given that we know require("d3") has the side effect of setting window.d3, can you not simply check whether window.d3 is defined, and require("d3") if not? e.g.

if(window.d3 === undefined) {
  require("d3");
}
module.exports = window.d3;