From 58b77d44aef2abcc8bbde0fe9445fbcd25968005 Mon Sep 17 00:00:00 2001 From: Brent Vatne Date: Fri, 9 Feb 2018 18:20:01 -0800 Subject: [PATCH] Make Router({ RouteName: Component }) a valid way to configure a router (#3486) * Make Router({routeName: Component}) a valid way to instantiate a route * Update App.js in NavigationPlayground * Fix route config flow type --- examples/NavigationPlayground/js/App.js | 59 +++++++------------ flow/react-navigation.js | 5 +- .../__tests__/createConfigGetter-test.js | 20 ------- .../__tests__/validateRouteConfigMap-test.js | 4 +- src/routers/getScreenForRouteName.js | 2 +- src/routers/validateRouteConfigMap.js | 39 +++++------- 6 files changed, 43 insertions(+), 86 deletions(-) diff --git a/examples/NavigationPlayground/js/App.js b/examples/NavigationPlayground/js/App.js index 7ee68abc..f7aa1561 100644 --- a/examples/NavigationPlayground/js/App.js +++ b/examples/NavigationPlayground/js/App.js @@ -96,39 +96,19 @@ const ExampleInfo = { }, }; const ExampleRoutes = { - SimpleStack: { - screen: SimpleStack, - }, - SimpleTabs: { - screen: SimpleTabs, - }, - Drawer: { - screen: Drawer, - }, + SimpleStack: SimpleStack, + SimpleTabs: SimpleTabs, + Drawer: Drawer, // MultipleDrawer: { // screen: MultipleDrawer, // }, - TabsInDrawer: { - screen: TabsInDrawer, - }, - CustomTabs: { - screen: CustomTabs, - }, - CustomTransitioner: { - screen: CustomTransitioner, - }, - ModalStack: { - screen: ModalStack, - }, - StacksWithKeys: { - screen: StacksWithKeys, - }, - StacksInTabs: { - screen: StacksInTabs, - }, - StacksOverTabs: { - screen: StacksOverTabs, - }, + TabsInDrawer: TabsInDrawer, + CustomTabs: CustomTabs, + CustomTransitioner: CustomTransitioner, + ModalStack: ModalStack, + StacksWithKeys: StacksWithKeys, + StacksInTabs: StacksInTabs, + StacksOverTabs: StacksOverTabs, LinkStack: { screen: SimpleStack, path: 'people/Jordan', @@ -137,9 +117,7 @@ const ExampleRoutes = { screen: SimpleTabs, path: 'settings', }, - TabAnimations: { - screen: TabAnimations, - }, + TabAnimations: TabAnimations, }; type State = { @@ -231,11 +209,16 @@ class MainScreen extends React.Component { { - const { path, params, screen } = ExampleRoutes[routeName]; - const { router } = screen; - const action = - path && router.getActionForPathAndParams(path, params); - navigation.navigate(routeName, {}, action); + let route = ExampleRoutes[routeName]; + if (route.screen || route.path || route.params) { + const { path, params, screen } = route; + const { router } = screen; + const action = + path && router.getActionForPathAndParams(path, params); + navigation.navigate(routeName, {}, action); + } else { + navigation.navigate(routeName); + } }} > diff --git a/flow/react-navigation.js b/flow/react-navigation.js index a0cbace0..213de866 100644 --- a/flow/react-navigation.js +++ b/flow/react-navigation.js @@ -332,12 +332,15 @@ declare module 'react-navigation' { navigationOptions?: ?NavigationScreenConfig, }; - declare export type NavigationRouteConfig = { + declare export type NavigationRouteConfig = + | NavigationComponent + | { navigationOptions?: NavigationScreenConfig<*>, path?: string, } & NavigationScreenRouteConfig; declare export type NavigationScreenRouteConfig = + | NavigationComponent | { screen: NavigationComponent, } diff --git a/src/routers/__tests__/createConfigGetter-test.js b/src/routers/__tests__/createConfigGetter-test.js index 42c35f27..3852598f 100644 --- a/src/routers/__tests__/createConfigGetter-test.js +++ b/src/routers/__tests__/createConfigGetter-test.js @@ -175,23 +175,3 @@ test('should throw if the route does not exist', () => { "There is no route defined for key Settings.\nMust be one of: 'Home'" ); }); - -test('should throw if the screen is not defined under the route config', () => { - /* eslint-disable react/no-multi-comp */ - - const getScreenOptions = createConfigGetter({ - Home: {}, - }); - - const routes = [{ key: 'B', routeName: 'Home' }]; - - expect(() => - getScreenOptions( - addNavigationHelpers({ - state: routes[0], - dispatch: () => false, - addListener: dummyEventSubscriber, - }) - ) - ).toThrowError('Route Home must define a screen or a getScreen.'); -}); diff --git a/src/routers/__tests__/validateRouteConfigMap-test.js b/src/routers/__tests__/validateRouteConfigMap-test.js index 280af972..8537ad98 100644 --- a/src/routers/__tests__/validateRouteConfigMap-test.js +++ b/src/routers/__tests__/validateRouteConfigMap-test.js @@ -39,9 +39,7 @@ describe('validateRouteConfigMap', () => { Home: { screen: ProfileNavigator, }, - Chat: { - screen: ListScreen, - }, + Chat: ListScreen, }; validateRouteConfigMap(invalidMap); }); diff --git a/src/routers/getScreenForRouteName.js b/src/routers/getScreenForRouteName.js index fc429922..89b3fe7d 100644 --- a/src/routers/getScreenForRouteName.js +++ b/src/routers/getScreenForRouteName.js @@ -32,5 +32,5 @@ export default function getScreenForRouteName(routeConfigs, routeName) { return screen; } - throw new Error(`Route ${routeName} must define a screen or a getScreen.`); + return routeConfig; } diff --git a/src/routers/validateRouteConfigMap.js b/src/routers/validateRouteConfigMap.js index bb1f00b7..bfe085fa 100644 --- a/src/routers/validateRouteConfigMap.js +++ b/src/routers/validateRouteConfigMap.js @@ -14,44 +14,37 @@ function validateRouteConfigMap(routeConfigs) { routeNames.forEach(routeName => { const routeConfig = routeConfigs[routeName]; - if (!routeConfig.screen && !routeConfig.getScreen) { - throw new Error( - `Route '${routeName}' should declare a screen. ` + - 'For example:\n\n' + - "import MyScreen from './MyScreen';\n" + - '...\n' + - `${routeName}: {\n` + - ' screen: MyScreen,\n' + - '}' - ); - } else if (routeConfig.screen && routeConfig.getScreen) { - throw new Error( - `Route '${routeName}' should declare a screen or ` + - 'a getScreen, not both.' - ); - } + const screenComponent = routeConfig.screen + ? routeConfig.screen + : routeConfig; if ( - routeConfig.screen && - typeof routeConfig.screen !== 'function' && - typeof routeConfig.screen !== 'string' + screenComponent && + typeof screenComponent !== 'function' && + typeof screenComponent !== 'string' && + !routeConfig.getScreen ) { throw new Error( `The component for route '${routeName}' must be a ` + 'React component. For example:\n\n' + "import MyScreen from './MyScreen';\n" + '...\n' + - `${routeName}: {\n` + - ' screen: MyScreen,\n' + + `${routeName}: MyScreen,\n` + '}\n\n' + 'You can also use a navigator:\n\n' + "import MyNavigator from './MyNavigator';\n" + '...\n' + - `${routeName}: {\n` + - ' screen: MyNavigator,\n' + + `${routeName}: MyNavigator,\n` + '}' ); } + + if (routeConfig.screen && routeConfig.getScreen) { + throw new Error( + `Route '${routeName}' should declare a screen or ` + + 'a getScreen, not both.' + ); + } }); }