Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use location descriptor in onEnter redirect #2855

Merged
merged 1 commit into from
Jan 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/guides/basics/RouteConfiguration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
]
Expand Down
10 changes: 7 additions & 3 deletions examples/auth-flow/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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((
Expand Down
13 changes: 7 additions & 6 deletions examples/auth-with-shared-root/config/routes.js
Original file line number Diff line number Diff line change
@@ -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('/')
}
}

Expand Down
10 changes: 5 additions & 5 deletions modules/Redirect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
25 changes: 20 additions & 5 deletions modules/TransitionUtils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { loopAsync } from './AsyncUtils'
import warning from './warning'

function createEnterHook(hook, route) {
return function (a, b, callback) {
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions modules/__tests__/serverRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ describe('server rendering', function () {

const RedirectRoute = {
path: '/company',
onEnter(nextState, replaceState) {
replaceState(null, '/about')
onEnter(nextState, replace) {
replace('/about')
}
}

Expand Down
22 changes: 11 additions & 11 deletions modules/__tests__/transitionHooks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions modules/createTransitionManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion upgrade-guides/v2.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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
Expand Down