Merge pull request #2855 from taion/onEnter-location-descriptor

Use location descriptor in onEnter redirect

Author: timdorr

docs/guides/basics/RouteConfiguration.md

@@ -193,8 +193,8 @@ const routeConfig = [
         childRoutes: [
           { path: '/messages/:id', component: Message },
           { path: 'messages/:id',
-            onEnter: function (nextState, replaceState) {
-              replaceState(null, '/messages/' + nextState.params.id)
+            onEnter: function (nextState, replace) {
+              replace('/messages/' + nextState.params.id)
             }
           }
         ]

examples/auth-flow/app.js

@@ -117,9 +117,13 @@ const Logout = React.createClass({
   }
 })
 
-function requireAuth(nextState, replaceState) {
-  if (!auth.loggedIn())
-    replaceState({ nextPathname: nextState.location.pathname }, '/login')
+function requireAuth(nextState, replace) {
+  if (!auth.loggedIn()) {
+    replace({
+      pathname: '/login',
+      state: { nextPathname: nextState.location.pathname }
+    })
+  }
 }
 
 render((

examples/auth-with-shared-root/config/routes.js

@@ -1,16 +1,17 @@
 import auth from '../utils/auth.js'
 
-function redirectToLogin(nextState, replaceState) {
+function redirectToLogin(nextState, replace) {
   if (!auth.loggedIn()) {
-    replaceState({
-      nextPathname: nextState.location.pathname
-    }, '/login')
+    replace({
+      pathname: '/login',
+      state: { nextPathname: nextState.location.pathname }
+    })
   }
 }
 
-function redirectToDashboard(nextState, replaceState) {
+function redirectToDashboard(nextState, replace) {
   if (auth.loggedIn()) {
-    replaceState(null, '/')
+    replace('/')
   }
 }
 

modules/Redirect.js

@@ -23,7 +23,7 @@ const Redirect = React.createClass({
       if (route.from)
         route.path = route.from
 
-      route.onEnter = function (nextState, replaceState) {
+      route.onEnter = function (nextState, replace) {
         const { location, params } = nextState
 
         let pathname
@@ -38,11 +38,11 @@ const Redirect = React.createClass({
           pathname = formatPattern(pattern, params)
         }
 
-        replaceState(
-          route.state || location.state,
+        replace({
           pathname,
-          route.query || location.query
-        )
+          query: route.query || location.query,
+          state: route.state || location.state
+        })
       }
 
       return route

modules/TransitionUtils.js

@@ -1,4 +1,5 @@
 import { loopAsync } from './AsyncUtils'
+import warning from './warning'
 
 function createEnterHook(hook, route) {
   return function (a, b, callback) {
@@ -23,9 +24,9 @@ function getEnterHooks(routes) {
 
 /**
  * Runs all onEnter hooks in the given array of routes in order
- * with onEnter(nextState, replaceState, callback) and calls
+ * with onEnter(nextState, replace, callback) and calls
  * callback(error, redirectInfo) when finished. The first hook
- * to use replaceState short-circuits the loop.
+ * to use replace short-circuits the loop.
  *
  * If a hook needs to run asynchronously, it may use the callback
  * function. However, doing so will cause the transition to pause,
@@ -40,12 +41,26 @@ export function runEnterHooks(routes, nextState, callback) {
   }
 
   let redirectInfo
-  function replaceState(state, pathname, query) {
-    redirectInfo = { pathname, query, state }
+  function replace(location, deprecatedPathname, deprecatedQuery) {
+    if (deprecatedPathname) {
+      warning(
+        false,
+        '`replaceState(state, pathname, query) is deprecated; use `replace(location)` with a location descriptor instead. http://tiny.cc/router-isActivedeprecated'
+      )
+      redirectInfo = {
+        pathname: deprecatedPathname,
+        query: deprecatedQuery,
+        state: location
+      }
+
+      return
+    }
+
+    redirectInfo = location
   }
 
   loopAsync(hooks.length, function (index, next, done) {
-    hooks[index](nextState, replaceState, function (error) {
+    hooks[index](nextState, replace, function (error) {
       if (error || redirectInfo) {
         done(error, redirectInfo) // No need to continue.
       } else {

modules/__tests__/serverRendering-test.js

@@ -55,8 +55,8 @@ describe('server rendering', function () {
 
   const RedirectRoute = {
     path: '/company',
-    onEnter(nextState, replaceState) {
-      replaceState(null, '/about')
+    onEnter(nextState, replace) {
+      replace('/about')
     }
   }
 

modules/__tests__/transitionHooks-test.js

@@ -53,10 +53,10 @@ describe('When a router enters a branch', function () {
     NewsFeedRoute = {
       path: 'news',
       component: NewsFeed,
-      onEnter(nextState, replaceState) {
+      onEnter(nextState, replace) {
         expect(this).toBe(NewsFeedRoute)
         expect(nextState.routes).toContain(NewsFeedRoute)
-        expect(replaceState).toBeA('function')
+        expect(replace).toBeA('function')
       },
       onLeave() {
         expect(this).toBe(NewsFeedRoute)
@@ -66,10 +66,10 @@ describe('When a router enters a branch', function () {
     InboxRoute = {
       path: 'inbox',
       component: Inbox,
-      onEnter(nextState, replaceState) {
+      onEnter(nextState, replace) {
         expect(this).toBe(InboxRoute)
         expect(nextState.routes).toContain(InboxRoute)
-        expect(replaceState).toBeA('function')
+        expect(replace).toBeA('function')
       },
       onLeave() {
         expect(this).toBe(InboxRoute)
@@ -78,12 +78,12 @@ describe('When a router enters a branch', function () {
 
     RedirectToInboxRoute = {
       path: 'redirect-to-inbox',
-      onEnter(nextState, replaceState) {
+      onEnter(nextState, replace) {
         expect(this).toBe(RedirectToInboxRoute)
         expect(nextState.routes).toContain(RedirectToInboxRoute)
-        expect(replaceState).toBeA('function')
+        expect(replace).toBeA('function')
 
-        replaceState(null, '/inbox')
+        replace('/inbox')
       },
       onLeave() {
         expect(this).toBe(RedirectToInboxRoute)
@@ -92,10 +92,10 @@ describe('When a router enters a branch', function () {
 
     MessageRoute = {
       path: 'messages/:messageID',
-      onEnter(nextState, replaceState) {
+      onEnter(nextState, replace) {
         expect(this).toBe(MessageRoute)
         expect(nextState.routes).toContain(MessageRoute)
-        expect(replaceState).toBeA('function')
+        expect(replace).toBeA('function')
       },
       onLeave() {
         expect(this).toBe(MessageRoute)
@@ -105,10 +105,10 @@ describe('When a router enters a branch', function () {
     DashboardRoute = {
       path: '/',
       component: Dashboard,
-      onEnter(nextState, replaceState) {
+      onEnter(nextState, replace) {
         expect(this).toBe(DashboardRoute)
         expect(nextState.routes).toContain(DashboardRoute)
-        expect(replaceState).toBeA('function')
+        expect(replace).toBeA('function')
       },
       onLeave() {
         expect(this).toBe(DashboardRoute)

modules/createTransitionManager.js

@@ -45,10 +45,8 @@ export default function createTransitionManager(history, routes) {
     )
   }
 
-  function createLocationFromRedirectInfo({ pathname, query, state }) {
-    return history.createLocation(
-      history.createPath(pathname, query), state, REPLACE
-    )
+  function createLocationFromRedirectInfo(location) {
+    return history.createLocation(location, REPLACE)
   }
 
   let partialNextState

upgrade-guides/v2.0.0.md

@@ -229,7 +229,7 @@ const DeepComponent = React.createClass({
 }
 ```
 
-## `<Link to>` and `isActive` take location descriptors
+## `<Link to>`, `onEnter`, and `isActive` use location descriptors
 
 `<Link to>` can now take a location descriptor in addition to strings. The `query` and `state` props are deprecated.
 
@@ -244,6 +244,18 @@ const DeepComponent = React.createClass({
 <Link to="/foo"/>
 ```
 
+Likewise, redirecting from an `onEnter` hook now also uses a location descriptor.
+
+```js
+// v1.0.x
+(nextState, replaceState) => replaceState(null, '/foo')
+(nextState, replaceState) => replaceState(null, '/foo', { the: 'query' })
+
+// v2.0.0
+(nextState, replace) => replace('/foo')
+(nextState, replace) => replace({ pathname: '/foo', query: { the: 'query' } })
+```
+
 For custom link-like components, the same applies for `router.isActive`, previously `history.isActive`.
 
 ```js