supasate / connected-react-router

A Redux binding for React Router v4
MIT License
4.73k stars 593 forks source link

Push action not working properly #260

Open jancama2 opened 5 years ago

jancama2 commented 5 years ago

Package version: connected-react-router@6.3.1 I have: <Route path="/" component={HomePage} exact />

In saga I do push("/") and the location is changed but HomePage is not rendered. When I change in react-devtools exact on the Route to false, HomePage is rendered.

Downgrading to connected-react-router@6.0.0 helped me to solve the problem.

victorhqc commented 5 years ago

I'm having the same problem. Downgrading to 6.0.0 worked for me. Either documentation is not updated and we're doing something wrong, or there's a bug 🐛

cdtinney commented 5 years ago

Downgrading to 6.0.0 worked for me too.

acaravia commented 5 years ago

I downgraded to 6.0.0 and push now works however I run into the issue reported in #242. So I've got to chose at this point which issue I can live with till this gets resolved.

jancama2 commented 5 years ago

Btw version 6.0.0 works just fine while version 6.1.0 makes push and other action creators to break.

jancama2 commented 5 years ago

Using connected-react-router@6.3.1. together with react-router-dom@4.4.0-beta.7 solved all the problems for me. It solved #242 too, I guess because new react-router uses the new context API. It even the solves blocking updates issues react-router-dom@4.3.1 have.

Even tho it is just a beta it looks stable and I highly recommend to try it. Looks like this issue can be closed.

acaravia commented 5 years ago

@jancama2 upgrading to connected-react-router@6.3.1 together with react-router-dom@4.4.0-beta.7 now breaks my implementation with following errors:

image

and

image

Below is a little bit of my code:

App.js

import React from 'react';
import { connect } from 'react-redux';
import { ConnectedRouter } from 'connected-react-router'
import Routes from '../_components/routes';
import NavigationDrawer from '../_components/Navigation/NavigationDrawer'
import { MuiThemeProvider, createMuiTheme } from '@material-ui/core/styles';
import { withTheme } from '@material-ui/core/styles';
.....
render() {
        const { history } = this.props;
        return (
            <MuiThemeProvider theme={theme}>
                <ConnectedRouter history={history}>
                    <div className="App-container">
                        <NavigationDrawer>
                            <Routes/>
                        </NavigationDrawer>
                    </div>
                </ConnectedRouter>
            </MuiThemeProvider>
        );
    }

I'm using withRouter in NavigationDrawer to get the location information since it is outside Route information for setting navigation styling has:

NavigationDrawer component uses MaterialUI components:

import React from 'react';
import PropTypes from 'prop-types';
import { withStyles } from '@material-ui/core/styles';
import { Drawer, CssBaseline, MenuList, MenuItem, } from '@material-ui/core';
import { connect } from 'react-redux';
import AppHeader from './AppHeader';
import { Link, withRouter } from 'react-router-dom';
import {compose} from 'recompose';
......
export default compose(
    connect(mapStateToProps),
    withRouter,
    withStyles(styles)
)(NavigationDrawer);

my routes are pretty standard (Routes.js):

import React from 'react';
import { Route, Switch } from "react-router-dom";
...
export default function Routes(props) {
    return (
        <div>
            <Route path="/callback" exact component={LoginCallback} />
            <Switch>
                <PrivateRoute path="/" exact component={LandingPage} />
                ...
                <PrivateRoute path="/changepassword" exact component={ChangePassword} />
            </Switch>
        </div>
    );
}

any thoughts?

jancama2 commented 5 years ago

Dont you have react-router installed twice with diff versions? Would try "npm dedupe"

acaravia commented 5 years ago

@jancama2 unfortunately I don't have different versions.

jancama2 commented 5 years ago

@acaravia try this one https://github.com/ReactTraining/react-router/releases/tag/v4.4.0-beta.1

mpigsley commented 5 years ago

