xkawi / react-universal-saga

Universal React Starter Kit ft. Redux Saga
https://react-universal-saga.herokuapp.com
MIT License
186 stars 33 forks source link

In this configuration sagas cannot take router events #12

Closed alex-shamshurin closed 7 years ago

alex-shamshurin commented 7 years ago

For example saga will not get '@@router/LOCATION_CHANGE' event on initial loading, because 'store.runSaga(rootSaga);' happens after history is configured.

It must be smth like that:

const store   = configureStore(initialState, browserHistory)
store.runSaga(rootSaga, store.dispatch)

const history = syncHistoryWithStore(browserHistory, store)
const routes  = createRoutes(store)
xkawi commented 7 years ago

@alex-shamshurin PR is welcomed. 😃

MysticEyeBlink commented 7 years ago

Any progress with this?

xkawi commented 7 years ago

@MysticEyeBlink @alex-shamshurin was looking into this.

Want to clarify, is this related to the usage of react-router-redux in this project configuration?

If it is, this project configuration does not come with react-router-redux. It manages the routing event manually using the UPDATE_ROUTER_STATE action.

If you want to use react-router-redux, you have to install and set it up on your own, like below:

const dest = document.getElementById('content');
const store = configureStore(history, window.__data); // eslint-disable-line

GoogleAnalytics.initialize(config.app.googleAnalytics.appId);

store.runSaga(rootSaga);

// notice that this line is after store.runSaga
const syncedHistory = syncHistoryWithStore(history, store);

// also note that now you are using syncedHistory rather than history
render(
  <Root
    store={store}
    history={syncedHistory}
    routes={getRoutes(store)}
  />,
  dest
);

Now, if you add a saga watcher to listen for @@router/LOCATION_CHANGE event provided by react-router-redux, the saga will be executed during initial loading, here is an example of such saga:

// src/sagas/index.js

function* watchReactRouterLocationChange() {
  while (true) {
    const newLocation = yield take('@@router/LOCATION_CHANGE');
    console.log('@@router/LOCATION_CHANGE is called', newLocation);
  }
}

export default function* root() {
  yield [
    fork(watchNavigate),
    fork(watchLoadUserPage),
    fork(watchLoadRepoPage),
    fork(watchLoadMoreStarred),
    fork(watchLoadMoreStargazers),
    fork(watchReactRouterLocationChange)
  ];
}

I guess the confusion here is this line:

const store = configureStore(history, window.__data);

Because if you look at configureStore function @ src/store/configureStore.dev.js or src/store/configureStore.prod.js, the first history argument is not actually being used. Will fix this confusion in the next patch release.

In general, don't think it's necessary to configure react-router-redux in this setup, as stated at react-router-redux readme:

This library is not necessary for using Redux together with React Router. You can use the two together just fine without any additional libraries. It is useful if you care about recording, persisting, and replaying user actions, using time travel. If you don't care about these features, just use Redux and React Router directly.

xkawi commented 7 years ago

going to close this for now since no activity so far. feel free to open it again if would like to continue the discussion.