davidbau / seedrandom

seeded random number generator for Javascript
2.06k stars 160 forks source link

add identifier to AMD module definition #4

Closed yanne closed 10 years ago

yanne commented 10 years ago

If the module definition lack id, the only way to require seedrandom is by path.

By adding the id, it is possible to load seedrandom with script tag and use require('seedrandom') for require.

jugglinmike commented 10 years ago

This is not necessarily an improvement. From the RequireJS documentation on named modules:

You may encounter some define() calls that include a name for the module as the first argument to define():

    //Explicitly defines the "foo/title" module:
    define("foo/title",
        ["my/cart", "my/inventory"],
        function(cart, inventory) {
            //Define foo/title object in here.
       }
    );

These are normally generated by the optimization tool. You can explicitly name modules yourself, but it makes the modules less portable -- if you move the file to another directory you will need to change the name. It is normally best to avoid coding in a name for the module and just let the optimization tool burn in the module names. The optimization tool needs to add the names so that more than one module can be bundled in a file, to allow for faster loading in the browser.

davidbau commented 10 years ago

I'm inclined to agree with jugglinmike, that it's cleaner for us to leave ourselves as an internally unnammed AMD module. I feel like "being named" should be a trick for hugely popular scripts like jquery.js. A minor utility like seedrandom is better of staying anonymous and being renamable by users.

seedrandom.js, of course (because of its historical design), is still always usable just as a <script> include without going through require, because it deposits itself on Math.seedrandom. If you know you're going to have it included that way, you can always say seedrandom = Math.seedrandom. That should be equivalent to seedrandom = require('seedrandom').

I'll leave this pull request open for a bit for any further discussion.

davidbau commented 10 years ago

OK, closing this pull request unmerged. Thanks yanne and jugglinmike.