Add key support to StackRouter navigate (#3393)

src/NavigationActions.js

@@ -37,6 +37,9 @@ const navigate = createAction(NAVIGATE, payload => {
   if (payload.action) {
     action.action = payload.action;
   }
+  if (payload.key) {
+    action.key = payload.key;
+  }
   return action;
 });
 

src/addNavigationHelpers.js

@@ -19,10 +19,26 @@ export default function(navigation) {
         NavigationActions.back({ key: actualizedKey })
       );
     },
-    navigate: (routeName, params, action) =>
-      navigation.dispatch(
-        NavigationActions.navigate({ routeName, params, action })
-      ),
+    navigate: (navigateTo, params, action) => {
+      if (typeof navigateTo === 'string') {
+        return navigation.dispatch(
+          NavigationActions.navigate({ routeName: navigateTo, params, action })
+        );
+      }
+      invariant(
+        typeof navigateTo === 'object',
+        'Must navigateTo an object or a string'
+      );
+      invariant(
+        params == null,
+        'Params must not be provided to .navigate() when specifying an object'
+      );
+      invariant(
+        action == null,
+        'Child action must not be provided to .navigate() when specifying an object'
+      );
+      return navigation.dispatch(NavigationActions.navigate(navigateTo));
+    },
     /**
      * For updating current route params. For example the nav bar title and
      * buttons are based on the route params.

src/routers/KeyGenerator.js

@@ -0,0 +1,11 @@
+let uniqueBaseId = `id-${Date.now()}`;
+let uuidCount = 0;
+
+export function _TESTING_ONLY_normalize_keys() {
+  uniqueBaseId = 'id';
+  uuidCount = 0;
+}
+
+export function generateKey() {
+  return `${uniqueBaseId}-${uuidCount++}`;
+}

src/routers/StackRouter.js

@@ -7,12 +7,7 @@ import StateUtils from '../StateUtils';
 import validateRouteConfigMap from './validateRouteConfigMap';
 import getScreenConfigDeprecated from './getScreenConfigDeprecated';
 import invariant from '../utils/invariant';
-
-const uniqueBaseId = `id-${Date.now()}`;
-let uuidCount = 0;
-function _getUuid() {
-  return `${uniqueBaseId}-${uuidCount++}`;
-}
+import { generateKey } from './KeyGenerator';
 
 function isEmpty(obj) {
   if (!obj) return true;
@@ -109,9 +104,10 @@ export default (routeConfigs, stackConfig = {}) => {
             index: 0,
             routes: [
               {
-                ...action,
+                routeName: action.routeName,
+                params: action.params,
                 type: undefined,
-                key: `Init-${_getUuid()}`,
+                key: `Init-${generateKey()}`,
               },
             ],
           };
@@ -134,7 +130,7 @@ export default (routeConfigs, stackConfig = {}) => {
         route = {
           ...route,
           routeName: initialRouteName,
-          key: `Init-${_getUuid()}`,
+          key: `Init-${generateKey()}`,
           ...(params ? { params } : {}),
         };
         // eslint-disable-next-line no-param-reassign
@@ -175,19 +171,55 @@ export default (routeConfigs, stackConfig = {}) => {
       ) {
         const childRouter = childRouters[action.routeName];
         let route;
+
+        // The key may be provided for pushing, or to navigate back to the key
+        if (action.key) {
+          const lastRouteIndex = state.routes.findIndex(
+            r => r.key === action.key
+          );
+          if (lastRouteIndex !== -1) {
+            // If index is unchanged and params are not being set, leave state identity intact
+            if (state.index === lastRouteIndex && !action.params) {
+              return state;
+            }
+            const routes = [...state.routes];
+            // Apply params if provided, otherwise leave route identity intact
+            if (action.params) {
+              const route = state.routes.find(r => r.key === action.key);
+              routes[lastRouteIndex] = {
+                ...route,
+                params: {
+                  ...route.params,
+                  ...action.params,
+                },
+              };
+            }
+            // Return state with new index. Change isTransitioning only if index has changed
+            return {
+              ...state,
+              isTransitioning:
+                state.index !== lastRouteIndex
+                  ? action.immediate !== true
+                  : undefined,
+              index: lastRouteIndex,
+              routes,
+            };
+          }
+        }
+        const key = action.key || generateKey();
         if (childRouter) {
           const childAction =
             action.action || NavigationActions.init({ params: action.params });
           route = {
             params: action.params,
             ...childRouter.getStateForAction(childAction),
-            key: _getUuid(),
+            key,
             routeName: action.routeName,
           };
         } else {
           route = {
             params: action.params,
-            key: _getUuid(),
+            key,
             routeName: action.routeName,
           };
         }
@@ -234,7 +266,7 @@ export default (routeConfigs, stackConfig = {}) => {
             if (routeToPush) {
               return StateUtils.push(state, {
                 ...routeToPush,
-                key: _getUuid(),
+                key: generateKey(),
                 routeName: childRouterName,
               });
             }
@@ -274,12 +306,12 @@ export default (routeConfigs, stackConfig = {}) => {
                 ...childAction,
                 ...router.getStateForAction(childAction),
                 routeName: childAction.routeName,
-                key: _getUuid(),
+                key: generateKey(),
               };
             }
             const route = {
               ...childAction,
-              key: _getUuid(),
+              key: generateKey(),
             };
             delete route.type;
             return route;

src/routers/__tests__/StackRouter-test.js

@@ -4,9 +4,14 @@ import React from 'react';
 
 import StackRouter from '../StackRouter';
 import TabRouter from '../TabRouter';
+import { _TESTING_ONLY_normalize_keys } from '../KeyGenerator';
 
 import NavigationActions from '../../NavigationActions';
 
+beforeEach(() => {
+  _TESTING_ONLY_normalize_keys();
+});
+
 const ListScreen = () => <div />;
 
 const ProfileNavigator = () => <div />;
@@ -349,7 +354,7 @@ describe('StackRouter', () => {
     expect(initState).toEqual({
       index: 0,
       isTransitioning: false,
-      routes: [{ key: 'Init-id-0-0', routeName: 'foo' }],
+      routes: [{ key: 'Init-id-0', routeName: 'foo' }],
     });
     const pushedState = TestRouter.getStateForAction(
       NavigationActions.navigate({ routeName: 'qux' }),
@@ -360,6 +365,46 @@ describe('StackRouter', () => {
     expect(pushedState.routes[1].routes[1].routeName).toEqual('qux');
   });
 
+  test('Navigate Pushes duplicate routeName', () => {
+    const TestRouter = StackRouter({
+      foo: { screen: () => <div /> },
+      bar: { screen: () => <div /> },
+    });
+    const initState = TestRouter.getStateForAction(NavigationActions.init());
+    const pushedState = TestRouter.getStateForAction(
+      NavigationActions.navigate({ routeName: 'bar' }),
+      initState
+    );
+    expect(pushedState.index).toEqual(1);
+    expect(pushedState.routes[1].routeName).toEqual('bar');
+    const pushedTwiceState = TestRouter.getStateForAction(
+      NavigationActions.navigate({ routeName: 'bar' }),
+      pushedState
+    );
+    expect(pushedTwiceState.index).toEqual(2);
+    expect(pushedTwiceState.routes[2].routeName).toEqual('bar');
+  });
+
+  test('Navigate with key is idempotent', () => {
+    const TestRouter = StackRouter({
+      foo: { screen: () => <div /> },
+      bar: { screen: () => <div /> },
+    });
+    const initState = TestRouter.getStateForAction(NavigationActions.init());
+    const pushedState = TestRouter.getStateForAction(
+      NavigationActions.navigate({ routeName: 'bar', key: 'a' }),
+      initState
+    );
+    expect(pushedState.index).toEqual(1);
+    expect(pushedState.routes[1].routeName).toEqual('bar');
+    const pushedTwiceState = TestRouter.getStateForAction(
+      NavigationActions.navigate({ routeName: 'bar', key: 'a' }),
+      pushedState
+    );
+    expect(pushedTwiceState.index).toEqual(1);
+    expect(pushedTwiceState.routes[1].routeName).toEqual('bar');
+  });
+
   test('Handle basic stack logic for plain components', () => {
     const FooScreen = () => <div />;
     const BarScreen = () => <div />;
@@ -377,7 +422,7 @@ describe('StackRouter', () => {
       isTransitioning: false,
       routes: [
         {
-          key: 'Init-id-0-4',
+          key: 'Init-id-0',
           routeName: 'Foo',
         },
       ],
@@ -404,7 +449,7 @@ describe('StackRouter', () => {
       isTransitioning: false,
       routes: [
         {
-          key: 'Init-id-0-4',
+          key: 'Init-id-0',
           routeName: 'Foo',
         },
       ],
@@ -465,7 +510,7 @@ describe('StackRouter', () => {
       isTransitioning: false,
       routes: [
         {
-          key: 'Init-id-0-8',
+          key: 'Init-id-0',
           routeName: 'Foo',
         },
       ],
@@ -492,7 +537,7 @@ describe('StackRouter', () => {
       isTransitioning: false,
       routes: [
         {
-          key: 'Init-id-0-8',
+          key: 'Init-id-0',
           routeName: 'Foo',
         },
       ],
@@ -565,7 +610,7 @@ describe('StackRouter', () => {
       isTransitioning: false,
       routes: [
         {
-          key: 'Init-id-0-14',
+          key: 'Init-id-0',
           routeName: 'Bar',
         },
       ],
@@ -673,14 +718,14 @@ describe('StackRouter', () => {
       {
         type: NavigationActions.SET_PARAMS,
         params: { name: 'foobar' },
-        key: 'Init-id-0-19',
+        key: 'Init-id-0',
       },
       state
     );
     expect(state2 && state2.index).toEqual(0);
     expect(state2 && state2.routes[0].routes[0].routes).toEqual([
       {
-        key: 'Init-id-0-19',
+        key: 'Init-id-0',
         routeName: 'Quux',
         params: { name: 'foobar' },
       },