Support loading of snapjs from npm.

[richardvanbergen]

There are some really awesome browser package managers for loading node modules out. Unfortunately, angular-snap.js relies on loading snap.js from the window object only. This wont work if you are using a tool like browserify or jspm because snap.js will instead export the Snap object as a module.export

There is a simple workaround for people coming from the Googles:

import Snap from 'snapjs';
window.Snap = Snap;
import 'angular-snap-js';

But that's pretty ugly it would be nicer if angular-snap could detect the environment it is in. In the SnapConstuctor provider, instead of this:

angular.module('snap')
  .provider('SnapConstructor', function() {
    'use strict';
    var constructor = window.Snap;

    this.use = function(Snap) {
      constructor = Snap;
    };

    this.$get = function() {
      return constructor;
    };
  });

We could do something like this:

angular.module('snap')
  .provider('SnapConstructor', function() {
    'use strict';

    var constructor;
    if (typeof require == 'function') {
      constructor = require('snapjs');
    }

    if (!constructor) {
      constructor = window.Snap;
    }

    this.use = function(Snap) {
      constructor = Snap;
    };

    this.$get = function() {
      return constructor;
    };
  });

This is probably not the correct way of detecting if require is supported and it leaves AMD users out in the cold (which snap.js also supports) but it's a start. If I worked on a proper solution would you be interested in including it?

[jtrussell]

I'd certainly be interested in including it.

Just curious though - are you not able to use the snapConstructorProvider to supply Snap in whichever way you want to pull it in? It was designed to be a generic, module loader agnostic interface. Though it's true we do look for Snap on window by default. In general though I'm open to making people's lives easier :).

import Snap from 'snapjs';
import 'angular-snap';

// ...
angular.module('myApp').config(function(snapConstructorProvider) {
  snapConstructorProvider.use(Snap);
});

[richardvanbergen]

That's actually pretty nifty.

This would be nice but to be honest, my main reservation was exporting Snap to the window object. I guess I should have taken more time to read the code on the provider.

[jtrussell]

Cool. Well I can definitely get behind at least trying to anticipate the module loaders that Snap.js allows for, so if you wanted to put together a PR I'd be up for it.

[richardvanbergen]

I actually started writing tests but the more I think about it. The more I think that loading Snap implicitly is not the right solution at all.

Obviously I'm not advocating breaking backwards compatibility by disabling loading from the window object but I think it would be a better idea to require the developer to be explicit. Maybe just throw a more descriptive error if SnapConstructorProvider is not able to load Snap from window.Snap.

Let me know what you think.

[jtrussell]

I'd lean against throwing an error if window.Snap is undefined. But it might make sense to throw (or at least $log.debug an error) if constructor is still undefined when the provider's $get method is called. That would at least be after the time to supply your own Snap object has passed. I think that's what you were suggesting?

[richardvanbergen]

Makes sense. Just a little bit of feedback as it did waste almost an hour for me so it makes a nice first impression if the user doesn't have to go though the source to find out what went wrong. :)

[jtrussell]

Cool - adding some new tests and while I'm at it I'll have it look or Snap on $window instead of window so you could also add it in a run if that's more your style.

Something along these lines:

angular.module('snap')
.provider('SnapConstructor', function() {
  'use strict';
  var constructor;

  this.use = function(MySnap) {
    constructor = MySnap;
  };

  this.$get = ['$window', function($window) {
    var S = constructor || $window.Snap;
    if(angular.isUndefined(S)) {
      throw new Error('Snap constructor is not defined. Make sure ' +
          'window.Snap is defined or supply your own with ' +
          'SnapConstructorProvider.use(MySnap).');
    }
    return S;
  }];
});

[richardvanbergen]

Perfect. Thanks for the fix, I was going to attempt it myself but you're too fast for me. You also seem to know the Angular API better. :+1: