fix: fixed Tab/SwitchRouter incorrectly switching children on "ba… (#74)

Author: hsource

src/routers/SwitchRouter.js

@@ -4,6 +4,7 @@ import createConfigGetter from './createConfigGetter';
 
 import * as NavigationActions from '../NavigationActions';
 import * as SwitchActions from './SwitchActions';
+import * as StackActions from './StackActions';
 import validateRouteConfigMap from './validateRouteConfigMap';
 import { createPathParser } from './pathUtils';
 
@@ -248,8 +249,6 @@ export default (routeConfigs, config = {}) => {
           const routeKey =
             state.routeKeyHistory[state.routeKeyHistory.length - 2];
           activeChildIndex = order.indexOf(routeKey);
-        } else {
-          return state;
         }
       }
 
@@ -337,44 +336,57 @@ export default (routeConfigs, config = {}) => {
         return { ...state };
       }
 
+      const isActionBackOrPop =
+        action.type === NavigationActions.BACK ||
+        action.type === StackActions.POP ||
+        action.type === StackActions.POP_TO_TOP;
+      const sendActionToInactiveChildren =
+        !isActionBackOrPop ||
+        (action.type === NavigationActions.BACK && action.key != null);
+
       // Let other children handle it and switch to the first child that returns a new state
-      let index = state.index;
-      let routes = state.routes;
-      order.find((childId, i) => {
-        const childRouter = childRouters[childId];
-        if (i === index) {
+      // Do not do this for StackActions.POP or NavigationActions.BACK actions without a key:
+      // it would be unintuitive for these actions to switch to another tab just because that tab had a Stack that could accept a back action
+      if (sendActionToInactiveChildren) {
+        let index = state.index;
+        let routes = state.routes;
+        order.find((childId, i) => {
+          const childRouter = childRouters[childId];
+          if (i === index) {
+            return false;
+          }
+          let childState = routes[i];
+          if (childRouter) {
+            childState = childRouter.getStateForAction(action, childState);
+          }
+          if (!childState) {
+            index = i;
+            return true;
+          }
+          if (childState !== routes[i]) {
+            routes = [...routes];
+            routes[i] = childState;
+            index = i;
+            return true;
+          }
           return false;
+        });
+
+        // Nested routers can be updated after switching children with actions such as SET_PARAMS
+        // and COMPLETE_TRANSITION.
+        if (action.preserveFocus) {
+          index = state.index;
         }
-        let childState = routes[i];
-        if (childRouter) {
-          childState = childRouter.getStateForAction(action, childState);
-        }
-        if (!childState) {
-          index = i;
-          return true;
-        }
-        if (childState !== routes[i]) {
-          routes = [...routes];
-          routes[i] = childState;
-          index = i;
-          return true;
-        }
-        return false;
-      });
 
-      // Nested routers can be updated after switching children with actions such as SET_PARAMS
-      // and COMPLETE_TRANSITION.
-      if (action.preserveFocus) {
-        index = state.index;
+        if (index !== state.index || routes !== state.routes) {
+          return getNextState(action, prevState, {
+            ...state,
+            index,
+            routes,
+          });
+        }
       }
 
-      if (index !== state.index || routes !== state.routes) {
-        return getNextState(action, prevState, {
-          ...state,
-          index,
-          routes,
-        });
-      }
       return state;
     },
 

src/routers/__tests__/SwitchRouter-test.js

@@ -139,6 +139,100 @@ describe('SwitchRouter', () => {
     expect(getSubState(2).routeName).toEqual('B1');
   });
 
+  it('handles back and does not apply back action to inactive child', () => {
+    const { navigateTo, back, getSubState } = getRouterTestHelper(
+      getExampleRouter({
+        backBehavior: 'initialRoute',
+        resetOnBlur: false, // Don't erase the state of substack B when we switch back to A
+      })
+    );
+
+    expect(getSubState(1).routeName).toEqual('A');
+
+    navigateTo('B');
+    navigateTo('B2');
+    expect(getSubState(1).routeName).toEqual('B');
+    expect(getSubState(2).routeName).toEqual('B2');
+
+    navigateTo('A');
+    expect(getSubState(1).routeName).toEqual('A');
+
+    // The back action should not switch to B. It should stay on A
+    back({ key: null });
+    expect(getSubState(1).routeName).toEqual('A');
+  });
+
+  it('handles pop and does not apply pop action to inactive child', () => {
+    const { navigateTo, pop, getSubState } = getRouterTestHelper(
+      getExampleRouter({
+        backBehavior: 'initialRoute',
+        resetOnBlur: false, // Don't erase the state of substack B when we switch back to A
+      })
+    );
+
+    expect(getSubState(1).routeName).toEqual('A');
+
+    navigateTo('B');
+    navigateTo('B2');
+    expect(getSubState(1).routeName).toEqual('B');
+    expect(getSubState(2).routeName).toEqual('B2');
+
+    navigateTo('A');
+    expect(getSubState(1).routeName).toEqual('A');
+
+    // The pop action should not switch to B. It should stay on A
+    pop();
+    expect(getSubState(1).routeName).toEqual('A');
+  });
+
+  it('handles popToTop and does not apply popToTop action to inactive child', () => {
+    const { navigateTo, popToTop, getSubState } = getRouterTestHelper(
+      getExampleRouter({
+        backBehavior: 'initialRoute',
+        resetOnBlur: false, // Don't erase the state of substack B when we switch back to A
+      })
+    );
+
+    expect(getSubState(1).routeName).toEqual('A');
+
+    navigateTo('B');
+    navigateTo('B2');
+    expect(getSubState(1).routeName).toEqual('B');
+    expect(getSubState(2).routeName).toEqual('B2');
+
+    navigateTo('A');
+    expect(getSubState(1).routeName).toEqual('A');
+
+    // The popToTop action should not switch to B. It should stay on A
+    popToTop();
+    expect(getSubState(1).routeName).toEqual('A');
+  });
+
+  it('handles back and does switch to inactive child with matching key', () => {
+    const { navigateTo, back, getSubState } = getRouterTestHelper(
+      getExampleRouter({
+        backBehavior: 'initialRoute',
+        resetOnBlur: false, // Don't erase the state of substack B when we switch back to A
+      })
+    );
+
+    expect(getSubState(1).routeName).toEqual('A');
+
+    navigateTo('B');
+    navigateTo('B2');
+    expect(getSubState(1).routeName).toEqual('B');
+    expect(getSubState(2).routeName).toEqual('B2');
+    const b2Key = getSubState(2).key;
+
+    navigateTo('A');
+    expect(getSubState(1).routeName).toEqual('A');
+
+    // The back action should switch to B and go back from B2 to B1
+    back(b2Key);
+    expect(getSubState(1).routeName).toEqual('B');
+    expect(getSubState(2).routeName).toEqual('B1');
+  });
+
   it('handles nested actions', () => {
     const { navigateTo, getSubState } = getRouterTestHelper(getExampleRouter());
 

src/routers/__tests__/routerTestHelper.js

@@ -1,5 +1,6 @@
 import * as NavigationActions from '../../NavigationActions';
 import * as SwitchActions from '../../routers/SwitchActions';
+import * as StackActions from '../../routers/StackActions';
 
 // A simple helper that makes it easier to write basic routing tests
 // We generally want to apply one action after the other and check router returns correct state
@@ -34,9 +35,20 @@ export const getRouterTestHelper = (router, options = defaultOptions) => {
       ...otherActionAttributes,
     });
 
-  const back = () =>
+  const back = key =>
     applyAction({
       type: NavigationActions.BACK,
+      key,
+    });
+
+  const pop = () =>
+    applyAction({
+      type: StackActions.POP,
+    });
+
+  const popToTop = () =>
+    applyAction({
+      type: StackActions.POP_TO_TOP,
     });
 
   const getState = () => state;
@@ -45,7 +57,16 @@ export const getRouterTestHelper = (router, options = defaultOptions) => {
     return getSubStateRecursive(state, level);
   };
 
-  return { applyAction, navigateTo, jumpTo, back, getState, getSubState };
+  return {
+    applyAction,
+    navigateTo,
+    jumpTo,
+    back,
+    pop,
+    popToTop,
+    getState,
+    getSubState,
+  };
 };
 
 const getSubStateRecursive = (state, level = 1) => {