Reloading causes "Warning: You cannot PUSH the same path using hash history" with react-router-redux #121

Closed
opened 2016-03-30 12:49:25 +00:00 by scarlac · 7 comments
scarlac commented 2016-03-30 12:49:25 +00:00 (Migrated from github.com)

This is a bug that arises from using popular packages together. I'm posting the bug here, and hoping you can cooperate with the involved parties.

If a user reloads an app that uses redux-storage & redux-storage-engine-localstorage & react-router & react-router-redux then the app will eventually complain with the error Warning: You cannot PUSH the same path using hash history.

Cause: React-storage triggers react-router-redux's enhanced react-router history, since it's causing a change in state. This in turns causes a history.push(...) to the restored URL, which is the same since it's a reload of the same page, thus causing the warning.

This issue doesn't seem to be directly addressed in the other projects since it's caused by using this plugin which automatically loads router state. Note that loading the state before enhanced router will avoid this bug, but then redux-storage fails to restore previous state on the second reload.

This is a bug that arises from using popular packages together. I'm posting the bug here, and hoping you can cooperate with the involved parties. If a user reloads an app that uses redux-storage & redux-storage-engine-localstorage & react-router & react-router-redux then the app will eventually complain with the error `Warning: You cannot PUSH the same path using hash history`. Cause: React-storage triggers react-router-redux's enhanced react-router history, since it's causing a change in state. This in turns causes a `history.push(...)` to the restored URL, which is the same since it's a reload of the same page, thus causing the warning. This issue doesn't seem to be directly addressed in the other projects since it's caused by using this plugin which automatically loads router state. Note that loading the state before enhanced router will avoid this bug, but then redux-storage fails to restore previous state on the second reload.
export-mike commented 2016-05-06 12:41:05 +00:00 (Migrated from github.com)

hi have you managed to fix this error? I'm rolling my own state serialise and deserialise but sounds like the same issue.

hi have you managed to fix this error? I'm rolling my own state serialise and deserialise but sounds like the same issue.
scarlac commented 2016-05-06 14:19:50 +00:00 (Migrated from github.com)

Sorry, I did not find an exact fix. In your case, when writing your own serializer, you can make explicit exceptions to what gets saved and loaded. I'd consider it somewhat of a hack but simply ignoring restoring URL would be a way to make it work if you're already at the URL.

Sorry, I did not find an exact fix. In your case, when writing your own serializer, you can make explicit exceptions to what gets saved and loaded. I'd consider it somewhat of a hack but simply ignoring restoring URL would be a way to make it work if you're already at the URL.
export-mike commented 2016-05-06 15:20:57 +00:00 (Migrated from github.com)

@scarlac thats exactly what I have in mind. but feels like I'm attempting to fix something that someone else has fixed more elegantly. I've still got the warning, might come back to it on a rainy day.

@scarlac thats exactly what I have in mind. but feels like I'm attempting to fix something that someone else has fixed more elegantly. I've still got the warning, might come back to it on a rainy day.
michaelcontento commented 2016-05-16 14:34:29 +00:00 (Migrated from github.com)

Maybe I don't understand the problem properly .. but .. well, it sounds like redux-storage-decorator-filter could be the missing piece!?

Maybe I don't understand the problem properly .. but .. well, it sounds like [redux-storage-decorator-filter](https://github.com/michaelcontento/redux-storage-decorator-filter) could be the missing piece!?
scarlac commented 2016-05-16 15:09:43 +00:00 (Migrated from github.com)

Unfortunately I don't think so @michaelcontento. It's a "orchestration" issue that follows from blindly restoring state to all components. I do have to say that I am not sure there's an easy fix.

Here's more verbose use-case-like explaination:

  1. User visits /mysite. Browser finishes loading.
  2. Redux-storage saves current state, including state of react-router which contain current URL.
  3. User presses a React router link that is pointing to /mysite/dashboard
    3a. react-router calls setState with currentUrl: "/mysite/dashboard" which triggers an internal process that then calls history.push(this.state.currentUrl).
    3b. Redux-storage is subscribed to state updates and is thus triggered to save current state, including state of react-router which contain current URL, now /mysite/dashboard.
  4. User pressede the reload button. The browser starts reloading /mysite/dashboard...
    4a. react-router(-redux) initializes with a store-synched history
    4b. redux-storage initializes, reads from storage and restores currentUrl to /mysite/dashboard which causes a state change trigger...
    4c. react-router is then re-rendered with the new state object because of react-router-redux's synched hsitory, but the same currentUrl as browser is already on which causes it to log a warning since browser is already at that URL.

