Skip to content
This repository has been archived by the owner on Oct 26, 2018. It is now read-only.

@@router/INIT_PATH doesn't work with initalState? #122

Closed
imevro opened this issue Dec 16, 2015 · 4 comments
Closed

@@router/INIT_PATH doesn't work with initalState? #122

imevro opened this issue Dec 16, 2015 · 4 comments

Comments

@imevro
Copy link

imevro commented Dec 16, 2015

Hi, I have this setup of my store:

const finalCreateStore = compose(
  autoRehydrate(),
  applyMiddleware(thunk, logger),
)(createStore);

const store = finalCreateStore(reducer, {
  routing: {
    path: `/contacts/30/about`,
  },
});

and then at app.jsx I use syncReduxAndRouter(history, store).
When I start my app redux-simple-router ignores my initialState with routing.path. Why?

localhost 3000 2015-12-16 04-03-43

@kimjoar
Copy link
Collaborator

kimjoar commented Dec 16, 2015

Hi @theaqua, thanks for the input. I just don't think we ever thought about this use-case. I played around with this now, but got into a problem with our tests that makes this difficult right now.

So it's definitely possible to implement, but I think we need to solve the test issue before adding this (but we're very open to PRs, so if someone has an idea, please try it out and see if it works!)

@jlongster The problem as far as I can see is that initialState is "global state" across invocations of syncReduxAndRouter. My initial idea was to do something like this:

if (!lastRoute) {
  // if getRouterState() is set, default to that
  // otherwise do what we do today
}

But that won't work in the tests as getRouterState() will only be "correct" on the first invocation of syncReduxAndRouter (so I got the feature working, but the tests failed, but they didn't fail "in the right way", as the test ran when I only ran the failing test).

I think we need to consider moving everything into a function that you invoke (similar to createHistory), so we get an "instance" of redux-simple-router that you work with. I think that might simplify some cases — but I really haven't gone through all the details, it just "feels" right. My next week will be busy, but I'll try to play around with ideas when I've got the time.

@jlongster
Copy link
Member

Yeah, it's not great that we change initialState from a single syncReduxAndRouter. Although I'd argue that you really can only sync once across your whole app. I'm open to other ways to set it up though.

This might be fixed by adding:

  if(getRouterState().path) {
    lastRoute = getRouterState();
  }

Right when syncReduxAndRouter is called. But I'm not sure. I'm going through issues and trying to build up context for where we stand.

@kimjoar
Copy link
Collaborator

kimjoar commented Dec 18, 2015

Yep, no problem in apps, only a problem in the tests

@timdorr
Copy link
Member

timdorr commented Jan 14, 2016

This is fixed in 2.0.0 AFAIK.

@timdorr timdorr closed this as completed Jan 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants