Fix remaining issues with #3556 (#3561)

Author: taion

docs/API.md

@@ -85,7 +85,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`](#getcomponentsnextstate-callback), [`route.getIndexRoute`](#getindexroutelocation-callback), and [`route.getChildRoutes`](#getchildroutespartialnextstate-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`](#getindexroutepartialnextstate-callback), and [`route.getChildRoutes`](#getchildroutespartialnextstate-callback).
 
 ##### `onUpdate()`
 Called whenever the router updates its state in response to URL changes.
@@ -317,8 +317,7 @@ class Users extends React.Component {
 ```
 
 ##### `getComponent(nextState, callback)`
-Same as `component` but asynchronous, useful for
-code-splitting.
+Same as `component` but asynchronous, useful for code-splitting.
 
 ###### `callback` signature
 `cb(err, component)`
@@ -369,7 +368,7 @@ A plain JavaScript object route definition. `<Router>` turns JSX `<Route>`s into
 An array of child routes, same as `children` in JSX route configs.
 
 ##### `getChildRoutes(partialNextState, callback)`
-Same as `childRoutes` but asynchronous and receives the `partialNextState`. Useful for code-splitting and dynamic route matching (given some state or session data to return a different set of child routes).
+Same as `childRoutes` but asynchronous and receives `partialNextState`. Useful for code-splitting and dynamic route matching (given some state or session data to return a different set of child routes).
 
 ###### `callback` signature
 `cb(err, routesArray)`
@@ -414,9 +413,9 @@ let myRoute = {
 ##### `indexRoute`
 The [index route](/docs/guides/IndexRoutes.md). This is the same as specifying an `<IndexRoute>` child when using JSX route configs.
 
-##### `getIndexRoute(location, callback)`
+##### `getIndexRoute(partialNextState, callback)`
 
-Same as `indexRoute`, but asynchronous and receives the `location`. As with `getChildRoutes`, this can be useful for code-splitting and dynamic route matching.
+Same as `indexRoute`, but asynchronous and receives `partialNextState`. As with `getChildRoutes`, this can be useful for code-splitting and dynamic route matching.
 
 ###### `callback` signature
 `cb(err, route)`
@@ -435,7 +434,7 @@ let myRoute = {
 // async index route
 let myRoute = {
   path: 'courses',
-  getIndexRoute(location, cb) {
+  getIndexRoute(partialNextState, cb) {
     // do something async here
     cb(null, myIndexRoute)
   }

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#getchildroutespartialnextstate-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.
+Routes may define [`getChildRoutes`](/docs/API.md#getchildroutespartialnextstate-callback), [`getIndexRoute`](/docs/API.md#getindexroutepartialnextstate-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.
 
@@ -28,7 +28,7 @@ const CourseRoute = {
     })
   },
 
-  getIndexRoute(location, callback) {
+  getIndexRoute(partialNextState, callback) {
     require.ensure([], function (require) {
       callback(null, {
         component: require('./components/Index'),

modules/__tests__/isActive-test.js

@@ -628,7 +628,7 @@ describe('isActive', function () {
       childRoutes: [
         { path: 'foo' }
       ],
-      getIndexRoute(location, callback) {
+      getIndexRoute(partialNextState, callback) {
         setTimeout(() => callback(null, {}))
       }
     }

modules/__tests__/matchRoutes-test.js

@@ -291,67 +291,70 @@ describe('matchRoutes', function () {
   })
 
   describe('an asynchronous JSX route config', function () {
-    let getChildRoutes, getIndexRoute, jsxRoutes, makeJsxNestedRoutes
+    let getChildRoutes, getIndexRoute, jsxRoutes
 
     beforeEach(function () {
-      getChildRoutes = function (partialNextState, callback) {
-        setTimeout(function () {
-          callback(null, <Route path=":userID" />)
-        })
-      }
+      getChildRoutes = expect.createSpy().andCall(
+        function (partialNextState, callback) {
+          setTimeout(function () {
+            callback(null, <Route path=":userId" />)
+          })
+        }
+      )
 
-      getIndexRoute = function (location, callback) {
-        setTimeout(function () {
-          callback(null, <Route name="jsx" />)
-        })
-      }
+      getIndexRoute = expect.createSpy().andCall(
+        function (location, callback) {
+          setTimeout(function () {
+            callback(null, <Route name="jsx" />)
+          })
+        }
+      )
 
       jsxRoutes = createRoutes([
-        <Route name="users"
-               path="users"
-               getChildRoutes={getChildRoutes}
-               getIndexRoute={getIndexRoute} />
+        <Route
+          name="users"
+          path=":groupId/users"
+          getChildRoutes={getChildRoutes}
+          getIndexRoute={getIndexRoute}
+        />
       ])
-
-      makeJsxNestedRoutes = () => {
-        const spy = expect.spyOn({ getChildRoutes }, 'getChildRoutes').andCallThrough()
-        const routes = createRoutes([
-          <Route name="users"
-                path="users/:id">
-            <Route name="topic"
-                  path=":topic"
-                  getChildRoutes={spy} />
-          </Route>
-        ])
-        return { spy, routes }
-      }
     })
 
     it('when getChildRoutes callback returns reactElements', function (done) {
-      matchRoutes(jsxRoutes, createLocation('/users/5'), function (error, match) {
-        expect(match).toExist()
-        expect(match.routes.map(r => r.path)).toEqual([ 'users', ':userID' ])
-        expect(match.params).toEqual({ userID: '5' })
-        done()
-      })
+      matchRoutes(
+        jsxRoutes, createLocation('/foo/users/5'),
+        function (error, match) {
+          expect(match).toExist()
+          expect(match.routes.map(r => r.path)).toEqual([
+            ':groupId/users', ':userId'
+          ])
+          expect(match.params).toEqual({ groupId: 'foo', userId: '5' })
+
+          expect(getChildRoutes.calls[0].arguments[0].params).toEqual({
+            groupId: 'foo'
+          })
+          expect(getIndexRoute).toNotHaveBeenCalled()
+
+          done()
+        }
+      )
     })
 
     it('when getIndexRoute callback returns reactElements', function (done) {
-      matchRoutes(jsxRoutes, createLocation('/users'), function (error, match) {
-        expect(match).toExist()
-        expect(match.routes.map(r => r.name)).toEqual([ 'users', 'jsx' ])
-        done()
-      })
-    })
-    
-    it('when getChildRoutes callback returns partialNextState', function (done) {
-      const jsxNestedRoutes = makeJsxNestedRoutes()
-      matchRoutes(jsxNestedRoutes.routes, createLocation('/users/5/details/others'), function () {
-        const state = jsxNestedRoutes.spy.calls[0].arguments[0]
-        expect(state).toExist()
-        expect(state.params).toEqual({ id: '5', topic: 'details' })
-        done()
-      })
+      matchRoutes(
+        jsxRoutes, createLocation('/bar/users'),
+        function (error, match) {
+          expect(match).toExist()
+          expect(match.routes.map(r => r.name)).toEqual([ 'users', 'jsx' ])
+
+          expect(getIndexRoute.calls[0].arguments[0].params).toEqual({
+            groupId: 'bar'
+          })
+          expect(getChildRoutes).toNotHaveBeenCalled()
+
+          done()
+        }
+      )
     })
   })
 

modules/deprecateLocationProperties.js

@@ -1,6 +1,5 @@
 import { mapAsync } from './AsyncUtils'
-import { canUseMembrane } from './deprecateObjectProperties'
-import deprecateLocationProperties from './deprecateLocationProperties'
+import makeStateWithLocation from './makeStateWithLocation'
 
 function getComponentsForRoute(nextState, route, callback) {
   if (route.component || route.components) {
@@ -15,13 +14,7 @@ function getComponentsForRoute(nextState, route, callback) {
   }
 
   const { location } = nextState
-  let nextStateWithLocation
-
-  if (__DEV__ && canUseMembrane) {
-    nextStateWithLocation = deprecateLocationProperties(nextState, location)
-  } else {
-    nextStateWithLocation = { ...nextState, ...location }
-  }
+  const nextStateWithLocation = makeStateWithLocation(nextState, location)
 
   getComponent.call(route, nextStateWithLocation, callback)
 }

modules/getComponents.js

@@ -0,0 +1,28 @@
+import { canUseMembrane } from './deprecateObjectProperties'
+import warning from './routerWarning'
+
+export default function makeStateWithLocation(state, location) {
+  if (__DEV__ && canUseMembrane) {
+    const stateWithLocation = { ...state }
+
+    // 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(stateWithLocation, prop, {
+        get() {
+          warning(false, 'Accessing location properties directly from the first argument to `getComponent`, `getComponents`, `getChildRoutes`, and `getIndexRoute` is deprecated. That argument is now the router state (`nextState` or `partialNextState`) rather than the location. To access the location, use `nextState.location` or `partialNextState.location`.')
+          return location[prop]
+        }
+      })
+    }
+
+    return stateWithLocation
+  }
+
+  return { ...state, ...location }
+}