From cf2e74978a1fbcdbd67b72a77745b8a8ec4d2275 Mon Sep 17 00:00:00 2001 From: Eric Vicenti Date: Wed, 31 Jan 2018 19:48:04 -0800 Subject: [PATCH] Add key support to StackRouter navigate (#3393) --- src/NavigationActions.js | 3 ++ src/addNavigationHelpers.js | 24 +++++++-- src/routers/KeyGenerator.js | 11 ++++ src/routers/StackRouter.js | 60 ++++++++++++++++------ src/routers/__tests__/StackRouter-test.js | 61 ++++++++++++++++++++--- 5 files changed, 133 insertions(+), 26 deletions(-) create mode 100644 src/routers/KeyGenerator.js diff --git a/src/NavigationActions.js b/src/NavigationActions.js index b19505efd6..18116f8e23 100644 --- a/src/NavigationActions.js +++ b/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; }); diff --git a/src/addNavigationHelpers.js b/src/addNavigationHelpers.js index 743a1c7688..36aea009f4 100644 --- a/src/addNavigationHelpers.js +++ b/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. diff --git a/src/routers/KeyGenerator.js b/src/routers/KeyGenerator.js new file mode 100644 index 0000000000..9a919555c6 --- /dev/null +++ b/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++}`; +} diff --git a/src/routers/StackRouter.js b/src/routers/StackRouter.js index 7d09bd2607..5de4ea131e 100644 --- a/src/routers/StackRouter.js +++ b/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; diff --git a/src/routers/__tests__/StackRouter-test.js b/src/routers/__tests__/StackRouter-test.js index 6b4e2a4d97..99015912dc 100644 --- a/src/routers/__tests__/StackRouter-test.js +++ b/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 = () =>
; const ProfileNavigator = () =>
; @@ -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: () =>
}, + bar: { screen: () =>
}, + }); + 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: () =>
}, + bar: { screen: () =>
}, + }); + 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 = () =>
; const BarScreen = () =>
; @@ -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' }, },