Failing unit test demonstrating undefined 'getState' in transition hooks

[davemaurakis]

Failing test case to demonstrate that getState throw as error when evaluated in the context of an onEnter transition hook.

[acdlite]

Thank you! I <3 pull requests with failing tests. I'll get to this very soon.

[chentsulin]

It seems onEnter will be called in createRoutes(routes) and before redux store ready.

// routerReplacement.js#L58
store = compose(
  applyMiddleware(
      replaceRoutesMiddleware(replaceRoutes)
  ),
  next({
      ...options,
      routes: createRoutes(routes)
  })
)(createStore)(reducer, initialState);

It's a hard problem with circular dependencies between redux store and react-router routes, @acdlite @gaearon Any ideas here?

[chentsulin]

Would provide something like this solve the problem?

function createRoutes({ getState, isReady, addReadyListener }) {
  function requireAuth(nextState, replaceState, callback) {
    if (isReady()) {
       // ...
       callback();
    } else {
       addReadyListener(function() {
          // ...
          callback();
       });
    }
  }
  //...
}

But It looks not ideal...

[ddgromit]

I'm running into this as well because dispatch also can't be called.

[chentsulin]

Found another workaround:

import { Route, Redirect } from 'react-router';
import * as containers from './containers';

const {
  App,
  LoginPage,
  DashboardPage,
  NotFoundPage
} = containers;

export function createRoutes({ getState }) {
  function shouldNotAuth(nextState, replaceState, cb) {
    setTimeout(() => {
      const state = getState();
      if (state.user.current) {
        replaceState(null, '/dashboard');
      }
      cb();
    }, 0);
  }

  function requireAuth(nextState, replaceState, cb) {
    setTimeout(() => {
      const state = getState();
      if (!state.user.current) {
        replaceState(null, '/login');
      }
      cb();
    }, 0);
  }
  return (
    <Route component={App}>
      <Redirect from="/" to="login" />
      <Route path="login" component={LoginPage} onEnter={shouldNotAuth} />
      <Route onEnter={requireAuth}>
        <Route path="dashboard" component={DashboardPage} />
      </Route>
      <Route path="*" component={NotFoundPage} />
    </Route>
  );
}

But some warnings is inevitable:

Warning: Failed propType: Required prop `location` was not specified in `RoutingContext`. Check the render method of `ReduxRouterContext`.
Warning: Failed propType: Required prop `routes` was not specified in `RoutingContext`. Check the render method of `ReduxRouterContext`.
Warning: Failed propType: Required prop `params` was not specified in `RoutingContext`. Check the render method of `ReduxRouterContext`.
Warning: Failed propType: Required prop `components` was not specified in `RoutingContext`. Check the render method of `ReduxRouterContext`.
Warning: Failed Context Types: Required child context `location` was not specified in `RoutingContext`. Check the render method of `ReduxRouterContext`.

[tappleby]

Was looking into this issue a bit this evening.

It is caused by the history.listen call in the clients historySynchronization function, which results in redux-router match being called from useRoutes (this is called by the history listen function).

Changing the order of the compose arguments in client.js allows the tests to pass:

export default compose(
  useDefaults,
  historySynchronization,
  routeReplacement
)(reduxReactRouter);

I think this might still be an issue though since the store is being accessed before the state is fully initialized (router is null in the state object).

Any thoughts @acdlite?

[anthonator]

Using setImmediate also works similarly to @chentsulin's suggestion.

[lynndylanhurley]

Thanks for the workaround, @chentsulin!

Has anyone figured out how to get rid of the warnings?

[cerisier]

This is a dirty workaround and I'm not sure why it works but doing the following removes the warnings... cc @chentsulin @lynndylanhurley :

function requireAuth() {
    setTimeout(() => {
        let [nextState, replaceState, cb] = arguments;

        const state = getState();
        if (!state.user.current) {
            replaceState(null, '/login');
        }
        cb();
    }, 0);
}

Does anyone have a clue on first steps to fix this issue ?

[lynndylanhurley]

@cerisier - how and where are you defining getState?

[cerisier]

@lynndylanhurley In the createRoutes function.
Here is the complete working example:

export function createRoutes({ getState }) {

  function requireAuth() {
    setTimeout(() => {
      let [nextState, replaceState, cb] = arguments;

      const state = getState();
      if (!state.user.current) {
          replaceState(null, '/login');
      }
      cb();
    }, 0);
  }

  return (
    <Route component={App}>
      <Route onEnter={requireAuth}>
        <Route path="dashboard" component={DashboardPage} />
      </Route>
      <Route path="*" component={NotFoundPage} />
    </Route>
  );
}

[chentsulin]

@cerisier don't know why this can works ha

