Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initializing bug in 6.3.2? #266

Closed
bmueller-sykes opened this issue Mar 11, 2019 · 4 comments
Closed

Initializing bug in 6.3.2? #266

bmueller-sykes opened this issue Mar 11, 2019 · 4 comments

Comments

@bmueller-sykes
Copy link

Hi,

After upgrading to 6.3.2 today, my published react project started acting wonky. What appears to be happening is that if I open my app in a browser, it will be re-directed to the last opened page. For example, let's say browser tab A is opened to:

http://mysite.com/path/to/something

...and then I open browser tab B and navigate to:

http://mysite.com/totally/different/path

...browser tab B will be redirected to

http://mysite.com/path/to/something

...immediately. I wasn't able to run any extensive testing I'm afraid, as I had to downgrade right away to get to some pressing work. I'm running React 16.8.4, react-router 4.3.1, and connected-react-router 6.3.2 (before the downgrade).

My config is as follows.

I have a custom history setup like this:

import createBrowserHistory from 'history/createBrowserHistory';

const history = createBrowserHistory({ basename: '/my/basepath/' });

export const navigate = (url) => {
  history.push(url);
};

export default history;

...and then my (somewhat stripped down) store config looks like this:

import { persistCombineReducers, persistStore } from 'redux-persist';
import sessionStorage from 'redux-persist/es/storage/session';

const allReducers = {
	...myAppReducers,
	router: connectRouter(customHistory)
}
const reducers = persistCombineReducers(persistConfig, allReducers);

export default function configureStore() {

	const store = createStore (
	 connectRouter(customHistory)(rootReducer),
	 {},
	 composeEnhancers (
	   responsiveStoreEnhancer,
	   applyMiddleware (
			 sagaMiddleware,
			 routerMiddleware(customHistory)
	   )
	 )
	)

	const persistor = persistStore (store)
	sagaMiddleware.run(rootSaga)
	return { persistor, store }
}

Hmm...could this be a bad interaction between redux-persist, I wonder? As I said at the top, this doesn't happen under 6.3.1, so it seems like the problem is localized to connected-react-router, but I could imagine some bad interaction between the different libraries.

Hope this is useful!

@mdupont
Copy link

mdupont commented Mar 13, 2019

I had a somewhat related issue (#267) caused by the fix present on v6.3.2 that makes isFirstRendering work properly (in earlier versions like v6.3.1, it was always false).

Since you say you're opening a new tab (therefore causing a "first rendering" of the page), I suspect it's the same cause.

I guess the problem comes from this statement in the reducer code: https://github.com/supasate/connected-react-router/blob/v6.3.2/src/reducer.js#L24

        return isFirstRendering
          ? state
          : merge(state, { location: fromJS(location), action })
}
  • when isFirstRendering is true, the reducer just returns the current state, when it's false, it uses the location from the payload
  • I don't know how redux-persist works, but I see it stores into Session Storage, which is shared across tabs for the same origin (domain + port).

I suspect something like:

  • you open the tab B, requesting a path
  • the router kicks in, see it's the first rendering (isFirstRendering = true, only in v6.3.2)
  • the reducer sees isFirstRendering = true so it returns the state as-is
  • alas the state comes from Session Storage, shared across all tabs accessing you app, and therefore contains the latest state of tab A, causing a redirect to tab A location.

@bmueller-sykes
Copy link
Author

@mdupont That sounds like a plausible explanation. I don't see how this isn't bad behavior, though. In the world of a single page app, there are still times where you legitimately want to open a second window of the app, and you certainly don't want that new window navigating somewhere you didn't tell it to. In my case, I open that second window to a specific route (e.g. /my/second/route), and connected-react-router navigates away from that route upon load.

One potential solution here is that redux-persist shouldn't persist the data connected-react-router stores. So, when redux-persist rehydrates the app from its data store (which can be whatever you want it to be, but in my case is indeed Session Storage, as you note), it just skips the stuff connected-react-router stores. That seems like it might work when opening a new window, but it seems like you wouldn't want that behavior if you simple reload the existing window, and therefore are just reloading the main instance of your React app.

So it still seems to me like the behavior in 6.3.2 is broken, but I'd be happy to be wrong here, so long as there's a solution that gets me where I need to be.

FWIW, I use connected-react-router primarily so I can trigger route changes from outside router calls (e.g. I sometimes do route changes from inside redux-saga saga functions.)

@bmueller-sykes
Copy link
Author

Okay, so it looks like adding blacklist: ['router'] to the redux-persist config options does the trick. I'll close this for now, and re-open if that turns out not to fix the issue.

@mbifulco
Copy link

@bmueller-sykes - dropping in to say that the blacklist fix worked for me as well. If you're running a production app, you may also want to add a migration to remove router from your existing users' persisted states:

Mine looks like this, and seems to do the trick:

state => {
  const { router, ...restOfState } = state;
  return restOfState;
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants