Always call listenBefore hooks on browser navigation

Fixes remix-run#71
stevoland · Oct 7, 2015 · 39e080a · 39e080a
1 parent eb05768
commit 39e080a
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 13 deletions.
diff --git a/modules/__tests__/BrowserHistory-test.js b/modules/__tests__/BrowserHistory-test.js
@@ -5,6 +5,7 @@ import describeInitialLocation from './describeInitialLocation'
 import describeTransitions from './describeTransitions'
 import describePushState from './describePushState'
 import describeReplaceState from './describeReplaceState'
+import describePopState from './describePopState'
 import describeHashSupport from './describeHashSupport'
 import describeBasename from './describeBasename'
 import describeQueries from './describeQueries'
@@ -20,6 +21,7 @@ describe('browser history', function () {
     describeTransitions(createBrowserHistory)
     describePushState(createBrowserHistory)
     describeReplaceState(createBrowserHistory)
+    describePopState(createBrowserHistory)
     describeHashSupport(createBrowserHistory)
     describeBasename(createBrowserHistory)
     describeQueries(createBrowserHistory)
@@ -30,6 +32,7 @@ describe('browser history', function () {
       describeTransitions(createBrowserHistory)
       describePushState(createBrowserHistory)
       describeReplaceState(createBrowserHistory)
+      describePopState(createBrowserHistory)
       describeHashSupport(createBrowserHistory)
       describeBasename(createBrowserHistory)
       describeQueries(createBrowserHistory)

diff --git a/modules/__tests__/HashHistory-test.js b/modules/__tests__/HashHistory-test.js
@@ -6,6 +6,7 @@ import describeInitialLocation from './describeInitialLocation'
 import describeTransitions from './describeTransitions'
 import describePushState from './describePushState'
 import describeReplaceState from './describeReplaceState'
+import describePopState from './describePopState'
 import describeQueryKey from './describeQueryKey'
 import describeBasename from './describeBasename'
 import describeQueries from './describeQueries'
@@ -21,6 +22,7 @@ describe('hash history', function () {
   describeTransitions(createHashHistory)
   describePushState(createHashHistory)
   describeReplaceState(createHashHistory)
+  describePopState(createHashHistory)
   describeBasename(createHashHistory)
   describeQueries(createHashHistory)
 

diff --git a/modules/__tests__/describePopState.js b/modules/__tests__/describePopState.js
@@ -0,0 +1,50 @@
+/*eslint-env mocha */
+
+function describePopState(createHistory) {
+  describe('when a listenBefore hook is added', function () {
+    let history, unlisten
+
+    beforeEach(function () {
+      history = createHistory()
+      history.pushState(null, '/home')
+    })
+
+    afterEach(function () {
+      if (unlisten)
+        unlisten()
+    })
+
+    it('is called when browser navigation is used', function (done) {
+      unlisten = history.listenBefore(function () {
+        done()
+      })
+
+      window.history.back()
+    })
+  })
+
+  describe('when a deprecated transition hook is added', function () {
+    let history, listener
+
+    beforeEach(function () {
+      history = createHistory()
+      history.pushState(null, '/home')
+    })
+
+    afterEach(function () {
+      history.unregisterTransitionHook(listener)
+    })
+
+    it('is called when browser navigation is used', function (done) {
+      listener = function () {
+        done()
+      }
+
+      history.registerTransitionHook(listener)
+
+      window.history.back()
+    })
+  })
+}
+
+export default describePopState
diff --git a/modules/__tests__/describeTransitions.js b/modules/__tests__/describeTransitions.js
@@ -6,12 +6,15 @@ import execSteps from './execSteps'
 
 function describeTransitions(createHistory) {
   describe('a synchronous transition hook', function () {
-    let history, unlisten
+    let history, unlisten, unlistenBefore
     beforeEach(function () {
       history = createHistory()
     })
 
     afterEach(function () {
+      if (unlistenBefore)
+        unlistenBefore()
+
       if (unlisten)
         unlisten()
     })
@@ -27,7 +30,7 @@ function describeTransitions(createHistory) {
       ]
 
       let nextLocation
-      history.listenBefore(function (location) {
+      unlistenBefore = history.listenBefore(function (location) {
         nextLocation = location
       })
 
@@ -36,12 +39,15 @@ function describeTransitions(createHistory) {
   })
 
   describe('an asynchronous transition hook', function () {
-    let history, unlisten
+    let history, unlisten, unlistenBefore
     beforeEach(function () {
       history = createHistory()
     })
 
     afterEach(function () {
+      if (unlistenBefore)
+        unlistenBefore()
+
       if (unlisten)
         unlisten()
     })
@@ -57,7 +63,7 @@ function describeTransitions(createHistory) {
       ]
 
       let nextLocation
-      history.listenBefore(function (location, callback) {
+      unlistenBefore = history.listenBefore(function (location, callback) {
         nextLocation = location
         setTimeout(callback)
       })
@@ -67,7 +73,7 @@ function describeTransitions(createHistory) {
   })
 
   describe('when the user confirms a transition', function () {
-    let confirmationMessage, location, history, unlisten
+    let confirmationMessage, location, history, unlisten, unlistenBefore
     beforeEach(function () {
       location = null
       confirmationMessage = 'Are you sure?'
@@ -79,7 +85,7 @@ function describeTransitions(createHistory) {
         }
       })
 
-      history.listenBefore(function () {
+      unlistenBefore = history.listenBefore(function () {
         return confirmationMessage
       })
 
@@ -89,6 +95,9 @@ function describeTransitions(createHistory) {
     })
 
     afterEach(function () {
+      if (unlistenBefore)
+        unlistenBefore()
+
       if (unlisten)
         unlisten()
     })
@@ -108,7 +117,7 @@ function describeTransitions(createHistory) {
   })
 
   describe('when the user cancels a transition', function () {
-    let confirmationMessage, location, history, unlisten
+    let confirmationMessage, location, history, unlisten, unlistenBefore
     beforeEach(function () {
       location = null
       confirmationMessage = 'Are you sure?'
@@ -120,7 +129,7 @@ function describeTransitions(createHistory) {
         }
       })
 
-      history.listenBefore(function () {
+      unlistenBefore = history.listenBefore(function () {
         return confirmationMessage
       })
 
@@ -130,6 +139,9 @@ function describeTransitions(createHistory) {
     })
 
     afterEach(function () {
+      if (unlistenBefore)
+        unlistenBefore()
+
       if (unlisten)
         unlisten()
     })
@@ -142,13 +154,13 @@ function describeTransitions(createHistory) {
   })
 
   describe('when the transition hook cancels a transition', function () {
-    let location, history, unlisten
+    let location, history, unlisten, unlistenBefore
     beforeEach(function () {
       location = null
 
       history = createHistory()
 
-      history.listenBefore(function () {
+      unlistenBefore = history.listenBefore(function () {
         return false
       })
 
@@ -158,6 +170,9 @@ function describeTransitions(createHistory) {
     })
 
     afterEach(function () {
+      if (unlistenBefore)
+        unlistenBefore()
+
       if (unlisten)
         unlisten()
     })

diff --git a/modules/createBrowserHistory.js b/modules/createBrowserHistory.js
@@ -98,6 +98,20 @@ function createBrowserHistory(options) {
 
   let listenerCount = 0, stopPopStateListener
 
+  function listenBefore(listener) {
+    if (++listenerCount === 1)
+      stopPopStateListener = startPopStateListener(history)
+
+    let unlisten = history.listenBefore(listener)
+
+    return function () {
+      unlisten()
+
+      if (--listenerCount === 0)
+        stopPopStateListener()
+    }
+  }
+
   function listen(listener) {
     if (++listenerCount === 1)
       stopPopStateListener = startPopStateListener(history)
@@ -112,9 +126,28 @@ function createBrowserHistory(options) {
     }
   }
 
+  // deprecated
+  function registerTransitionHook(hook) {
+    if (++listenerCount === 1)
+      stopPopStateListener = startPopStateListener(history)
+
+    history.registerTransitionHook(hook)
+  }
+
+  // deprecated
+  function unregisterTransitionHook(hook) {
+    history.unregisterTransitionHook(hook)
+
+    if (--listenerCount === 0)
+      stopPopStateListener()
+  }
+
   return {
     ...history,
-    listen
+    listenBefore,
+    listen,
+    registerTransitionHook,
+    unregisterTransitionHook
   }
 }
 

diff --git a/modules/createHashHistory.js b/modules/createHashHistory.js
@@ -49,7 +49,7 @@ function createHashHistory(options={}) {
 
   function getCurrentLocation() {
     let path = getHashPath()
-    
+
     let key, state
     if (queryKey) {
       key = getQueryStringValueFromPath(path, queryKey)
@@ -129,6 +129,20 @@ function createHashHistory(options={}) {
 
   let listenerCount = 0, stopHashChangeListener
 
+  function listenBefore(listener) {
+    if (++listenerCount === 1)
+      stopHashChangeListener = startHashChangeListener(history)
+
+    let unlisten = history.listenBefore(listener)
+
+    return function () {
+      unlisten()
+
+      if (--listenerCount === 0)
+        stopHashChangeListener()
+    }
+  }
+
   function listen(listener) {
     if (++listenerCount === 1)
       stopHashChangeListener = startHashChangeListener(history)
@@ -176,13 +190,32 @@ function createHashHistory(options={}) {
     return '#' + history.createHref(path)
   }
 
+  // deprecated
+  function registerTransitionHook(hook) {
+    if (++listenerCount === 1)
+      stopHashChangeListener = startHashChangeListener(history)
+
+    history.registerTransitionHook(hook)
+  }
+
+  // deprecated
+  function unregisterTransitionHook(hook) {
+    history.unregisterTransitionHook(hook)
+
+    if (--listenerCount === 0)
+      stopHashChangeListener()
+  }
+
   return {
     ...history,
+    listenBefore,
     listen,
     pushState,
     replaceState,
     go,
-    createHref
+    createHref,
+    registerTransitionHook,
+    unregisterTransitionHook
   }
 }