[lynndylanhurley]

I've tried this, but I still get the warnings. I should add that I'm using server side rendering, and the warnings are only thrown by the client after a server-side redirect.

[stevenleeg]

Any progress on this issue? I'm running into the same thing and I believe the discussion in #66 is mostly based around this as well.

[lynndylanhurley]

It would be really nice if the store was just available to the onEnter function. For example:

function requireAuth(nextState, replaceState, store, cb) {
  let state = store.getState();
  //...
}

The fact that it's not seriously complicates the redux configuration.

[ferdinandsalis]

Another way is to wrap onEnter to make it safe, like @stevoland has done here — https://github.com/erikras/react-redux-universal-hot-example/blob/master/src/helpers/makeRouteHooksSafe.js

[CrisLi]

I am expecting a same API just like @lynndylanhurley suggested. This is a very normal requirement for onEnter hook access the state tree.

[lynndylanhurley]

Thanks @ferdinandsalis - I'll try that ASAP.

[dsteinbach]

It would be nice to have access to dispatch as well as state.

[lynndylanhurley]

@dsteinbach - the access to the store value would give both dispatch and getState. For example:

function requireAuth(nextState, replaceState, store, cb) {
  // access current state
  let state = store.getState();

  // dispatch event
  store.dispatch(someAction());
  //...
}

Whoever is maintaining this - will you accept a PR for this feature?

[hsin421]

For anybody who uses the workaround by @chentsulin , does the restricted route render first before it gets redirected to log in page?

[lynndylanhurley]

@hsin421 - it doesn't for me.

[joshgeller]

I'm using a workaround based on a higher-order component that wraps any components/views requiring authentication. I'm pretty new to this so if anyone has any comments or feedback I'd appreciate it.

routes.js

import {requireAuthentication} from 'components/AuthComponent';

export default (
  <Route path='/' component={CoreLayout}>
    <Route path="login" component={LoginView} />
    <Route path="protected" component={requireAuthentication(ProtectedView)}/>
  </Route>
);

AuthComponent.js

import React from 'react';
import { connect } from 'react-redux';
import { pushState } from 'redux-router';

export function requireAuthentication(Component) {

    class AuthComponent extends React.Component {

        componentWillMount() {
            if (!this.props.isAuthenticated) {
                // redirect to login and add next param so we can redirect again after login
                this.props.dispatch(pushState(null, `/login?next=${this.props.location.pathname}`));
            }
        }

        render() {
            // render the component that requires auth (passed to this wrapper)
            return (
                <Component  {...this.props} />
            )
        }
    }

    const mapStateToProps =
        (state) => ({
            token: state.auth.token,
            isAuthenticated: state.auth.isAuthenticated
        });

    return connect(mapStateToProps)(AuthComponent);

}

AuthComponent wraps the protected view and prevents it from rendering if the auth props do not indicate a successful login. In the componentDidMount of my protected views I am fetching data using the token passed by AuthComponent.

Anyone see any issues with this approach? Seems to be working well for me though I admit I have not spent a lot of time with it.

[hsin421]

@joshgeller your higher component works like a charm. Really appreciate it !!

I'll spend some more time with it and report any issues/improvements.

[dsteinbach]

You'll also have to do the check in componentWillReceiveProps so if the user logouts or an API responds with 401 you can respond client-side. This works great for me thanks @joshgeller !

[LucaColonnello]

Solved!!!!!!!!!!!!!!!
It seems a problem on the configuration of the examples...

You have to set the routes with getRoutes function when you render the ReduxRouter component instead of doing this when you compose the store...

So that when you create the store, the routes configuration will not be calculated.
In the Root component creation, you finally create the routes configuration, passing the store as parameter of the getRoutes function.

Simple two phase configuration.

Root.jsx

import React, { Component, PropTypes } from 'react';
import { Provider } from 'react-redux';
import { ReduxRouter } from 'redux-router';
import getRoutes from '../routes';

export default class Root extends Component {
  render() {
    const { store } = this.props;
    return (
      <Provider store={store}>
        <ReduxRouter>
          {getRoutes(store)}
        </ReduxRouter>
      </Provider>
    );
  }
}

Root.propTypes = {
  store: PropTypes.object.isRequired,
};

createStore.js

import createHistory from 'history/lib/createBrowserHistory';
import { createStore, applyMiddleware, compose } from 'redux';
import { apiMiddleware } from 'redux-api-middleware';
import createLogger from 'redux-logger';
import { reduxReactRouter } from 'redux-router';
import thunk from 'redux-thunk';
import DevTools from '../containers/DevTools';
import rootReducer from '../reducers';

