supasate / connected-react-router

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

push a new route only triggers URL change but not location change #159

Open chuanxie opened 5 years ago

chuanxie commented 5 years ago

set up the connected-react-router as doc. but it only update the url rather than the location object when yield put(push(/somewhere)) in saga. so it won't re-render the component.

try to resolve this via Update Blockingsection in react-router but it works fine outside the redux-saga. which means the push method from connected-react-router doesn't work as expected.

any suggestions?

Thanks

akhil-gautam commented 5 years ago

If possible, can you check it once in redux-thunk? Is it working there? In my case also, it was not changing the location from redux-saga, so I just moved it in thunk. May be it doesn't fit your use-case, but just have a try. Ex:

const mapDispatchToProps = dispatch => {
    dispatch(push(/somewhere/))
}
kurt-hardy commented 5 years ago

I have this same issue. I can confirm dispatching a push does not render anything. This is my component:

import React from "react";
import { connect } from "react-redux";
import { Switch, Route, Redirect, withRouter } from "react-router-dom";

import SideBar from "Components/SideBar";
import Dashboard from "Features/Dashboard";
import Team from "Features/Team";
import userActions from "store/user/actions";
import styles from "./styles.scss";

class AppRouter extends React.PureComponent {
    componentDidMount() {
        if (!this.props.hasUserDataLoaded) {
            this.props.loadUserData();
        }
    }

    render() {
        return (
            <div className={styles.container}>
                <SideBar pathname={this.props.location.pathname} />
                <Switch>
                    <Route path="/dashboard" Component={Dashboard} />
                    <Route path="/team" Component={Team} />
                    <Redirect to="/dashboard" />
                </Switch>
            </div>
        );
    }
}

const mapStateToProps = state => ({
    hasUserDataLoaded: state.user.hasUserDataLoaded,
});

const mapDispatchToProps = dispatch => ({
    loadUserData: () => dispatch(userActions.loadUserData()),
});

export default withRouter(connect(mapStateToProps, mapDispatchToProps)(AppRouter));
kurt-hardy commented 5 years ago

Figured my issue out. Notice how I've used title case for the component prop in the Route. Double check your not doing something stupid like me :-)

fbove commented 5 years ago

Same is happening inside a saga, doing: yield put(push('/home')) just changes the URL, but it does not render the component.

My config is the one from react-boilerplate plus this pr.

kurt-hardy commented 5 years ago

Does it render a default component or if you change the URL manually? My default route wasn't being rendered even after I disconnected it from the store.

If you have a repo or codepen, I can take a look.

fbove commented 5 years ago

Solved Saga was cancelled after LOCATION_CHANGE.

@kurt-hardy Thanks for answering and willing to help!

florianchevallier commented 5 years ago

Hey, got the same issue here. The Links are working, but I can't push new URL.

I have a working example here : https://codesandbox.io/s/vvznjm8mz3

kurt-hardy commented 5 years ago

Did you get that working? I see that your not using connected router, your using browser router.

florianchevallier commented 5 years ago

I go it working when doing like this : https://codesandbox.io/s/p2wp49mmp0

I didn't get that I should not use BrowserRouter. Thanks

kurt-hardy commented 5 years ago

Your not actually using the connected-router. Using history.push() is fine but if you need to start using push inside reducers, epics or sagas, you will need to use the connected router (or come up with your own routing solution).

Take a look here if you need help getting the connected router working. https://github.com/kurt-hardy/react-router-redux-issue

Feel free to question me on this too, not that I'm an expert but I'll try. :-)

florianchevallier commented 5 years ago

Yeah that's what I thought too, I felt like cheating; I've updated the codebox :)

I was doing two things wrong :

See if something is wrong with the codesandbox, but else, it's ok for me :)

kurt-hardy commented 5 years ago

No everything looks good there. That's the same setup as I have it.

Glad you got it working. 👍

Absvep commented 5 years ago

