react-router: React Router 4 (beta 8) won't render components if using redux connect
Version
4.0.0-beta.8
If a component containing <Route> components is exported with a Redux connect wrapper non of the components mapped to those routes components are rendered. Let’s say you have
<BrowserRouter>
<App />
</BrowserRouter>
and App looks like:
class App extends Component {
render() {
return (
<div>
<Route exact path="/" component={Home} />
<Route path="/Test1" component={Test1}/>
<Route path="/Test2" component={Test2} />
</div>
);
}
}
export default connect(mapStateToProps, mapDispatchToProps)(App);
then none of the components get rendered on route changes. If you remove the connect wrapper all works as expected.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 104
- Comments: 40 (14 by maintainers)
Commits related to this issue
- Fix React Router not updating routes https://github.com/ReactTraining/react-router/issues/4671 — committed to sharetribe/ftw-daily by kpuputti 7 years ago
- Fix React Router not updating routes https://github.com/ReactTraining/react-router/issues/4671 — committed to sharetribe/ftw-daily by kpuputti 7 years ago
Hey klase, I may be mistaken but I think this is exactly the problem I had these previous days. I was used to the v4 alpha API where doing what you shown above worked fine.
In the latest beta API however they kind of went back to their previous way of doing things (that is using react context exclusively to communicate changes), connect() prevents that from working by ways of shouldComponentUpdate.
They have provided again in the beta the withRouter() HOC that solves this nicely, just import it and do
export default withRouter(connect(mapStateToProps, mapDispatchToProps)(App));This should work just fine after that. Note that you would also need to wrap any redux connected component that has router components inside it with a withRouter(). That means if you have router Links in your redux connected Nav component for example, you would need to wrap it with withRouter() as well.
Hope this helps.
Here is a helpful discussion over on the react-redux repo
This MUST be mentioned very prominently in the docs, it’s an issue many will run into.
I spent a long time scratching my head, trying to figure out why my routes aren’t working.
@timdorr
There’s a guide to help with this: https://github.com/ReactTraining/react-router/blob/v4/packages/react-router/docs/guides/blocked-updates.md
I’m looking into removing this issue from react-redux’s end soon.
I just battled with this for a few hours, reading up on context and the various arguments for why subscriptions are bad/good. I believe that removing subscriptions was a step back for this library, for the reasons mentioned in this thread and elsewhere. I’ll add my 2 cents.
I think it’s worth pointing out that the React docs have an extra warning for trying to update context:
This warning is in addition to the usual “context is not the React way, context has a volatile API and will probably change” warning.
They also link to this blog post, indicating their support that subscriptions are the correct way to handle changing context at the moment. This means that React Router is using a feature that React is actively saying “do not use”, which is similar to using a deprecated feature in my eyes. If this dropped warnings at runtime instead of in documentation, it’d probably be dropped without a second thought.
From a quick glance, I believe implementing a working subscription system is possible, and the problems mentioned in @pshrmn’s article (stale context and re-renders) could be fixed with never changing context and some immutable objects or shallow compares. But I am not that clued-in on the inner-workings of the module so I can’t speak much to that.
However, this line of the article stuck out:
Sometimes, we have to write hacky solutions to problems (called “tricks” here). React’s context feature is kind of broken so this is one of those cases. Abstraction lets us tuck those hacks away and not have to expose them. Removing subscriptions has exposed that hack and requires every developer to implement it in one way or another (albeit wrapped in a single function). We now have to add a “trick” to arbitrary components in our hierarchy, which definitely doesn’t feel right to me (to use the words from the article).
Furthermore on abstraction, developer experience aside, it forces knowledge of implementation details that the consumers of these libraries don’t need to know. Such as:
react-routeruses context to share information with componentsreact-redux(or other libraries) useshouldComponentUpdatefor optimisationThis leads to code that is difficult to understand, maintain and is likely to contain hard-to-find bugs. I believe that this is going against the vision of React Router in a big way, but in general is something we as developers are trying to avoid.
Just wanted to add that you should wrap the
connectHOC withwithRouterand not the other way around. ie.withRouter(connect()(Component))and notconnect()(withRouter(Component))Like all complex things sometimes the right answer is both. As you can tell from react-redux the api supports a few key optional properties (removing pure component logic for instance). I don’t see why this library can’t take a similarly pragmatic approach. Both sides can make compelling cases for whether subscription model is appropriate / desirable / safe / pointless / wasteful etc… I personally am desperately in need of the subscription style but I understand that it has a tradeoff.
Is there any appetite for supporting this via configuration?
I realise this is a closed issue but is there no interest in solving this problem? Or is it a feature? I really like React Router v4 but the way that I would have built every app in the past this feature would break them and I gave up and reverted to v3 for deadline pressure on my project atm.
The above solutions probably will work but feel like patches and or major mixing of concerns (
withRouterfor example).hey if anyone is still blocked by this (I was for a solid 3 hours I hate to admit), a work around I found was to wrap my root component with
withRouterthis allows the location context to be passed to every component. It’s not perfect, but it beats tracking every route down through your component tree. Also I found that using webpack dev server with hot reloading somehow bypasses this issue, so I didn’t run into it until it came time to deploy. Definitely needs to be all over the docs. Thanks for all the help though everyone!@antonmedv - this is actually covered in the documentation - and if it wasn’t - why not try to rectify the perceived problem by submitting a PR?
This is really annoying. Every time need to google to find proper docs hidden in issues on github. 😦
Hi!
I’m having trouble working around this issue like suggested in blocked-updates.md. It seems that a pure component at any level of the component tree will prevent updates for react-router. I’m using react-router 4.0.0.
I have multiple connect()s or other pure components all over my app so it’s not very feasible go and wrap them with
withRouter(). Most of them have otherwise nothing to do with react-router.I wonder why react-router uses context values to propagate the route changes to begin with? React Redux for example circumvents this issue by listening the store on each connect() so nesting it under other pure components does not matter. So why not to listen to the
historyobject on everyRoutecomponent?As a proof of concept I created this workaround (mentioned previously in https://github.com/ReactTraining/react-router/issues/4676#issuecomment-286140154):
Seems to solve the issue very nicely in my app.
UPDATE: This has some issues. Read on…
UPDATE2: React Redux wrapper https://gist.github.com/epeli/ddd916d568ed2c1b30a7c714effabd01
Why not wrap them all into a single container and get it from there? Just one key for the context. No need to pass them individually. Or just regenerate them when
Routemounts from thehistoryobject?You are right about the performance tradeoff but you are also doing another more concerning tradeoff: Compatiblity with the rest of the React ecosystem.
React.PureComponentis part of the React core and React Router does not work with it unless React Router specific hack is added to it. That’s not good tradeoff in my opinion.I threw together this module if anyone else needs a connected route or switch whilst using redux https://github.com/team-griffin/react-router-connected/tree/develop
https://www.npmjs.com/package/@team-griffin/react-router-connected
@hihuz No worries. Believe me, I have seen some terrible comments 😄
Using subscriptions/observables to avoid sCU blocking and to use the context API correctly seems like a best practice. I see that pattern in other major libraries. Recently MUI and JSS both added a theming solution that uses theme listeners and it works beautifully, successfully avoiding the pitfalls of context while providing expected functionality and great DX. RR should not punt on this issue.
But when I put a
<Route>somewhere I feel like I’m saying “but as exception under this always render when the route changes”As stated before that’s not always the choice of the user. Some 3rd party component might make that decision for you. Also in our code bases we would have to put the location prop into dozens of components using React Redux
connect()which otherwise have no references to React Router.@epeli with redux, there is only one object that needs to be referenced, the store. With react router, there is only one history, but each
<Route>also has its ownmatchobject that its children need to reference.If react router implemented a subscription system, every
<Route>would need to have its location-aware children subscribe to it. Then, when the<Route>updates, it would force update the subscribers. The context would need to be re-written so that children “pull” the context value from their parent, because they would not be able to rely onthis.context.(React Router does actually use a subscription system, but only through the
<Router>component.)A complicated subscription solution makes sense for redux because of the frequency of re-renders that the store might cause. Every time an action is dispatched to the redux store, the application needs to re-render. This may very well be 60 or more times per second. This means that react-redux has to minimize the scope of each update to maximize performance. It isn’t as picky as mobx, but it still wants to limit what is updated based on what has changed since the last render.
When does react router require re-renders? When the location changes. That isn’t something we have to worry about happening dozens of times a second (unless someone gets themselves in a redirect loop…). That isn’t to say that react router doesn’t care about efficiency, just that the tradeoff of a complicated subsription system doesn’t justify any performance gains from only updating location-aware components. Instead, react router’s concern is that every component that is location-aware re-renders using this new information. If we can ensure a full component tree re-render (i.e., what React does naturally) by getting components with shallow prop checks to re-render when the location changes, then we can be sure that every location-aware component updates properly.
Oh wow, you’ve got a whole article ready! Now that’s some support, haha. Thank you!
I resolved my issue by breaking up
SKRouteinto two components. I made the connected component a sub-component of<Route>, receiving the route props and eliminating the need forwithRouter.Now the component tree only includes one
Route.Huzzah for composability!
We’re doing a similar thing to @epeli - fortunately we were already wrapping every
Routeso we only have to make the change in one place. The suggested workaround isn’t an option for us - you can’t expect developers on a large codebase to remember to add an unrelated higher order component every timePureComponentorshouldComponentUpdateis used.I’m very grateful for react-router and acknowledge that I don’t necessarily know best but it does seem odd to stick to a feature that the react documentation says “Don’t do it” whilst forcing a workaround on a useful and documented feature.
@hihuz your comment was very helpful to me, appreciate it.
@timdorr if I understand correctly, are you saying that react-router simply isn’t going to be compatible with shouldComponentUpdate? react-redux isn’t the only thing that uses that function. I’d rather not have to use that workaround every time I’ve used
React.PureComponent