RequireJS compiler compability broken

[antris]

Bonzo defines itself to RequireJS as an anonymous module. The RequireJS library (require.js), when requiring the modules from separated files, takes the module name from the file name and it works perfectly. However, the compiler is dumb, and it cannot detect bonzo's module name from the filename because the define() call isn't in the beginning of the file.

This change makes bonzo pass the module name to RequireJS manually, fixing the problem:

diff --git a/src/bonzo.js b/src/bonzo.js
index 99e66d5..b5dc08a 100644
--- a/src/bonzo.js
+++ b/src/bonzo.js
@@ -1,5 +1,5 @@
 !function (name, definition){
-  if (typeof define == 'function') define(definition)
+  if (typeof define == 'function') define(name, definition)
   else if (typeof module != 'undefined') module.exports = definition()
   else this[name] = definition()
 }('bonzo', function() {

[ded]

cc @rpflorence

[ryanflorence]

Yeah, I just ran into this also. Been meaning to connect with @jrburke about it.

[ryanflorence]

I think this is a sound solution though (for now at least), then you can simply require(['bonzo'])

[jrburke]

The requirejs optimizer tries not to step on any define() that may be declared as part of some non-AMD code, so it only matches for very specific signatures. In this case, it wants to see a function literal in the define call. This signature works best:

if (typeof define == 'function' && define.amd) define('bonzo', [], function () { return definition(); })

The string literal is in the define function so that it is clear this is a named module, and that the optimizer does not need to generate a define call with that name. Defining a named module also works best if this code is optimized by other concat tools but then loaded in a page in a way that does not use an AMD loader, but an AMD loader does exist on the page. In that case using an anonymous call to define will probably result in an error since the module cannot be tied to a module name.

The optimizer will avoid processing the define() call if the name is a variable, in case this is an app-defined define(). Similar reason (but for runtime) for changing the define test to include define.amd. However, if you prefer to accept the risk calling a non-AMD define(), that && define.amd can be removed.

[antris]

Oh, and I forgot to mention that this problem also affects at least the qwery library.

[ded]

nearly all my modules use the exact syntax shown here already in Bonzo. So if it's a problem, I'd have to fix them all. So I mostly want to double check that this would be the exact syntax we want.

[jrburke]

I have updated the requirejs 0.27.1 (the latest release) optimizer and relaxed the define matching a bit to allow for smaller code patterns, so this should be sufficient:

Pattern 1:

    if (typeof define == 'function' && define.amd) define(name, definition)

However, since the optimizer does not know what name is, will add an extra define('bonzo',...) to the built file indicating that 'bonzo' has in the optimized layer, but the above works and is minimal impact on the bonzo source.

If you prefer to not have the extra built code inserted, then this pattern is the recommended one:

Pattern 2:

    if (typeof define == 'function' && define.amd) define('bonzo', function(){return definition()})

I have also been working out suggested syntax for universal modules, but that may be seen as too much change.

So, use Pattern 2 unless the aesthetics of it bother you. In that case, use Pattern 1.

[ded]

thanks @jrburke i'll go with the first. it looks sexier.

[mattandrews]

Not sure if this is directly related to this issue, but I'm having issues when using bonzo with requireJS. When I pass bonzo as an argument to require's callback function, it outputs null when I run console.log(bonzo), but if I rename the argument to _bonzo_ or something, my call to console.log(bonzo) returns the correct object.

This could be due to the way I'm using require() -- I map my URLs using require's config/paths tool, so call bonzo like this:

require(["bonzo"], function(bonzo) {
    console.log(bonzo);
});

Anyway: am I wrong, or is Bonzo?

[ryanflorence]

Bonzo is checking if define is 'function' instead of 'typeof define', making a pull request real quick.

[mattandrews]

Thanks for this -- can confirm that it fixes my issue above.