Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support AMD/RequireJS. #1689

Closed
wants to merge 2 commits into from
Closed

Support AMD/RequireJS. #1689

wants to merge 2 commits into from

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Jan 9, 2014

When d3.js is loaded, it now prefers the AMD define function or module.exports if available; the global d3 is only set if neither of these are available. A side benefit of this change is that browserify, bower and component can now load the D3 library directly rather than needing a custom definition.

When d3.js is loaded, it now prefers the AMD define function or module.exports
if available; the global `d3` is only set if neither of these are available. A
side benefit of this change is that browserify, bower and component can now load
the D3 library directly rather than needing a custom definition.
@sheppard
Copy link

sheppard commented Jan 9, 2014

Cool, thanks!

I was just about to throw out another option based on this example.

Pros: works with the return from the old closure, and puts all the moduley stuff at the top of the built file.
Cons: a bit more convoluted

(function (root, factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module.
        define(factory);
    /* EDIT: instead of this: 
    } else if (typeof exports === 'object') {
    do this: */
    } else if (typeof module === "object" && module.exports) {
        // Node. Does not work with strict CommonJS, but
        // only CommonJS-like enviroments that support module.exports,
        // like Node.
        module.exports = factory();
    } else {
        // Browser globals (root is window)
        root.d3 = factory();
  }
}(this, function () {
    var d3 = {};
    // make d3
    return d3;
}));

I'm fine leaving it as you have it now.

@mbostock
Copy link
Member Author

mbostock commented Jan 9, 2014

The only effective difference is using typeof exports === 'object' rather than our current longer typeof module === "object" && module.exports, right? I feel like the latter is more robust, but then again this whole thing seems like a cludge anyway. :)

@sheppard
Copy link

sheppard commented Jan 9, 2014

Yes, I just noticed that after copying the example. I agree that typeof module === "object" && module.exports is a better test - I meant for the example to be effectively the same. My main point was just to show the difference in where the cludge actually appears in the file.

@sheppard
Copy link

sheppard commented Jan 9, 2014

It'd be less cludgy if everyone would just use AMD :)

Ok I'll stop

@mbostock
Copy link
Member Author

mbostock commented Jan 9, 2014

Obligatory XKCD reference: 😁

xkcd:927

@mbostock
Copy link
Member Author

Folded into #1690.

@mbostock mbostock closed this Jan 10, 2014
@mbostock mbostock deleted the define branch January 10, 2014 00:39
tbranyen added a commit to tbranyen/d3 that referenced this pull request Jun 24, 2014
With regards to d3#1689 adding AMD compatibility, it seemed odd to me that
D3 opt'd out of exposing itself globally for third-party plugins.

Within a discussion about this very problem, found here:
mpld3/mpld3#33 (comment)

the argument is made that exposing the global defeats the point of
module systems.  I agree that module systems help avoid global
pollution, but that is not something I'd consider the point of module
systems, especially since they are registered in a global namespace
themselves.

This patch will fix third-party modules that are shimmed.
rpoconn pushed a commit to rpoconn/d3 that referenced this pull request Aug 20, 2014
With regards to d3#1689 adding AMD compatibility, it seemed odd to me that
D3 opt'd out of exposing itself globally for third-party plugins.

Within a discussion about this very problem, found here:
mpld3/mpld3#33 (comment)

the argument is made that exposing the global defeats the point of
module systems.  I agree that module systems help avoid global
pollution, but that is not something I'd consider the point of module
systems, especially since they are registered in a global namespace
themselves.

This patch will fix third-party modules that are shimmed.
rpoconn pushed a commit to rpoconn/d3 that referenced this pull request Aug 20, 2014
With regards to d3#1689 adding AMD compatibility, it seemed odd to me that
D3 opt'd out of exposing itself globally for third-party plugins.

Within a discussion about this very problem, found here:
mpld3/mpld3#33 (comment)

the argument is made that exposing the global defeats the point of
module systems.  I agree that module systems help avoid global
pollution, but that is not something I'd consider the point of module
systems, especially since they are registered in a global namespace
themselves.

This patch will fix third-party modules that are shimmed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants