chaplinjs / chaplin

HTML5 application architecture using Backbone.js
http://chaplinjs.org
Other
2.85k stars 232 forks source link

AMD Sugar #12

Closed WikiBruce closed 12 years ago

WikiBruce commented 12 years ago

Great effort. Thanks a lot.

After some deliberation we decided to use the "simplified CommonJS wrapping" notation as it is advocated by J. Burke of requirejs: http://requirejs.org/docs/whyamd.html#sugar for our project. We actually prefer it because it feels somewhat easier to read and it may help to avoid unnecessary module loading as it seems to happen in your view.coffee class.

So we were wondering if there are some factors that we are overlooking or not taking into account. And since you really seem to know what your are doing here we would very much appreciate it if you could briefly explain to us why your are sticking to the classic notation.

molily commented 12 years ago

Nice, I didn’t know about this “sugar” possibility.

In Chaplin, the import lists are quite short but this is just because we cheated. Libraries like jQuery, Backbone and Underscore aren’t wrapped with AMD at the moment. That is, we’re not using things like https://github.com/geddesign/wrapjs or https://github.com/tbranyen/use.js at the moment.

Also, @paulmillr did a great job improving the code readability regarding the import headers at least in the CoffeeScripts.

Actually I don’t get the sugar example on requirejs.org. Sure, this is a whole mess:

define([ "require", "jquery", "blade/object", "blade/fn", "rdapi",
         "oauth", "blade/jig", "blade/url", "dispatch", "accounts",
         "storage", "services", "widgets/AccountPanel", "widgets/TabButton",
         "widgets/AddAccount", "less", "osTheme", "jquery-ui-1.8.7.min",
         "jquery.textOverflow"],
function (require,   $,        object,         fn,         rdapi,
          oauth,   jig,         url,         dispatch,   accounts,
          storage,   services,   AccountPanel,           TabButton,
          AddAccount,           less,   osTheme) {

});

But if I include 17 other modules I guess the code won’t get nicer if I insert 17 lines of var module = require('module'); instead. This wouldn’t improve the overview at all. If I ran into the problem of including so many modules I would first question my modularization approach.

For example, on moviepilot.com, we’re loading jQuery from a CDN without an AMD wrapper. jQuery is a hard dependency for our app as a whole and I don’t have a problem with jQuery being global and not taking part in our app module dependancy tree.

In general, I don’t get the reason behind wrapping all code into AMD modules. I have a more pragmatic view on RequireJS, looking at the benefits. It’s great to specify a clean dependancy tree, lazy-load modules during runtime and pre-compile packages. Most libraries today create only one global object, do not pollute the global space beyond that and do not modify ECMAScript core prototypes.

Of course wrapping jQuery etc. makes it easy to compile your whole application into one file using the r.js optimization tool alone. But the r.js packager is mostly just one step in a more complex build process. Adding jQuery and other prerequisite scripts to the package isn’t rocket science.

The require.js documentation also mentions the drawbacks of the “sugar” imports: After loading the code via XHR, it relies on function decompilation and JavaScript code parsing to extract the require statements. Decompilation is slow and – as the documentation already states – not available on all JavaScript engines. It’s a non-standard feature which a conformant ECMAScript engine does not have to implement. So this cannot be a general and standard-conformant approach.

To sum it up, I’m generally I’m sceptic about this sugar and and I’d first try to work around the problem. Since I don’t know your specific application and its modularization concept, I surely don’t want to condemn the whole sugar idea completely. Probably it’s a useful solution in your case.

paulmillr commented 12 years ago

IMO this is useful because it doesn't add another indentation level to your files if you'll add module.exports wrapper automatically. E.g. https://github.com/brunch/todos/tree/gh-pages/app/views.

WikiBruce commented 12 years ago

Hi molily, thanks a lot for you considerate reply!

We appreciate your impartial feedback and your additional hints towards wrapjs and use.js. Very nice.

@paulmillr: Thank you for for pointing out the indention level aspect.

So, at the very least this sugar thing

For us this warrants its use as long as we are using it with the requirejs optimizer, which we are.

Sugar or not. Keep up the great work! We like your project and we will follow as closely and contribute as much as we can!

molily commented 12 years ago

@paulmillr Sure, Brunch code is clearly superior in this regard since Stitch automatically wraps the code into CommonJS modules. But when using RequireJS/AMD, AFAICS it’s not possible to get rid of the define([…], function (…) {…}) wrapper, even with the sugar approach and the r.js optimizer. (Except you write an additional build script.) So I don’t see how the sugary imports could remove the initial indentation level. (Please correct me if I’m wrong.)

paulmillr commented 12 years ago

Yep, you're right, in this case chaplin cannot get rid of indentation.

(side note: it's not stitch, it's a custom thing).

nfedyashev commented 12 years ago

@molily

I like the way you explained why Chaplin is opinionated in that sense and does not use AMD modules for hard dependencies.

Maybe this is about time to put this explanation in README and remove it from https://github.com/moviepilot/chaplin/blob/master/index.html#L32 ?

This is a simple solution. We use the non-AMD versions of the core libraries
because nearly every module uses these libraries. For this demo, it's okay
that they create global objects (jQuery, $, _, Backbone, Handlebars etc.).

It seems like a temporary comment/solution but it's not. Maybe that is why @WikiBruce asked this(I was not sure about the reasoning before you explained it in this issue either). It is final conscious decision for the Chaplin as you explained earlier.

molily commented 12 years ago

Chaplin shouldn’t be opionated in this regard, it’s just me who is. ;)

It’s not part of Chaplin to favor either of the solutions. Since Chaplin is just an example structure, it leaves out how you build and package everything for production. RequireJS will help you but cannot deal with all tasks (for example, precompiling templates). If you stick with RequireJS, you will at least need some plugins to handle that.

In a real-life public application, I would wrap Backbone and Underscore in AMD, package with r.js while loading jQuery from a CDN. But this totally depends on your application. Since most people seem to expect that all libraries are loaded with RequireJS, we will probably include use.js/wrap.js so beginners find a consistent modularization in Chaplin.

paulmillr commented 12 years ago

By the way — what do you think about writing a simple build script that would wrap modules into require.js closures?

paulmillr commented 12 years ago

This was pretty boring, but I've replaced all module definitions / requirements with sugar version in brunch with chaplin (see app/ dir).

Looks & feels awesome so far.

burin commented 12 years ago

@paulmillr the sugared version of chaplin looks great!