Merge pull request #3107 from taion/leave-hook-teardown

Don't tear down leave hooks when not leaving

Author: taion

modules/__tests__/transitionHooks-test.js

@@ -8,13 +8,15 @@ import Router from '../Router'
 
 describe('When a router enters a branch', function () {
   let
-    node, leaveHookSpy, removeLeaveHook,
-    DashboardRoute, NewsFeedRoute, InboxRoute, RedirectToInboxRoute, MessageRoute,
+    node,
+    newsLeaveHookSpy, removeNewsLeaveHook, userLeaveHookSpy,
+    DashboardRoute, NewsFeedRoute, InboxRoute, RedirectToInboxRoute, MessageRoute, UserRoute,
     routes
 
   beforeEach(function () {
     node = document.createElement('div')
-    leaveHookSpy = expect.createSpy()
+    newsLeaveHookSpy = expect.createSpy()
+    userLeaveHookSpy = expect.createSpy()
 
     class Dashboard extends Component {
       render() {
@@ -29,9 +31,9 @@ describe('When a router enters a branch', function () {
 
     class NewsFeed extends Component {
       componentWillMount() {
-        removeLeaveHook = this.context.router.setRouteLeaveHook(
+        removeNewsLeaveHook = this.context.router.setRouteLeaveHook(
           this.props.route,
-          () => leaveHookSpy() // Break reference equality.
+          () => newsLeaveHookSpy() // Break reference equality.
         )
       }
 
@@ -50,6 +52,23 @@ describe('When a router enters a branch', function () {
       }
     }
 
+    class User extends Component {
+      componentWillMount() {
+        this.context.router.setRouteLeaveHook(
+          this.props.route,
+          userLeaveHookSpy
+        )
+      }
+
+      render() {
+        return <div>User</div>
+      }
+    }
+
+    User.contextTypes = {
+      router: React.PropTypes.object.isRequired
+    }
+
     NewsFeedRoute = {
       path: 'news',
       component: NewsFeed,
@@ -102,6 +121,11 @@ describe('When a router enters a branch', function () {
       }
     }
 
+    UserRoute = {
+      path: 'users/:userId',
+      component: User
+    }
+
     DashboardRoute = {
       path: '/',
       component: Dashboard,
@@ -113,7 +137,7 @@ describe('When a router enters a branch', function () {
       onLeave() {
         expect(this).toBe(DashboardRoute)
       },
-      childRoutes: [ NewsFeedRoute, InboxRoute, RedirectToInboxRoute, MessageRoute ]
+      childRoutes: [ NewsFeedRoute, InboxRoute, RedirectToInboxRoute, MessageRoute, UserRoute ]
     }
 
     routes = [
@@ -139,17 +163,14 @@ describe('When a router enters a branch', function () {
   it('calls the route leave hooks when leaving the route', function (done) {
     const history = createHistory('/news')
 
-    // Stub this function to exercise the code path.
-    history.listenBeforeUnload = () => (() => {})
-
     render(<Router history={history} routes={routes}/>, node, function () {
-      expect(leaveHookSpy.calls.length).toEqual(0)
+      expect(newsLeaveHookSpy.calls.length).toEqual(0)
       history.push('/inbox')
-      expect(leaveHookSpy.calls.length).toEqual(1)
+      expect(newsLeaveHookSpy.calls.length).toEqual(1)
       history.push('/news')
-      expect(leaveHookSpy.calls.length).toEqual(1)
+      expect(newsLeaveHookSpy.calls.length).toEqual(1)
       history.push('/inbox')
-      expect(leaveHookSpy.calls.length).toEqual(2)
+      expect(newsLeaveHookSpy.calls.length).toEqual(2)
       done()
     })
   })
@@ -158,9 +179,25 @@ describe('When a router enters a branch', function () {
     const history = createHistory('/news')
 
     render(<Router history={history} routes={routes}/>, node, function () {
-      removeLeaveHook()
+      removeNewsLeaveHook()
       history.push('/inbox')
-      expect(leaveHookSpy).toNotHaveBeenCalled()
+      expect(newsLeaveHookSpy).toNotHaveBeenCalled()
+      done()
+    })
+  })
+
+  it('does not remove route leave hooks when changing params', function (done) {
+    const history = createHistory('/users/foo')
+
+    // Stub this function to exercise the code path.
+    history.listenBeforeUnload = () => (() => {})
+
+    render(<Router history={history} routes={routes}/>, node, function () {
+      expect(userLeaveHookSpy.calls.length).toEqual(0)
+      history.push('/users/bar')
+      expect(userLeaveHookSpy.calls.length).toEqual(1)
+      history.push('/users/baz')
+      expect(userLeaveHookSpy.calls.length).toEqual(2)
       done()
     })
   })

modules/createTransitionManager.js

@@ -72,7 +72,9 @@ export default function createTransitionManager(history, routes) {
     runLeaveHooks(leaveRoutes)
 
     // Tear down confirmation hooks for left routes
-    leaveRoutes.forEach(removeListenBeforeHooksForRoute)
+    leaveRoutes
+      .filter(route => enterRoutes.indexOf(route) === -1)
+      .forEach(removeListenBeforeHooksForRoute)
 
     runEnterHooks(enterRoutes, nextState, function (error, redirectInfo) {
       if (error) {