remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.02k stars 10.3k forks source link

react-router-redux's push() not rendering new route #4924

Closed dominikbulaj closed 7 years ago

dominikbulaj commented 7 years ago

First of all, sorry if title is misleading.

I've problems with using react-router-redux@next with react-router-dom@4.0.0 I set up all correctly (I'm migrating from V3) and when I dispatch(push(..)) I can see in DevTools action @@router/LOCATION_CHANGE is triggered. Also URL changes. But components are not rendered. However it works without issues with router's Link

Version

react-router-rom 4.0.0 react-router-redux 5.0.0-alpha.4

Steps to reproduce

I'm attaching file with simple App containing 3 stateless components and defined 3 routes. Also there're 3 links using Link component and 3 using react-router-redux' push (links to the same components). I couldn't use codepen since I haven't found react-router-redux and react-redux on cdn. Sorry!

File: https://gist.github.com/dominikbulaj/33c37099a0073278eebbba3a0226f2d2

Expected Behavior

Render components when using react-router-redux

Actual Behavior

I see action @@router/LOCATION_CHANGE being triggered. URL changes but nothing changes on page. Looks like it doesn't push anything to history object.

timdorr commented 7 years ago

Read this guide: https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/guides/blocked-updates.md

dominikbulaj commented 7 years ago

Thanks! Actually this helped me much more: https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/guides/redux.md

danyim commented 7 years ago

I tried wrapping all my connected components with withRouter, but it still doesn't seem to work. My structure is the following:

App.jsx:

<Route component={Home} />

Home.jsx:

class Home extends React.Component {
  ...
}
export default withRouter(connect(mapStateToProps, mapDispatchToProps)(Home))

The URL changes but the route does not render.

wellyshen commented 7 years ago

The export default withRouter(connect(mapStateToProps, mapDispatchToProps)(Component)) doesn't work for me too :(

react-router-rom: 4.0.0 react-router-redux: 5.0.0-alpha.4

danyim commented 7 years ago

@dominikbulaj Could you share how you got your Gist example to work with the withRouter higher order component?

dominikbulaj commented 7 years ago

@danyim in my case in index.js (entry point) I've:

ReactDOM.render(
  <Provider store={store}>
    <App>
      <Router history={history}/>
    </App>
  </Provider>,
  document.getElementById('root')
)

