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

popTo causes an infinite loop when route is not found #3026

Closed
Jarred-Sumner opened this issue May 8, 2018 · 5 comments
Closed

popTo causes an infinite loop when route is not found #3026

Jarred-Sumner opened this issue May 8, 2018 · 5 comments
Labels

Comments

@Jarred-Sumner
Copy link

Jarred-Sumner commented May 8, 2018

Version

Tell us which versions you are using:

  • react-native-router-flux v4.0.0 beta 27 (v3 is not supported)
  • react-native v0.5.5

Expected behaviour

  • popTo returns null or throws an Error of some kind

Actual behaviour

When calling Actions.popTo on a route that is not in the navigation stack, it infinite loops (inside of the Reducer)

Additionally, I expected it to "just work" given a parent route. More context:

  • I have some tabs and some modals of $N depth. One of the modals are the current scene. I want to dismiss the modal. Going back just once might not be enough. The route I want to go back to is whichever was last selected on a tab. To do that, I wrongly assumed I could pass Actions.popTo the tabs route and it would "just work", instead it infinite loops.
  • My solution is to manually track what the last child route of the tab bar that was active, and popTo that child route

Steps to reproduce

For non-obvious bugs, please fork this component, modify Example project to reproduce your issue and include link here.

  1. Open your app
  2. Call `Actions.popTo("AnyRouteThatDoesntExist")
  3. Infinite loop
@daviscabral
Copy link
Collaborator

@Jarred-Sumner I am able to reproduce the error - but it seems to be related to react-navigation. But I believe something can be done over it to avoid that. I will spare some time this week to investigate this issue. Thank you.

@daviscabral
Copy link
Collaborator

Alright - I believe I found the cause of the issue. When reaching the top level, the state is always the same - never finding the undefined route. That might also happen with a defined route in a different stack level.

Adjusted to verify if the current state is the same as the top level from the current router - if does, just break the loop.

@AlexDM0
Copy link

AlexDM0 commented May 30, 2018

I solved this by overloading the popTo in the reducer that you give to the Router.createReducer.

export const reducerCreate = (params) => {
  const defaultReducer = Reducer(params, {});
  return (state, action)=>{
    if (action && action.type == "REACT_NATIVE_ROUTER_FLUX_POP_TO") {
      // check if we can see the key in the list of items, if not, do a back.
      let popCount = 0;
      let success = false;
      // search throught all routes in the state, starting from the top, for one that matches the route name
      for (let i = state.index; i >= 0; i--) {
        if (state.routes[i].routeName == action.routeName) {
          // we found it!
          success = true
          break;
        }
        // how many pops do we need for this operation?
        popCount++;
      }
      if (success) {
        // if our routing stack is [1, 2, 3, 4, 5] and we're in 5 and want to go back to 1, we remove [2,3,4] so [1,5] are left, then do a nav/back for animated transition
        // cut out the intermediates ([2,3,4])
        if (popCount == 1) {
          return defaultReducer(state, {type: "Navigation/BACK"});
        }
        else if (popCount > 1) {
          let newState = {...state};
          newState.routes = newState.routes.slice(0, newState.routes.length - popCount);
          // push the current scene back on top of the stack.
          newState.routes.push(state.routes[state.routes.length-1])

          // have the index point to a the correct scene.
          newState.index -= (popCount - 1);

          // go back one step to go from 5 to 1
          return defaultReducer(newState, {type: "Navigation/BACK"});
        }
        else {
          // popCount = 0, we're already there?
          LOGw.info("navigation.ts: Tried PopTo with name", action.routeName, " while already on that route. Popping once.")
          return defaultReducer(state, {type: "Navigation/BACK"});
        }
      }
      else {
        // just go back one if we can't find the target?
        LOGw.info("navigation.ts: Tried PopTo with name", action.routeName, " but could not find target route. Popping once.")
        return defaultReducer(state, {type: "Navigation/BACK"});
      }
    }
    
    return defaultReducer(state, action);
  }
};

edit:
usage:

<Router createReducer={reducerCreate} ... >

Hope it helps!

aksonov pushed a commit that referenced this issue Jun 6, 2018
…) (#3043)

* popTo causes an infinite loop when route is not found (as seen on #3026)

* Added warning
@daviscabral
Copy link
Collaborator

@AlexDM0 / @Jarred-Sumner the PR that fixes this issue just got merged. Now, instead of the loop, you will be warned (console.warn) about the unknown route. I would appreciate the extra eyes on this from you guys. Thank you.

@AlexDM0
Copy link

AlexDM0 commented Jun 7, 2018

Hi,

I've had issues with Android (using a Drawer with a nested scene) and iOS using a tabBar with nested scenes together with normal scenes (router here:https://github.com/crownstone/CrownstoneApp/blob/master/js/router/RouterIOS.js#L35)

I solved this by going into nested sets to find the key:
https://github.com/crownstone/CrownstoneApp/blob/master/js/router/store/reducers/navigation.ts

Basically, I reconstruct the state to have only the scene I'm at and the target scene, then use a normal back action to have the animation.

I think it's a little shortsighted to only compare the routename and not account for nesting... If you have a router like:

<Router>
  <Scene key="1" />
  <Drawer key="drawer">
    <Scene key="2" initial={true} />
  </Drawer>
</Router>

Then do Actions.1(), to go from 2 -> 1, then Actions.popTo(2) and it fails.

Cheers

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

No branches or pull requests

3 participants