Skip to content
This repository

Unified registration as browser/AMD/CommonJS module. #710

Closed
wants to merge 7 commits into from
James Burke

This is an improvement over the #688 based on feedback from @tbranyen and @jdalton. It is a much simpler registration approach.

One of the simplifications over #688 was to not to try to load jquery in a node/commonjs environment since the current Backbone code does not do that.

This pull request supersedes #688, I will close 688.

Sam Breed
Collaborator

that's really clever!

John-David Dalton

The super weak inferences aside, I dig this :D :+1:

lifesinger

IMO, these wrapper code is nonsense for Backbone. In this case, AMD is a transport format and only meaningful for AMD loaders. In addition to AMD loaders, there ary many other loaders. AMD should keep a transport version by itself.

For example, SeaJS -- a CommonJS-like loader focusing on the web, its community maintains a transport file for Backbone:

https://github.com/seajs/modules/blob/gh-pages/backbone/transport.js

when a new version of backbone is out, just run spm transport backbone/transport.js to generate the transport version of backbone. It is very convenient.

Furthermore, please do NOT add "backbone" to define. In this way, you must put backbone.js under the baseUrl, or config paths to use it. This will cause problems when multi versions of a module exist in one page.

For libraries maintenance, I suggest to put libraries files under version, such as:

libs
  -- jquery
      -- 1.6.4/jquery.js
      -- 1.7.0/jquery.js
  -- backbone
     -- 0.5.3/backbone.js
     -- 0.6.0/backbone.js
  -- requirejs
     -- 1.0.0/require.js
     -- 1.0.1/require.js
...

A example: https://github.com/seajs/modules

In this way, when one library has new version, just transport it. And modify the config file in your project to update:

seajs.config({
    jquery: 'jquery/1.6.4/jquery'
});

When you decide to update jquery to 1.7.0, the only work you need to do, is to modify the config file:

seajs.config({
    jquery: 'jquery/1.7.0/jquery'
});

It is more robust, and has no side effects to other projects.

Finally, please do NOT add these transporting code to source code.

Mike Knoop

Tonight I slugged through integrating the latest RequireJS (1.0.1), Underscore (1.2.1), jQuery (1.7.0), and Backbone (0.5.3-optamd3, this pull request 710). This may be obvious in retrospect, config.paths keys for Underscore and jQuery must be undercase to match the AMD module code. For example,

require.config(
 paths: {
  jQuery: 'library/jquery/1.7.0/jquery',
  Underscore: 'library/underscore/1.2.1/underscore',
  Backbone: 'library/backbone/0.5.3-amd/backbone'
 });

does not work -- RequireJS will try to load "jquery.js" and "underscore.js" from the rootUrl instead of the defined paths. Undercase all keys as follows:

require.config(
 paths: {
  jquery: 'library/jquery/1.7.0/jquery',
  underscore: 'library/underscore/1.2.1/underscore',
  backbone: 'library/backbone/0.5.3-amd/backbone'
 });

Now the new Backbone code properly detects the modules. Hopefully this comment serves useful to someone who stumbles across this in the future.

Miller Medeiros

wasn't it supposed to register the factory value somehow? or by assigning it to the root object or returning the factory value?

return factory(root, exports, _, $);

same thing for node:

module.exports = factory(root, exports, require('underscore'), undefined);

forget... just realized you are passing exports as Backbone so it is already returning proper value...

James Burke

@lifesinger: maintaining a github repo of all modules that may want to register as modules is not scalable, and not very friendly to the web's decentralized nature. In addition, a file needs to indicate its dependencies properly and, if using a modular pattern, not use globals. The file ends up needing to declare what it needs.

Your concerns about transport code seem to echo CommonJS arguments, but CommonJS was not designed for browsers today, and to get true adoption on the web there needs to be a standard transport format understood by loaders. AMD is that format. This patch has a path for CommonJS-based code.

You are bringing up foundational issues that resulted in there being a separate AMD and CommonJS efforts. It is best to discuss your concerns on amd-implement and not in this ticket.

On a named define() call: it is OK for base libraries of the web, like jQuery, underscore, Backbone, to register as a named module, since they will be referenced by other modules by this name. Case in point, how this patch for Backbone references 'jquery' as a dependency.

