Make a higher-order component to get the router from context:

[ryanflorence]

When React documented context we were like "YAY! we can just tell everybody to use context for the router." But we were probably over-confident in its stability and people's comfort with that API. I think we can wrap up everything into a higher-order component like:

import { withRouter } from 'react-router'
withRouter(SomeComponent)

// in SomeComponent
this.props.router

• We'll shield our users from context changes in the future and save much JSF
• We won't have to document the infinite ways that React + JavaScript + Babel can get router from context (Expanded API docs for context.router #3346)

We could probably even provide a this.props.confirmOnLeaveRoute shortcut like the Lifecycle mixin used to provide.

Whose with me?

(yes, this has been talked about before, thanks to those who tried to get this in before :)

Potential names:

• connectRouter like Redux. Seems weird to me, not sure if that name is about connecting to the store, or to the data in the store.
• injectRouter like React Intl. I'm also iffy on this since it's not technically "injection", you're asking for it, not injecting it.
• I like withRouter but whatever ... is there a community name for this yet? If not, can we start lobbying for this one?

[timdorr]

Adding it now...

[gaearon]

Yes please
(I like withRouter!)

[ryanflorence]

While we're on the topic, this is my new fav for context: https://gist.github.com/ryanflorence/1e1290571337ebcea1c5a748e8f5b37d

[ryanflorence]

@timdorr awesome :)

[benlesh]

What is the migration strategy for those currently using context? Will there be depreciation warnings with guidance?

[timdorr]

@Blesh Keep using it. This is just a convenience.

[gaearon]

This would be an addition—not breaking existing code. However I’d encourage to use it in case React changes the exact context API.

[ryanflorence]

@Blesh We can't deprecate React's context API, and we'll continue to provide the same object on context as we provide in withRouter, so both APIs are valid. The only problem would be if we decide to provide something different in withRouter than the object we put on context, which is hard for me to ever imagine being the case.

I'd probably suggest using withContext though, so when React change's context again, you can update to the latest React and Router w/o changing your own components.

[dlmanning]

Sounds oddly familiar

[ryanflorence]

@dlmanning

(yes, this has been talked about before, thanks to those who tried to get this in before :)

Was thinking of you :P At the time I don't think what we provided on context was as clean as it is now.

[Download]

I also like withRouter. Simple and clear.