Just ran into this today. connected-react-router@6.3.1 with react-router-dom@4.3.1 is not working properly. Downgraded to v6.0.0 fixed the issue.

acaravia commented 5 years ago

@jancama2 still does not work I get a different set of errors:

image

image

I don't reference location in my route.js but I do reference it in my NavigationDrawer to get the selected location. I read this on beta 0 release notes:

We also made it explicit in this release that you can't use contextTypes to access properties on this.context.router anymore. If you try, you'll get a warning. That's our private API! If you need stuff on context, just use a or withRouter instead. It's all the same stuff, and that's our public API

I see in the ConnectedRouter code that it is trying to access context directly through props:

const ConnectedRouterWithContext = props => {
    const Context = props.context || ReactReduxContext

    if (Context == null) {
      throw 'Please upgrade to react-redux v6'
    }

    return (
      <Context.Consumer>
        {({ store }) => <ConnectedRouter store={store} {...props} />}
      </Context.Consumer>
    )
  }

could this be the issue? As it stands right now using I'm using the latest official versions of connected-react-router and react-router-dom and my application works with the exception of push functionality. If I upgrade to any of the the v4.4.0 beta versions of react-router-dom it all breaks. If I downgrade connected-react-router to 6.0.0 I still get issue #242, so I feel a little stuck. :)

jancama2 commented 5 years ago

Well, they access props.context to get the consumer, I don't think it has something to do with not accessing this.context.

Have you tried to debug it using some minimal repo with your use case? (Would suggest trying to do it in codesandbox.io).

I have these deps and everything works just fine.

        "antd": "3.x",
        "connected-react-router": "^6.3.1",
        "copy-to-clipboard": "^3.0.8",
        "fela": "10.x",
        "fela-monolithic": "10.x",
        "fela-plugin-fallback-value": "^10.2.1",
        "fela-plugin-friendly-pseudo-class": "10.x",
        "fela-plugin-named-keys": "10.x",
        "fela-plugin-unit": "10.x",
        "history": "4.x",
        "localforage": "1.x",
        "lodash": "4.17.x",
        "normalizr": "^3.3.0",
        "query-string": "^6.2.0",
        "react": "^16.8.2",
        "react-dom": "^16.8.2",
        "react-fela": "10.x",
        "react-intl": "2.x",
        "react-redux": "6.x",
        "react-router-dom": "^4.4.0-beta.7",
        "react-virtualized-auto-sizer": "^1.0.2",
        "react-window": "^1.5.2",
        "redux": "4.x",
        "redux-form": "^8.1.0",
        "redux-saga": "0.x",
        "redux-sentry-middleware": "0.x",
        "reselect": "4.x",
acaravia commented 5 years ago

@jancama2 I'll see about getting something in codesandbox. My app is a good size so I'll have to find time to do it here soon. The thing is it works with connected-react-router 6.3.1 and react-router-dom 4.3.1 so something was done to break this existing functionality.

jerrygreen commented 5 years ago

I agree it's broken with the pair of:

"connected-react-router": "6.3.1",
"react-router-dom": "4.3.1",

However, its works in:

"connected-react-router": "6.0.0",
"react-router-dom": "4.3.1",

It also works in:

"connected-react-router": "6.3.1",
"react-router-dom": "4.4.0-beta.7",

I've chosen the last one. Thanks for the issue and the info!

acaravia commented 5 years ago

@jancama2 here is a link to a quick sandbox I created that is throwing the same error I get now. When I change react-router-dom to use v4.3.1 it works. I've left it at v4.4.0-beta.7 for now so you can see the issue.

https://codesandbox.io/s/3kv928j1r1

jancama2 commented 5 years ago

@acaravia The problem is that you have specified react-router version in package.json to be 4.3.1. Install 4.4.0-beta.7 too or not install any. It will be installed properly by react-router-dom.

acaravia commented 5 years ago

@jancama2 DOH!!! It's usually something simple like that. I really appreciate you helping me out with this. I was pulling what little hair I have left out and I'm bald! :) As far as I can tell looks like all issues are resolved for me using react-router-dom@4.4.0-beta.7.

