From f800f87edc57b3a6383e585c3ffa626aac6bc1be Mon Sep 17 00:00:00 2001
From: David Calhoun <dpcalhoun@gmail.com>
Date: Sat, 30 Sep 2017 20:58:59 -0500
Subject: [PATCH] Unsubscribe from config changes on dispose

---
 index.js                          |  7 ++++---
 modules/__tests__/dispose.test.js | 14 +++++++++++++-
 modules/app.js                    |  3 +--
 modules/dispose.js                |  5 ++++-
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/index.js b/index.js
index 95b15e3..38a3725 100644
--- a/index.js
+++ b/index.js
@@ -2,12 +2,13 @@ const dispose = require('./modules/dispose')
 const { generateActivateCallback, onApp } = require('./modules/app')
 const { generateBlurCallback, hideWindows, showWindows } = require('./modules/windows')
 
-let handleActivate, handleBlur
+let cfgUnsubscribe, handleActivate, handleBlur
 
 exports.onApp = app => {
   handleBlur = generateBlurCallback(hideWindows)(app)
   handleActivate = generateActivateCallback(showWindows)(app)
 
-  onApp(app, handleActivate, handleBlur)
+  cfgUnsubscribe = onApp(app, handleActivate, handleBlur)
 }
-exports.onUnload = app => dispose(app, handleActivate, handleBlur)
+
+exports.onUnload = app => dispose(app, { cfgUnsubscribe, handleActivate, handleBlur })
diff --git a/modules/__tests__/dispose.test.js b/modules/__tests__/dispose.test.js
index d717144..f272a74 100644
--- a/modules/__tests__/dispose.test.js
+++ b/modules/__tests__/dispose.test.js
@@ -9,10 +9,18 @@ jest.mock('../windows')
 const app = generateApp()
 const handleActivateMock = jest.fn()
 const handleBlurMock = jest.fn()
+const cfgUnsubscribeMock = jest.fn()
 
 describe('dispose', () => {
   beforeEach(() => {
-    dispose(app, handleActivateMock, handleBlurMock)
+    dispose(
+      app,
+      {
+        cfgUnsubscribe: cfgUnsubscribeMock,
+        handleActivate: handleActivateMock,
+        handleBlur: handleBlurMock
+      }
+    )
   })
 
   it('shows all windows', () => {
@@ -23,6 +31,10 @@ describe('dispose', () => {
     expect(app.dock.show).toHaveBeenCalledTimes(1)
   })
 
+  it('unsubscribes from config changes', () => {
+    expect(cfgUnsubscribeMock).toHaveBeenCalledTimes(1)
+  })
+
   xit('unregisters the shortcut', () => {
     // expect(unregisterShortcut).toHaveBeenCalledTimes(1)
   })
diff --git a/modules/app.js b/modules/app.js
index e7b0862..e72ceff 100644
--- a/modules/app.js
+++ b/modules/app.js
@@ -29,8 +29,7 @@ const generateActivateCallback = callback => app => event => callback(app)
 const onApp = (app, handleActivate, handleBlur) => {
   app.on('activate', handleActivate)
   applyConfig(app, handleBlur)
-  // TODO: Is there a way to unsubscribe within dispose?
-  app.config.subscribe(() => applyConfig(app, handleBlur))
+  return app.config.subscribe(() => applyConfig(app, handleBlur))
 }
 
 module.exports = {
diff --git a/modules/dispose.js b/modules/dispose.js
index 82f72ab..ed455a4 100644
--- a/modules/dispose.js
+++ b/modules/dispose.js
@@ -1,11 +1,14 @@
 // const { unregisterShortcut } = require('hyperterm-register-shortcut')
 const { showWindows } = require('./windows')
 
-module.exports = (app, handleActivate, handleBlur) => {
+module.exports = (app, callbacks) => {
+  const { handleActivate, handleBlur, cfgUnsubscribe } = callbacks
+
   showWindows(app)
   app.dock.show()
   // TODO: Un-register shortcut when supported
   // unregisterShortcut()
+  cfgUnsubscribe()
   app.removeListener('activate', handleActivate)
   app.removeListener('browser-window-blur', handleBlur)
 }