React-router: [3.0] Put `params`, `location`, and `route` on `context.router`

Created on 15 Apr 2016  Â·  23Comments  Â·  Source: ReactTraining/react-router

These are all really useful deep in the tree, especially when building links. Instead of asking folks to take them from their route components and put them on context there, we should probably just do it.

Thoughts?

feature

Most helpful comment

This is all on next except for route

All 23 comments

We want to be careful about what we mean by location – the location that the router sees is not the location that history sees, in the case of an async transition.

Otherwise it probably makes sense to put these on the context.

That location is the one after we have resolved the async stuff, correct? That would seem to be safer than history's, since it doesn't ever get out of sync with what the router is attempting to display. (re: the react-router-redux 4.0 changes).

"the location we used to render", whichever one RouterContext got from Router.

Makes sense to me. @timdorr, you'll want to add a caveat in React Router Redux that this won't match whatever location ends up in the Redux store (err, does React Router Redux still have an option to do that?).

Looking at the code, this will be a lot simpler after 3.0. Not that it's _hard_, it's just that where we provide context is pretty messy from all the deprecations. Maybe we should just ship this w/ 3.0 when we can clean all that stuff up.

May 9th is when we can release 3.0, so maybe let's figure out everything we want for the next minor release, release it, and then make a 3.0 branch to start removing deprecations and such (use a branch so that we can do some bug fixe releases from master if we need to).

What do we think of putting router on the props for route components as well, then?

Since we're doing that via withRouter, it makes sense to get it for "free" with a route component.

3442

We’ll be putting route later right? We can’t do it yet.

I think we defer that until we do relative links... they just come together really naturally.

Whoops, forgot about 'route' (and Dre...)

We can track this in a separate issue since it’s implemented separately and after 3.0?

It'd be really great if we could get relative links in 3.0... just a question of time. Ideally they'd be part of core rather than an add-on, which would be a breaking change (but one that wouldn't break much).

Can we track the remaining bit on the "relative <Link to>" issue? From a code perspective, they work out very similarly.

What do we think of putting router on the props for route components as well, then?

Since we're doing that via withRouter, it makes sense to get it for "free" with a route component.

Any chance this a concrete goal for 3.0? Having to wrap my routed components with withRouter so they can do router.push feels awkward when there's other router props already injected by RouterContext.

This is all on next except for route

I don't think I see router on props. It would be here right? https://github.com/reactjs/react-router/blob/next/modules/RouterContext.js#L57-L63

I could take a stab at it if you'd take a PR.

Sounds reasonable to me, but you'll need one more person to approve the change.

We want to bring withRouter() to have parity with route handlers right? It would make sense to put .router there as well then.

My thoughts exactly. Sounds like that's the requisite 2 votes, then.

:+1: from me so we have something "in-house"

Yeah the original justification is that we didn't want router in two places but that's irrelevant now that we have withRouter.

I'm closing this for now, given that the work to put route on context.router is closely tied to the relative link work.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

imWildCat picture imWildCat  Â·  3Comments

misterwilliam picture misterwilliam  Â·  3Comments

Waquo picture Waquo  Â·  3Comments

stnwk picture stnwk  Â·  3Comments

andrewpillar picture andrewpillar  Â·  3Comments