Merge pull request #1306 from DAB0mB/fix/double-stop

fix(autorun): destroy event listener once autorun has been stopped

Author: Urigo

src/modules/core.js

@@ -34,7 +34,8 @@ angular.module(name, [
       }
 
       const computation = Tracker.autorun(fn, options);
-      this.$$autoStop(computation);
+      // Reset to a function that will also stop the listener we just added
+      computation.stop = this.$$autoStop(computation);
       return computation;
     };
 
@@ -89,8 +90,20 @@ angular.module(name, [
       return Meteor.apply(...args, fn);
     };
 
+    // Stops a process once the scope has been destroyed
     $$Core.$$autoStop = function(stoppable) {
-      this.$on('$destroy', stoppable.stop.bind(stoppable));
+      let removeListener;
+      const baseStop = stoppable.stop.bind(stoppable);
+
+      // Once the process has been stopped the destroy event listener will be removed
+      // to avoid memory leaks and unexpected behaviours
+      const stop = (...args) => {
+        removeListener();
+        return baseStop(...args);
+      };
+
+      removeListener = this.$on('$destroy', stop);
+      return stop;
     };
 
     // Digests scope only if there is no phase at the moment

tests/integration/core.spec.js

@@ -26,31 +26,34 @@ describe('angular-meteor.core', function() {
       });
 
       it('should call Tracker.autorun()', function() {
-        var stoppable = { stop: jasmine.createSpy('stop') };
+        var stop = jasmine.createSpy('stop');
+        var stoppable = { stop: stop };
         spyOn(Tracker, 'autorun').and.returnValue(stoppable);
 
         scope.autorun(function() {});
         expect(Tracker.autorun).toHaveBeenCalled();
       });
 
       it('should autostop computation', function() {
-        var stoppable = { stop: jasmine.createSpy('stop') };
+        var stop = jasmine.createSpy('stop');
+        var stoppable = { stop: stop };
         spyOn(Tracker, 'autorun').and.returnValue(stoppable);
 
         scope.autorun(angular.noop);
         scope.$destroy();
 
-        expect(stoppable.stop).toHaveBeenCalled();
+        expect(stop).toHaveBeenCalled();
       });
 
       it('should stop computation manually', function() {
-        var stoppable = { stop: jasmine.createSpy('stop') };
+        var stop = jasmine.createSpy('stop');
+        var stoppable = { stop: stop };
         spyOn(Tracker, 'autorun').and.returnValue(stoppable);
 
         var computation = scope.autorun(angular.noop);
         computation.stop();
 
-        expect(stoppable.stop).toHaveBeenCalled();
+        expect(stop).toHaveBeenCalled();
       });
 
       it('should call autorun function using view model as context', function() {
@@ -66,6 +69,18 @@ describe('angular-meteor.core', function() {
         scope.autorun(angular.noop);
         expect(scope.$digest).toHaveBeenCalled();
       });
+
+      it('should remove the destroy event listener once the computation has been stopped', function() {
+        var stop = jasmine.createSpy('stop');
+        var stoppable = { stop: stop };
+        spyOn(Tracker, 'autorun').and.returnValue(stoppable);
+
+        var computation = scope.autorun(angular.noop);
+        computation.stop();
+        scope.$destroy();
+
+        expect(stop.calls.count()).toEqual(1);
+      });
     });
 
     describe('subscribe()', function() {
@@ -87,23 +102,25 @@ describe('angular-meteor.core', function() {
       });
 
       it('should autostop subscription', function() {
-        var stoppable = { stop: jasmine.createSpy('stop') };
+        var stop = jasmine.createSpy('stop');
+        var stoppable = { stop: stop };
         spyOn(Tracker, 'autorun').and.returnValue(stoppable);
 
         scope.subscribe('test');
         scope.$destroy();
 
-        expect(stoppable.stop).toHaveBeenCalled();
+        expect(stop).toHaveBeenCalled();
       });
 
       it('should stop subscription manually', function() {
-        var stoppable = { stop: jasmine.createSpy('stop') };
+        var stop = jasmine.createSpy('stop');
+        var stoppable = { stop: stop };
         spyOn(Tracker, 'autorun').and.returnValue(stoppable);
 
         var subscription = scope.subscribe('test');
         subscription.stop();
 
-        expect(stoppable.stop).toHaveBeenCalled();
+        expect(stop).toHaveBeenCalled();
       });
 
       it('should return subscription ready and subscriptionId properties', function(done) {