StackRouter block actions while transitioning (#3469)

The most straightforward fix for two issues is to block all navigation actions while mid-transition of a stack navigator. This will fix:

The double-navigate on double tap issue, because the first navigation will start the transition and the second action will be ignored.

Will fix the buggy header experience that you can see when going back and forward to a different route quickly. This happens because the next navigate action happens before the completion action. After the fix, the navigate action will be ignored, the user will tap again, and will see a good transition

src/NavigationActions.js

@@ -45,6 +45,9 @@ const navigate = createAction(NAVIGATE, payload => {
   if (payload.key) {
     action.key = payload.key;
   }
+  if (payload.immediate) {
+    action.immediate = payload.immediate;
+  }
   return action;
 });
 
@@ -70,6 +73,9 @@ const push = createAction(PUSH, payload => {
   if (payload.action) {
     action.action = payload.action;
   }
+  if (payload.immediate) {
+    action.immediate = payload.immediate;
+  }
   return action;
 });
 

src/__tests__/NavigationContainer-test.js

@@ -109,7 +109,7 @@ describe('NavigationContainer', () => {
       // First dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'bar' })
+          NavigationActions.navigate({ routeName: 'bar', immediate: true })
         )
       ).toEqual(true);
 
@@ -119,7 +119,7 @@ describe('NavigationContainer', () => {
       // Second dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'baz' })
+          NavigationActions.navigate({ routeName: 'baz', immediate: true })
         )
       ).toEqual(true);
 
@@ -147,7 +147,7 @@ describe('NavigationContainer', () => {
       // First dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'bar' })
+          NavigationActions.navigate({ routeName: 'bar', immediate: true })
         )
       ).toEqual(true);
 
@@ -157,28 +157,28 @@ describe('NavigationContainer', () => {
       // Second dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'baz' })
+          NavigationActions.navigate({ routeName: 'baz', immediate: true })
         )
       ).toEqual(true);
 
       // Third dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'car' })
+          NavigationActions.navigate({ routeName: 'car', immediate: true })
         )
       ).toEqual(true);
 
       // Fourth dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'dog' })
+          NavigationActions.navigate({ routeName: 'dog', immediate: true })
         )
       ).toEqual(true);
 
       // Fifth dispatch
       expect(
         navigationContainer.dispatch(
-          NavigationActions.navigate({ routeName: 'elk' })
+          NavigationActions.navigate({ routeName: 'elk', immediate: true })
         )
       ).toEqual(true);
 

src/routers/StackRouter.js

@@ -148,6 +148,17 @@ export default (routeConfigs, stackConfig = {}) => {
         };
       }
 
+      if (
+        action.type === NavigationActions.COMPLETE_TRANSITION &&
+        (action.key == null || action.key === state.key) &&
+        state.isTransitioning
+      ) {
+        return {
+          ...state,
+          isTransitioning: false,
+        };
+      }
+
       // Check if a child scene wants to handle the action as long as it is not a reset to the root stack
       if (action.type !== NavigationActions.RESET || action.key !== null) {
         const keyIndex = action.key
@@ -218,6 +229,10 @@ export default (routeConfigs, stackConfig = {}) => {
         behavesLikePushAction(action) &&
         childRouters[action.routeName] !== undefined
       ) {
+        if (state.isTransitioning) {
+          return { ...state };
+        }
+
         const childRouter = childRouters[action.routeName];
         let route;
 
@@ -290,17 +305,6 @@ export default (routeConfigs, stackConfig = {}) => {
         };
       }
 
-      if (
-        action.type === NavigationActions.COMPLETE_TRANSITION &&
-        (action.key == null || action.key === state.key) &&
-        state.isTransitioning
-      ) {
-        return {
-          ...state,
-          isTransitioning: false,
-        };
-      }
-
       // Handle navigation to other child routers that are not yet pushed
       if (behavesLikePushAction(action)) {
         const childRouterNames = Object.keys(childRouters);

src/routers/__tests__/StackRouter-test.js

@@ -493,6 +493,23 @@ describe('StackRouter', () => {
     expect(poppedImmediatelyState.isTransitioning).toBe(false);
   });
 
+  test('Navigate does not happen while transitioning', () => {
+    const TestRouter = StackRouter({
+      foo: { screen: () => <div /> },
+      bar: { screen: () => <div /> },
+    });
+    const initState = {
+      ...TestRouter.getStateForAction(NavigationActions.init()),
+      isTransitioning: true,
+    };
+    const pushedState = TestRouter.getStateForAction(
+      NavigationActions.navigate({ routeName: 'bar' }),
+      initState
+    );
+    expect(pushedState.index).toEqual(0);
+    expect(pushedState.routes.length).toEqual(1);
+  });
+
   test('Navigate Pushes duplicate routeName', () => {
     const TestRouter = StackRouter({
       foo: { screen: () => <div /> },
@@ -500,13 +517,13 @@ describe('StackRouter', () => {
     });
     const initState = TestRouter.getStateForAction(NavigationActions.init());
     const pushedState = TestRouter.getStateForAction(
-      NavigationActions.navigate({ routeName: 'bar' }),
+      NavigationActions.navigate({ routeName: 'bar', immediate: true }),
       initState
     );
     expect(pushedState.index).toEqual(1);
     expect(pushedState.routes[1].routeName).toEqual('bar');
     const pushedTwiceState = TestRouter.getStateForAction(
-      NavigationActions.navigate({ routeName: 'bar' }),
+      NavigationActions.navigate({ routeName: 'bar', immediate: true }),
       pushedState
     );
     expect(pushedTwiceState.index).toEqual(2);
@@ -540,7 +557,7 @@ describe('StackRouter', () => {
     });
     const initState = TestRouter.getStateForAction(NavigationActions.init());
     const pushedState = TestRouter.getStateForAction(
-      NavigationActions.push({ routeName: 'bar' }),
+      NavigationActions.push({ routeName: 'bar', immediate: true }),
       initState
     );
     expect(pushedState.index).toEqual(1);