Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not triggering Redirect with multiple route matches #5155

Closed
ghost opened this issue May 21, 2017 · 6 comments
Closed

Not triggering Redirect with multiple route matches #5155

ghost opened this issue May 21, 2017 · 6 comments

Comments

@ghost
Copy link

ghost commented May 21, 2017

Version

4.1.1

Test Case

https://github.com/milanvdmria/MyFirstReactProject (npm start)
https://github.com/milanvdmria/MyFirstReactProject/blob/master/src/components/Authentication.jsx
https://github.com/milanvdmria/MyFirstReactProject/blob/master/src/scenes/app/routes.jsx

simplified: https://codepen.io/anon/pen/YVROMK (not known with codepen)

Steps to reproduce

When loading a page, the Authentication component is triggered. When you are not logged in, it does a <Redirect to='/login' />.
Everything works when going to the pages. When you are on a page, and you click on the Home icon in the left top corner, (a Link component to '/') it triggers all the needed components (including the Authentication) but it just renders the Home page without redirecting to the /login page.

Expected Behavior

When clicking the Home button, it links to the home page and should trigger the Authentication and redirect to the login page

Actual Behavior

It goes to the Home page, although Authentication returns a component.

@BTMPL
Copy link

BTMPL commented May 21, 2017

As a followup, a minimal demo of the issue:

import React from 'react';
import ReactDOM from 'react-dom';

import { BrowserRouter as Router, Route, Redirect, Link } from 'react-router-dom';

const Authentication = () => <Redirect to="/login" />;

class App extends React.Component {
  render() {
    return (
        <Router>
          <div>
            <Link to="/">Home</Link>
            <Route exact path="/" component={() => <p>Home</p>} />
            <Route path="/" component={Authentication} />
            <Route path="/login" component={() => <p>Login plx</p>} />
          </div>
        </Router>
    )
  }
}

ReactDOM.render(<App />, document.querySelector('.container'));

Even though when navigating from /login to / the Authentication component runs, the route is not updated back to /login

@BTMPL
Copy link

BTMPL commented May 21, 2017

I believe that this is a result of some optimalization on either React or react-router side - in this scenario the Redirect component is not destroyed an recreated (even though it's wrapped in a SFC) and the same compnent is reused.

A solution to this error is adding

  componentDidUpdate() { 
    const to = (typeof this.props.to === "string") ? this.props.to : this.props.to.pathname;
    if(this.context.router.route.location.pathname !== to) {
      this.perform();
    }
  }

to the Redirect component ( https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Redirect.js ).

Not sure if this use-cause is expectd and if such a solution is accepted.

@a-x-
Copy link

a-x- commented May 25, 2017

maybe rewrite it like this?:

            <Route exact path="/" component={() => <p>Home</p>} />
            <Route path="/" component={Authentication} />

-->

            <Route path="/" component={Authentication} />
const Authentication = () => isLoggedIn ? <Home/> : <Redirect to="/login" />;

@a-x-
Copy link

a-x- commented May 25, 2017

my theoretical suggest:

const Home = (params) => <p>Home</p>;

// hoc
const Auth = Component => ({isLoggedIn}) => isLoggedIn ? Component : <Redirect to="/login" />;

const AuthorizedHome = Auth(<Home/>);
const ConnectedAuthrizedHome = connect(store => ({isLoggedIn: store.isLoggedIn}))(AuthorizedHome);

<Route path="/" component={ConnectedAuthrizedHome} />

@ghost
Copy link
Author

ghost commented May 25, 2017

I will have a try and seems to be a valid solution from an architectural view. Does not solve the issue with react router itself though :)

@pshrmn
Copy link
Contributor

pshrmn commented May 25, 2017

This is related to #5162/#5003. I'm going to close this so that discussions of the redirect issue can be concentrated to over there.

I'm not really a fan of the route layout that you are using and I think some simple changes would help you avoid the redirect issue.

The approach that I would take is to:

  1. When the user is not authenticated, you should only have two routes: /login and a catch all for everything else that redirects to /login.
  2. Once they are authenticated,switch over to the normal routes.
const NormalRoutes = () => (
  <Switch>
    <Route exact path="/" component={Home} />
    <Route path="/person/:id" component={Person} />
  </Switch>
)

const ForceAuthRoutes = () => (
  <Switch>
    <Route path="/login" component={Login} />
    <Redirect to='/login' />
  </Switch>
)

const Routes = props => props.activeUser.jwt ? <NormalRoutes /> : <ForceAuthRoutes />

@pshrmn pshrmn closed this as completed May 25, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants