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

Browser history not synched anymore with new router location starting from 6.6.0 #395

Open
ziriax opened this issue Feb 18, 2020 · 7 comments

Comments

@ziriax
Copy link

ziriax commented Feb 18, 2020

When reducing a state on some of our actions, we also output a new router location state using the following TS code:

interface RootState {
  router: RouterState;
  game: GameState;
}

function syncRouterLocation(state: RootState, pathname: string): RootState {
  const { router } = state;
  if (router.location.pathname !== pathname) {
    const location = createLocation(pathname);
    state = { ...state, router: { ...router, location } };
  }
  return state;
}

This code doesn't always work anymore with V6.6.0 and newer, the browser is sometimes not navigating to the new page (it depends on the starting page), I'm trying to find out what version broke it.

Our maybe our function above is just wrong, do we need to include other properties in the state? Do you have a helper function for this?

I guess it is this line that breaks our code, but just adding action: "PUSH" to the state doesn't seem to fix it.

What is the correct way to reduce the router location as a "side effect" in our own reducers?

@agriffis
Copy link

Time travelling is only intended for debugging with redux-devtools. Normally you should never write to the router's state from your app reducer. If this was working for you previously, I don't think it's how connected-react-router is intended to be used.

If you want to modify the location bar along with some of your own actions, you have a few choices:

  • You could use dispatch your action, then dispatch a push or replace

  • You could use https://github.com/tshelburne/redux-batched-actions to dispatch them both simultaneously

  • You could write a middleware that notices the actions that should change the location, and injects the appropriate router action

What you have in the description is kind of like the last option, but you tried to do it in a reducer, which only worked by chance.

@ziriax
Copy link
Author

ziriax commented Feb 20, 2020

Okay, I have worked around it, thanks for the info.

This does feel strange to me, the whole idea of Redux is that you have a state, and the view renders the state. I guess this paradigm breaks appart for browser history, since it is a stack, and can't just be "rendered".

@ziriax ziriax closed this as completed Feb 20, 2020
@ziriax ziriax reopened this Feb 20, 2020
@ziriax ziriax closed this as completed Feb 20, 2020
@ziriax
Copy link
Author

ziriax commented Feb 20, 2020

I'm reopening this issue, since I'm running circles with the proposed solutions, I guess I'm tackling this incorrectly.

The situation is as follows:

  • the Redux store has a {game, router} state.
  • each game has a name
  • when the user navigates to /play/{name}, the Play page is rendered. If the {name} doesn't corresponded to the current game state, it will fetch the game state from the server, and dispatch the SET_GAME action with the loaded game state

This works fine.

However, a SET_GAME action can also be dispatched differently, e.g. when the user loads a game by clicking a button. The reducer will update the game state, but now the Play page is rendered again, still with the old {name}, and it will load the old game again...

Setting the history location before SET_GAME is reduced won't work, since the Play page will then fetch the old game state.

Setting the history location after SET_GAME is reduced won't work, since the Play page will immediately render after the reduction took place with the wrong name...

So basically, I need to change the game state and the router state atomically, something that worked fine pre 6.6.0, even though it wasn't designed for this.

I could make different actions, but that doesn't seem to solve the problem.

I could pass the game state as the router location state, but that feels wrong, it should be independent...

This feels such a common scenario that a solution must exist for this?

For now I will revert to 6.5.2, since that solved all my problems ;-)

@ziriax ziriax reopened this Feb 20, 2020
@agriffis
Copy link

However, a SET_GAME action can also be dispatched differently, e.g. when the user loads a game by clicking a button. The reducer will update the game state, but now the Play page is rendered again, still with the old {name}, and it will load the old game again...

Instead of SET_GAME can you push here?

@ziriax
Copy link
Author

ziriax commented Feb 21, 2020

Instead of SET_GAME can you push here?

Yes, my example was simplified too much. SET_GAME is also used for game states that aren't saved on the server yet. I also want to be able to replay the redux logs, SET_GAME always carries a full state, nothing is ever loaded from the server when replaying actions. So basically, SET_GAME is the only action to provide a new state to the app.

I could solve my issue if I could detect if a page was rendered because a user navigated to it, because in all other cases, the program controls the state, and the page shouldn't alter it. Like the HTML input element, it only fires the change event when the user changed the element, not when code changed the value (as demonstrated here).

Maybe I could use the location state to detect the above? So when my middleware pushes the URL that corresponds to the next new game state that will be set, I pass a location state that says hey page, a new state is coming in, don't load one. Yes, that would actually work I guess? Amazing how just trying to explain a problem often results in finding a solution yourself, LOL ;-) The proof of the pudding is in the eating, I'm going to try this asap

Thanks for the feedback!

@ziriax
Copy link
Author

ziriax commented Feb 21, 2020

Yes, that seems to work!

EDIT: No it doesn't, it seems a page refresh keeps the router location state somehow?

One small problem remains (independently from this): when replaying actions with the redux devtools, the initial page render doesn't realize it is being played back, and it will fetch the state from the server, dispatching a new action...

This is another issue: IMHO fetching state should rarely be done in a view, instead it should always be the result of a user or external event (e.g. the user clicking a button, the user navigating to a page).

But I can't find an event on the <Route/> for that, but that's a question for the React Router devs.

@ziriax
Copy link
Author

ziriax commented Feb 21, 2020

It seems I was just using the wrong library for my architecture, https://github.com/faceyspacey/redux-first-router might suit me better?

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

2 participants