diff --git a/docs/API.md b/docs/API.md index 9c24789877..6e6d2895ae 100644 --- a/docs/API.md +++ b/docs/API.md @@ -84,7 +84,7 @@ A function used to convert an object from [``](#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. @@ -300,7 +300,7 @@ class Users extends React.Component { } ``` -##### `getComponent(location, callback)` +##### `getComponent(nextState, callback)` Same as `component` but asynchronous, useful for code-splitting. @@ -308,13 +308,13 @@ code-splitting. `cb(err, component)` ```js - { + { // 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. @@ -322,7 +322,7 @@ code-splitting. `cb(err, components)` ```js - { + { // do asynchronous stuff to find the components cb(null, {sidebar: CourseSidebar, content: Course}) }} /> diff --git a/docs/guides/DynamicRouting.md b/docs/guides/DynamicRouting.md index 171f1314fe..267fba0199 100644 --- a/docs/guides/DynamicRouting.md +++ b/docs/guides/DynamicRouting.md @@ -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. @@ -36,7 +36,7 @@ const CourseRoute = { }) }, - getComponents(location, callback) { + getComponents(nextState, callback) { require.ensure([], function (require) { callback(null, require('./components/Course')) }) diff --git a/examples/auth-with-shared-root/config/routes.js b/examples/auth-with-shared-root/config/routes.js index fc12ccc93a..683a41bdac 100644 --- a/examples/auth-with-shared-root/config/routes.js +++ b/examples/auth-with-shared-root/config/routes.js @@ -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')) }) @@ -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')) }) @@ -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')) }) @@ -63,7 +63,7 @@ export default { }, { path: '/', - getComponent: (location, cb) => { + getComponent: (nextState, cb) => { // Share the path // Dynamically load the correct component if (auth.loggedIn()) { @@ -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) => { @@ -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')) }) diff --git a/examples/huge-apps/routes/Calendar/index.js b/examples/huge-apps/routes/Calendar/index.js index 8313f51399..e51a82304e 100644 --- a/examples/huge-apps/routes/Calendar/index.js +++ b/examples/huge-apps/routes/Calendar/index.js @@ -1,6 +1,6 @@ module.exports = { path: 'calendar', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Calendar')) }) diff --git a/examples/huge-apps/routes/Course/index.js b/examples/huge-apps/routes/Course/index.js index dc6d7dcba1..5cd6da42ff 100644 --- a/examples/huge-apps/routes/Course/index.js +++ b/examples/huge-apps/routes/Course/index.js @@ -11,7 +11,7 @@ module.exports = { }) }, - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Course')) }) diff --git a/examples/huge-apps/routes/Course/routes/Announcements/index.js b/examples/huge-apps/routes/Course/routes/Announcements/index.js index 6c782387fb..c72d0db11b 100644 --- a/examples/huge-apps/routes/Course/routes/Announcements/index.js +++ b/examples/huge-apps/routes/Course/routes/Announcements/index.js @@ -9,7 +9,7 @@ module.exports = { }) }, - getComponents(location, cb) { + getComponents(nextState, cb) { require.ensure([], (require) => { cb(null, { sidebar: require('./components/Sidebar'), diff --git a/examples/huge-apps/routes/Course/routes/Announcements/routes/Announcement/index.js b/examples/huge-apps/routes/Course/routes/Announcements/routes/Announcement/index.js index 63adf2432b..88ca7242c5 100644 --- a/examples/huge-apps/routes/Course/routes/Announcements/routes/Announcement/index.js +++ b/examples/huge-apps/routes/Course/routes/Announcements/routes/Announcement/index.js @@ -1,7 +1,7 @@ module.exports = { path: ':announcementId', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Announcement')) }) diff --git a/examples/huge-apps/routes/Course/routes/Assignments/index.js b/examples/huge-apps/routes/Course/routes/Assignments/index.js index 47c987c336..475907494c 100644 --- a/examples/huge-apps/routes/Course/routes/Assignments/index.js +++ b/examples/huge-apps/routes/Course/routes/Assignments/index.js @@ -9,7 +9,7 @@ module.exports = { }) }, - getComponents(location, cb) { + getComponents(nextState, cb) { require.ensure([], (require) => { cb(null, { sidebar: require('./components/Sidebar'), diff --git a/examples/huge-apps/routes/Course/routes/Assignments/routes/Assignment/index.js b/examples/huge-apps/routes/Course/routes/Assignments/routes/Assignment/index.js index dd43094688..00be04446a 100644 --- a/examples/huge-apps/routes/Course/routes/Assignments/routes/Assignment/index.js +++ b/examples/huge-apps/routes/Course/routes/Assignments/routes/Assignment/index.js @@ -1,6 +1,6 @@ module.exports = { path: ':assignmentId', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Assignment')) }) diff --git a/examples/huge-apps/routes/Course/routes/Grades/index.js b/examples/huge-apps/routes/Course/routes/Grades/index.js index 5fbd681378..c44141dba4 100644 --- a/examples/huge-apps/routes/Course/routes/Grades/index.js +++ b/examples/huge-apps/routes/Course/routes/Grades/index.js @@ -1,6 +1,6 @@ module.exports = { path: 'grades', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Grades')) }) diff --git a/examples/huge-apps/routes/Grades/index.js b/examples/huge-apps/routes/Grades/index.js index 5fbd681378..c44141dba4 100644 --- a/examples/huge-apps/routes/Grades/index.js +++ b/examples/huge-apps/routes/Grades/index.js @@ -1,6 +1,6 @@ module.exports = { path: 'grades', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Grades')) }) diff --git a/examples/huge-apps/routes/Messages/index.js b/examples/huge-apps/routes/Messages/index.js index 0d01e3b71b..cfcd44ac5f 100644 --- a/examples/huge-apps/routes/Messages/index.js +++ b/examples/huge-apps/routes/Messages/index.js @@ -1,6 +1,6 @@ module.exports = { path: 'messages', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Messages')) }) diff --git a/examples/huge-apps/routes/Profile/index.js b/examples/huge-apps/routes/Profile/index.js index 3cae1893ad..29def45934 100644 --- a/examples/huge-apps/routes/Profile/index.js +++ b/examples/huge-apps/routes/Profile/index.js @@ -1,6 +1,6 @@ module.exports = { path: 'profile', - getComponent(location, cb) { + getComponent(nextState, cb) { require.ensure([], (require) => { cb(null, require('./components/Profile')) }) diff --git a/modules/__tests__/Router-test.js b/modules/__tests__/Router-test.js index f1610eddcc..52456c1cfc 100644 --- a/modules/__tests__/Router-test.js +++ b/modules/__tests__/Router-test.js @@ -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 () { @@ -329,7 +331,8 @@ describe('Router', function () { it('should support getComponent', function (done) { const Component = () =>
- const getComponent = (_, callback) => { + const getComponent = (nextState, callback) => { + expect(nextState.location.pathname).toBe('/') setTimeout(() => callback(null, Component)) } @@ -349,7 +352,8 @@ describe('Router', function () { const foo = () =>
const bar = () =>
- const getComponents = (_, callback) => { + const getComponents = (nextState, callback) => { + expect(nextState.location.pathname).toBe('/') setTimeout(() => callback(null, { foo, bar })) } @@ -364,6 +368,30 @@ describe('Router', function () { }) }) }) + + it('should supply location properties to getComponent', function (done) { + if (canUseMembrane) { + shouldWarn('deprecated') + } + + const Component = () =>
+ const getComponent = (location, callback) => { + expect(location.pathname).toBe('/') + setTimeout(() => callback(null, Component)) + } + + render(( + + + + ), node, function () { + setTimeout(function () { + expect(componentSpy).toHaveBeenCalledWith([ Component ]) + done() + }) + }) + }) + }) describe('error handling', function () { diff --git a/modules/deprecateObjectProperties.js b/modules/deprecateObjectProperties.js index 740a4b88da..f1309c50d3 100644 --- a/modules/deprecateObjectProperties.js +++ b/modules/deprecateObjectProperties.js @@ -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 = {} @@ -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] diff --git a/modules/getComponents.js b/modules/getComponents.js index 724c20344f..6731c69691 100644 --- a/modules/getComponents.js +++ b/modules/getComponents.js @@ -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() } /** @@ -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) } diff --git a/upgrade-guides/v2.2.0.md b/upgrade-guides/v2.2.0.md new file mode 100644 index 0000000000..21f1d90a32 --- /dev/null +++ b/upgrade-guides/v2.2.0.md @@ -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.