const finalCreateStore = compose(
  applyMiddleware(thunk),
  applyMiddleware(createLogger()),
  reduxReactRouter( { createHistory } ),
  applyMiddleware(apiMiddleware),
  DevTools.instrument()
)(createStore);

export default function configureStore(initialState) {
  const store = finalCreateStore(rootReducer, initialState);

  if (module.hot) {
    // Enable Webpack hot module replacement for reducers
    module.hot.accept('../reducers', () => {
      const nextRootReducer = require('../reducers');
      store.replaceReducer(nextRootReducer);
    });
  }

  return store;
}

[alexbeletsky]

@joshgeller I really liked the approach of high-level component. Looks very clear and solid. Seems like @LucaColonnello also makes a lot of sense, will try that out.

[Scarysize]

@joshgeller Pretty sweet approach. I really like how readable the routes declaration is, you can easily see which routes require authentication.

So this is a issue coming up pretty often: It would be really appreciated if somebody could write something up, describing the workarounds and adding a simple example.

[joshgeller]

I put together a sample repo and quick explanation of the higher-order component approach here:

https://github.com/joshgeller/react-redux-jwt-auth-example

[alexbeletsky]

@joshgeller It looks like an amazing boilerplate.. Thanks for sharing!

[Scarysize]

@joshgeller Thank your very much. I added your repo to our README. Of course this won´t fix the issue itself, but it is a pretty solid workaround for now.

[scottnonnenberg]

To add more fuel to the fire... the closure solution works for me, except for one client/server rendering scenario. It's the standard onEnter={requireAuth} setup, but the user has loaded, straight from the server, a protected URL because they're already logged in.

In that case, my onEnter handler is run synchronously on the client at this line, and the store isn't available yet since the router needs to be supplied to my configureStore call

const router = reduxReactRouter({
  createHistory,
  getRoutes
});

So for now, my solution is this, not the prettiest, but it works client and server:

let getState = () => initialState;
const routes = createRoutes(() => getState());
const router = reduxReactRouter({
  createHistory,
  routes
});
const store = configureStore({initialState, router});
getState = () => store.getState();

[Wuvnen]

Any news on this?

[alexbeletsky]

@ysudo I you fall in this problem to implement auth in your app, the boilerplate from @joshgeller is just great. I've tried that and it works. I don't have other cases to use getState, so workaround works for me.

[Scarysize]

@ysudo I hope @joshgeller workaround works for you. I can´t predict if we will fix this any time soon. See #172

[petrometro]

With @joshgeller 's solution we have to load the to-be-authorized component to see if the user is allowed to view it, which isn't great for dynamic routing/code splitting... ideally, i don't want to load the component from the server at all unless the user is authorized to see it.
So in this case @LucaColonnello 's solution looks more promising, the router has a reference to state and so 'this' inside of onEnter callback has the state reference too...

[LucaColonnello]

Thank you @petrometro . Now I'm looking at redux-simple-router, that is a great and simple implementation to let communication between Redux and React Router ...
I don't wanna make some SPAM, but I think it could be more flexible, as mentioned in the README of this repo.

[Elyx0]

@LucaColonnello did you find how to access your store in redux-simple-router in onEnter ?

[LucaColonnello]

@Elyx0 according to the react-router-redux documentation, seems that no hack is needed.
You can generate your routes as you want, in all the way react-router let you do it.

This is because you simply use Router component from react-router and all the react-router-redux parts are only middlewares, reducers and action creators that doesn't act in the bootstrap phase of your app.

import React from 'react'
import ReactDOM from 'react-dom'
import { createStore, combineReducers, applyMiddleware } from 'redux'
import { Provider } from 'react-redux'
import { Router, Route, browserHistory } from 'react-router'
import { syncHistory, routeReducer } from 'react-router-redux'
import reducers from '<project-path>/reducers'

const reducer = combineReducers(Object.assign({}, reducers, {
  routing: routeReducer
}))

// Sync dispatched route actions to the history
const reduxRouterMiddleware = syncHistory(browserHistory)
const createStoreWithMiddleware = applyMiddleware(reduxRouterMiddleware)(createStore)

const store = createStoreWithMiddleware(reducer)

// Required for replaying actions from devtools to work
reduxRouterMiddleware.listenForReplays(store)

ReactDOM.render(
  <Provider store={store}>
    <Router history={browserHistory}>
      <Route path="/" component={App}>
        <Route path="foo" component={Foo}/>
        <Route path="bar" component={Bar}/>
      </Route>
    </Router>
  </Provider>,
  document.getElementById('mount')
)

[catamphetamine]

It's funny how this long standing issue is fixed by simply swapping two lines of code.
https://github.com/acdlite/redux-router/pull/272/files