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

syncHistoryWithStore does not respect redux state #534

Open
pronebird opened this issue Feb 15, 2017 · 6 comments
Open

syncHistoryWithStore does not respect redux state #534

pronebird opened this issue Feb 15, 2017 · 6 comments

Comments

@pronebird
Copy link

pronebird commented Feb 15, 2017

Hi,

I am trying to restore the displayed page on app startup but it's always /, redux store is preloaded from local storage. HoweversyncHistoryWithStore erases the state.

I use redux-localstorage to persist redux to local storage which seems to be merging state from local storage into initialState therefore restoring the state without telling anyone which shouldn't be any different for the app when the concept of single source of truth is applied.

I am sure this issue can be reproduced with initialState with hardcoded route.

import { routerMiddleware, routerReducer as routing } from 'react-router-redux';
import persistState from 'redux-localstorage';
import thunk from 'redux-thunk';
import routes from './routes';

const router = routerMiddleware(hashHistory);

const reducers = {
  routing
};

const middlewares = [ thunk, router ];

function configureStore(initialState) {
  const enhancer = compose(applyMiddleware(...middlewares), persistState());
  const rootReducer = combineReducers(reducers);
  
  return createStore(rootReducer, initialState, enhancer);
}

const initialState = {};
const store = configureStore(initialState);
const routerHistory = syncHistoryWithStore(hashHistory, store);

ReactDOM.render(
  <Provider store={ store }>
    <Router history={ routerHistory } routes={ routes } />
  </Provider>,
  rootElement
);

I tried adjustUrlOnReplay without luck:

const routerHistory = syncHistoryWithStore(hashHistory, store, { adjustUrlOnReplay: true });

Thanks

@pronebird
Copy link
Author

pronebird commented Feb 15, 2017

Debugging through syncHistoryWithStore reveals that the following statement breaks:

if (currentLocation === locationInStore || initialLocation === locationInStore) {
        return;
}

in my case initialLocation restored from state and is /connect and is the same as locationInStore, therefore syncHistoryWithStore does not attempt to restore the location.
That seems to be error prone in its essence:

// Init initialLocation with potential location in store
initialLocation = getLocationInStore();

initialLocation === locationInStore will always be true on cold restart with state restored from local storage. This code will only work if we push new location first.

Why not to consult history for initial location? I mean, browser should know better right? When the app cold starts / is initial location of browser, but state has /connect set.

@pronebird
Copy link
Author

pronebird commented Feb 15, 2017

As a workaround we could simply enforce the transition to new route right after syncHistoryWithStore. Since syncHistoryWithStore will always reset the route to /, we could save the last visited location prior to the call and then feed it back, i.e:

const recentLocation = (store.getState().routing || {}).locationBeforeTransitions;
const routerHistory = syncHistoryWithStore(hashHistory, store, { adjustUrlOnReplay: true });
if(recentLocation && recentLocation.pathname) {
  routerHistory.replace(recentLocation.pathname);
}

@rob2d
Copy link

rob2d commented Mar 27, 2017

Just a random note, it seems the syncHistoryWithStore method isn't a necessity in v5.0.0 as the middleware takes care of it. So perhaps if migrating to the alpha for that, no more issues? [outside of... the alpha thing]

@MacKentoch
Copy link

It seems like react-router-redux v5.0.0 will fix a lot of things when using react-router 4.
But what about using immutable state, will a custom routerReducer do the job:

import { fromJS }           from 'immutable';
import { LOCATION_CHANGE }  from 'react-router-redux';

const initialState = fromJS({
  locationBeforeTransitions: null
});

export default (state = initialState, action) => {
  if (action.type === LOCATION_CHANGE) {
    return state.set('locationBeforeTransitions', action.payload);
  }

  return state;
};

Or middleware will take care of it?

@rowlandekemezie
Copy link

@MacKentoch have you gotten a workaround for your question ☝️

@MacKentoch
Copy link

@mentrie I ended my starter with react-router-redux 4.x.x and wrote my own reducer.

I did not give a try to 5.x since I did have the use case in private or pro project then.

But, I keep interesting in new about it

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