Merge pull request #15627 from atom/mb-async-will-destroy-pane-item-hook

Wait for async onWillDestroyPaneItem handlers before destroying items

package.json

@@ -29,7 +29,7 @@
     "dedent": "^0.6.0",
     "devtron": "1.3.0",
     "etch": "^0.12.6",
-    "event-kit": "^2.3.0",
+    "event-kit": "^2.4.0",
     "find-parent-dir": "^0.3.0",
     "first-mate": "7.0.7",
     "focus-trap": "^2.3.0",
@@ -70,7 +70,7 @@
     "service-hub": "^0.7.4",
     "sinon": "1.17.4",
     "temp": "^0.8.3",
-    "text-buffer": "13.1.15",
+    "text-buffer": "13.2.1",
     "typescript-simple": "1.0.0",
     "underscore-plus": "^1.6.6",
     "winreg": "^1.2.1",
@@ -134,7 +134,7 @@
     "tree-view": "0.217.9",
     "update-package-dependencies": "0.12.0",
     "welcome": "0.36.5",
-    "whitespace": "0.37.2",
+    "whitespace": "0.37.3",
     "wrap-guide": "0.40.2",
     "language-c": "0.58.1",
     "language-clojure": "0.22.4",

spec/pane-spec.js

@@ -3,7 +3,7 @@ const {Emitter} = require('event-kit')
 const Grim = require('grim')
 const Pane = require('../src/pane')
 const PaneContainer = require('../src/pane-container')
-const {it, fit, ffit, fffit, beforeEach} = require('./async-spec-helpers')
+const {it, fit, ffit, fffit, beforeEach, timeoutPromise} = require('./async-spec-helpers')
 
 describe('Pane', () => {
   let confirm, showSaveDialog, deserializerDisposable
@@ -491,14 +491,21 @@ describe('Pane', () => {
       expect(pane.getActiveItem()).toBeUndefined()
     })
 
-    it('invokes ::onWillDestroyItem() observers before destroying the item', () => {
+    it('invokes ::onWillDestroyItem() observers before destroying the item', async () => {
+      jasmine.useRealClock()
+
+      let handlerDidFinish = false
       const events = []
-      pane.onWillDestroyItem(function (event) {
+      pane.onWillDestroyItem(async (event) => {
         expect(item2.isDestroyed()).toBe(false)
         events.push(event)
+        await timeoutPromise(50)
+        expect(item2.isDestroyed()).toBe(false)
+        handlerDidFinish = true
       })
 
-      pane.destroyItem(item2)
+      await pane.destroyItem(item2)
+      expect(handlerDidFinish).toBe(true)
       expect(item2.isDestroyed()).toBe(true)
       expect(events).toEqual([{item: item2, index: 1}])
     })

src/pane-container.js

@@ -267,7 +267,7 @@ class PaneContainer {
   }
 
   willDestroyPaneItem (event) {
-    this.emitter.emit('will-destroy-pane-item', event)
+    return this.emitter.emitAsync('will-destroy-pane-item', event)
   }
 
   didDestroyPaneItem (event) {

src/pane.coffee

@@ -624,18 +624,25 @@ class Pane
       if not force and @getContainer()?.getLocation() isnt 'center' and item.isPermanentDockItem?()
         return Promise.resolve(false)
 
-      @emitter.emit 'will-destroy-item', {item, index}
-      @container?.willDestroyPaneItem({item, index, pane: this})
-      if force or not item?.shouldPromptToSave?()
-        @removeItem(item, false)
-        item.destroy?()
-        Promise.resolve(true)
+      callback = =>
+        @container?.willDestroyPaneItem({item, index, pane: this})
+        if force or not item?.shouldPromptToSave?()
+          @removeItem(item, false)
+          item.destroy?()
+          true
+        else
+          @promptToSaveItem(item).then (result) =>
+            if result
+              @removeItem(item, false)
+              item.destroy?()
+            result
+
+      # In the case where there are no `onWillDestroyPaneItem` listeners, preserve the old behavior
+      # where `Pane.destroyItem` and callers such as `Pane.close` take effect synchronously.
+      if @emitter.listenerCountForEventName('will-destroy-item') is 0
+        return Promise.resolve(callback())
       else
-        @promptToSaveItem(item).then (result) =>
-          if result
-            @removeItem(item, false)
-            item.destroy?()
-          result
+        @emitter.emitAsync('will-destroy-item', {item, index}).then(callback)
 
   # Public: Destroy all items.
   destroyItems: ->

src/workspace.js

@@ -820,7 +820,8 @@ module.exports = class Workspace extends Model {
   // Extended: Invoke the given callback when a pane item is about to be
   // destroyed, before the user is prompted to save it.
   //
-  // * `callback` {Function} to be called before pane items are destroyed.
+  // * `callback` {Function} to be called before pane items are destroyed. If this function returns
+  //   a {Promise}, then the item will not be destroyed until the promise resolves.
   //   * `event` {Object} with the following keys:
   //     * `item` The item to be destroyed.
   //     * `pane` {Pane} containing the item to be destroyed.