React-router: New Route children API does not forward RouterContext props

[dylinmaust]

Version

react-router-dom 5.1.2

Test Case

https://codesandbox.io/embed/loving-mcnulty-ygk9u

Steps to reproduce

1. Navigate to Topics route

Expected Behavior

Topics function renders and displays nested routes using the match param provided via Router context.

Actual Behavior

Topics function throws an error because match is undefined. Indeed, no props are forwarded to the Topics function.

The error reads: Cannot read property 'url' of undefined

[leoyli]

I think this is expected. Here is the pattern shifts.

Previously it was using the render prop pattern, or pass component with component prop to inject the routing contexts. Now to make it more declarative, we can use children + hooks. In the code sandbox, the Topic component need to call useRouteMatch hook to get the match object.

[dylinmaust]

Interesting. The CodeSandbox code above is taken directly from https://reacttraining.com/react-router/web/guides/quick-start where match is provided to the child component without any use of hooks. Additionally, I thought the main purpose of useRouteMatch was to replace floating <Route /> components that were used solely for getting match data.

Additionally, it looks like React Router should still be passing the RouterContext props to Route's children, unless I'm misunderstanding the source: https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Route.js#L56

[MeiKatz]

@dylinmaust <Route /> passes the routing props to children if (and only if) children is a function. Any documentation that say something else is wrong and needs to be fixed.

[dylinmaust]

@leoyli Good call! I didn't realize you could just use useRouteMatch without passing params to get the current match. It seems that this is a documentation issue rather than a bug within React Router.

[leoyli]

There are currently 4 patterns:

1. render props, children as a function:

<Route render={(props) => <Child {...props} />} />

<Route>{(props) => <Child {...props} />}</Route>

They are, in general, the same beings.

1. component (the augmentation pattern):

<Route component={Child} />

Please note, it is Child, not <Child />. This one is very useful when we do code splitting with React.lazy.

1. element:

<Route>
  <Child /> {/* call hooks from the Child to get the routing contexts instead. */}
</Route>

1. HOC (withRouter).

This one is very very bad, IMHO. (And the blog post have said to be deprecated.)

So basically, the 1st and 2nd are the old patterns we are used to. The third one is special in the way it now can be passed as an element, and since we did not provide any props, we need to "inject" the contexts from the hooks.

Yes, so I think the doc can be improved...

[MeiKatz]

The 3rd one is the one that will stay in the future. render and component may be dropped in the future, because you can do both also with children.

[leoyli]

@MeiKatz, I think we should consider to keep 2., which keep the API surface consistent too.

<Route component={React.lazy(() => import('...'))}>

Of course, we can assign another local variable and pass as an element, but I would rather to avoid so it become clear which component is async.

[MeiKatz]

@leoyli You're right: we should add a detection for lazy loaded components so we could do something like this:

<Route path="/foo">
  {React.lazy(() => import("./foo"))}
</Route>

[timdorr]

Fixed by #6952. Might just need to be published.