In fact for base libraries it is best to register as a named module because they are more likely to be loaded on a page by higher level third party libraries on a page, but not by an AMD loader. If there is an AMD loader on that page, and that module is not named, there will be an error because the AMD loader does not have enough context to give the module a name. There is more discussion about this in the context of jQuery.

As a counterpoint, if Zepto adopts AMD registration, it should not register as a named module because it wants to be used in place of jQuery, and it is a smaller, niche loader.

Your example of libraries under versioned directories also work with AMD loaders. However I consider it configuration overkill. Best to use a convention based on file names. If jquery.js and backbone.js are placed in the baseUrl for an AMD project, things just work without the developer having to type a bunch of config.

Note that this does not argue for using an anonymous define() call, since libraries (like Backbone referencing 'jquery') need to use generic names, not versioned ones.

Thanks for pointing out that I should write up a doc so that when people have these questions we can just refer to the document. I put an issue in my task list to do this.

I believe that doc also would have helped @mikeknoop and I'll make a point to call out what he talked about, in particular the use of lower case names is so that there is an easy convention for loading scripts that match file names that does not require a configuration block for developers to type out.

lifesinger

@jrburke: Thanks for your detailed explanation, but I still don't agree some points with you.

I don't mean to maintain all modules in a github repos. It is just an example. The real purpose here is to demonstrate how to use spm(some like r.js) to transport a library file to a CommonJS or AMD module:

$ spm transport xx/transport.js

Developers can write their own tranport.js, and then use spm to transport it. In this way, you can convert all existed javascript files what you want to modules rather than modify the source code directly.

For the multiple version issues, I will discuss on the amd-implement list. Thanks for your reminding.

Finally, CommonJS is not designed for browsers, but AMD is not the only format designed for browsers. There are many other loaders such as BravoJS, FlyScript, SeaJS etc. They support CommonJS/Modules 2.0, Modules/Wrappings etc. Those formats are also designed for Web Browsers! As base library, Backbone should not add some special code for only one format. AMD is popular, but does not mean AMD is the best and the common module format for the web. It needs more time to prove.

Mike Knoop
James Burke

@lifesinger: The problem with module formats is that it is easy to invent them but hard to actually get adoption. AMD has been adopted by the Dojo, MooTools and jQuery communities.

The draft of a CommonJS 2.0 that you mention is just a draft, nothing ratified, and it is doubtful it will be. Its syntax is more verbose than AMD. At this point it is a bikeshed argument, AMD is blue, the draft you mention is red. It is time one was just chosen so we can move on and start sharing code and solving end users' problems. AMD has been chosen by more web-based library authors. Since the jQuery community likes AMD, it makes even more sense for Backbone to favor that format.

It is possible for command line tools like spm to convert AMD to another syntax. There is nothing saying there cannot be another format for a developer's niche needs. However there is great value in having at least one commonly understood one. At this point, the adoption points to AMD.

Matthew Mirande

Agreed. I use AMD because I do most of my work in the browser. If I did more node work, I might have favored CommonJS. What's important is being able to build and share more flexible code. Also remember that this patch doesn't favor any one format.

TrisMcC

I have been maintaining my own patched backbone 0.5.3 with the changes in the optamd3 branch to allow my continued development. Adhering to the AMD standard would make many developers' lives easier. Backbone.js with AMD is a very strong use case for asynchronous modules.

The AMD bootstrap code is minimal and does not impinge on any other aspect of a JavaScript library's inner-workings. Many other libraries, including jQuery and Underscore, are now AMD compliant.

Aeron Glemann

@jrburke

I think this patch breaks Backbone if using with Zepto and RequireJS (assumes jQuery on line 15).

James Burke

@aglemann this is by design -- the module name can be fulfilled by a different implementation. Right now AMD users would need to create an adapter module that either wraps zepto in an AMD wrapper or that adapter module can require zepto and then return it as the value for jQuery.

I have started looking at a patch for zepto to call define() as an anonymous module, then no adapter module is needed. However, Zepto's code layout and construction is different than other code I have seen, so it may take a bit for me to work out what kind of patch would be best for it.

Depending on how this pull request goes, I'll make a note to write up how to use AMD with Backbone and Zepto in the meantime via the adapter module approach.

Rainer Wittmann

+1 Underscore does it, jQuery does it... so PLEASE make backone AMD compliant as well. As said before: It makes developers lives (and mine) easier :).

