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

Make params available to getChildRoutes providers #3556

Merged
31 changes: 31 additions & 0 deletions modules/deprecateLocationProperties.js
Original file line number Diff line number Diff line change
@@ -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
20 changes: 2 additions & 18 deletions modules/getComponents.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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 }
}
Expand Down
20 changes: 17 additions & 3 deletions modules/matchRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
}
Expand All @@ -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 ]
Expand Down Expand Up @@ -160,7 +162,19 @@ function matchRouteDeep(
}
}

const result = getChildRoutes(route, location, onChildRoutes)
const progressState = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases the route will not use this object; no reason to do stuff like call createParams unless we know we need to call getChildRoutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so you would like the call to createParams shifted to between lines 15-18?

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)
}
Expand Down