acdlite / redux-router

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

Better warning if store has a initialState for routes #203

Closed zimond closed 8 years ago

zimond commented 8 years ago

If the store is initiated with following state:

{
  router: {},
  app: { /* general app state data*/ }
}

redux-router will throw error in useDefault.js because it could not get the next history state. Maybe this could be better warned.

Scarysize commented 8 years ago

@daiheitan By "warned" you mean added to the documentation more explicitly?

zimond commented 8 years ago

Output warning to console, or maybe just throw an error when user defines 'router' like this in the initial state.

Scarysize commented 8 years ago

On the todo-list for the next release.

Scarysize commented 8 years ago

Sorry, I need some clarification. Default initialization looks something like this:

const reducer = combineReducers({
  router: routerStateReducer
});

const store = compose(
  reduxReactRouter({ createHistory }),
  devTools()
)(createStore)(reducer);

What would yours - producing the error - look like? @daiheitan

zimond commented 8 years ago

@Scarysize I think that you can send a second param to store

const store = compose(...)(createStore)(reducer, initState)

and if the initState has router: {}, the error comes. Sorry that I could not test it now.

Scarysize commented 8 years ago

Hmmm. Do you have this problem on the server-side? I tried to reproduce it on the client-side, but I´m not running into any errors.

const store = compose(
  reduxReactRouter({ createHistory }),
  devTools()
)(createStore)(reducer, {
  router: {},
  app: {},
});

I only get an error for the app key, but this expected as I don´t have a corresponding reducer. The router sets up correctly and works. The best thing you could do is to submit a either a complete gist or - even better - a failing test.

zimond commented 8 years ago

I checked again and found that the bug was fixed in 80299c9. Sorry for the late report and I will close this issue.

Scarysize commented 8 years ago

@daiheitan Good to hear!