Router is component with ConnectedRouter root element (this comes from react-router-redux

export default ({history}) => <ConnectedRouter history={history}>
  <Switch>
  <Route exact path='/' component={Home}/>
  <Route path="/login" component={Login}/>
  <Route path="/logout" component={Logout}/>
{* ... *}
</Switch>
</ConnectedRouter>

NOTE not every comonent needs withRouter (I'd say I use it in just ~30%)

danyim commented 7 years ago

@dominikbulaj Thanks. We were able to figure it out. The boilerplate we were using nested a BrowserRouter inside of a ConnectedRouter that needed to be passed into the withRouter higher order component.

viennv1709 commented 7 years ago

@dominikbulaj In the latest version of react-router, they removed ConnectedRouter. I also try withRouter but it doesn't working

dominikbulaj commented 7 years ago

@viennv1709 Hmm, I'm still using it. Here's what I import in my Router component:

import { Redirect, Route, Switch } from 'react-router-dom'
import { ConnectedRouter } from 'react-router-redux'

ConnectedRouter is part of react-router-redux (sorry if I confused anyone)

MtDalPizzol commented 7 years ago

If my opinion matters to anyone, I got no luck using withRouter HOC. As @dominikbulaj , I got everything working replacing BrowserRouter with ConnectedRouter from react-router-redux. Simply follow the example in the README. To me, this seems the way to go if you're using Redux. First because it enables time traveling and keeps state in sync. Second and most important, it forces components to re-render since the state changes. This way, you don't need to worry about keeping track of the components you need to wrap with withRoute. I also found this video on YouTube that can clarify things a bit.

naderchehab commented 7 years ago

I figured out the best way to do this using react-router 4. BrowserRouter internally creates its own history object that does not work as expected. That's what was causing the problem for me. I solved it by avoiding BrowserRouter, using Router instead and creating my own history object:

import { Router, Route } from 'react-router-dom';
import createHistory from 'history/createBrowserHistory';

const history = createHistory();

const App= () => (
   <Router history={history}>
      <div>
         <Route path='/mypath' component={<MyComponent history={history} />}
      </div>
   </Router>
);

And then inside MyComponent, use this to navigate to a new URL:

this.props.history.push('/myurl');
josephMG commented 7 years ago

@dominikbulaj Do you call createHistory() multiple times? I created History from different files and I had same issue. And I solved it after I import history from myHistory.js file.

// myHistory.js
import createHistory from 'history/createBrowserHistory';
const history = createHistory();
export default history;

import myHistory.js anywhere you called createHistory():

// example: AdminRoutes.js
import { ConnectedRouter } from 'react-router-redux';
import history from './myHistory';
import App from '../containers/AppContainer';

/////////////////////////////////// I did create here, now removed!!
//import createHistory from 'history/createBrowserHistory';
//const history = createHistory(});
///////////////////////////////////

const AdminRoutes = (props) => {
    return (
           <ConnectedRouter history={history}>
                <App muiTheme={muiTheme} {...props} />
        </ConnectedRouter>
    );
}
dominikbulaj commented 7 years ago

@josephMG no I call createHistory() once than pass it to routerMiddleware from react-router-redux

So my example is as follows:

// index.js

import createHistory from 'history/createBrowserHistory'
import { Provider } from 'react-redux'
import configureStore from './configureStore'
// ...

const history = createHistory() // <-- createHistory() call
const store = configureStore(history) // <-- createHistory() response passed to create store

ReactDOM.render(
  <Provider store={store}>
    <App>
      <Router history={history}/>
    </App>
  </Provider>,
  document.getElementById('root')
)

And inside configureStore I have just:

// configureStore.js

import multi from 'redux-multi'
import thunkMiddleware from 'redux-thunk'
import { routerMiddleware } from 'react-router-redux'
// ...

const configureStore = (browserHistory) => {

  let middleware = [
    thunkMiddleware, // lets us dispatch() functions
    routerMiddleware(browserHistory), // <-- here I put createHistory() response/value
    multi
  ]

    return createStore(
    reducers,
    applyMiddleware(
      ...middleware
    ))
}

export default configureStore
s-chand commented 7 years ago

@naderchehab's solution worked for me. Wohoo! What a day!

codeBelt commented 7 years ago

@MtDalPizzol solution worked for me. Check out the video.

Basically instead of using: import {BrowserRouter as Router} from 'react-router-dom';

Use: import {ConnectedRouter as Router} from 'react-router-redux';

<Provider store={props.store}>
    <Router history={history}>
    </Router>
</Provider>
naeem-gitonga commented 7 years ago

@naderchehab first off thanks for commenting your post helped me to think in the right direction but there is an error in your snippet <div> <Route path='/mypath' component={<MyComponent history={history} />} </div> that will throw an error for sure since is one glaring syntax error in the code a missing /> to close your <Route /> tag

I fixed mine like so

`render() { return (

);` 

and in the component I did this:

<h1 className='title link' onClick={() => { if (x.user) { x.history.push(/home/${x.user._id}) } else{x.history.push('/')} }}> Connex </h1>

it worked like a charm once i passed the history in as a prop. once again thanks @naderchehab for helping me think in the right direction

eseQ commented 6 years ago
<Router history={history}>
    <App>
      <Switch>
        <Route exact path="/" component={indexComponent} />
        <Route path="/login" component={LoginView} />
        <Route path="/partner" render={renderPartner} />
      </Switch>
    </App>
</Router>

In case if someone has a code like this, it's important to use WithRouter with a parent component, for example, App. So, the parent component will be able to detect changes in location object.

johndevedu commented 6 years ago

@naderchehab 's solution worked for me as well. @timdorr Is there a plan to possibly reopen this issue? We did not have any shouldComponentUpdate or PureComponents in the component tree that were affecting this issue. withRouter alone did not help, but switching from BrowserRouter to Router w/withHistory definitely worked.

bmmathe commented 6 years ago

Since you guys are talking about react-router-redux is using contextTypes not the preferred solution anymore?
MyComponent.contextTypes = { router: PropTypes.object };

Then you just simply call it like this: this.context.router.history.push(location);