Register as a browser global or an AMD module

[jrburke]

This is in response to pull request #6. I believe this one will work better as it removes an extra set of arg passing in the globals case. This pattern was based on this general UMD pattern:

https://github.com/umdjs/umd/blob/master/amdWeb.js

@peterlong: if you want to confirm this works for you, that would be good. I tried this file in the example.html in this repo and it works fine for the browser globals case.

[peterlong]

@jrburke Thank you for taking the time to look at this. Unfortunately this version did not work for me right out the box as a requirejs module. I expect this code to extend the Backbone global by adding Backbone.GoogleMaps to it.

I prefer your pull request to mine in general since there are far fewer diffs and it is clearer what you have changed. One way I think it could be made to work is by changing line 15 to:

  // Browser globals
  factory(root.Backbone, root._, root.jQuery || root.Zepto || root.ender);

and then inside the factory function just before returning GoogleMaps, add it to Backbone:

  Backbone.GoogleMaps = GoogleMaps;
  return Backbone.GoogleMaps;

That should set it correctly whether used as a AMD module or directly. Would you agree or is there a better way to do this?

[jrburke]

@peterlong ideally these sorts of things would not need to hang off other dependencies, but since projects that use this file might also use other Backbone plugins or code that work only with the Backbone code, I added the change you suggested, if you would like to confirm it.

[peterlong]

Thank you, it is working for me now.

I think I understand your reservation with this change. Do you mean that a file like this should be included as a module and referenced directly? In other words it would be better to reference the dependency than to use its side-effect via Backbone.GoogleMap:

define(['backbone','backbone.googlemaps'], function(Backbone, BackboneGoogleMaps) {
  // some code...
  var places = new BackboneGoogleMaps.LocationCollection();
  // some more code...
});

That is probably correct and as this code stands it is still possible to do this, just not required.

@eschwartz I hope Mr Burke's pull request meets with your approval.

[eschwartz]

Thanks for the pull request -- I'll take a look when I have a few minutes. I just got a new job (building a google maps + backbone API, of all things), so I'm keeping pretty busy.

[eschwartz]

Sorry to be so slow on this. It's been a busy summer...

On my todo list still is to break this library up into separate AMD modules for each Backbone.GoogleMaps class, then include AlmondJS in the build. If anyone wants to take a stab at that, I'll try to move quicker on your pull request this time :)