acdlite / redux-router

Redux bindings for React Router – keep your router state inside your Redux store
MIT License
2.3k stars 200 forks source link

Problem with dispatching pushState(...) using redux-thunk #191

Closed mohebifar closed 8 years ago

mohebifar commented 8 years ago

I use redux-thunk which offers dispatch and getState in the action creator.

loginSuccess is a redux duck function which dispatches two actions, the second of which is pushState that is supposed to redirect to the attempted path after successful login.

export function loginSuccess(result) {
  return (dispatch, getState) => {
    dispatch({type: LOGIN_SUCCESS, result});
    const router = getState().router;
    dispatch(pushState(null, router.location.query.redirect))
  };
}

I can see an action of type @@reduxReactRouter/historyAPI is dispatched with this payload:

{"method": "pushState", "args": [null, "/path"]}

And nothing happens in the history!

However, when I dispatch the action creator in a component the route changes successfully and a @@reduxReactRouter/routeDidChange action is dispatched and everything is fine.

timaschew commented 8 years ago

try to check which part is not called via log statements

export function loginSuccess(result) {
  console.log('outer')
  return (dispatch, getState) => {
    console.log('inner')
    dispatch({type: LOGIN_SUCCESS, result});
    const router = getState().router;
    dispatch(pushState(null, router.location.query.redirect))
   console.log('end')
  };
}
mohebifar commented 8 years ago

All parts are called via log statements. Simply, I expect to see an action of type @@reduxReactRouter/routeDidChange on redux devtools monitor but what I see is @@reduxReactRouter/historyAPI and it doesn't transit.

timaschew commented 8 years ago

does pushState come from import { pushState } from 'redux-router'?

can you try to put a literal string to the url state like pushState(null, '/foo')

mohebifar commented 8 years ago

Yes. That comes from redux-router.

I tried this:

dispatch(pushState(null, '/events'));

Still no transition and the result is:

Redux Devtools Monitor

lennardv2 commented 8 years ago

Same problem here

timaschew commented 8 years ago

Which version of redux, and redux-router are you using? Maybe show what's your npm ls or even better create a shrinkwrap file npm shrinwrap.

If you have problems with these commands, it's actually a npm issue, just install a new npm version via npm i npm@3.* -g

manukall commented 8 years ago

i think this is depends on the order in which middlewares are applied. I get the same (non)-result when I have

const finalCreateStore = compose(
  reduxReactRouter({createHistory})
  applyMiddleware(thunk),
 )(createStore);

When I change the order to

const finalCreateStore = compose(
  applyMiddleware(thunk),
  reduxReactRouter({createHistory})
 )(createStore);

the history is changed as intended.

timaschew commented 8 years ago

that's true, I had some issues with the wrong order as well. It's always good to check out some examples in the redux repo: real-world-rexample

Scarysize commented 8 years ago

Can confirm that the order matters, other people had some similar problems! @mohebifar Hope this solves your issue :-)

mohebifar commented 8 years ago

This is what I have while the problem exists:

let finalCreateStore = applyMiddleware(...middleware)(createStore);
finalCreateStore = reduxReactRouter({getRoutes, createHistory})(finalCreateStore);
timaschew commented 8 years ago

This looks wrong, you don't use compose and what is middleware? what do you export?

Either show the complete snippet or just use the snippet of @manukall or the real-world example

thaerlabs commented 8 years ago

As the topic is still open, I'd like to share my issue and maybe get some help. Basically my scenario is the same, I'm trying to call pushState from my action creator using redux-thunk.

export function onSubmit(data) {
  const _data = normalizeRequest(data);
  return (dispatch) => {
    dispatch(pushState(null, 'do-search'));
    performSearch(_data, dispatch);
  };
}

and my createStore.js is

import { createStore as _createStore, applyMiddleware, compose } from 'redux';
import createMiddleware from './middleware/clientMiddleware';
import transitionMiddleware from './middleware/transitionMiddleware';
import thunkMiddleware from 'redux-thunk';