Would it be possible to make this component in such a way that it can be used as a decorator (but doesn't have to be)? I really like how that works with connect from redux-router. I can just see it now:

@connect(myApp.todos.connector)
@withRouter
class Todos extends React.Component {
  clickHandler() {
    this.props.router.push('/yeah');
  }
  // ...
}

The name connectRouter doesn't really make sense. In Redux that term implies that we are listening to changes from the store. withRouter is better (and shorter! :)

[benlesh]

We can't deprecate React's context API,

Yeah, it's problematic that it's not opt-in when it comes to wanting to deprecate usage.

@ryanflorence couldn't you return a proxied router instance from getChildContext that puts a deprecation message in console while in development mode when its methods are called?

getChildContext() {
   var proxiedRouter = {
      actual: router,
      push() {
        console.warn('using router from context is deprecated, please use `withRouter` (docs link here)');
        this.actual.push.apply(this.actual, arguments);
      }
   };
}

(Sorry, I'm not familiar with the code-base, it's just a suggestion).

[taion]

I'd prefer we keep context.router around. I understand the appeal of withRouter, and am 👍 on adding it, but I'd prefer to continue using context.router myself.

We just don't want to force all users to use context.

[benlesh]

I'd prefer we keep context.router around.

Multiple ways to do the same thing might be problematic on larger codebases and bigger teams.

[nfcampos]

should this be included in the router when it is available here?
https://github.com/acdlite/recompose/blob/master/src/packages/recompose/getContext.js

const withRouter = getContext({ router: React.PropTypes.object })

[timdorr]

I'd prefer we keep context.router around.

Multiple ways to do the same thing might be problematic on larger codebases and bigger teams.

This is my thinking too. But I'm in the camp of keeping context.router and not introducing props.router.

[ryanflorence]

@nfcampos that's a great question.

I have no way of knowing our users as compared to react at large, but we have 1/2-2/3 the downloads of react from npm. I think that means a lot of folks are coming to React and Router together, and it would be nice to not send them on dependency goose-chase to use our API.

"Before you can navigate around in your component, you need to install recompose and then create a withRouter higher order component with the getContext higher order function, and then create your component with it"

Is a lot to ask a new user.

[taion]

@Blesh

I think that's on you (or your architects, rather).

As it stands, we already support both doing e.g. context.router.push and browserHistory.push, and that's in some sense much worse than just small questions over how you ultimately get the router object from context.

I'm very much 👍 on not forcing users to use a feature documented as "experimental" just to be able to call isActive, but I don't really want to add another component into my render hierarchy just to be able to call isActive myself.

[ryanflorence]

I think all of our docs should use withRouter and this.props.router everywhere, with one note that says "If you want to get router from context, it's the same object as the one from withRouter, so knock yourself out."

[timdorr]

"If you want to get router from context, it's the same object as the one from withRouter, so knock yourself out."

Hmm, this is true. It's not like we're wrapping up props.router in some special sauce. It's literally the exact same thing.

I just don't want to do all that rewriting... 😫

[nfcampos]

@ryanflorence Good point, probably worth it to add it in then.

[Download]

I agree with @timdorr. I think introducing props.router would be a bad idea.

First of all, there is a reason 'we' (i you'll permit me :) have router on context right now. It sucks to have to pass it down manually via props. The good folks at Facebook recognized this problem and introduced a specific mechanism to deal with this. React router jumped on that (and rightly so I think).

The fact that stateless function components are the recommended way to write React components now and have a context argument next to props is a testament as to how big Facebook's commitment to context really is. Removing it or changing it in some big way would potentially break all those new components.

Currently, React Router places a router object on context that we can use. Almost as easily as props. Almost, because the contextTypes static is required, as opposed to the propTypes. This makes use of context declarative.

By making a withRouter that just adds that contextTypes static, we just add sugar for doing something the exact way it was intended by React.

You want composition because the idea is to make consumers not care about context.

If you want to avoid the side effect, you can make a HoC that has childContextTypes and getChildContext and achieve the same thing. I don't really care but I respect that it could be important.
EDIT: I don't think it can be done actually. :(

Either way, please make sure the child component can just use context.router. Because the effort we would have here to change all the docs would be echoed on their end to change all their code.

Furthermore, choosing for props.context would lock you in to withRouter (effectively, or write the boilerplate yourself). But how is that better than being locked in to context?

My reasoning is that, given React's user base, context is more dependable as an API than withRouter. Using context directly keeps my component a standard component whereas using props means that now I am almost forced to wrap it with withRouter. Instead of a 'dependency' on context I now have a 'dependency' on withRouter.

EDIT:
I realize now I made a mistake in my thinking. We can't sugar the use of context.router without side effects.

Still, it begs the question if it's important enough to split the API so to say? I think it will be confusing to see code using context.router in one file and props.router in another,

[taion]

The idea here is just to do what Redux, React Intl, and most other libraries do.

If you use withRouter (either as an explicit HoC or as a decorator if declaring your component as a class), you get the router object passed to you as props.router. That context is involved at all is completely hidden from you.

You may not feel that withRouter is necessary for you – in which case you should feel free to continue to use context.router. In fact I will continue to use context.router myself. However, we don't want context.router to be the only API, given the following warnings in the React docs:

Context is an advanced and experimental feature. The API is likely to change in future releases.
...
If you have to use context, use it sparingly.
Regardless of whether you're building an application or a library, try to isolate your use of context to a small area and avoid using the context API directly when possible so that it's easier to upgrade when the API changes.

[Download]

@taion I get it and I even sorta like it. From the perspective of the author of that component, it's great!

The problem comes when we mix and match components from different authors. Some like withContext and use it all over, others stick to grabbing the router from context...

I just have this feeling coming up that when I copy and paste some code, I'm gonna keep having to search-replace props.router with context.router and vice versa. And why exactly? What is the gain?

Personally I think either would be fine, but having both is sorta... Yuck. And since it's on context right now, why not just leave that?

Re the 'experimental' quote. Mmm I'm not sure what to think of that.

Having the signature for the recommended way to writing components be

function MyComponent(props, context) {
  // render here using props and.... context!
}

... and then acting like the API could change at any time is quite at odds with each other in my book. And in situations like these I tend to go with the code and not the docs. Whether Facebook likes it or not, changing context in any significant way right now will break a lot of code. Including all code that uses React Router.

[ryanflorence]

The fact that stateless function components are the recommended way to write React components now

Nope. There are tons of use-cases for component state, you might be reading too many blog posts :P

I just have this feeling coming up that when I copy and paste some code, I'm gonna keep having to search-replace props.router with context.router and vice versa. And why exactly? What is the gain?

Our docs will say props.router everywhere. If you use context, you've bailed from the React Router happy path.

We've been traveling down the context-as-our-public-api-not-just-implementation road you're talking about, we have bruises, aches, and pains, and we might need our left leg removed.

We look over at redux and react-intl and they don't have any bruises, but still get to use context for their libs.

Had we used this API in the first place:

1. almost all of the [3.0] Remove deprecated code #3336 deprecations would not have had to happen
2. Expanded API docs for context.router #3346 wouldn't have come up
3. a bajillion questions in our issue tracker and mine and michael's workshops wouldn't have come up
4. I'm aware of a future change to context in React that will affect everybody using context, and I'd like to get people ready now, rather than later. When it happens, we can make a minor release to router and everything is good, v. us having to make a major release and everybody having to update their code all over the place.

[Download]

Nope. There are tons of use-cases for component state, you might be reading too many blog posts :P

React API documentation, actually:

In an ideal world, most of your components would be stateless functions because in the future we’ll also be able to make performance optimizations specific to these components by avoiding unnecessary checks and memory allocations. This is the recommended pattern, when possible.

https://facebook.github.io/react/docs/reusable-components.html#stateless-functions
(emphasis mine)

[taion]

I'll also add that I can't think of any React Router extensions that expose their use of the router context object, either via context directly or via the wrapper. Most of them fully encapsulate that use, so you won't even see this inconsistency through pulling in third-party code.

[benlesh]

@ryanflorence ... regarding me previous comments. I was thinking about it last night, and someone could probably make a Babel plugin that warns about context use that would be much more generally useful than per-library deprecation warnings.

So, if you were worried/thinking about what I had said (I don't think you were, but just in case), disregard.

[ryanflorence]

Unless you get rid of context support

We can't. Link needs it.

Because code using context would break

Context is React's API, not ours. Ours is the thing we put on context, however that gets accessed in React isn't our problem, our problem is simply the shape and function of that object.

Aargh! Spill it already, what is it?? :)

This may or may not happen. But context right now has the potential for naming collisions, which would go away if a component was only allowed to provide one context type, rather than multiple contextTypes. Similarly, components would only be able to ask for a single contextType, thus preventing naming collisions (but still allowing shadowing). I don't know if anybody is actively working on it.

[ryanflorence]

@insin was it mooted? where?

[Download]

Sorry I did not mean to confuse like I did :) I get that you don't want to remove support technically.

Just saying that the problems you are citing with context.router will only go away if React Router users end up not using it. So you'd probably need to explicitly warn them (in the docs) that it's use as a React Router API is 'deprecated' and will not be 'supported' (as in issues etc) in the future and can break at any time. Otherwise, using props.router would only add to the surface area, causing more issues / support, not less.

EDIT:

We only have to release a minor version bump, and make the change in withRouter to accomodate the new way context works

Currently, context.router is a public, documented feature of React Router. It should be expected that people are using it. So I don't see how adding an extra API would have the effect of only having to bump the minor version number, unless you 'deprecate' context.router, by saying it should no longer be considered a public API. In strict semver, you should only do that during a major version bump. Once you have 'banned' context.router's use as an API, then yes you could do just a minor version bump. But it would only become possible (within semver) from v3.x onwards.

Taion is suggesting that context.router will remain a public API. If that is indeed true then I don't think bumping only the minor version would work within semver.

Context is React's API, not ours.

Not according to this project's API docs. context.router is listed right there as a public API. This is the downside of exposing parts of the project your project is depending on; you inherit their API as it becomes part of your own API.

EDIT 2:
I realize now what you meant by 'shape of the object we put there'. And yes, if React would change to only one context item per component, then that would be a React problem... sort of. Of course people will probably still make issues here because 'React Router not working with React v16' etc :(

[acdlite]

Just my 2 cents: in my own project, I already use a thing called withRouter() that works exactly like this proposal. It's a one-liner because I'm already using Recompose a la @nfcampos's comment above, but still really handy. So I'm 👍

[taion]

withRouter is a new feature. That's exactly what semver minor bumps allow.

[Download]

I'm 👍 too.

withRouter is a new feature. That's exactly what semver minor bumps allow.

Yup, you are right. But 'deprecating' context.router would require a major bump Before you say that the plan is not to deprecate context.router, I was reacting to Ryan's comment that

Because when React changes how context works (again)
...
If you access it with withRouter
We only have to release a minor version bump, and make the change in withRouter to accomodate.

I'm not too sure about that because if Facebook would, say, rename context to protext (extreme example but bear with me) in v17... Would you say you could just change the docs of context.router to read protext.router and bump a minor version?

I'm hoping you agree that, no, that wouldn't be cool.

unless we would remove all mention of context.router or explicitly list it as 'experimental' or whatever. However, since it's already public without such a notice right now, that would in turn require a major version bump.

It sounds to me that you guys are basically saying you are only adding withRouter but still somehow will not have to do major bumps because of changes to context in the future... Those two are incompatible is what I'm saying. Either ditch context.router (again, as an API, can still be there as impl. detail) and avoid major version bumps, or keep it and do major version bumps.

[Download]

I just realize that the API docs in it's current form don't even mention that Facebook considers context an experimental API... Maybe we should cite their warning?

[insin]

@ryanflorence From The Notghost of Reactmas Yet to Come:

https://twitter.com/sebmarkbage/status/669313585940553728

The big change will be that you can only read one context at a time. No more merged objects. this.context.x.y -> this.context.y

https://twitter.com/sebmarkbage/status/717384903763959808

I think a single HoC is probably the safest way to consume context, in terms of upgradability.

[taion]

This discussion is going way, way off track. Here's the plan:

1. We add withRouter
2. We update the docs to point users at withRouter instead of telling them to use context as the preferred API
3. We continue to support context for users (like me) who are comfortable with that API

As our underlying implementation relies on context, if React makes the aforementioned change to context, we'll need to release a breaking change to be compatible with the new version of React anyway.

There's no difference – this is just adding an API for users who want to avoid potential instability with context.

We're going to try to land this in the next minor release.

[Download]

Ok you're going to cheat :)
(Technically, you are breaking semver by silently removing a part of the public API)

👍

[taion]

No part of the API is being removed. context.router will remain a public API – we'll just point users at withRouter instead, because we expect the majority of users to prefer withRouter over context.router.

[Download]

Yeah, especially after having read those tweets (thanks @insin ) I think I will prefer it as well.

Since it is not getting removed, that means breaking changes to context will still propagate to this project and cause breaking changes here. But probably a lot less people will be affected and that's a very good thing.

[taion]

Any breaking changes to context will require a major version upgrade anyway, since we'd need to change the implementation of e.g. <Link> to work with the changed context, and bump the peer dependency (not that we have one).

The only difference is that if you're using withRouter, you just bump the versions and go, and you won't need to rewrite any of your code – but very likely React Router will have to do a major version bump for a sufficiently significant change in the behavior of context – especially something like that discussed above.

[Download]

The only difference is that if you're using withRouter, you just bump the versions and go, and you won't need to rewrite any of your code

... which in semver is exactly the difference between a major and a minor bump. You could do a minor bump if you would remove context.router from the API. This is what @ryanflorence was hinting about (correct me if I'm wrong).

@taion Sorry for being pedantic but if you read carefully you'll see that you and Ryan are not saying the same thing; Ryan is implying context.router will be dropped from the API, because he is saying he will get to do only minor version bumps. You are saying context.router will stay and that the major version will be bumped.

I'm in your camp :) If React is bumping the major, why would it be strange that React Router follows suit? Keep it around and wait to see what happens with that experimental status before deciding here. Still, a warning in the docs about that experimental status might be good.

[vcarl]

@Download I see many places in this discussion where Ryan says that context.router will stick around and this will be in addition to, not instead of.

we'll continue to provide the same object on context as we provide in withRouter, so both APIs are valid.

[taion]

I don't think you're understanding my point.

If React completely changes how context works in v17, such that we have to rewrite <Link> (and withRouter), and as such make React Router no longer compatible with versions of React < v17, then this really ought to be a major version bump, since we're breaking backward compatibility with older versions of React.

I'd prefer not to do this, and potentially it might not be strictly needed, but we probably want to treat (nontrivial) breaking changes to peer dependencies as breaking changes anyway.

[Download]

but we probably want to treat (nontrivial) breaking changes to peer dependencies as breaking changes anyway.

Great point. I hadn't thought about that.

@vcarl
And there are also many places where he is saying he will get the benefits he can only get when removing it as a public API...

But how is that better than being locked in to context?
Because when React changes how context works (again)

We will have to do a major version bump because we will have to change what we put on context
You will have to change your code

If you access it with withRouter

We only have to release a minor version bump, and make the change in withRouter to accomodate the new way context works
You won't have to change your code.

Somehow, making withRouter set props.router i.s.o. context.router alleviates the need for major version bumps? This implies that context.router would no longer be a public API does it not?

[taion]

It does not alleviate the need for major version bumps.

What it means is that if you use withRouter, it is possible that you might not need to rewrite any of your own code.

But we'd still need to bump the major version, &c.

[taion]

Released on v2.4.0.

[jonaswindey]

how do you get the current location when using withRouter() ?

this.props.router is my router object, but this.props.router.location is undefined

same ngoes for this.props.router.pathname and this.props.router.currentPathname

[plava]

You can access current location via this.props.location

[jonaswindey]

this.props.location is also undefined when using withRouter()

[timdorr]

@jonaswindey @lastnamelava It's available on 3.0.0-beta.1: https://github.com/ReactTraining/react-router/blob/v3/modules/withRouter.js#L33

[mayerwin]

The documentation is not clear at all on how withRouteris supposed to replace browserHistory(if I understood correctly). I wish there was a good canonical way shown on how to use react-router. The tutorial seems outdatded.