@fbove I have same problem where my saga gets cancelled when I do yield put(push('/newlocation'). How did you solve it? Pls check my question https://stackoverflow.com/questions/53223958/saga-always-cancelled?noredirect=1#comment93342944_53223958

fbove commented 5 years ago

My issue had to do with doing a LOCATION_CHANGE and the saga being cancelled cause of that. In fact I do agree with "Andrey Moiseev", you should check for something like

yield take(LOCATION_CHANGE)
yield cancel(watcher)

or if the component that's mounting that saga is still mounted also.

If you are doing a yield put(push('/dashboard')) and the component for that route is in a different branch of your "routing tree", I'll guess the saga won't be listening. If that's the case, you'll have to hook the saga to an upper level component, which contains the other two. Hope this makes sense.

Absvep commented 5 years ago

I have the same issue where LOCATION_CHANGE is cancelling my saga. But how did you solve it in detail? I also use react boilerplate and its newly added connected-react-router. I use withRouter in base App component as well, else the redirect does not happen. Please help, I've been looking for answer for 2 days:(

fbove commented 5 years ago

I solved it by removing:

yield take(LOCATION_CHANGE)
yield cancel(watcher)

And placing the watcher on app/containers/App/saga.js as I needed it to be global and persist through location changes.

p/s: Instead of removing it, you could use an other action to cancel it.

Absvep commented 5 years ago

@fbove Thanks a lot man! It solved it to move saga to global scope! Thank you!

Galya-IT commented 5 years ago

I have a similar problem with redux-observable and I explained it in details in Stackoverflow. If anyone can help?

monestereo commented 5 years ago

I had the same problem today. It seems to break with react-dom >= 16.5. Downgrading it to 16.4.1 solved it for me.

Any Idea why?

bndynet commented 5 years ago

I had the same problem and resolved it for some days. Finally, I found no history specified in tag <Router> and Router is BrowserRouter. I change it to <Router history={history}> (Router is default, not BrowserRouter) and the history is shared with router reducer. Code as below:

history.tsx

import { createBrowserHistory } from 'history';
export default createBrowserHistory();

store.tsx

import { routerMiddleware } from 'connected-react-router';

import history from './history';
import { createRootReducer } from './reducer';
const appRootReducer = createRootReducer(history);
const appRouterMiddleware = routerMiddleware(history);
//...

!App.tsx

//...
import { BrowserRouter as Router } from 'react-router-dom';
class AppComponent extends React.Component {
    render() {
        return (
              <Router>
                    <main>{renderRoutes(routes)}</main>
              </Router>
        );
    }
}

CHANGE TO:

//...
import { Router } from 'react-router-dom';

class AppComponent extends React.Component {
    render() {
        return (
              <Router history={history}>
                    <main>{renderRoutes(routes)}</main>
              </Router>
        );
    }
}
//...
fizzvr commented 5 years ago

Ok its working on development, but I have a problem with serve the statics index the console says and didn't display anything.

Uncaught TypeError: Cannot read property 'location' of undefined

On deploy the statics and serve with express with

app.get('/*', (req, res) => {
  res.sendFile(path.join(__dirname, 'index.html'))
})

Follow these steps works well but serving not working IDK what happens.

My dispatch

theruziev commented 5 years ago

Make sure in component you are declare routes, wrapped with withRouter:

export default withRouter(
  connect(
    mapStateToProps,
    mapDispatchToProps,
  )(App),
);
rm-rf-etc commented 5 years ago

Was having this issue. In case it may help someone, here's what caused the issue for me:

First: Actions weren't dispatching. Issue was that I was using BrowserRouter, I didn't know about ConnectedRouter.

Second: Got actions to dispatch and URL would update, but the page wouldn't change. Issue was that I needed to add the exact prop on every Route.

addamove commented 5 years ago

Same for me: location changed, action dispatched - component not changed

yash2code commented 5 years ago

I go it working when doing like this : https://codesandbox.io/s/p2wp49mmp0

I didn't get that I should not use BrowserRouter. Thanks

thanks. can you refactor it with typescript please.

CycoPH commented 5 years ago

Similar problem. Looked into the solutions listed above and they all stop working when you switch to the latest connected-react-router. https://codesandbox.io/s/2190y39qxy

when loaded should point to /login but points to /

navarroaxel commented 5 years ago

I have found that the error is related to notifyListeners on the history package, with a delay on the redirect this is resolved.

Bug demo repo: https://github.com/navarroaxel/react-history-redirect

bashleigh commented 5 years ago

I was using sagas to change the location. First I was using the push method from connected react router within a saga. I tried absolutely everything to make my route change. The URL would change but the component wouldn't. I found removing the BrowserRouter component fixed my issue. Guessing it's because I was using a switch and a browserRouter component.

<ConnectedRouter history={history}>
    <BrowserRouter> {/* Removing this component solved my issue. Possibly because it's using it's own history instance. I dunno. React + it's packages change too quickly for me to keep up and stay sane at the same time, given up caring... */}
      <Switch>
        <PrivateRouter exact path="/" component={App} />
        <Route exact path="/login" component={Login} />
        <Route component={NotFound} />
     </Switch>
   <BrowserRouter>
</ConnectedRouter>
florianmartens commented 5 years ago

For everyone looking for a quick (and probably dirty) fix using BrowserRouter ... (at least it worked for me). BrowerRouter is a forceRefresh prop. You can set this true.

<BrowserRouter forceRefresh={true}></BrowserRouter

bishopZ commented 5 years ago

Using connected-react-router v6.4.0, If I don't use a Switch then react-router doesn't trigger the route. If I use Switch, I wasn't getting any "Location_Change" events.

The solution for me was to put the Connected-Router inside the Switch

<Provider store={store}>
    <Switch>
      <ConnectedRouter history={history}>
nicgirault commented 4 years ago

I found this issue which match pretty well my issue although I'm not using connected-react-router. I am indeed in a vicious circle where a history.push in my saga triggers the saga cancellation.

To give some context, I'm using redux-saga in a quite unusual way: in my components I run this kind of effect:

useEffect(() => {
  const task = sagaMiddleware.run(mySaga)

  return () => {
      if (task.isRunning()) {
        task.cancel()
      }
    }
}, [])

because I want to be sure my saga is properly attached to a component. But this leads to bugs when the saga triggers the component to unmount.

At first I wrote in my saga history.push('/some-redirection') but this lead to an error: "Generator is already running" (see https://github.com/redux-saga/redux-saga/issues/703).

As advised in the linked issue, I changed my code to call({context: history, fn: history.push}, '/some-redirection') but this leads to the problem described here.

Since an action in the saga that makes the component unmount seems to be a valid case to me, I don't want to remove the code that cancel the saga on unmount.

My solution to this issue is to spawn the history.push.

export const historyPushFromSaga = (path: string) =>
  spawn(() => history.push(path))

Interestingly spawn(function*() { yield call({context: history, fn: history.push}, '/some-redirection')}) still give me a white screen. But I didn't investigate why. Instead I just call directly history.push.

Bielik20 commented 4 years ago

Had the same issue, @bashleigh has helped. It looks like it is not related to any specific middleware used - or we are dealing with two distinct problems here and we should split the issue.

As far as removing router part is concerned it is either a bug or it is intended to be used this way. If it is the latter we ought to change documentation cause it takes too much time to find this issue and a working solution.

facingmonday commented 4 years ago

Just wanted to share that I had this issue and what fixed it was updating to react-router@5 and react-router-dom@5.

droconnel22 commented 4 years ago

I have this same issue. I can confirm dispatching a push does not render anything. This is my component:

import React from "react";
import { connect } from "react-redux";
import { Switch, Route, Redirect, withRouter } from "react-router-dom";

import SideBar from "Components/SideBar";
import Dashboard from "Features/Dashboard";
import Team from "Features/Team";
import userActions from "store/user/actions";
import styles from "./styles.scss";

class AppRouter extends React.PureComponent {
    componentDidMount() {
        if (!this.props.hasUserDataLoaded) {
            this.props.loadUserData();
        }
    }

    render() {
        return (
            <div className={styles.container}>
                <SideBar pathname={this.props.location.pathname} />
                <Switch>
                    <Route path="/dashboard" Component={Dashboard} />
                    <Route path="/team" Component={Team} />
                    <Redirect to="/dashboard" />
                </Switch>
            </div>
        );
    }
}

const mapStateToProps = state => ({
    hasUserDataLoaded: state.user.hasUserDataLoaded,
});

const mapDispatchToProps = dispatch => ({
    loadUserData: () => dispatch(userActions.loadUserData()),
});

export default withRouter(connect(mapStateToProps, mapDispatchToProps)(AppRouter));

wow this solved my issue. Was using BrowserRouter AND connected router. Didn't put 2 and 2 together! Thank you!!

PratapAkshay commented 4 years ago

Solved Saga was cancelled after LOCATION_CHANGE.

@kurt-hardy Thanks for answering and willing to help!

can you please explain how you solved this ?

hasan-aa commented 4 years ago

Hello everyone, In case someone is struggling with this, none of the above worked for me. Finally I've reinstalled the packages and it started to work.

yarn remove react-router-dom @types/react-router-dom yarn add react-router-dom @types/react-router-dom

evey201 commented 4 years ago

I go it working when doing like this : https://codesandbox.io/s/p2wp49mmp0

I didn't get that I should not use BrowserRouter. Thanks



This method worked very well
Mwaseemzakir commented 4 years ago

Same issue , just URL chnages but page does't

adamcaron commented 4 years ago

As per this SO answer:

Changing from this:

    <Switch>
      <Route path="/">
        <Welcome />
      </Route>
      <Redirect to="/" />
    </Switch>

To this:

    <Switch>
      <>
        <Route path="/">
          <Welcome />
        </Route>
        <Redirect to="/" />
      </>
    </Switch>

... worked for me.

I was trying to implement a basic fallback redirect for any paths not explicitly specified, so random paths like http://localhost:3000/askfjasdf would redirect to http://localhost:3000. For some reason adding the fragment as a top level child of <Switch> did the trick.

linhtran-agilityio commented 3 years ago

Just try around and this work for me.

<Provider store={store}>
    <ConnectedRouter history={history}>
      <BrowserRouter> {/* Add this */
        <Switch>
          <Route exact={true} path={navigation.login.path} component={Login} />
          <Route path={navigation.posts.path} component={Posts} />
        </Switch>
      </BrowserRouter>
    </ConnectedRouter>
  </Provider>
DutchJelly commented 3 years ago

For me it worked to set exact to true:

ReactDOM.render(
  <Router>
    <Switch>
      <Route exact path="/">
        <RecipePicker />
      </Route>
      <Route exact path="/recipesettings">
        <RecipeSettings />
      </Route>
      <Route exact path="/recipeguide" component={RecipeGuide}/>
      <Route>Page not found</Route>
    </Switch>
  </Router>,
  document.getElementById('root')
);
Marcus2810 commented 3 years ago

My advice is use Router, not BrowserRouter or anything else, just Router, and do () => return createHistoryBrowser(); in separate file then also in the Route Tags do . then import history from you're comps folder, then just history.push() because doesn't need called already an object.

radulfr96 commented 2 years ago

Using "react-router-dom": "^6.2.1", "react": "^17.0.2" and "connected-react-router": "^6.9.2". But using push('/') is not working. Any advice?

Marcus2810 commented 2 years ago

You’re router is wrong then, either you haven’t setup history or you’re having wrapped you’re routs in Switch

On Thu, 30 Dec 2021 at 06:31, Wade Russell @.***> wrote:

Using "react-router-dom": "^6.2.1", "react-router": "^5.0.0", "react": "^17.0.2" and '"connected-react-router": "^6.9.2"'. But using push('/') is not working. Any advice?

— Reply to this email directly, view it on GitHub https://github.com/supasate/connected-react-router/issues/159#issuecomment-1002891970, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOVTE3UKA7B2PYTIWBUJJRTUTP4F7ANCNFSM4F6E44PA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

Marcus2810 commented 2 years ago

*Haven’t

On Fri, 31 Dec 2021 at 14:28, marcus rose @.***> wrote:

You’re router is wrong then, either you haven’t setup history or you’re having wrapped you’re routs in Switch

On Thu, 30 Dec 2021 at 06:31, Wade Russell @.***> wrote:

Using "react-router-dom": "^6.2.1", "react-router": "^5.0.0", "react": "^17.0.2" and '"connected-react-router": "^6.9.2"'. But using push('/') is not working. Any advice?

— Reply to this email directly, view it on GitHub https://github.com/supasate/connected-react-router/issues/159#issuecomment-1002891970, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOVTE3UKA7B2PYTIWBUJJRTUTP4F7ANCNFSM4F6E44PA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

radulfr96 commented 2 years ago

Here is a sample of my code. I do have a switch at the moment.

ReactDOM.render(
    <React.StrictMode>
        <Provider store={store}>
            <OidcProvider store={store} userManager={userManager}>
                <AppContextProvider>
                    <ThemeProvider theme={theme}>
                        <SnackbarProvider maxSnack={10}>
                            <Layout>
                                <ConnectedRouter history={history}>
                                    <Switch>
                                        <Route exact path="/" component={Home} />
                                        <Route path="/callback" component={CallbackPage} />
                                        <Route path="/user" component={Users} />
                                        <Route path="/login" component={Login} />
                                        <Route path="/register" component={Register} />
                                        <Route path="/account" component={MyAccount} />
                                        <Route path="/userdetails/:id" component={UserPage} />
                                    </Switch>
                                </ConnectedRouter>
                            </Layout>
                        </SnackbarProvider>
                    </ThemeProvider>
                </AppContextProvider>
            </OidcProvider>
        </Provider>
    </React.StrictMode>,
    document.getElementById('root'),
);
Marcus2810 commented 2 years ago

In you're routes use exact Path instead of just path because you're only doing it in one of them, also make sure history is createBrowserHistory() and also Change connectedRouter to BrowerRouter or Router because these are the normal ones.

On Sun, 2 Jan 2022 at 05:07, Wade Russell @.***> wrote:

Here is a sample of my code. I do have a switch at the moment.

— Reply to this email directly, view it on GitHub , or unsubscribe . Triage notifications on the go with GitHub Mobile for iOS or Android . You are receiving this because you commented.Message ID: ***@***.***>
Marcus2810 commented 2 years ago

Okay so could you please try doing exact path for all the ones except for default, so ‘exact path=/login’, Because you have done it for only you’re Home component.

On Sun, 2 Jan 2022 at 12:33, marcus rose @.***> wrote:

In you're routes use exact Path instead of just path because you're only doing it in one of them, also make sure history is createBrowserHistory() and also Change connectedRouter to BrowerRouter or Router because these are the normal ones.

On Sun, 2 Jan 2022 at 05:07, Wade Russell @.***> wrote:

Here is a sample of my code. I do have a switch at the moment.

— Reply to this email directly, view it on GitHub , or unsubscribe . Triage notifications on the go with GitHub Mobile for iOS or Android . You are receiving this because you commented.Message ID: ***@***.***>
skiller70 commented 2 years ago

try this method https://github.com/skiller70/reduxRouter/blob/master/reduxRouting.js