export default function createStore(reduxReactRouter, getRoutes, createHistory, client, data) {

  const middleware = [thunkMiddleware, createMiddleware(client), transitionMiddleware];

  let finalCreateStore;
  if (__DEVELOPMENT__ && __CLIENT__ && __DEVTOOLS__) {
    const { persistState } = require('redux-devtools');
    const DevTools = require('../containers/DevTools/DevTools');
    const logger = require('redux-logger')();
    finalCreateStore = compose(
      applyMiddleware(...middleware, logger),
      window.devToolsExtension ? window.devToolsExtension() : DevTools.instrument(),
      persistState(window.location.href.match(/[?&]debug_session=([^&]+)\b/))
    )(_createStore);
  } else {
    finalCreateStore = applyMiddleware(...middleware)(_createStore);
  }

  finalCreateStore = reduxReactRouter({ getRoutes, createHistory })(finalCreateStore);

  const reducer = require('./modules/reducer');
  const store = finalCreateStore(reducer, data);

  if (__DEVELOPMENT__ && module.hot) {
    module.hot.accept('./modules/reducer', () => {
      store.replaceReducer(require('./modules/reducer'));
    });
  }

  return store;
}

Looking and the logs I can see the @@reduxReactRouter/historyAPI action being dispatched, but no changes in url or in state..

NOTE: the base project is this https://github.com/erikras/react-redux-universal-hot-example

Any help is appreciated

timaschew commented 8 years ago

The universal hot example has really lot of stuff in it. If you don't know how to use, start with this example instead: https://github.com/rackt/redux/tree/master/examples/real-world

I can say it's working fine with this snippet here:

const finalCreateStore = compose(
  applyMiddleware(thunk, api),
  reduxReactRouter({ routes, createHistory }),
  applyMiddleware(foobar, createLogger()), // do your stuff, then log
)(createStore)
thaerlabs commented 8 years ago

@timaschew figured out.. actually the issue was in the ordering of the middleware. thanks anyway

mohebifar commented 8 years ago

@thaerlabs Can you show how you fixed this issue in react-redux-universal-hot-example?

thaerlabs commented 8 years ago

@mohebifar This is the modified create.js

import { createStore as _createStore, applyMiddleware, compose } from 'redux';
import clientMiddleware from './middleware/clientMiddleware';
import transitionMiddleware from './middleware/transitionMiddleware';

export default function createStore(reduxReactRouter, getRoutes, createHistory, client, data) {

  const middleware = [clientMiddleware(client), transitionMiddleware];

  let finalCreateStore;

  if (__SERVER__) {
    finalCreateStore = compose(
      reduxReactRouter({ getRoutes, createHistory }),
      applyMiddleware(...middleware)
    )(_createStore);
  } else if (__CLIENT__ && __DEVELOPMENT__ && __DEVTOOLS__) {
    const { persistState } = require('redux-devtools');
    const DevTools = require('../containers/DevTools/DevTools');
    const logger = require('redux-logger')();
    finalCreateStore = compose(
      applyMiddleware(...middleware),
      reduxReactRouter({ getRoutes, createHistory }),
      applyMiddleware(logger),
      window.devToolsExtension ? window.devToolsExtension() : DevTools.instrument(),
      persistState(window.location.href.match(/[?&]debug_session=([^&]+)\b/))
    )(_createStore);
  } else if (__CLIENT__) {
    finalCreateStore = compose(
      applyMiddleware(...middleware),
      reduxReactRouter({ getRoutes, createHistory })
    )(_createStore);
  }

  const reducer = require('./modules/reducer');
  const store = finalCreateStore(reducer, data);

  if (__DEVELOPMENT__ && module.hot) {
    module.hot.accept('./modules/reducer', () => {
      store.replaceReducer(require('./modules/reducer'));
    });
  }

  return store;
}

I also removed redux-thunk, and in clientMiddleware.js I did this:

export default function clientMiddleware(client) {
  return ({dispatch, getState}) => {
    return next => action => {
      if (typeof action === 'function') {
        return action(dispatch, getState, client);
      }

      const { promise, types, ...rest } = action; // eslint-disable-line no-redeclare
      if (!promise) {
        return next(action);
      }

      const [REQUEST, SUCCESS, FAILURE] = types;
      next({...rest, type: REQUEST});
      return promise(client).then(
        (result) => next({...rest, result, type: SUCCESS})
      ).catch((error)=> {
        console.error('MIDDLEWARE ERROR:', error);
        next({...rest, error, type: FAILURE});
      });
    };
  };
}

Added the client as a third parameter of the actions passed as a function.. to be used like this in the action creator. eg. redux/modules/auth.js

export function login(email, password) {
  return (dispatch, getState, client) => {
    dispatch({ type: LOGIN });
    client.post('/api/auth/local', {
      data: { email, password }
    })
    .then(() => {
      dispatch({ type: LOGIN_SUCCESS });
      dispatch(load());
    })
    .catch((error) => {
      dispatch({ type: LOGIN_FAIL, error });
    });
  };
}

Cheers