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!