Clean up my own messes

lib/github-package.js

@@ -188,6 +188,10 @@ export default class GithubPackage {
 
   @autobind
   rerender(callback) {
+    if (this.workspace.isDestroyed()) {
+      return;
+    }
+
     if (!this.element) {
       this.element = document.createElement('div');
       this.subscriptions.add(new Disposable(() => {
@@ -391,6 +395,10 @@ export default class GithubPackage {
   }
 
   async updateActiveContext(savedState = {}) {
+    if (this.workspace.isDestroyed()) {
+      return;
+    }
+
     this.switchboard.didBeginActiveContextUpdate();
 
     const nextActiveContext = await this.getNextContext(savedState);

lib/models/repository-states/cloning.js

@@ -13,14 +13,18 @@ export default class Cloning extends State {
 
   async start() {
     await mkdirs(this.workdir());
-    await this.git().clone(this.remoteUrl, {recursive: true});
+    await this.doClone(this.remoteUrl, {recursive: true});
 
     await this.transitionTo('Loading');
   }
 
   showGitTabLoading() {
     return true;
   }
+
+  directClone(remoteUrl, options) {
+    return this.git().clone(remoteUrl, options);
+  }
 }
 
 State.register(Cloning);

lib/models/repository-states/initializing.js

@@ -5,14 +5,18 @@ import State from './state';
  */
 export default class Initializing extends State {
   async start() {
-    await this.git().init(this.workdir());
+    await this.doInit(this.workdir());
 
     await this.transitionTo('Loading');
   }
 
   showGitTabLoading() {
     return true;
   }
+
+  directInit(workdir) {
+    return this.git().init(workdir);
+  }
 }
 
 State.register(Initializing);

lib/models/repository-states/loading.js

@@ -6,7 +6,7 @@ import State from './state';
  */
 export default class Loading extends State {
   async start() {
-    if (await this.git().isGitRepository()) {
+    if (await this.isGitRepository()) {
       const history = await this.loadHistoryPayload();
       return this.transitionTo('Present', history);
     } else {
@@ -31,6 +31,18 @@ export default class Loading extends State {
   showGitTabLoading() {
     return true;
   }
+
+  directIsGitRepository() {
+    return this.git().isGitRepository();
+  }
+
+  directGetConfig(key, options) {
+    return this.git().getConfig(key, options);
+  }
+
+  directGetBlobContents(sha) {
+    return this.git().getBlobContents(sha);
+  }
 }
 
 State.register(Loading);

lib/models/repository-states/state.js

@@ -407,6 +407,12 @@ export default class State {
     return this.repository.getWorkingDirectoryPath();
   }
 
+  // Call methods on the active Repository state, even if the state has transitioned beneath you.
+  // Use this to perform operations within `start()` methods to guard against interrupted state transitions.
+  current() {
+    return this.repository.state;
+  }
+
   // Return a Promise that will resolve once the state transitions from Loading.
   getLoadPromise() {
     return this.repository.getLoadPromise();
@@ -439,16 +445,55 @@ export default class State {
     return this.repository.emitter.emit('did-update');
   }
 
+  // Direct git access
+  // Non-delegated git operations for internal use within states.
+
+  directIsGitRepository() {
+    return Promise.resolve(false);
+  }
+
+  directGetConfig(key, options = {}) {
+    return Promise.resolve(null);
+  }
+
+  directGetBlobContents() {
+    return Promise.reject(new Error('Not a valid object name'));
+  }
+
+  directInit() {
+    return Promise.resolve();
+  }
+
+  directClone(remoteUrl, options) {
+    return Promise.resolve();
+  }
+
+  // Deferred operations
+  // Direct raw git operations to the current state, even if the state has been changed. Use these methods within
+  // start() methods.
+
+  isGitRepository() {
+    return this.current().directIsGitRepository();
+  }
+
+  doInit(workdir) {
+    return this.current().directInit();
+  }
+
+  doClone(remoteUrl, options) {
+    return this.current().directClone(remoteUrl, options);
+  }
+
   // Parse a DiscardHistory payload from the SHA recorded in config.
   async loadHistoryPayload() {
-    const historySha = await this.git().getConfig('atomGithub.historySha');
+    const historySha = await this.current().directGetConfig('atomGithub.historySha');
     if (!historySha) {
       return {};
     }
 
     let blob;
     try {
-      blob = await this.git().getBlobContents(historySha);
+      blob = await this.current().directGetBlobContents(historySha);
     } catch (e) {
       if (/Not a valid object name/.test(e.stdErr)) {
         return {};

lib/models/repository.js

@@ -67,11 +67,18 @@ export default class Repository {
   // State management //////////////////////////////////////////////////////////////////////////////////////////////////
 
   transition(currentState, StateConstructor, ...payload) {
+    if (currentState !== this.state) {
+      // Attempted transition from a non-active state, most likely from an asynchronous start() method.
+      return Promise.resolve();
+    }
+
     const nextState = new StateConstructor(this, ...payload);
     this.state = nextState;
 
     this.emitter.emit('did-change-state', {from: currentState, to: this.state});
-    this.emitter.emit('did-update');
+    if (!this.isDestroyed()) {
+      this.emitter.emit('did-update');
+    }
 
     return this.state.start();
   }

lib/views/dock-item.js

@@ -70,9 +70,9 @@ export default class DockItem extends React.Component {
         this.activate();
       }
     } else {
-      Promise.resolve(this.props.workspace.open(itemToAdd))
-        .then(item => { this.dockItem = item; })
-        .then(() => {
+      Promise.resolve(this.props.workspace.open(itemToAdd, {activatePane: false}))
+        .then(item => {
+          this.dockItem = item;
           if (this.props.activate) { this.activate(); }
         });
     }
@@ -102,7 +102,9 @@ export default class DockItem extends React.Component {
 
   activate() {
     setTimeout(() => {
-      if (!this.dockItem) { return; }
+      if (!this.dockItem || this.didCloseItem || this.props.workspace.isDestroyed()) {
+        return;
+      }
 
       const pane = this.props.workspace.paneForItem(this.dockItem);
       if (pane) {

test/github-package.test.js

@@ -410,6 +410,7 @@ describe('GithubPackage', function() {
     beforeEach(function() {
       // Necessary since we skip activate()
       githubPackage.savedState = {};
+      githubPackage.useLegacyPanels = !workspace.getLeftDock;
     });
 
     it('prefers the context of the active pane item', async function() {