React-router: Docs: clarify that <BrowserRouter> must (currently) have one child

[tj]

Just started porting to v4 and ran into vendor.bundle.js:8780 Uncaught Invariant Violation: React.Children.only expected to receive a single React element child. and its crazy stack trace with the following:

<BrowserRouter>
  <Match exactly pattern="/" component={_ => <div>test</div>} />
  <Miss component={NotFound} />
</BrowserRouter>

Otherwise it's looking sweet so far! Not really a big deal but React's stack traces are a little insane so it might be worth the mention.

On a side note it would be kind of cool to allow for:

<Match pattern="/docs">
  <h1>Whatever</h1>
  <Match ...>
  <Match ...>
</Match>

[tj]

Nevermind, I guess that doesn't make sense if you can't get at the props!

[ryanflorence]

We just need React.Children.only(this.props.children) ... or allow <BrowserRouter/> to wrap its children in a div.

[rickharrison]

I'm trying to learn from the codebase and I just ran into this issue as well. For educational purposes, why do you need to call React.Children.only in StaticRouter instead of just rendering {this.props.children} ?

[alisd23]

@rickharrison React elements must have one top-level element. You can't do the following:

const MyComponent = () => (
  <One />
  <Two />
)

Instead you'd have to do this:

const MyComponent = () => (
  <div>
    <One />
    <Two />
  </div>
)

React.Children.only throws a meaningful error if anything other than a single element is passed down. Without that it would still bomb anyway.

[rickharrison]

I understand that, but this isn't the return of a function? It is as a child of another react element.

<BrowserRouter>
  <Match exactly pattern="/" component={Foo} />
  <Match pattern="/bar" component={Bar} />
</BrowserRouter>

[timdorr]

@rickharrison Normally, we end up near the top of the render tree, so you'll see something like this:

ReactDOM.render(
  <BrowserRouter>
    <Match pattern="/baz" component={Foo} />
    <Match pattern="/baz" component={Bar} />
  </BrowserRouter>,
  docRoot
)

That would return multiple elements at the root and React will throw an error about that. This is a short circuit to prevent that from happening.

[rickharrison]

Ahhh ok. And that is because BrowserRouter renders StaticRouter, which uses LocationBroadcast and MatchProvider which render their children directly? So BrowserRouter doesn't actually result in any dom elements, which is why there are 2 top level parents?

EDIT: Is this something that could be detected and then react-router automatically wraps it in a div? I had some trouble figuring out this error as I couldn't figure out what wasn't working as I thought there was just one top level component, the BrowserRouter. I'd love to contribute this change if it's technically possible so other people don't have this error :)

[ryanflorence]

We don't want react-dom to be a dependency (so it works for react-native effortlessly). So, we don't want to be creating any dom elements. But browser router could, I'm still back and forth about splitting into three packages :\ (react-router, react-router-dom, react-router-native)

[timdorr]

I think someone else suggested it, but what about making that a prop on StaticRouter?

StaticRouter.defaultProps.wrapperComponent = 'div'

const { wrapperComponent:WrapperComponent, children } = this.props
if (WrapperComponent) return (<WrapperComponent>{children}</WrapperComponent>)
else return React.Children.only(children)

Then you could set wrapperComponent={null} in environments where you don't have a DOM or don't want/need a wrapping div.

[ryanflorence]

interesting

[rickharrison]

@timdorr Would that be a default, or would users need to set StaticRouter.defaultProps.wrapperComponent = 'div' themselves ?

Could we detect if react-dom exists somehow and then set that automatically?

[sikanhe]

What about having a <NativeRouter>and <BrowserRouter> where the BrowserRouter auto wraps <div> and maybe NativeRouter auto wraps with <view>? I really really think we should not have seperate packages for this. Even import from 'react-router/native' is better.

[ryanflorence]

we do React.Children.only() now, so you'll get a warning. Maybe we'll add automatic wrapping later, but for now we're just going to keep it as it is.