From a81788b57428850c90dc08e14ab40f0c48614214 Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 10:59:21 -0700 Subject: [PATCH 01/13] Initial POC of making params and remainingPathname available to getChildRoutes providers --- modules/deprecateLocationProperties.js | 31 ++++++++++++++++++++++++++ modules/getComponents.js | 20 ++--------------- modules/matchRoutes.js | 20 ++++++++++++++--- 3 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 modules/deprecateLocationProperties.js diff --git a/modules/deprecateLocationProperties.js b/modules/deprecateLocationProperties.js new file mode 100644 index 0000000000..be3179062f --- /dev/null +++ b/modules/deprecateLocationProperties.js @@ -0,0 +1,31 @@ +import warning from './routerWarning' +import { canUseMembrane } from './deprecateObjectProperties' + +// No-op by default. +let deprecateLocationProperties = () => {} + +if (__DEV__ && canUseMembrane) { + deprecateLocationProperties = (nextState, location) => { + const nextStateWithLocation = { ...nextState } + + // 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] + } + }) + } + + return nextStateWithLocation + } +} + +export default deprecateLocationProperties diff --git a/modules/getComponents.js b/modules/getComponents.js index 06a663fe5c..7caa6c0b73 100644 --- a/modules/getComponents.js +++ b/modules/getComponents.js @@ -1,6 +1,6 @@ import { mapAsync } from './AsyncUtils' import { canUseMembrane } from './deprecateObjectProperties' -import warning from './routerWarning' +import deprecateLocationProperties from './deprecateLocationProperties' function getComponentsForRoute(nextState, route, callback) { if (route.component || route.components) { @@ -18,23 +18,7 @@ function getComponentsForRoute(nextState, route, callback) { let nextStateWithLocation if (__DEV__ && canUseMembrane) { - nextStateWithLocation = { ...nextState } - - // 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] - } - }) - } + nextStateWithLocation = deprecateLocationProperties(nextState, location) } else { nextStateWithLocation = { ...nextState, ...location } } diff --git a/modules/matchRoutes.js b/modules/matchRoutes.js index 37bde417a2..892213cbc1 100644 --- a/modules/matchRoutes.js +++ b/modules/matchRoutes.js @@ -2,8 +2,10 @@ import warning from './routerWarning' import { loopAsync } from './AsyncUtils' import { matchPattern } from './PatternUtils' import { createRoutes } from './RouteUtils' +import { canUseMembrane } from './deprecateObjectProperties' +import deprecateLocationProperties from './deprecateLocationProperties' -function getChildRoutes(route, location, callback) { +function getChildRoutes(route, progressState, callback) { if (route.childRoutes) { return [ null, route.childRoutes ] } @@ -13,7 +15,7 @@ function getChildRoutes(route, location, callback) { let sync = true, result - route.getChildRoutes(location, function (error, childRoutes) { + route.getChildRoutes(progressState, function (error, childRoutes) { childRoutes = !error && createRoutes(childRoutes) if (sync) { result = [ error, childRoutes ] @@ -160,7 +162,19 @@ function matchRouteDeep( } } - const result = getChildRoutes(route, location, onChildRoutes) + const progressState = { + params: createParams(paramNames, paramValues), + remainingPathname + } + let progressStateWithLocation + + if (__DEV__ && canUseMembrane) { + progressStateWithLocation = deprecateLocationProperties(progressState, location) + } else { + progressStateWithLocation = { ...progressState, ...location } + } + + const result = getChildRoutes(route, progressStateWithLocation, onChildRoutes) if (result) { onChildRoutes(...result) } From 2c90d043127e35c7ea1586346ff055aafe9dfdbf Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 13:10:34 -0700 Subject: [PATCH 02/13] Update given feedback on when to call createParams --- modules/matchRoutes.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/modules/matchRoutes.js b/modules/matchRoutes.js index 892213cbc1..d2d14c6ca3 100644 --- a/modules/matchRoutes.js +++ b/modules/matchRoutes.js @@ -5,7 +5,7 @@ import { createRoutes } from './RouteUtils' import { canUseMembrane } from './deprecateObjectProperties' import deprecateLocationProperties from './deprecateLocationProperties' -function getChildRoutes(route, progressState, callback) { +function getChildRoutes(route, location, paramNames, paramValues, remainingPathname, callback) { if (route.childRoutes) { return [ null, route.childRoutes ] } @@ -13,9 +13,19 @@ function getChildRoutes(route, progressState, callback) { return [] } - let sync = true, result + let sync = true, result, progressStateWithLocation + const progressState = { + params: createParams(paramNames, paramValues), + remainingPathname + } + + if (__DEV__ && canUseMembrane) { + progressStateWithLocation = deprecateLocationProperties(progressState, location) + } else { + progressStateWithLocation = { ...progressState, ...location } + } - route.getChildRoutes(progressState, function (error, childRoutes) { + route.getChildRoutes(progressStateWithLocation, function (error, childRoutes) { childRoutes = !error && createRoutes(childRoutes) if (sync) { result = [ error, childRoutes ] @@ -162,19 +172,7 @@ function matchRouteDeep( } } - const progressState = { - params: createParams(paramNames, paramValues), - remainingPathname - } - let progressStateWithLocation - - if (__DEV__ && canUseMembrane) { - progressStateWithLocation = deprecateLocationProperties(progressState, location) - } else { - progressStateWithLocation = { ...progressState, ...location } - } - - const result = getChildRoutes(route, progressStateWithLocation, onChildRoutes) + const result = getChildRoutes(route, location, paramNames, paramValues, remainingPathname, onChildRoutes) if (result) { onChildRoutes(...result) } From 8e2576f2d3b917a1be7b236364615f971d23757a Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 13:32:23 -0700 Subject: [PATCH 03/13] Update documentation --- docs/API.md | 10 +++++----- docs/guides/DynamicRouting.md | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/API.md b/docs/API.md index 549f8055c4..81e59de686 100644 --- a/docs/API.md +++ b/docs/API.md @@ -85,7 +85,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`](#getcomponentsnextstate-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`](#getchildroutesprogressstate-callback). ##### `onUpdate()` Called whenever the router updates its state in response to URL changes. @@ -368,8 +368,8 @@ A plain JavaScript object route definition. `` turns JSX ``s into ##### `childRoutes` An array of child routes, same as `children` in JSX route configs. -##### `getChildRoutes(location, callback)` -Same as `childRoutes` but asynchronous and receives the `location`. Useful for code-splitting and dynamic route matching (given some state or session data to return a different set of child routes). +##### `getChildRoutes(progressState, callback)` +Same as `childRoutes` but asynchronous and receives the `progressState`. 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)` @@ -399,8 +399,8 @@ let myRoute = { let myRoute = { path: 'picture/:id', - getChildRoutes(location, cb) { - let { state } = location + getChildRoutes(progressState, cb) { + let { state } = progressState if (state && state.fromDashboard) { cb(null, [dashboardPictureRoute]) diff --git a/docs/guides/DynamicRouting.md b/docs/guides/DynamicRouting.md index 267fba0199..233ba1fade 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#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#getchildroutesprogressstate-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. @@ -18,7 +18,7 @@ Coupled with a smart code splitting tool like [webpack](http://webpack.github.io const CourseRoute = { path: 'course/:courseId', - getChildRoutes(location, callback) { + getChildRoutes(progressState, callback) { require.ensure([], function (require) { callback(null, [ require('./routes/Announcements'), From 61c64f7fd377b6e7c2f361cc96d64f7b4706e46c Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 13:36:14 -0700 Subject: [PATCH 04/13] Update samples and tests usage of location to progressState --- examples/huge-apps/routes/Course/index.js | 2 +- .../huge-apps/routes/Course/routes/Announcements/index.js | 2 +- examples/huge-apps/routes/Course/routes/Assignments/index.js | 2 +- modules/__tests__/matchRoutes-test.js | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/examples/huge-apps/routes/Course/index.js b/examples/huge-apps/routes/Course/index.js index 5cd6da42ff..7df553645b 100644 --- a/examples/huge-apps/routes/Course/index.js +++ b/examples/huge-apps/routes/Course/index.js @@ -1,7 +1,7 @@ module.exports = { path: 'course/:courseId', - getChildRoutes(location, cb) { + getChildRoutes(progressState, cb) { require.ensure([], (require) => { cb(null, [ require('./routes/Announcements'), diff --git a/examples/huge-apps/routes/Course/routes/Announcements/index.js b/examples/huge-apps/routes/Course/routes/Announcements/index.js index c72d0db11b..2bcaf4276e 100644 --- a/examples/huge-apps/routes/Course/routes/Announcements/index.js +++ b/examples/huge-apps/routes/Course/routes/Announcements/index.js @@ -1,7 +1,7 @@ module.exports = { path: 'announcements', - getChildRoutes(location, cb) { + getChildRoutes(progressState, cb) { require.ensure([], (require) => { cb(null, [ require('./routes/Announcement') diff --git a/examples/huge-apps/routes/Course/routes/Assignments/index.js b/examples/huge-apps/routes/Course/routes/Assignments/index.js index 475907494c..03328e1ef5 100644 --- a/examples/huge-apps/routes/Course/routes/Assignments/index.js +++ b/examples/huge-apps/routes/Course/routes/Assignments/index.js @@ -1,7 +1,7 @@ module.exports = { path: 'assignments', - getChildRoutes(location, cb) { + getChildRoutes(progressState, cb) { require.ensure([], (require) => { cb(null, [ require('./routes/Assignment') diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index b43574925b..0cc4ea5306 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -262,7 +262,7 @@ describe('matchRoutes', function () { if (childRoutes) { delete route.childRoutes - route.getChildRoutes = function (location, callback) { + route.getChildRoutes = function (progressState, callback) { setTimeout(function () { callback(null, childRoutes) }) @@ -294,7 +294,7 @@ describe('matchRoutes', function () { let getChildRoutes, getIndexRoute, jsxRoutes beforeEach(function () { - getChildRoutes = function (location, callback) { + getChildRoutes = function (progressState, callback) { setTimeout(function () { callback(null, ) }) From d3b7ce5e569c559f25877b93eed5695c3ca0b0ff Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 14:02:38 -0700 Subject: [PATCH 05/13] Rename from partialState to partialNextState --- docs/API.md | 10 +++++----- docs/guides/DynamicRouting.md | 4 ++-- examples/huge-apps/routes/Course/index.js | 2 +- .../routes/Course/routes/Announcements/index.js | 2 +- .../routes/Course/routes/Assignments/index.js | 2 +- modules/__tests__/matchRoutes-test.js | 4 ++-- modules/matchRoutes.js | 10 +++++----- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/API.md b/docs/API.md index 81e59de686..d017cad7f2 100644 --- a/docs/API.md +++ b/docs/API.md @@ -85,7 +85,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`](#getcomponentsnextstate-callback), [`route.getIndexRoute`](#getindexroutelocation-callback), and [`route.getChildRoutes`](#getchildroutesprogressstate-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`](#getchildroutespartialnextstate-callback). ##### `onUpdate()` Called whenever the router updates its state in response to URL changes. @@ -368,8 +368,8 @@ A plain JavaScript object route definition. `` turns JSX ``s into ##### `childRoutes` An array of child routes, same as `children` in JSX route configs. -##### `getChildRoutes(progressState, callback)` -Same as `childRoutes` but asynchronous and receives the `progressState`. Useful for code-splitting and dynamic route matching (given some state or session data to return a different set of child routes). +##### `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). ###### `callback` signature `cb(err, routesArray)` @@ -399,8 +399,8 @@ let myRoute = { let myRoute = { path: 'picture/:id', - getChildRoutes(progressState, cb) { - let { state } = progressState + getChildRoutes(partialNextState, cb) { + let { state } = partialNextState if (state && state.fromDashboard) { cb(null, [dashboardPictureRoute]) diff --git a/docs/guides/DynamicRouting.md b/docs/guides/DynamicRouting.md index 233ba1fade..8853664f12 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#getchildroutesprogressstate-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#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. @@ -18,7 +18,7 @@ Coupled with a smart code splitting tool like [webpack](http://webpack.github.io const CourseRoute = { path: 'course/:courseId', - getChildRoutes(progressState, callback) { + getChildRoutes(partialNextState, callback) { require.ensure([], function (require) { callback(null, [ require('./routes/Announcements'), diff --git a/examples/huge-apps/routes/Course/index.js b/examples/huge-apps/routes/Course/index.js index 7df553645b..6e55b8c1e8 100644 --- a/examples/huge-apps/routes/Course/index.js +++ b/examples/huge-apps/routes/Course/index.js @@ -1,7 +1,7 @@ module.exports = { path: 'course/:courseId', - getChildRoutes(progressState, cb) { + getChildRoutes(partialNextState, cb) { require.ensure([], (require) => { cb(null, [ require('./routes/Announcements'), diff --git a/examples/huge-apps/routes/Course/routes/Announcements/index.js b/examples/huge-apps/routes/Course/routes/Announcements/index.js index 2bcaf4276e..903bc6d7d3 100644 --- a/examples/huge-apps/routes/Course/routes/Announcements/index.js +++ b/examples/huge-apps/routes/Course/routes/Announcements/index.js @@ -1,7 +1,7 @@ module.exports = { path: 'announcements', - getChildRoutes(progressState, cb) { + getChildRoutes(partialNextState, cb) { require.ensure([], (require) => { cb(null, [ require('./routes/Announcement') diff --git a/examples/huge-apps/routes/Course/routes/Assignments/index.js b/examples/huge-apps/routes/Course/routes/Assignments/index.js index 03328e1ef5..bd06da1dbb 100644 --- a/examples/huge-apps/routes/Course/routes/Assignments/index.js +++ b/examples/huge-apps/routes/Course/routes/Assignments/index.js @@ -1,7 +1,7 @@ module.exports = { path: 'assignments', - getChildRoutes(progressState, cb) { + getChildRoutes(partialNextState, cb) { require.ensure([], (require) => { cb(null, [ require('./routes/Assignment') diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index 0cc4ea5306..e8f25fd8bf 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -262,7 +262,7 @@ describe('matchRoutes', function () { if (childRoutes) { delete route.childRoutes - route.getChildRoutes = function (progressState, callback) { + route.getChildRoutes = function (partialNextState, callback) { setTimeout(function () { callback(null, childRoutes) }) @@ -294,7 +294,7 @@ describe('matchRoutes', function () { let getChildRoutes, getIndexRoute, jsxRoutes beforeEach(function () { - getChildRoutes = function (progressState, callback) { + getChildRoutes = function (partialNextState, callback) { setTimeout(function () { callback(null, ) }) diff --git a/modules/matchRoutes.js b/modules/matchRoutes.js index d2d14c6ca3..ffe4f0e706 100644 --- a/modules/matchRoutes.js +++ b/modules/matchRoutes.js @@ -13,19 +13,19 @@ function getChildRoutes(route, location, paramNames, paramValues, remainingPathn return [] } - let sync = true, result, progressStateWithLocation - const progressState = { + let sync = true, result, partialNextStateWithLocation + const partialNextState = { params: createParams(paramNames, paramValues), remainingPathname } if (__DEV__ && canUseMembrane) { - progressStateWithLocation = deprecateLocationProperties(progressState, location) + partialNextStateWithLocation = deprecateLocationProperties(partialNextState, location) } else { - progressStateWithLocation = { ...progressState, ...location } + partialNextStateWithLocation = { ...partialNextState, ...location } } - route.getChildRoutes(progressStateWithLocation, function (error, childRoutes) { + route.getChildRoutes(partialNextStateWithLocation, function (error, childRoutes) { childRoutes = !error && createRoutes(childRoutes) if (sync) { result = [ error, childRoutes ] From bfb18e676a48bd4858cced93c7dabeb5bc6682bc Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 14:04:32 -0700 Subject: [PATCH 06/13] Remove usage of remainingPathname for the moment --- modules/matchRoutes.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/matchRoutes.js b/modules/matchRoutes.js index ffe4f0e706..1a9b3b6f12 100644 --- a/modules/matchRoutes.js +++ b/modules/matchRoutes.js @@ -5,7 +5,7 @@ import { createRoutes } from './RouteUtils' import { canUseMembrane } from './deprecateObjectProperties' import deprecateLocationProperties from './deprecateLocationProperties' -function getChildRoutes(route, location, paramNames, paramValues, remainingPathname, callback) { +function getChildRoutes(route, location, paramNames, paramValues, callback) { if (route.childRoutes) { return [ null, route.childRoutes ] } @@ -15,8 +15,7 @@ function getChildRoutes(route, location, paramNames, paramValues, remainingPathn let sync = true, result, partialNextStateWithLocation const partialNextState = { - params: createParams(paramNames, paramValues), - remainingPathname + params: createParams(paramNames, paramValues) } if (__DEV__ && canUseMembrane) { @@ -172,7 +171,7 @@ function matchRouteDeep( } } - const result = getChildRoutes(route, location, paramNames, paramValues, remainingPathname, onChildRoutes) + const result = getChildRoutes(route, location, paramNames, paramValues, onChildRoutes) if (result) { onChildRoutes(...result) } From 400734e4bd552c82665ecffdddd4a754a5b2b27f Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 14:58:24 -0700 Subject: [PATCH 07/13] Add unit tests --- modules/__tests__/matchRoutes-test.js | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index e8f25fd8bf..4f64715435 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -332,6 +332,35 @@ describe('matchRoutes', function () { }) }) + describe('a nested route with a getChildRoutes callback', function () { + let getChildRoutes, jsxRoutes + + beforeEach(function () { + getChildRoutes = function (partialNextState, callback) { + setTimeout(function () { + callback(null, partialNextState) + }) + } + + jsxRoutes = createRoutes([ + + + + ]) + }) + + it('when getChildRoutes callback returns partialNextState', function (done) { + matchRoutes(jsxRoutes, createLocation('/users/5/details'), function (error, partialNextState) { + expect(partialNextState).toExist() + expect(partialNextState.params).toEqual({ id: '5', topic: 'details' }) + done() + }) + }) + }) + it('complains about invalid index route with path', function (done) { shouldWarn('path') From b8c18207c90a1c20b621d853203ca0042ccb6d7d Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 17:04:10 -0700 Subject: [PATCH 08/13] Update and consolidate tests --- modules/__tests__/matchRoutes-test.js | 47 ++++++++++----------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index 4f64715435..c728a8dde9 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -291,15 +291,16 @@ describe('matchRoutes', function () { }) describe('an asynchronous JSX route config', function () { - let getChildRoutes, getIndexRoute, jsxRoutes + let getChildRoutes, getIndexRoute, jsxRoutes, jsxNestedRoutes, state beforeEach(function () { getChildRoutes = function (partialNextState, callback) { + state = partialNextState setTimeout(function () { callback(null, ) }) } - + getIndexRoute = function (location, callback) { setTimeout(function () { callback(null, ) @@ -310,9 +311,18 @@ describe('matchRoutes', function () { + getIndexRoute={getIndexRoute} /> ]) - }) + + jsxNestedRoutes = createRoutes([ + + + + ]) + }) it('when getChildRoutes callback returns reactElements', function (done) { matchRoutes(jsxRoutes, createLocation('/users/5'), function (error, match) { @@ -330,32 +340,11 @@ describe('matchRoutes', function () { done() }) }) - }) - - describe('a nested route with a getChildRoutes callback', function () { - let getChildRoutes, jsxRoutes - - beforeEach(function () { - getChildRoutes = function (partialNextState, callback) { - setTimeout(function () { - callback(null, partialNextState) - }) - } - - jsxRoutes = createRoutes([ - - - - ]) - }) - + it('when getChildRoutes callback returns partialNextState', function (done) { - matchRoutes(jsxRoutes, createLocation('/users/5/details'), function (error, partialNextState) { - expect(partialNextState).toExist() - expect(partialNextState.params).toEqual({ id: '5', topic: 'details' }) + matchRoutes(jsxNestedRoutes, createLocation('/users/5/details/others'), function () { + expect(state).toExist() + expect(state.params).toEqual({ id: '5', topic: 'details' }) done() }) }) From 3bf3fa95ada47d12b142d60f618ae1f81273d3a8 Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 21:02:20 -0700 Subject: [PATCH 09/13] Fixup whitespace issues --- modules/__tests__/matchRoutes-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index c728a8dde9..bf529ee2a9 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -300,7 +300,7 @@ describe('matchRoutes', function () { callback(null, ) }) } - + getIndexRoute = function (location, callback) { setTimeout(function () { callback(null, ) From 414b61228f5569e10c5495a306a245a32c944f7c Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 22:38:40 -0700 Subject: [PATCH 10/13] Setup spy to observe changes --- modules/__tests__/matchRoutes-test.js | 29 ++++++++++++++++----------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index bf529ee2a9..b78383d44f 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -291,11 +291,10 @@ describe('matchRoutes', function () { }) describe('an asynchronous JSX route config', function () { - let getChildRoutes, getIndexRoute, jsxRoutes, jsxNestedRoutes, state + let getChildRoutes, getIndexRoute, jsxRoutes, makeJsxNestedRoutes beforeEach(function () { getChildRoutes = function (partialNextState, callback) { - state = partialNextState setTimeout(function () { callback(null, ) }) @@ -313,15 +312,19 @@ describe('matchRoutes', function () { getChildRoutes={getChildRoutes} getIndexRoute={getIndexRoute} /> ]) - - jsxNestedRoutes = createRoutes([ - - - - ]) + + makeJsxNestedRoutes = () => { + const spy = expect.spyOn({ getChildRoutes }, 'getChildRoutes').andCallThrough() + const routes = createRoutes([ + + + + ]) + return { spy, routes } + } }) it('when getChildRoutes callback returns reactElements', function (done) { @@ -342,7 +345,9 @@ describe('matchRoutes', function () { }) it('when getChildRoutes callback returns partialNextState', function (done) { - matchRoutes(jsxNestedRoutes, createLocation('/users/5/details/others'), function () { + 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() From f566bf4af0769f4ce72a36bb4cfc3377f7769d68 Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 22:40:22 -0700 Subject: [PATCH 11/13] Fix extra white space --- modules/__tests__/matchRoutes-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index b78383d44f..6049a14f95 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -310,7 +310,7 @@ describe('matchRoutes', function () { + getIndexRoute={getIndexRoute} /> ]) makeJsxNestedRoutes = () => { From 56353f4f539e5f8f0a9f7c221ede40fc79c80fbc Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 22:41:24 -0700 Subject: [PATCH 12/13] Hopefully last one --- modules/__tests__/matchRoutes-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/__tests__/matchRoutes-test.js b/modules/__tests__/matchRoutes-test.js index 6049a14f95..73db3ce958 100644 --- a/modules/__tests__/matchRoutes-test.js +++ b/modules/__tests__/matchRoutes-test.js @@ -325,7 +325,7 @@ describe('matchRoutes', function () { ]) return { spy, routes } } - }) + }) it('when getChildRoutes callback returns reactElements', function (done) { matchRoutes(jsxRoutes, createLocation('/users/5'), function (error, match) { From 2aa9a9ea1079c64466cf9ebeec88fb1f65b0f644 Mon Sep 17 00:00:00 2001 From: Anthony van der Hoorn Date: Mon, 20 Jun 2016 22:43:05 -0700 Subject: [PATCH 13/13] Nope 2 more --- modules/getComponents.js | 2 +- modules/matchRoutes.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/getComponents.js b/modules/getComponents.js index 7caa6c0b73..d38bb3e8e4 100644 --- a/modules/getComponents.js +++ b/modules/getComponents.js @@ -1,6 +1,6 @@ import { mapAsync } from './AsyncUtils' import { canUseMembrane } from './deprecateObjectProperties' -import deprecateLocationProperties from './deprecateLocationProperties' +import deprecateLocationProperties from './deprecateLocationProperties' function getComponentsForRoute(nextState, route, callback) { if (route.component || route.components) { diff --git a/modules/matchRoutes.js b/modules/matchRoutes.js index 1a9b3b6f12..be5377ca02 100644 --- a/modules/matchRoutes.js +++ b/modules/matchRoutes.js @@ -3,7 +3,7 @@ import { loopAsync } from './AsyncUtils' import { matchPattern } from './PatternUtils' import { createRoutes } from './RouteUtils' import { canUseMembrane } from './deprecateObjectProperties' -import deprecateLocationProperties from './deprecateLocationProperties' +import deprecateLocationProperties from './deprecateLocationProperties' function getChildRoutes(route, location, paramNames, paramValues, callback) { if (route.childRoutes) {