connected-react-router icon indicating copy to clipboard operation
connected-react-router copied to clipboard

Push action not working properly

Open jancama2 opened this issue 5 years ago • 37 comments

Package version: [email protected] 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 [email protected] helped me to solve the problem.

jancama2 avatar Feb 20 '19 11:02 jancama2

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 🐛

victorhqc avatar Feb 21 '19 12:02 victorhqc

Downgrading to 6.0.0 worked for me too.

cdtinney avatar Feb 23 '19 04:02 cdtinney

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.

acaravia avatar Feb 26 '19 18:02 acaravia

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

jancama2 avatar Feb 28 '19 10:02 jancama2

Using [email protected]. together with [email protected] 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 [email protected] 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.

jancama2 avatar Mar 04 '19 07:03 jancama2

@jancama2 upgrading to [email protected] together with [email protected] 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?

acaravia avatar Mar 04 '19 19:03 acaravia

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

jancama2 avatar Mar 05 '19 10:03 jancama2

@jancama2 unfortunately I don't have different versions.

acaravia avatar Mar 05 '19 19:03 acaravia

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

jancama2 avatar Mar 06 '19 07:03 jancama2

Just ran into this today. [email protected] with [email protected] is not working properly. Downgraded to v6.0.0 fixed the issue.

mpigsley avatar Mar 06 '19 16:03 mpigsley

@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 <Route> 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. :)

acaravia avatar Mar 06 '19 16:03 acaravia

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",

jancama2 avatar Mar 07 '19 07:03 jancama2

@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.

acaravia avatar Mar 07 '19 08:03 acaravia

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!

jerrygreen avatar Mar 07 '19 13:03 jerrygreen

@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

acaravia avatar Mar 08 '19 00:03 acaravia

@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.

jancama2 avatar Mar 08 '19 08:03 jancama2

@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 [email protected].

acaravia avatar Mar 08 '19 08:03 acaravia

@acaravia No problem, glad to help :))

jancama2 avatar Mar 08 '19 09:03 jancama2

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

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

marko-ignjatovic avatar Mar 15 '19 20:03 marko-ignjatovic

@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.

acaravia avatar Mar 15 '19 23:03 acaravia

@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();

marko-ignjatovic avatar Mar 16 '19 07:03 marko-ignjatovic

@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 = {}.

acaravia avatar Mar 16 '19 08:03 acaravia

@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 avatar Mar 16 '19 09:03 marko-ignjatovic

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

marko-ignjatovic avatar Mar 16 '19 12:03 marko-ignjatovic

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.

rivernews avatar Mar 22 '19 00:03 rivernews

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>
);

evandrogrm avatar May 10 '19 03:05 evandrogrm

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.

JulienMelissas avatar May 10 '19 23:05 JulienMelissas

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.

vBorgesAlb avatar Jun 13 '19 13:06 vBorgesAlb

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

imbhargav5 avatar Jul 16 '19 18:07 imbhargav5

@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.

andreitodoruts avatar Sep 21 '19 10:09 andreitodoruts

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.

peterdee avatar Oct 28 '19 14:10 peterdee

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 },
            }}
          />
        )
      }
    />
  );
}

Juan-Gargiulo avatar Apr 07 '20 13:04 Juan-Gargiulo

These versions worked for me:

"connected-react-router": "6.8.0",
"react-router": "5.2.0",
"react-router-dom": "5.2.0",

pixochi avatar Jul 20 '20 09:07 pixochi

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 !

maximelebastard avatar Jul 30 '20 03:07 maximelebastard

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",

thodwris avatar Oct 30 '20 22:10 thodwris

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

ihormaslov avatar Mar 08 '21 09:03 ihormaslov

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

roweldeguzman avatar May 14 '21 01:05 roweldeguzman