Jeremy Ashkenas
Owner

Point of curiosity -- is there anyway to revise this patch to avoid the tricky function-passed-into-a-function pattern?

It would be great if all this could be accomplished with simple assignments and conditionals at the top of the vanilla closure wrapper.

James Burke

@jashkenas: the issue lies in the synchronous nature of CommonJS and window global paths -- they assume synchronous access to their dependencies -- where AMD recognizes that dependencies may not be immediately available, even if bundled in the same file, but later in the file. The bottom function should not be executed until the dependencies are available. That is the main hurdle.

There was a different version of this patch that puts the work more at the bottom of the page, with dependencies grabbed inside the main function, but it could result in another level of indentation (I avoided it in the patch for diff clarity) and @tbranyen was not too keen on the tertiary use to pass define. @jdalton also had some issues with that approach.

I now find that previous, different version of the patch a bit ugly compared to this latest version, although it could be cut down a bit, by removing the require('jquery') for the CommonJS branch.

This latest version feels closer to idiomatic web style where dependencies are passed as named arguments to an anonymous function. It is also about the same code weight as before patch, and does not require another level of indentation.

But if you want me to revisit the path in that first patch, I can set up another branch to show it without the extra try/catch for jquery in the CommonJS path. Your thoughts on the possible extra level of indentation would also be appreciated in that case.

Aaron Hardy

+1. Would love to see backbone work appropriately with require.js.

James Morrin

It has been a while since i have seen discussion on this pull request. Is it not going to be accepted? Right now I am working with a fork of Backbone to stay compatible with underscore with an AMD loader. I would like to get back to using the main Backbone and get off of the fork. What can we do to get some form of AMD compatible Backbone accepted? This wouldnt be much of an issue if underscore didnt already implement AMD compatibility.

Jeremy Ashkenas
Owner

Yes, some form of this will probably be accepted for the next version of Backbone.

Romain

+1

I use AMD, and start using backbone with underscore. I don't want to stay on a fork.

František Hába

+1

Thomas Davis

Looking forward to this, you guys are making history! +1

Aeron Glemann
James Burke

@aglemann, probably based on @lifesinger's comments earlier in this thread. I'm sure they still stand for him. Same for me, in my responses.

Tomasz Tunik

:+1: for 0.6

Dave Geddes

+1 for AMD support.

Kevin Wade

+1

Denis

Big +1

Any idea on 0.6 eta/roadmap? Searching brings my nothing but a discussion on 0.5s 'roadmap'.

Thanks

Thomas Hollstegge

+1

S.Korth

+1

Tauren Mills

+1

Deleted user

The content you are editing has changed. Reload the page and try again.

+1

Sending Request…

Attach images by dragging & dropping or selecting them. Octocat-spinner-32 Uploading your images… Unfortunately, we don't support that file type. Try again with a PNG, GIF, or JPG. Yowza, that's a big file. Try again with an image file smaller than 10MB. This browser doesn't support image attachments. We recommend updating to the latest Internet Explorer, Google Chrome, or Firefox. Something went really wrong, and we can't process that image. Try again.

John Whitley jwhitley referenced this pull request in jwhitley/requirejs-rails January 06, 2012
Closed

Require / Underscore / Backbone #23

Ryan Florence

Currently backbone tries to export as an AMD module. So does underscore. Since underscore is a dependency, and since backbone checks for root._ it's all busted. So those of us using AMD just hack the source of both libraries.

If you are going to support define, might as well do it right, and with fewer lines of code to boot :)

It's a disappointing amount of boilerplate, but it accomplishes a lot. In the end, you're writing the backbone code inside one closure or the other, all it changes is the line number the code starts at.

If this isn't merged, then don't try to support AMD at all (because it doesn't work atm). Export both Backbone and _ to the global object (underscore doesn't if define is detected) so people can do a quick "shim" module with the order plugin to set up the backbone dependency for other modules without hacking the source of either library.

Jeremy Ashkenas
Owner

After much back and forth and soul-searching, Backbone and Underscore aren't going to support AMD out of the box -- you can add it yourself if you like. There's a bit more detail on the Underscore ticket:

jashkenas/underscore#431

I'd like to think that this will place Backbone.js on the right side of JS history, but only time will tell ;)

Closing this pull request as a "wontfix".

