diff --git a/lib/github-package.js b/lib/github-package.js index 5f727b7e8c..5b87b11331 100644 --- a/lib/github-package.js +++ b/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); diff --git a/lib/models/repository-states/cloning.js b/lib/models/repository-states/cloning.js index 8afe1c081a..368af07f45 100644 --- a/lib/models/repository-states/cloning.js +++ b/lib/models/repository-states/cloning.js @@ -13,7 +13,7 @@ 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'); } @@ -21,6 +21,10 @@ export default class Cloning extends State { showGitTabLoading() { return true; } + + directClone(remoteUrl, options) { + return this.git().clone(remoteUrl, options); + } } State.register(Cloning); diff --git a/lib/models/repository-states/initializing.js b/lib/models/repository-states/initializing.js index 385203d6ca..e96af6a2da 100644 --- a/lib/models/repository-states/initializing.js +++ b/lib/models/repository-states/initializing.js @@ -5,7 +5,7 @@ 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'); } @@ -13,6 +13,10 @@ export default class Initializing extends State { showGitTabLoading() { return true; } + + directInit(workdir) { + return this.git().init(workdir); + } } State.register(Initializing); diff --git a/lib/models/repository-states/loading.js b/lib/models/repository-states/loading.js index a70420f557..00fe19d758 100644 --- a/lib/models/repository-states/loading.js +++ b/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); diff --git a/lib/models/repository-states/state.js b/lib/models/repository-states/state.js index 95a8dd743c..9fb78f5b36 100644 --- a/lib/models/repository-states/state.js +++ b/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 {}; diff --git a/lib/models/repository.js b/lib/models/repository.js index e3f8d340a7..347d9af56e 100644 --- a/lib/models/repository.js +++ b/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(); } diff --git a/lib/views/dock-item.js b/lib/views/dock-item.js index d6ac2a1c25..960720a74c 100644 --- a/lib/views/dock-item.js +++ b/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) { diff --git a/test/github-package.test.js b/test/github-package.test.js index 254aef9ff1..b8ccd66db2 100644 --- a/test/github-package.test.js +++ b/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() {