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

Call getComponent with nextState instead of location #3298

Merged
merged 1 commit into from
Apr 13, 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
10 changes: 5 additions & 5 deletions docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ A function used to convert an object from [`<Link>`](#link)s or calls to
A function used to convert a query string into an object that gets passed to route component props.

##### `onError(error)`
While the router is matching, errors may bubble up, here is your opportunity to catch and deal with them. Typically these will come from async features like [`route.getComponents`](#getcomponentslocation-callback), [`route.getIndexRoute`](#getindexroutelocation-callback), and [`route.getChildRoutes`](#getchildrouteslocation-callback).
While the router is matching, errors may bubble up, here is your opportunity to catch and deal with them. Typically these will come from async features like [`route.getComponents`](#getcomponentsnextstate-callback), [`route.getIndexRoute`](#getindexroutelocation-callback), and [`route.getChildRoutes`](#getchildrouteslocation-callback).

##### `onUpdate()`
Called whenever the router updates its state in response to URL changes.
Expand Down Expand Up @@ -300,29 +300,29 @@ class Users extends React.Component {
}
```

##### `getComponent(location, callback)`
##### `getComponent(nextState, callback)`
Same as `component` but asynchronous, useful for
code-splitting.

###### `callback` signature
`cb(err, component)`

```js
<Route path="courses/:courseId" getComponent={(location, cb) => {
<Route path="courses/:courseId" getComponent={(nextState, cb) => {
// do asynchronous stuff to find the components
cb(null, Course)
}} />
```

##### `getComponents(location, callback)`
##### `getComponents(nextState, callback)`
Same as `components` but asynchronous, useful for
code-splitting.

###### `callback` signature
`cb(err, components)`

```js
<Route path="courses/:courseId" getComponents={(location, cb) => {
<Route path="courses/:courseId" getComponents={(nextState, cb) => {
// do asynchronous stuff to find the components
cb(null, {sidebar: CourseSidebar, content: Course})
}} />
Expand Down
4 changes: 2 additions & 2 deletions docs/guides/DynamicRouting.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ A router is the perfect place to handle code splitting: it's responsible for set

React Router does all of its [path matching](/docs/guides/RouteMatching.md) and component fetching asynchronously, which allows you to not only load up the components lazily, *but also lazily load the route configuration*. You really only need one route definition in your initial bundle, the router can resolve the rest on demand.

Routes may define [`getChildRoutes`](/docs/API.md#getchildrouteslocation-callback), [`getIndexRoute`](/docs/API.md#getindexroutelocation-callback), and [`getComponents`](/docs/API.md#getcomponentslocation-callback) methods. These are asynchronous and only called when needed. We call it "gradual matching". React Router will gradually match the URL and fetch only the amount of route configuration and components it needs to match the URL and render.
Routes may define [`getChildRoutes`](/docs/API.md#getchildrouteslocation-callback), [`getIndexRoute`](/docs/API.md#getindexroutelocation-callback), and [`getComponents`](/docs/API.md#getcomponentsnextstate-callback) methods. These are asynchronous and only called when needed. We call it "gradual matching". React Router will gradually match the URL and fetch only the amount of route configuration and components it needs to match the URL and render.

Coupled with a smart code splitting tool like [webpack](http://webpack.github.io/), a once tiresome architecture is now simple and declarative.

Expand All @@ -36,7 +36,7 @@ const CourseRoute = {
})
},

getComponents(location, callback) {
getComponents(nextState, callback) {
require.ensure([], function (require) {
callback(null, require('./components/Course'))
})
Expand Down
14 changes: 7 additions & 7 deletions examples/auth-with-shared-root/config/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ export default {
component: require('../components/App'),
childRoutes: [
{ path: '/logout',
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
require.ensure([], (require) => {
cb(null, require('../components/Logout'))
})
}
},
{ path: '/about',
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
require.ensure([], (require) => {
cb(null, require('../components/About'))
})
Expand All @@ -38,7 +38,7 @@ export default {
// Unauthenticated routes
// Redirect to dashboard if user is already logged in
{ path: '/login',
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
require.ensure([], (require) => {
cb(null, require('../components/Login'))
})
Expand All @@ -52,7 +52,7 @@ export default {
childRoutes: [
// Protected routes that don't share the dashboard UI
{ path: '/user/:id',
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
require.ensure([], (require) => {
cb(null, require('../components/User'))
})
Expand All @@ -63,7 +63,7 @@ export default {
},

{ path: '/',
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
// Share the path
// Dynamically load the correct component
if (auth.loggedIn()) {
Expand All @@ -76,7 +76,7 @@ export default {
})
},
indexRoute: {
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
// Only load if we're logged in
if (auth.loggedIn()) {
return require.ensure([], (require) => {
Expand All @@ -91,7 +91,7 @@ export default {
childRoutes: [
// Protected nested routes for the dashboard
{ path: '/page2',
getComponent: (location, cb) => {
getComponent: (nextState, cb) => {
require.ensure([], (require) => {
cb(null, require('../components/PageTwo'))
})
Expand Down
2 changes: 1 addition & 1 deletion examples/huge-apps/routes/Calendar/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
path: 'calendar',
getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Calendar'))
})
Expand Down
2 changes: 1 addition & 1 deletion examples/huge-apps/routes/Course/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = {
})
},

getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Course'))
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports = {
})
},

getComponents(location, cb) {
getComponents(nextState, cb) {
require.ensure([], (require) => {
cb(null, {
sidebar: require('./components/Sidebar'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
path: ':announcementId',

getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Announcement'))
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports = {
})
},

getComponents(location, cb) {
getComponents(nextState, cb) {
require.ensure([], (require) => {
cb(null, {
sidebar: require('./components/Sidebar'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
path: ':assignmentId',
getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Assignment'))
})
Expand Down
2 changes: 1 addition & 1 deletion examples/huge-apps/routes/Course/routes/Grades/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
path: 'grades',
getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Grades'))
})
Expand Down
2 changes: 1 addition & 1 deletion examples/huge-apps/routes/Grades/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
path: 'grades',
getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Grades'))
})
Expand Down
2 changes: 1 addition & 1 deletion examples/huge-apps/routes/Messages/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
path: 'messages',
getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Messages'))
})
Expand Down
2 changes: 1 addition & 1 deletion examples/huge-apps/routes/Profile/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
path: 'profile',
getComponent(location, cb) {
getComponent(nextState, cb) {
require.ensure([], (require) => {
cb(null, require('./components/Profile'))
})
Expand Down
32 changes: 30 additions & 2 deletions modules/__tests__/Router-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import expect from 'expect'
import React, { Component } from 'react'
import { render, unmountComponentAtNode } from 'react-dom'
import createHistory from '../createMemoryHistory'
import { canUseMembrane } from '../deprecateObjectProperties'
import Route from '../Route'
import Router from '../Router'
import shouldWarn from './shouldWarn'

describe('Router', function () {

Expand Down Expand Up @@ -329,7 +331,8 @@ describe('Router', function () {

it('should support getComponent', function (done) {
const Component = () => <div />
const getComponent = (_, callback) => {
const getComponent = (nextState, callback) => {
expect(nextState.location.pathname).toBe('/')
setTimeout(() => callback(null, Component))
}

Expand All @@ -349,7 +352,8 @@ describe('Router', function () {
const foo = () => <div />
const bar = () => <div />

const getComponents = (_, callback) => {
const getComponents = (nextState, callback) => {
expect(nextState.location.pathname).toBe('/')
setTimeout(() => callback(null, { foo, bar }))
}

Expand All @@ -364,6 +368,30 @@ describe('Router', function () {
})
})
})

it('should supply location properties to getComponent', function (done) {
if (canUseMembrane) {
shouldWarn('deprecated')
}

const Component = () => <div />
const getComponent = (location, callback) => {
expect(location.pathname).toBe('/')
setTimeout(() => callback(null, Component))
}

render((
<Router history={createHistory('/')} render={renderSpy}>
<Route path="/" getComponent={getComponent} />
</Router>
), node, function () {
setTimeout(function () {
expect(componentSpy).toHaveBeenCalledWith([ Component ])
done()
})
})
})

})

describe('error handling', function () {
Expand Down
10 changes: 4 additions & 6 deletions modules/deprecateObjectProperties.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import warning from './routerWarning'

export let canUseMembrane = false

// No-op by default.
let deprecateObjectProperties = object => object

if (__DEV__) {
let useMembrane = false

try {
if (Object.defineProperty({}, 'x', { get() { return true } }).x) {
useMembrane = true
canUseMembrane = true
}
/* eslint-disable no-empty */
} catch(e) {}
/* eslint-enable no-empty */

if (useMembrane) {
if (canUseMembrane) {
deprecateObjectProperties = (object, message) => {
// Wrap the deprecated object in a membrane to warn on property access.
const membrane = {}
Expand All @@ -39,8 +39,6 @@ if (__DEV__) {
// ownKeys trap on proxies is not universal, even among browsers that
// otherwise support proxies.
Object.defineProperty(membrane, prop, {
configurable: false,
enumerable: false,
get() {
warning(false, message)
return object[prop]
Expand Down
44 changes: 36 additions & 8 deletions modules/getComponents.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,43 @@
import { mapAsync } from './AsyncUtils'
import { canUseMembrane } from './deprecateObjectProperties'
import warning from './routerWarning'

function getComponentsForRoute(location, route, callback) {
function getComponentsForRoute(nextState, route, callback) {
if (route.component || route.components) {
callback(null, route.component || route.components)
} else if (route.getComponent) {
route.getComponent(location, callback)
} else if (route.getComponents) {
route.getComponents(location, callback)
} else {
callback()
return
}

const getComponent = route.getComponent || route.getComponents
if (getComponent) {
let nextStateWithLocation = { ...nextState }
const { location } = nextState

if (__DEV__ && canUseMembrane) {
// I don't use deprecateObjectProperties here because I want to keep the
// same code path between development and production, in that we just
// assign extra properties to the copy of the state object in both cases.
for (const prop in location) {
if (!Object.prototype.hasOwnProperty.call(location, prop)) {
continue
}

Object.defineProperty(nextStateWithLocation, prop, {
get() {
warning(false, 'Accessing location properties from the first argument to `getComponent` and `getComponents` is deprecated. That argument is now the router state (`nextState`) rather than the location. To access the location, use `nextState.location`.')
return location[prop]
}
})
}
} else {
Object.assign(nextStateWithLocation, location)
}

getComponent(nextStateWithLocation, callback)
return
}

callback()
}

/**
Expand All @@ -21,7 +49,7 @@ function getComponentsForRoute(location, route, callback) {
*/
function getComponents(nextState, callback) {
mapAsync(nextState.routes, function (route, index, callback) {
getComponentsForRoute(nextState.location, route, callback)
getComponentsForRoute(nextState, route, callback)
}, callback)
}

Expand Down
25 changes: 25 additions & 0 deletions upgrade-guides/v2.2.0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# v2.2.0 Upgrade Guide

## `getComponent`, `getComponents` signature

**This is unlikely to affect you, even if you use `getComponent` and `getComponents`.**

The signature of `getComponent` and `getComponents` has been changed from `(location: Location, callback: Function) => any` to `(nextState: RouterState, callback: Function) => any`. That means that instead of writing

```js
getComponent(location, cb) {
cb(fetchComponent(location.query))
}
```

You would need to instead write

```js
getComponent(nextState, cb) {
cb(fetchComponent(nextState.location.query))
}
```

However, you new also have access to the matched `params` on `nextState`, and can use those to determine which component to return.

You will still be able to access location properties directly on `nextState` until the next breaking release (and in fact they will shadow router state properties, if applicable), but this will case a deprecation warning in development mode.