4
\$\begingroup\$

I created a little wrapper for one of my JavaScript libraries to enable Angular functionality. Are there any pitfalls I should be wary of with my code?

angular.module("signalR.eventAggregator", [])
    .run([
        "$rootScope", function($rootScope) {
        function createScope(scope) {
            scope.$on('$destroy', function() {
                signalR.eventAggregator.unsubscribe(scope);
            });

            return {
                subscribe: function(type, handler, constraint) {
                    signalR.eventAggregator.subscribe(type, function(e) {
                        handler(e);
                        if (scope.$$phase == null) {
                            scope.$digest();
                        }
                    }, scope, constraint);
                },
                publish: function(event) {
                    signalR.eventAggregator.publish(event);
                }
            }
        }

        $rootScope.eventAggregator = function() {
            return this.__eventAggregator = this.__eventAggregator || createScope(this);
        };
    }
]);

More info on the code itself here.

It's used like this:

$scope.eventAggregator().subscribe(MyApp.MyEvent, onEvent);

Other scenarios it supports are

Generic events, (all events are server events proxied to javascript through a dynamic Owin javascript)

$scope.eventAggregator().subscribe(MyApp.MyGenericEvent.of("System.String"), onEvent);

Constraints, in this case listen to all events with id 5

$scope.eventAggregator().subscribe(MyApp.MyEvent, onEvent, { id: 5 });

It wraps an event-driven library, and when an event is triggered, I use digest to update the view, and it's this part of the code that I wonder about.

handler(e); could return a promise that I currently ignore. I tested the ng-click directive and it also ignores any promise returned from the click handler. So at least it's consistent with the existing event handlers in Angular.

\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

Hmm... interesting use of run + $rootScope, but I would avoid it primarily because of $rootScope. It's essentially the "global space" of an Angular app. The only legit use I see for it is attaching event listeners ($rootScope.$on), and for broadcasting events ($rootScope.$emit).

As an alternative, you could wrap your event relay in a factory.

  • As a factory, the dependency is explicit. The dependent knows it's there and Angular will throw if the dependency is missing on initialization, unlike $rootScope where it only throws when you use it.

  • Factories are singletons. You don't have to worry about checking and using the existing or create a new one.

  • You're not polluting the $rootScope nor depending on something that might not be there or have been overridden by something in a lower, enclosing scope. This is a headache to debug, especially without tools like Batarang or ngInspector.

Additionally, you might also want to put your event names in a constant. That way, you have a global lookup of event names, and you won't be hardcoding string literals all over the app.

Here's an example of how it would look like as a factory (using implicit dependency injection syntax for brevity).

angular.module('SignalrModule', []);
  .constant('SIGNALR_EVENTS', {
    FOO_EVENT: 'fooevent',
    BAR_EVENT: 'barevent',
  })
  .factory('SignalrFactory', function(){
    return {
      publish: function(){...},
      subscribe: function(){...},
      unsubscribe: function(){...},
    };
  });

angular.module('app', ['SignalrModule'])
  .controller('MyController', function(SignalrFactory, SIGNALR_EVENTS){
    SignalrFactory.subscribe(SIGNALR_EVENTS.FOO_EVENT, function(){
      // on foo event
    });
  });

Now I did mention that angular has a built-in pub-sub system. We can just hook on to it so your controllers will simply use regular events over $rootScope (we're not adding stuff to $rootScope, just making it a relay for events). Not sure of the following works, but the concept is there.

angular.module('SignalrModule', []);
  .constant('SIGNALR_EVENTS', {
    FOO_EVENT: 'fooevent',
    BAR_EVENT: 'barevent',
  })
  .run(function($rootScope, SIGNALR_EVENTS){

    // Iterate through our registry of events
    Object.keys(SIGNALR_EVENTS).forEach(function(eventName){

      // Relay angular events to signalr over $rootScope
      $rootScope.$on(SIGNALR_EVENTS[eventName], function(){
        signalR.eventAggregator.publish(SIGNALR_EVENTS[eventName]);
      });

      // Relay signalr events to angular over $rootScope
      signalR.eventAggregator.subscribe(SIGNALR_EVENTS[eventName], function(e) {
        $rootScope.$emit(SIGNALR_EVENTS[eventName], e);
      });
    });

  });

angular.module('app', ['SignalrModule'])
  .controller('MyController', function($rootScope, SIGNALR_EVENTS){

    // Using regular angular-ish emit and on

    $rootScope.$emit(SIGNALR_EVENTS.FOO_EVENT, { foo: 'data' });

    $rootScope.$on(SIGNALR_EVENTS.BAR_EVENT, function(){
      // bar event emitted somewhere
    });
  });
\$\endgroup\$
7
  • \$\begingroup\$ This wont work because my event system is much more advanced than angulars, for example you can do $scope.eventAggregator().subscribe(MyApp.MyGenericEvent.of("System.String"), onEvent); etc. But interesting ideas, I will see what I can use. I hav already thought about creating a factory for the signalR.eventAggregator global. That way I can use it for stuff that does not have a scope. Good points, thanks \$\endgroup\$
    – Anders
    Commented Oct 14, 2015 at 6:46
  • \$\begingroup\$ Updated question with all valid scenarios \$\endgroup\$
    – Anders
    Commented Oct 14, 2015 at 6:55
  • \$\begingroup\$ hmm, also with your version there is not natural place to call unsubscribe. Without unsubscribe the server will keep sending events even if no one is listening client side which is a waste of resources \$\endgroup\$
    – Anders
    Commented Oct 14, 2015 at 7:00
  • \$\begingroup\$ @Anders Ahh yeah, wasn't really acquainted with SignalR really, other than that it was some for of event system. Also, usually one doesn't update a question when it has answers, otherwise the answers may become irrelevant. Usually you post a separate question as followup. \$\endgroup\$
    – Joseph
    Commented Oct 14, 2015 at 12:01
  • 1
    \$\begingroup\$ I think its good to keep it as is, it can help others even if it does not help me 100%. Btw, this is not actually SignalR related. My library is an abstraction ontop of SignalR \$\endgroup\$
    – Anders
    Commented Oct 14, 2015 at 12:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.