jancama2 commented 5 years ago

@acaravia No problem, glad to help :))

marko-ignjatovic commented 5 years ago

I still have a troubles with rendering and I am using:

"connected-react-router": "6.0.0", "react-router-dom": "4.3.1"

acaravia commented 5 years ago

@lolz42 if you upgrade to "connected-react-router": "6.3.1" and "react-router-dom": "4.4.0-beta.7"making sure that "react-router": "4.4.0-beta.7"as well. It all works, hope that helps.

marko-ignjatovic commented 5 years ago

@acaravia thanks for reply! I've tried "connected-react-router": "6.3.1", "react-router-dom": "4.4.0-beta.7" and "react-router": "4.4.0-beta.7" and it still doesn't work 🤦‍♂️ I guess that I am doing something wrong. Here is my config

// storeConfig.js

import { createBrowserHistory } from "history";
import { applyMiddleware, compose, createStore } from "redux";
import { routerMiddleware } from "connected-react-router";
import { rootReducer } from "./../reducers/index";
import thunk from "redux-thunk";
const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;
export const history = createBrowserHistory();

export default function configureStore(preloadedState) {
  const store = createStore(
    rootReducer(history), // root reducer with router state
    preloadedState,
    composeEnhancers(applyMiddleware(thunk, routerMiddleware(history)))
  );
  return store;
}
// reducers/index.js

import { combineReducers } from "redux";
import { connectRouter } from "connected-react-router";

const userReducer = (state = {}, { type, payload }) => {
  return state;
};

export const rootReducer = history =>
  combineReducers({
    router: connectRouter(history),
    userReducer: userReducer
  });
// index.js

import React from "react";
import ReactDOM from "react-dom";
import "./index.scss";
import App from "./containers/App";
import * as serviceWorker from "./serviceWorker";
import { ConnectedRouter } from "connected-react-router";
import "./scss/custom.scss";
import { Provider } from "react-redux";
import configureStore, { history } from "./plugins/storeConfig";

const store = configureStore();
ReactDOM.render(
  <Provider store={store}>
    <ConnectedRouter history={history}>
      <App />
    </ConnectedRouter>
  </Provider>,
  document.getElementById("root")
);

// If you want your app to work offline and load faster, you can change
// unregister() to register() below. Note this comes with some pitfalls.
// Learn more about service workers: https://bit.ly/CRA-PWA
serviceWorker.unregister();
acaravia commented 5 years ago

@lolz42 you check you don't have duplicate installs of react-router and react-router-dom with different versions?

I noticed is you have your configureStore setup as a function and the initial state of your store isn't being initialized since you never pass anything into preloadedState parameter. Maybe set a default (i.e. preloadedState = {}.

marko-ignjatovic commented 5 years ago

@acaravia

"dependencies": { "bootstrap": "^4.3.1", "connected-react-router": "^6.3.1", "firebase": "^5.9.0", "formik": "^1.5.1", "node-sass": "^4.11.0", "react": "^16.8.4", "react-bootstrap": "^1.0.0-beta.5", "react-custom-scrollbars": "^4.2.1", "react-dom": "^16.8.4", "react-redux": "^6.0.1", "react-redux-firebase": "^2.2.6", "react-router": "^4.4.0-beta.7", "react-router-dom": "^4.4.0-beta.7", "react-scripts": "2.1.8", "redux": "^4.0.1", "redux-firestore": "^0.7.2", "redux-thunk": "^2.3.0", "yup": "^0.27.0" },

I also tried to put default state but still doesn't work

marko-ignjatovic commented 5 years ago

Finally solved! This commented helped me a a lot https://github.com/supasate/connected-react-router/issues/159#issuecomment-442688247 😄

rivernews commented 5 years ago

Just to add one more successful case - previously push() won't work in my saga either, but now it navigates successfully. It turns out the way I dispatch in saga is wrong.

