Skip to content
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
12 changes: 11 additions & 1 deletion modules/Router.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ class Router extends Component {
history: object,
children: routes,
routes, // alias for children
RoutingContext: func.isRequired,
Copy link
Member

Choose a reason for hiding this comment

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

(nit) This should be camelCased as routingContext.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works for me - I've seen people do both for custom component classes depending on the context and don't have a personal preference.

Copy link
Member

Choose a reason for hiding this comment

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

I suppose it would be different if the DI here was done via module. But then you wouldn't even have this problem in the first place (you'd have others, but that's a different story...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implicitly I'm sorta re-using the module name here: https://github.com/rackt/react-router/pull/2383/files#diff-1063d4db6b1a5033d4afb74421a91ef4R31

It's not extremely clever, but it saves a tiny amount of typing.

createElement: func,
onError: func,
onUpdate: func,
parseQueryString: func,
stringifyQuery: func
}

static defaultProps = {
RoutingContext
}

constructor(props, context) {
super(props, context)

Expand Down Expand Up @@ -80,12 +85,17 @@ class Router extends Component {

render() {
let { location, routes, params, components } = this.state
let { createElement } = this.props
let { RoutingContext, createElement, ...props } = this.props

if (location == null)
return null // Async match

// Only forward non-Router-specific props to routing context, as those are
// the only ones that might be custom routing context props.
Object.keys(Router.propTypes).forEach(propType => delete props[propType])

return React.createElement(RoutingContext, {
...props,
history: this.history,
createElement,
location,
Expand Down
97 changes: 96 additions & 1 deletion modules/__tests__/Router-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import expect from 'expect'
import React, { Component } from 'react'
import { render, unmountComponentAtNode } from 'react-dom'
import createHistory from 'history/lib/createMemoryHistory'
import Router from '../Router'
import Route from '../Route'
import Router from '../Router'
import RoutingContext from '../RoutingContext'

describe('Router', function () {

Expand Down Expand Up @@ -248,4 +249,98 @@ describe('Router', function () {

})

describe('RoutingContext', function () {
it('applies custom RoutingContext', function (done) {
const Parent = ({ children }) => <span>parent:{children}</span>
const Child = () => <span>child</span>

class LabelWrapper extends Component {
static defaultProps = {
createElement: React.createElement
}

createElement = (component, props) => {
const { label, createElement } = this.props

return (
<span>
{label}-inner:{createElement(component, props)}
</span>
)
}

render() {
const { label, children } = this.props
const child = React.cloneElement(children, {
createElement: this.createElement
})

return (
<span>
{label}-outer:{child}
</span>
)
}
}

const CustomRoutingContext = props => (
<LabelWrapper label="m1">
<LabelWrapper label="m2">
<RoutingContext {...props} />
</LabelWrapper>
</LabelWrapper>
)

render((
<Router
history={createHistory('/child')}
RoutingContext={CustomRoutingContext}
>
<Route path="/" component={Parent}>
<Route path="child" component={Child} />
</Route>
</Router>
), node, function () {
// Note that the nesting order is inverted for `createElement`, because
// the order of function application is outermost-first.
expect(node.textContent).toBe(
'm1-outer:m2-outer:m2-inner:m1-inner:parent:m2-inner:m1-inner:child'
)
done()
})
})

it('passes router props to custom RoutingContext', function (done) {
const MyComponent = () => <div />
const route = { path: '/', component: MyComponent }

const Wrapper = (
{ routes, components, foo, RoutingContext, children }
) => {
expect(routes).toEqual([ route ])
expect(components).toEqual([ MyComponent ])
expect(foo).toBe('bar')
expect(RoutingContext).toNotExist()
done()

return children
}
const CustomRoutingContext = props => (
<Wrapper {...props}>
<RoutingContext {...props} />
</Wrapper>
)

render((
<Router
history={createHistory('/')}
routes={route}
RoutingContext={CustomRoutingContext}
foo="bar"
/>
), node)
})

})

})