cleaning up long running functions

src/js/App.jsx

@@ -11,6 +11,8 @@ import NotFoundPage from './pages/NotFoundPage.jsx';
 
 import actions from './actions/appActions.js';
 
+import appStore from './stores/appStore.js';
+
 // Will fire even if event is prevented from propagating.
 delete RouterMixin.handleClick;
 
@@ -117,7 +119,6 @@ export default React.createClass({
       return <div />;
     } else {
       blank = true;
-      actions.emit('system.loading', false);
       return this.renderCurrentRoute();
     }
   }

src/js/core/Store.js

@@ -13,6 +13,28 @@ export default class Store extends EventEmitter {
     super();
     // Initial payload.
     this[DATA] = data || {};
+    // Callbacks to cleanup.
+    this._cbs = {};
+  }
+
+  // Register an async function callback.
+  // TODO: unit-test.
+  cb(fn) {
+    let id = _.uniqueId();
+    return this._cbs[id] = (...args) => {
+      // Still running?
+      if (!(id in this._cbs)) return console.log(`stop ${id}`);
+      fn.apply(this, args);
+      delete this._cbs[id];
+    };
+  };
+
+  // Cleanup callbacks because a View has changed thus long-running
+  //  functions need to end. Unreference any onChange events too.
+  clean(onChange) {
+    console.log('cleaning up');
+    for (let id in this._cbs) delete this._cbs[id];
+    if (_.isFunction(onChange)) this.offAny(onChange);
   }
 
   // Set a value on a key. Pass falsy value as 3rd param to not emit changes.

src/js/mixins/Page.js

@@ -28,10 +28,7 @@ export default {
   },
 
   _onChange(store, val, key) {
-    // TODO: this is not the right approach!
-    if (this.isMounted()) {
-      this.setState(this._getData(store));
-    }
+    this.setState(this._getData(store));
   },
 
   getInitialState() {
@@ -49,7 +46,8 @@ export default {
   componentWillUnmount() {
     let key;
     for (key in stores) {
-      stores[key].offAny(this._onChange); 
+      console.log('cleanup');
+      stores[key].clean(this._onChange); 
     }
   }
 

src/js/stores/appStore.js

@@ -16,7 +16,7 @@ class AppStore extends Store {
   constructor() {
     super({
       system: {
-        loading: true
+        loading: false
       },
       user: {}
     });

src/js/stores/projectsStore.js

@@ -65,23 +65,24 @@ class ProjectsStore extends Store {
     let list = this.get('list');
 
     let done = (err) => {
-      actions.emit('system.loading', false);
+      // actions.emit('system.loading', false);
     };
 
     // Quit if we have no projects.
     if (!list.length) return done();
 
-    actions.emit('system.loading', true);
+    // actions.emit('system.loading', true);
 
     // Wait for the user to get resolved.
-    this.get('user', (user) => {
+    this.get('user', this.cb((user) => { // async
       // For all projects.
+      // TODO: this is generally bad since we are updating the store
+      //  piece by piece but can't recover if we end half-way through,
+      //  a better way would be to transact and save all the data as
+      //  they arrive completely, checking if we are hanging or not.
       async.map(list, (project, cb) => {
-        // Skip any projects that already have milestones.
-        if ('milestones' in project) return cb();
-
         // Fetch their milestones.
-        milestones.fetchAll(user, project, (err, list) => {
+        milestones.fetchAll(user, project, this.cb((err, list) => { // async
           // Save the error if project does not exist.
           if (err) {
             this.saveError(project, err);
@@ -90,10 +91,8 @@ class ProjectsStore extends Store {
 
           // Now add in the issues.
           async.each(list, (milestone, cb) => {
-            // Do we have this milestone already?
-            if (_.find(project.milestones, (arg) => {
-              var number;
-              number = arg.number;
+            // Do we have this milestone already? Skip fetching issues then.
+            if (_.find(project.milestones, ({ number }) => {
               return milestone.number === number;
             })) {
               return cb(null);
@@ -104,7 +103,7 @@ class ProjectsStore extends Store {
               'owner': project.owner,
               'name': project.name,
               'milestone': milestone.number
-            }, (err, obj) => {
+            }, this.cb((err, obj) => { // async
               // Save any errors on the project.
               if (err) {
                 this.saveError(project, err);
@@ -117,11 +116,11 @@ class ProjectsStore extends Store {
               this.addMilestone(project, milestone);
               // Done.
               cb();
-            });
+            }));
           }, cb);
-        });
+        }));
       }, done);
-    });
+    }));
   }
 
   // Push to the stack unless it exists already.
@@ -229,6 +228,7 @@ class ProjectsStore extends Store {
   }
 
   // Add a milestone for a project.
+  // TODO: check if this milestone exists already.
   addMilestone(project, milestone) {
     // Add in the stats.
     let i, j;