Jeremy Ashkenas jashkenas closed this January 11, 2012
Ryan Florence

I'd like to think that this will place Backbone.js on the right side of JS history, but only time will tell ;)

Hey now, don't need to get all elitist ...

We'll have a fork somewhere, hopefully it will become popular enough to get your attention again, but for now at least we don't have to hack the source of underscore or backbone to get it to work.

Dave Geddes

Yes, some form of this will probably be accepted for the next version of Backbone.

I'm curious what changed your mind? (Other than maybe being driven insane by all the +1s).

I believe there's a reason why so many people are rallying behind AMD right now: JavaScript needs a standard module format that works anywhere JS does. After developing in Node it's hard going to back to the browser where you don't have modules. As you pointed out in that other ticket native modules are coming in ES6 (planned for sometime in 2013). Then give it another year while the IE team says that feature is "not ready for primetime" while scrambling to implement it in their next version. So in reality we're looking at 2-3 years from now for native modules to be widely adopted. AMD modules already have the adoption. Developers are getting the benefits of JS modules right now. Native modules may or may not make the AMD standard obsolete- I don't really care either way. But why not use and improve on the standard (AMD) that we have right now instead of shooting it down because something potentially better is in the works? That's like refusing to use CoffeeScript because Harmony is coming. Like Brenden Eich said at JSConf last year and as you know well, CS is heavily influencing the JS standard. Likewise, AMD is influencing Dave Herman's native modules spec for JS. It isn't about sitting on the sidelines and hoping you chose the right "side". It's about uniting as a JS community and improving the language we all love. AMD is a huge step in the right direction. Removing support for it isn't.

John Hann

Hey @geddesign! Two points:

Last I looked at the native modules spec, it wasn't backward-compatible. This means we have to wait for IE9 to die off before we can actually use it. AMD as a definition format is meant to bridge the gap until that time, which is likely to be at least five years away.

Also, I still haven't seen any work from the ES team to help with dependency management or concatenation, packaging, or grouping of modules (or non-js resources). The reason the CommonJS people are still exploring a 2.0 Modules format is because these features are still sorely needed. AMD as a transport format has already solved most of these problems, and individual loaders/builders have already solved the rest of them.

AMD is influencing the CJS guys too. They see we've implemented something simple and efficient. (I know because they told me that it has. :) )

I know we'll see AMD support in @documentcloud's projects again soon. For now, as @rpflorence said, somebody will fork them and fix them.

-- John

Aaron Hardy

Ditto. ^^^^^^^^^^^^^^^

Jeremy Ashkenas
Owner

I'm curious what changed your mind?

... to be specific, what changed my mind was witnessing the volume of AMD-specific troubleshooting questions/tickets/complaints only rise after adding an AMD definition to Underscore ... and then later having websites break in production due to the AMD definition.

Previously, I was thinking "no harm, no foul" -- despite personal preferences, we might as well add it for the folks who want to use it. Those two things pushed it over the edge. More are listed here: jashkenas/underscore#431

And read @jrburke's great response, if you haven't already: http://tagneto.blogspot.com/2012/01/simplicity-and-javascript-modules.html

Ryan Florence

to be specific, what changed my mind was witnessing the volume of AMD-specific troubleshooting questions/tickets/complaints only rise after adding an AMD definition to Underscore

That's not fair. Backbone and other third-party libs that depended on it and tried to support amd didn't do it right. That was the problem, not underscore, not amd. If you'd done the node dependency support wrong you wouldn't blame node.

If underscore simply continued to export to window, or if the third-party libs that depended on underscore did it right then it would have all been fine.

Adam Pflug

@rpflorence +1. It's really a shame to see Backbone take this stance against AMD. Right now it completely breaks in an AMD environment because it uses "require" in an incompatible way.

Tim Branyen
Collaborator

@AdamPflug I've been using Backbone and Underscore without modification just fine: http://github.com/tbranyen/backbone-boilerplate

Sam Breed
Collaborator

@tbranyen also, isn't use! being pulled into require core? if so, case-closed :godmode:

Mike Knoop
John Whitley

@wookiehangover Almost. Functionality similar to the use! and wrap! plugins will gain first-class support in RequireJS 2.0 as the shim configuration option. Note that 2.0 is still under development and design changes have and will continue to occur until it's released. Interested folks can follow development via the draft spec link above and the requirejs dev2.0 branch.

