bernabe9 / redux-react-session

:key: Simple Session API storage for Redux and React
https://bernabe9.github.io/redux-react-session/
147 stars 41 forks source link

React Router v4 support #27

Closed freddyamsterdam closed 6 years ago

freddyamsterdam commented 6 years ago

Issue I was able to login to my application and click around my "authenticated routes" just fine. However, the second I tried to access a route directly, i.e. typing a url in the browser or refreshing the page, I would hit the login screen. I attempted to follow the React Router v4 Example, but it did not work.

Solution React Router v4 does not use hooks, but rather relies on the React Component lifecycle. Which means it not possible to call sessionService.checkAuth in the onEnter hook. The IndexRoute component has also been removed.

The following code works like a charm:

import React, { Component } from 'react'
import { BrowserRouter as Router, Route, Redirect, Switch } from 'react-router-dom'
import { connect } from 'react-redux'
import PropTypes from 'prop-types'

import Dashboard from './Dashboard'
import Home from './Home'
import SignIn from './SignIn'
import SignOut from './SignOut'

class Routes extends Component {
  constructor(props) {
    super(props)
    this.state = {
      loading: true,
      authenticated: false
    }
  }

  componentWillReceiveProps(nextProps) {
    const { session } = nextProps

    this.setState({
      loading: false,
      authenticated: session.authenticated
    })
  }

  render() {
    const state = this.state

    return (
      <div style={appStyles}>
      { state.loading === true ? (
        <p>Loading...</p>
      ) : (
        <div>
          <Router>
            <Switch>
              {/* Always accessible routes*/}
              <Route exact path="/" component={Home}/>
              <div>
                { state.authenticated === false ? (
                <div>
                  {/* Public only routes*/}
                  <Route path="/sign-in" component={SignIn}/>
                </div>
                ) : (
                <div>
                  {/* Private only routes*/}
                  <Route path="/dashboard" component={Dashboard}/>
                  <Route path="/profile" component={Profile}/>
                  <Route path="/sign-out" component={SignOut}/>
                </div>
                )}
              </div>
              <Redirect to='/' />
            </Switch>
          </Router>
        </div>
      )}
      </div>
    )
  }
}

Routes.propTypes = {
  session: PropTypes.object.isRequired
}

const mapStateToProps = (state, ownProps = {}) => {
  return state
}

export default connect(mapStateToProps)(Routes)
umpc commented 6 years ago

Hi,

I am glad that you got it working.

Is there is a performance-related reason for copying and storing the authenticated prop object value in the component's state after it has been passed down from redux?

Edit: It appears that the routes.js file, from which you were implementing, is unused by the newer React Router v4 example. The example does work as-is, though I have submitted a PR, #28, to remove this unused file, as it is certainly confusing without using the entire example.

Thanks for submitting this!

freddyamsterdam commented 6 years ago

Hi @umpc,

Thank you kindly for your reply.

With regards to your question about component state, there are no performance gains whatsoever – at least none which I could detect! In fact, I would say that the state came back to bite my in the rear further down the line. The authentication prop should not be passed to the component state, similarly loading should be passed as a prop from the store, as opposed to being included in the component state.

In hindsight I would equate the code snippet above as the essence of the brain fart which got the application working, rather than the best or the final solution.

Forgive me for any bugs as I did not test the highly redacted code. However, the following is closer to what I ended up doing:

./reducers/index.js

import { combineReducers } from "redux"
import { sessionReducer as session } from 'redux-react-session';
import app from "./appReducer"

export default combineReducers({
  session,
  app,
})

./reducers/appReducer

const initialState = {
  loading: true
}

export default function reducer(state = initialState, action) {
    switch (action.type) {
      case 'RECEIVE_PAGE_LOADED':
        return {
          ...state,
          loading: action.loading
        }
      default:
        return state
    }
}

./actions/appActions

const RECEIVE_PAGE_LOADED = 'RECEIVE_PAGE_LOADED'

export const  pageLoaded = (loaded = true) => ({
  type: RECEIVE_PAGE_LOADED,
  loading: !loaded
})

Routes.js

import React, { Component } from 'react'
import { BrowserRouter as Router, Route, Redirect, Switch } from 'react-router-dom'
import { connect } from 'react-redux'
import PropTypes from 'prop-types'

import { pageLoaded } from './actions/appActions'

import Dashboard from './Dashboard'
import Home from './Home'
import SignIn from './SignIn'
import SignOut from './SignOut'

class Routes extends Component {
  componentWillReceiveProps(nextProps) {
    const { app, session, dispatch } = nextProps

    if (session.authenticated && session.checked) {
      if (app.loading) {
        dispatch(pageLoaded())
      }
    }

    if (!session.authenticated && session.checked) {
      if (app.loading) {
        dispatch(pageLoaded())
      }
    }
  }

  render() {
    const { app, session } = this.props

    return (
      <div style={appStyles}>
      { app.loading ? (
        <p>Loading...</p>
      ) : (
        <div>
          <Router>
            <div>
            { !session.authenticated ? (
              <div>
                <Switch>
                  {/* Public only routes*/}
                  <Route path="/sign-in" component={SignIn}/>
                </Switch>
              </div>
              ) : (
              <div>
                <Switch>
                  {/* Private only routes*/}
                  <Route path="/dashboard" component={Dashboard}/>
                  <Route path="/profile" component={Profile}/>
                  <Route path="/sign-out" component={SignOut}/>
                </Switch>
              </div>
            )}
            </div>
            <Switch>
              {/* Always accessible routes*/}
              <Route exact path="/" component={Home}/>
              <Redirect to='/' />
            </Switch>
          </Router>
        </div>
      )}
      </div>
    )
  }
}

App.propTypes = {
  app: PropTypes.object.isRequired,
  session: PropTypes.object.isRequired,
}

const mapStateToProps = (state, ownProps = {}) => {
  return state
}

export default connect(mapStateToProps)(Routes)
umpc commented 6 years ago

Ah, that makes sense. Thanks for posting a complete answer with code.

freddyamsterdam commented 6 years ago

No problem!

bernabe9 commented 6 years ago

@freddyamsterdam it seems that you missed some important lines of code from the example that will solve your issue.

The issue is that your app is launched before the session, to fix this in the example I use the flag checked to start the routes: https://github.com/bernabe9/react-redux-base/blob/master/src/components/App.js

This way I make sure that when the app is launched the session is checked. I only render the routes once I have the variable checked.

Your solution is great, but a little bit more complex. The solution of the example should work fine :)

freddyamsterdam commented 6 years ago

@bernabe9 cheers for your reply! The application loaded state depends on a range of other props being truthy, too. The code above is heavily redacted. I imagine the solution you have provided should work for most others, too. Thanks! :-)

This issue can be closed as far as I'm concerned.