Merge pull request #3166 from reactjs/transition-bug

Transition bug

Author: ryanflorence

CHANGES.md

@@ -1,7 +1,10 @@
 ## [HEAD]
 > Unreleased
 
-- Nothing yet!
+- Fixed bug where transition hooks were not called on child routes of
+  parent's whose params changed but the child's did not. ([#3166])
+
+[#3166][https://github.com/reactjs/react-router/pull/3166]
 
 [HEAD]: https://github.com/reactjs/react-router/compare/v2.0.0...HEAD
 

modules/__tests__/transitionHooks-test.js

@@ -10,7 +10,7 @@ describe('When a router enters a branch', function () {
   let
     node,
     newsLeaveHookSpy, removeNewsLeaveHook, userLeaveHookSpy,
-    DashboardRoute, NewsFeedRoute, InboxRoute, RedirectToInboxRoute, MessageRoute, UserRoute,
+    DashboardRoute, NewsFeedRoute, InboxRoute, RedirectToInboxRoute, MessageRoute, UserRoute, AssignmentRoute,
     routes
 
   beforeEach(function () {
@@ -52,6 +52,12 @@ describe('When a router enters a branch', function () {
       }
     }
 
+    class UserAssignment extends Component {
+      render() {
+        return <div>assignment {this.props.params.assignmentId}</div>
+      }
+    }
+
     class User extends Component {
       componentWillMount() {
         this.context.router.setRouteLeaveHook(
@@ -61,7 +67,7 @@ describe('When a router enters a branch', function () {
       }
 
       render() {
-        return <div>User</div>
+        return <div>User {this.props.params.userId} {this.props.children}</div>
       }
     }
 
@@ -121,9 +127,19 @@ describe('When a router enters a branch', function () {
       }
     }
 
+    AssignmentRoute = {
+      path: 'assignments/:assignmentId',
+      component: UserAssignment,
+      onEnter() { expect(this).toBe(AssignmentRoute) },
+      onLeave() { expect(this).toBe(AssignmentRoute) }
+    }
+
     UserRoute = {
       path: 'users/:userId',
-      component: User
+      component: User,
+      childRoutes: [ AssignmentRoute ],
+      onEnter() { expect(this).toBe(UserRoute) },
+      onLeave() { expect(this).toBe(UserRoute) }
     }
 
     DashboardRoute = {
@@ -277,6 +293,38 @@ describe('When a router enters a branch', function () {
     })
   })
 
+  describe('and then navigates to the same branch, but with different parent params', function () {
+    it('calls the onLeave and onEnter hooks of the parent and children', function (done) {
+      const parentLeaveSpy = spyOn(UserRoute, 'onLeave').andCallThrough()
+      const parentEnterSpy = spyOn(UserRoute, 'onEnter').andCallThrough()
+      const childLeaveSpy = spyOn(AssignmentRoute, 'onLeave').andCallThrough()
+      const childEnterSpy = spyOn(AssignmentRoute, 'onEnter').andCallThrough()
+      const history = createHistory('/users/123/assignments/456')
+
+      const steps = [
+        function () {
+          expect(parentEnterSpy).toHaveBeenCalled()
+          expect(childEnterSpy).toHaveBeenCalled()
+          history.push('/users/789/assignments/456')
+        },
+        function () {
+          expect(parentLeaveSpy).toHaveBeenCalled()
+          expect(childLeaveSpy).toHaveBeenCalled()
+          expect(parentEnterSpy).toHaveBeenCalled()
+          expect(childEnterSpy).toHaveBeenCalled()
+        }
+      ]
+
+      const execNextStep = execSteps(steps, done)
+
+      render(
+        <Router history={history}
+                routes={routes}
+                onUpdate={execNextStep}
+        />, node, execNextStep)
+    })
+  })
+
   describe('and then the query changes', function () {
     it('calls the onEnter hooks of all routes in that branch', function (done) {
       const newsFeedRouteEnterSpy = spyOn(NewsFeedRoute, 'onEnter').andCallThrough()

modules/computeChangedRoutes.js

@@ -27,8 +27,16 @@ function computeChangedRoutes(prevState, nextState) {
 
   let leaveRoutes, enterRoutes
   if (prevRoutes) {
+    let parentIsLeaving = false
     leaveRoutes = prevRoutes.filter(function (route) {
-      return nextRoutes.indexOf(route) === -1 || routeParamsChanged(route, prevState, nextState)
+      if (parentIsLeaving) {
+        return true
+      } else {
+        const isLeaving = nextRoutes.indexOf(route) === -1 || routeParamsChanged(route, prevState, nextState)
+        if (isLeaving)
+          parentIsLeaving = true
+        return isLeaving
+      }
     })
 
     // onLeave hooks start at the leaf route.