If there was a way to inject a small piece of code into redux-storage to prevent it from restoring certain parts of the global state, using a callback, that would be a possible solution. The filter decorator is some of the fix, but you don't always want to ignore the values - only in the case where state.currentUrl == location.href.toString()-ish. But maybe it could be patched to do it?

Unfortunately I don't think so @michaelcontento. It's a "orchestration" issue that follows from blindly restoring state to all components. I do have to say that I am not sure there's an easy fix. Here's more verbose use-case-like explaination: 1. User visits `/mysite`. Browser finishes loading. 2. Redux-storage saves current state, including state of react-router which contain current URL. 3. User presses a React router link that is pointing to `/mysite/dashboard` 3a. react-router calls `setState` with `currentUrl: "/mysite/dashboard"` which triggers an internal process that then calls `history.push(this.state.currentUrl)`. 3b. Redux-storage is subscribed to state updates and is thus triggered to save current state, including state of react-router which contain current URL, now `/mysite/dashboard`. 4. User pressede the reload button. The browser starts reloading `/mysite/dashboard`... 4a. react-router(-redux) initializes with a store-synched history 4b. redux-storage initializes, reads from storage and restores `currentUrl` to `/mysite/dashboard` which causes a state change trigger... 4c. react-router is then re-rendered with the new state object because of react-router-redux's synched hsitory, but the same `currentUrl` as browser is already on which causes it to log a warning since browser is already at that URL. If there was a way to inject a small piece of code into redux-storage to prevent it from restoring certain parts of the global state, using a callback, that would be a possible solution. The filter decorator is some of the fix, but you don't _always_ want to ignore the values - only in the case where `state.currentUrl == location.href.toString()`-ish. But maybe it could be patched to do it?
michaelcontento commented 2016-05-16 15:26:56 +00:00 (Migrated from github.com)

I'm not sure if it's a good idea to persist currentUrl at all. What's the benefit of doing so?

Maybe I'm wrong but event at the earliest point possible (read: where you construct all the required objects (redux, redux-storage, react-router, ...) after a full reload) the destination url (=currentUrl) is already determined.

The loaded currentUrl could only be different if the user changes the address bar and hits enter. This would cause react-router to render /changed/by/user and, after redux-storage has received the old state from the engine, a re-render with /loaded/from/old/state. Is this even a desired behaviour?

But even IF this is a desired behaviour - use a custom merger! 😄
A merger is passed as second argument to storage.reducer and is responsible for properly merging currentState with loadedState. So this is exactly the point where you could perform the state.currentUrl == location.href.toString() comparison.

I'm not sure if it's a good idea to persist `currentUrl` at all. What's the benefit of doing so? Maybe I'm wrong but event at the earliest point possible (read: where you construct all the required objects (redux, redux-storage, react-router, ...) after a full reload) the destination url (=`currentUrl`) is already determined. The loaded `currentUrl` could only be different if the user changes the address bar and hits enter. This would cause `react-router` to render `/changed/by/user` and, after redux-storage has received the old state from the engine, a re-render with `/loaded/from/old/state`. Is this even a desired behaviour? But even IF this is a desired behaviour - use a custom merger! :smile: A merger is passed as second argument to [`storage.reducer`](https://github.com/michaelcontento/redux-storage/blob/master/src/reducer.js#L5) and is responsible for properly merging `currentState` with `loadedState`. So this is exactly the point where you could perform the `state.currentUrl == location.href.toString()` comparison.
scarlac commented 2016-05-16 15:44:03 +00:00 (Migrated from github.com)

I think it's realistic to assume that cherry-picking from react-router(-redux) state can cause some pretty exotic bugs that you can't issue reports for. One of the proposed features of using react-router-redux is that when your location is a part of the store, you can work with the time machine debugger, etc.

But! I didn't see the custom merger stuff! That could very well be the solution 👍

Perhaps someone will make a PR for documentation of storage.reducer ahem

I think it's realistic to assume that cherry-picking from react-router(-redux) state can cause some pretty exotic bugs that you can't issue reports for. One of the proposed features of using react-router-redux is that when your location is a part of the store, you can work with the time machine debugger, etc. But! I didn't see the custom merger stuff! That could very well be the solution 👍 Perhaps someone will make a PR for documentation of `storage.reducer` _ahem_
This discussion has been locked. Commenting is limited to contributors.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
MichaelContento/redux-storage#121
No description provided.