Sam Breed
Collaborator

@jwhitley that's great new, thanks for confirming. IMO that functionality is the lynch-pin to making require an absolute no-brainer :shipit:

Adam Pflug

@tbranyen that's a nice work-around. I hadn't see use! before. My point is just that without extra code like that, the "require" check breaks if you're using AMD, even if you don't use AMD to load backbone/underscore, or would like to use them just through the globals they register.

if (!_ && (typeof require !== 'undefined')) _ = require('underscore');

simply changing it to this:

if (!_ && (typeof require !== 'undefined') && !require.amd) _ = require('underscore');

would let them co-exist much more nicely.

Pete Hawkins

+1

Rico Sta. Cruz rstacruz referenced this pull request in theironcook/Backbone.ModelBinder October 02, 2012
Closed

Remove AMD/require.js support #83

Anzor Bashkhaz

Now that lodash can be used as a drop in replacement for underscore and it supports AMD (so does jQuery), all that's left is Backbone! +1, let's make this happen guys

Gurdas Nijor

+1

Timmy Willison timmywil referenced this pull request June 27, 2013
Closed

Reconsider AMD support #2641

James Burke jrburke referenced this pull request in jashkenas/underscore June 27, 2013
Closed

Register as AMD module, but still export a global #431

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 19 additions and 22 deletions. Show diff stats Hide diff stats

  1. 41  backbone.js
41  backbone.js
@@ -4,44 +4,40 @@
4 4
 //     For all details and documentation:
5 5
 //     http://documentcloud.github.com/backbone
6 6
 
7  
-(function(){
  7
+
  8
+(function(root, factory) {
  9
+  // Set up Backbone appropriately for the environment.
  10
+  if (typeof exports !== 'undefined') {
  11
+    // Node/CommonJS, no need for jQuery in that case.
  12
+    factory(root, exports, require('underscore'));
  13
+  } else if (typeof define === 'function' && define.amd) {
  14
+    // AMD
  15
+    define(['underscore', 'jquery', 'exports'], function(_, $, exports) {
  16
+      factory(root, exports, _, $);
  17
+    });
  18
+  } else {
  19
+    // Browser globals
  20
+    root.Backbone = factory(root, {}, root._, (root.jQuery || root.Zepto || root.ender));
  21
+  }
  22
+}(this, function(root, Backbone, _, $) {
8 23
 
9 24
   // Initial Setup
10 25
   // -------------
11 26
 
12  
-  // Save a reference to the global object.
13  
-  var root = this;
14  
-
15 27
   // Save the previous value of the `Backbone` variable.
16 28
   var previousBackbone = root.Backbone;
17 29
 
18 30
   // Create a local reference to slice.
19 31
   var slice = Array.prototype.slice;
20 32
 
21  
-  // The top-level namespace. All public Backbone classes and modules will
22  
-  // be attached to this. Exported for both CommonJS and the browser.
23  
-  var Backbone;
24  
-  if (typeof exports !== 'undefined') {
25  
-    Backbone = exports;
26  
-  } else {
27  
-    Backbone = root.Backbone = {};
28  
-  }
29  
-
30 33
   // Current version of the library. Keep in sync with `package.json`.
31 34
   Backbone.VERSION = '0.5.3';
32 35
 
33  
-  // Require Underscore, if we're on the server, and it's not already present.
34  
-  var _ = root._;
35  
-  if (!_ && (typeof require !== 'undefined')) _ = require('underscore')._;
36  
-
37  
-  // For Backbone's purposes, jQuery, Zepto, or Ender owns the `$` variable.
38  
-  var $ = root.jQuery || root.Zepto || root.ender;
39  
-
40 36
   // Runs Backbone.js in *noConflict* mode, returning the `Backbone` variable
41 37
   // to its previous owner. Returns a reference to this Backbone object.
42 38
   Backbone.noConflict = function() {
43 39
     root.Backbone = previousBackbone;
44  
-    return this;
  40
+    return Backbone;
45 41
   };
46 42
 
47 43
   // Turn on `emulateHTTP` to support legacy HTTP servers. Setting this option will
@@ -1139,4 +1135,5 @@
1139 1135
     };
1140 1136
   };
1141 1137
 
1142  
-}).call(this);
  1138
+  return Backbone;
  1139
+}));
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.