Instead of:

...
put(push("/home/"));
yield put(SuccessAuthAction());
...

You should write

...
yield put(SuccessAuthAction());
yield put(push("/home/"));
...

yield [put(SuccessAuthAction()), put(push("/home/"))]; won't work as well.

About version - I use React + Typescript with "react-router-dom": "^5.0.0", "connected-react-router": "^6.3.2", - all latest at this point.

I benefit a lot from what people mentioned above, but particularly I want to add - you do have to use <ConnectedRouter> over <Router>, at least with the same tech stack & version, otherwise navigation with push() won't work.

evandrogrm commented 5 years ago

Worked for me with these packages:

"react-router-dom": "4.4.0-beta.7",
"react-router-redux": "^4.0.8",
"connected-react-router": "6.0.0",

App.js

const App = () => (
  <Provider store={store}>
    <BrowserRouter>
      <GlobalStyle />
      <Container>
        <ReduxToastr />
        <Header />
        <Routes />
      </Container>
    </BrowserRouter>
  </Provider>
);

Routes.js

const Routes = () => (
  <ConnectedRouter history={history}>
    <Switch>
      <Route path="/login" component={SignIn} />
      <Route path="/signup" component={SignUp} />
      <Private path="/" exact component={Main} />
    </Switch>
  </ConnectedRouter>
);
JulienMelissas commented 5 years ago

Wow, was pulling my hair out over this one for a few hours. I'm using:

"connected-react-router": "^6.3.1",
"react-router-dom": "^5.0.0",

And all is working now, from what I can tell.

vBorgesAlb commented 5 years ago

I had the same problem and the solution here was just to remove "react-router-dom" from my package.json, because "react-router-redux" already has the properly dependencies.

imbhargav5 commented 5 years ago

I am facing this issue too. Any idea how I can contribute a fix?

andreitodoruts commented 5 years ago

@imbhargav5 please check out this answer on stackoverflow: https://stackoverflow.com/questions/42784798/react-router-redux-push-method-does-not-work/58039482#58039482 I bumped in this kind of issues with react-redux-router too and that's how I solved it.

peterdee commented 5 years ago

In case someone faces this issue...

I had an issue with redirects: I was not redirected to the destination page when using push:

yield put(push('/login'));

I was using both react-router-dom wrap (<Router>) and connected-react-router wrap (<ConnectedRouter history={history}>).

Removing the <Router> wrap helped, and everything is working as expected.

Juan-Gargiulo commented 4 years ago

In my case, the problem was in the PrivateRoute component that is breaking the flow. I was taking the token outside component from locale storage like a flag, to determine if the user is logged, like this:

// don't work
const token = localStorage.getItem('token')

// works
const getToken = () => localStorage.getItem('token')

export default function PrivateRoute({ children, ...rest }) {
  // works
  const [token] = React.useState(localStorage.getItem('token') || false);

  return (
    <Route
      {...rest}
      render={({ location }) =>
        token ? (
          children
        ) : (
          <Redirect
            to={{
              pathname: '/login',
              state: { from: location },
            }}
          />
        )
      }
    />
  );
}
pixochi commented 4 years ago

These versions worked for me:

"connected-react-router": "6.8.0",
"react-router": "5.2.0",
"react-router-dom": "5.2.0",
maximelebastard commented 4 years ago

Just faced this issue, I was just not using the same history object everywhere. I made my configureStore create and export the history, then used it in the ConnectedRouter and it works !

thodwris commented 4 years ago

Thank you @maximelebastard ! That was my issue which is not very well documented :/

"connected-react-router": "6.8.0",
"react-router": "5.2.0",
"react-router-dom": "5.2.0",
ihormaslov commented 3 years ago

This example solved my issue https://github.com/supasate/connected-react-router/blob/master/FAQ.md#with-storedispatch

roweldeguzman commented 3 years ago

If you are wrapping your Switch and Router using BrowserRouter, then try to remove that BrowserRouter wrapper