From af90697ef1d3e0bdb2ba854d14ed2f960d74a5bb Mon Sep 17 00:00:00 2001 From: Eric Vicenti Date: Fri, 8 Jun 2018 14:20:07 -0700 Subject: [PATCH] Refactor redo, child navigation to navigation prop (#4453) --- .../src/__tests__/getNavigation-test.js | 97 +++++++++++++ .../src/createNavigationContainer.js | 38 ++--- .../src/getChildNavigation.js | 88 ++++++++++++ .../react-navigation/src/getChildRouter.js | 9 ++ .../react-navigation/src/getNavigation.js | 54 +++++++ .../src/navigators/createNavigator.js | 132 ++++-------------- .../src/routers/StackRouter.js | 2 + .../src/routers/SwitchRouter.js | 2 + 8 files changed, 291 insertions(+), 131 deletions(-) create mode 100644 packages/react-navigation/src/__tests__/getNavigation-test.js create mode 100644 packages/react-navigation/src/getChildNavigation.js create mode 100644 packages/react-navigation/src/getChildRouter.js create mode 100644 packages/react-navigation/src/getNavigation.js diff --git a/packages/react-navigation/src/__tests__/getNavigation-test.js b/packages/react-navigation/src/__tests__/getNavigation-test.js new file mode 100644 index 00000000..5acaadbb --- /dev/null +++ b/packages/react-navigation/src/__tests__/getNavigation-test.js @@ -0,0 +1,97 @@ +import getNavigation from '../getNavigation'; + +test('getNavigation provides default action helpers', () => { + const router = { + getActionCreators: () => ({}), + getStateForAction(action, lastState = {}) { + return lastState; + }, + }; + + const dispatch = jest.fn(); + + const topNav = getNavigation( + router, + {}, + dispatch, + new Set(), + () => ({}), + () => topNav + ); + + topNav.navigate('GreatRoute'); + + expect(dispatch.mock.calls.length).toBe(1); + expect(dispatch.mock.calls[0][0].type).toBe('Navigation/NAVIGATE'); + expect(dispatch.mock.calls[0][0].routeName).toBe('GreatRoute'); +}); + +test('getNavigation provides router action helpers', () => { + const router = { + getActionCreators: () => ({ + foo: bar => ({ type: 'FooBarAction', bar }), + }), + getStateForAction(action, lastState = {}) { + return lastState; + }, + }; + + const dispatch = jest.fn(); + + const topNav = getNavigation( + router, + {}, + dispatch, + new Set(), + () => ({}), + () => topNav + ); + + topNav.foo('Great'); + + expect(dispatch.mock.calls.length).toBe(1); + expect(dispatch.mock.calls[0][0].type).toBe('FooBarAction'); + expect(dispatch.mock.calls[0][0].bar).toBe('Great'); +}); + +test('getNavigation get child navigation with router', () => { + const actionSubscribers = new Set(); + let navigation = null; + + const routerA = { + getActionCreators: () => ({}), + getStateForAction(action, lastState = {}) { + return lastState; + }, + }; + const router = { + childRouters: { + RouteA: routerA, + }, + getActionCreators: () => ({}), + getStateForAction(action, lastState = {}) { + return lastState; + }, + }; + + const initState = { + index: 0, + routes: [ + { key: 'a', routeName: 'RouteA' }, + { key: 'b', routeName: 'RouteB' }, + ], + }; + + const topNav = getNavigation( + router, + initState, + () => {}, + actionSubscribers, + () => ({}), + () => navigation + ); + + const childNavA = topNav.getChildNavigation('a'); + + expect(childNavA.router).toBe(routerA); +}); diff --git a/packages/react-navigation/src/createNavigationContainer.js b/packages/react-navigation/src/createNavigationContainer.js index 5cbbb59e..8c3db816 100644 --- a/packages/react-navigation/src/createNavigationContainer.js +++ b/packages/react-navigation/src/createNavigationContainer.js @@ -4,8 +4,8 @@ import { polyfill } from 'react-lifecycles-compat'; import { BackHandler } from './PlatformHelpers'; import NavigationActions from './NavigationActions'; +import getNavigation from './getNavigation'; import invariant from './utils/invariant'; -import getNavigationActionCreators from './routers/getNavigationActionCreators'; import docsUrl from './utils/docsUrl'; function isStateful(props) { @@ -358,34 +358,24 @@ export default function createNavigationContainer(Component) { return false; }; + _getScreenProps = () => this.props.screenProps; + render() { let navigation = this.props.navigation; if (this._isStateful()) { - const nav = this.state.nav; - if (!nav) { + const navState = this.state.nav; + if (!navState) { return this._renderLoading(); } - if (!this._navigation || this._navigation.state !== nav) { - this._navigation = { - dispatch: this.dispatch, - state: nav, - addListener: (eventName, handler) => { - if (eventName !== 'action') { - return { remove: () => {} }; - } - this._actionEventSubscribers.add(handler); - return { - remove: () => { - this._actionEventSubscribers.delete(handler); - }, - }; - }, - }; - const actionCreators = getNavigationActionCreators(nav); - Object.keys(actionCreators).forEach(actionName => { - this._navigation[actionName] = (...args) => - this.dispatch(actionCreators[actionName](...args)); - }); + if (!this._navigation || this._navigation.state !== navState) { + this._navigation = getNavigation( + Component.router, + navState, + this.dispatch, + this._actionEventSubscribers, + this._getScreenProps, + () => this._navigation + ); } navigation = this._navigation; } diff --git a/packages/react-navigation/src/getChildNavigation.js b/packages/react-navigation/src/getChildNavigation.js new file mode 100644 index 00000000..63c598e0 --- /dev/null +++ b/packages/react-navigation/src/getChildNavigation.js @@ -0,0 +1,88 @@ +import getChildEventSubscriber from './getChildEventSubscriber'; +import getChildRouter from './getChildRouter'; + +const createParamGetter = route => (paramName, defaultValue) => { + const params = route.params; + + if (params && paramName in params) { + return params[paramName]; + } + + return defaultValue; +}; + +function getChildNavigation(navigation, childKey, getCurrentParentNavigation) { + const children = + navigation._childrenNavigation || (navigation._childrenNavigation = {}); + + const route = navigation.state.routes.find(r => r.key === childKey); + + if (children[childKey] && children[childKey].state === route) { + return children[childKey]; + } + + const childRouter = getChildRouter(navigation.router, route.routeName); + + const actionCreators = { + ...navigation.actions, + ...navigation.router.getActionCreators(route, navigation.state.key), + }; + const actionHelpers = {}; + Object.keys(actionCreators).forEach(actionName => { + actionHelpers[actionName] = (...args) => { + const actionCreator = actionCreators[actionName]; + const action = actionCreator(...args); + navigation.dispatch(action); + }; + }); + + if (children[childKey]) { + children[childKey] = { + ...children[childKey], + ...actionHelpers, + state: route, + router: childRouter, + actions: actionCreators, + getParam: createParamGetter(route), + }; + return children[childKey]; + } + + const childSubscriber = getChildEventSubscriber( + navigation.addListener, + childKey + ); + + children[childKey] = { + ...actionHelpers, + + state: route, + router: childRouter, + actions: actionCreators, + getParam: createParamGetter(route), + + getChildNavigation: grandChildKey => + getChildNavigation(children[childKey], grandChildKey, () => + getCurrentParentNavigation().getChildNavigation(childKey) + ), + + isFocused: () => { + const currentNavigation = getCurrentParentNavigation(); + const { routes, index } = currentNavigation.state; + if (!currentNavigation.isFocused()) { + return false; + } + if (routes[index].key === childKey) { + return true; + } + return false; + }, + dispatch: navigation.dispatch, + getScreenProps: navigation.getScreenProps, + dangerouslyGetParent: getCurrentParentNavigation, + addListener: childSubscriber.addListener, + }; + return children[childKey]; +} + +export default getChildNavigation; diff --git a/packages/react-navigation/src/getChildRouter.js b/packages/react-navigation/src/getChildRouter.js new file mode 100644 index 00000000..74bfbc23 --- /dev/null +++ b/packages/react-navigation/src/getChildRouter.js @@ -0,0 +1,9 @@ +export default function getChildRouter(router, routeName) { + if (router.childRouters && router.childRouters[routeName]) { + return router.childRouters[routeName]; + } + + const Component = router.getComponentForRouteName(routeName); + + return Component.router; +} diff --git a/packages/react-navigation/src/getNavigation.js b/packages/react-navigation/src/getNavigation.js new file mode 100644 index 00000000..236de18d --- /dev/null +++ b/packages/react-navigation/src/getNavigation.js @@ -0,0 +1,54 @@ +import getNavigationActionCreators from './routers/getNavigationActionCreators'; +import getChildNavigation from './getChildNavigation'; + +export default function getNavigation( + router, + state, + dispatch, + actionSubscribers, + getScreenProps, + getCurrentNavigation +) { + const actions = router.getActionCreators(state, null); + + const navigation = { + actions, + router, + state, + dispatch, + getScreenProps, + getChildNavigation: childKey => + getChildNavigation(navigation, childKey, getCurrentNavigation), + isFocused: childKey => { + const { routes, index } = getCurrentNavigation().state; + if (childKey == null || routes[index].key === childKey) { + return true; + } + return false; + }, + addListener: (eventName, handler) => { + if (eventName !== 'action') { + return { remove: () => {} }; + } + actionSubscribers.add(handler); + return { + remove: () => { + actionSubscribers.delete(handler); + }, + }; + }, + dangerouslyGetParent: () => null, + }; + + const actionCreators = { + ...getNavigationActionCreators(navigation.state), + ...actions, + }; + + Object.keys(actionCreators).forEach(actionName => { + navigation[actionName] = (...args) => + navigation.dispatch(actionCreators[actionName](...args)); + }); + + return navigation; +} diff --git a/packages/react-navigation/src/navigators/createNavigator.js b/packages/react-navigation/src/navigators/createNavigator.js index a54416d1..525d591c 100644 --- a/packages/react-navigation/src/navigators/createNavigator.js +++ b/packages/react-navigation/src/navigators/createNavigator.js @@ -8,17 +8,13 @@ function createNavigator(NavigatorView, router, navigationConfig) { static router = router; static navigationOptions = null; - constructor(props) { - super(props); - - this.state = { - descriptors: {}, - childEventSubscribers: {}, - screenProps: props.screenProps, - }; - } + state = { + descriptors: {}, + screenProps: this.props.screenProps, + }; static getDerivedStateFromProps(nextProps, prevState) { + const prevDescriptors = prevState.descriptors; const { navigation, screenProps } = nextProps; const { dispatch, state, addListener } = navigation; const { routes } = state; @@ -29,119 +25,41 @@ function createNavigator(NavigatorView, router, navigationConfig) { } const descriptors = { ...prevState.descriptors }; - const childEventSubscribers = { ...prevState.childEventSubscribers }; routes.forEach(route => { if ( - !descriptors[route.key] || - descriptors[route.key].state !== route || - screenProps !== prevState.screenProps + prevDescriptors && + prevDescriptors[route.key] && + route === prevDescriptors[route.key].state && + screenProps === prevState.screenProps ) { - const getComponent = () => - router.getComponentForRouteName(route.routeName); - - if (!childEventSubscribers[route.key]) { - childEventSubscribers[route.key] = getChildEventSubscriber( - addListener, - route.key - ); - } - - const actionCreators = { - ...navigation.actions, - ...router.getActionCreators(route, state.key), - }; - const actionHelpers = {}; - Object.keys(actionCreators).forEach(actionName => { - actionHelpers[actionName] = (...args) => { - const actionCreator = actionCreators[actionName]; - const action = actionCreator(...args); - return dispatch(action); - }; - }); - const childNavigation = { - ...actionHelpers, - actions: actionCreators, - dispatch, - state: route, - addListener: childEventSubscribers[route.key].addListener, - getParam: (paramName, defaultValue) => { - const params = route.params; - - if (params && paramName in params) { - return params[paramName]; - } - - return defaultValue; - }, - }; - - const options = router.getScreenOptions(childNavigation, screenProps); - descriptors[route.key] = { - key: route.key, - getComponent, - options, - state: route, - navigation: childNavigation, - }; + descriptors[route.key] = prevDescriptors[route.key]; + return; } + const getComponent = () => + router.getComponentForRouteName(route.routeName); + const childNavigation = navigation.getChildNavigation(route.key); + const options = router.getScreenOptions(childNavigation, screenProps); + descriptors[route.key] = { + key: route.key, + getComponent, + options, + state: route, + navigation: childNavigation, + }; }); - return { - descriptors, - childEventSubscribers, - screenProps, - }; + return { descriptors, screenProps }; } - // Cleanup subscriptions for routes that no longer exist - componentDidUpdate() { - const activeKeys = this.props.navigation.state.routes.map(r => r.key); - let childEventSubscribers = { ...this.state.childEventSubscribers }; - Object.keys(childEventSubscribers).forEach(key => { - if (!activeKeys.includes(key)) { - delete childEventSubscribers[key]; - } - }); - if ( - childEventSubscribers.length !== this.state.childEventSubscribers.length - ) { - this.setState({ childEventSubscribers }); - } - } - - _isRouteFocused = route => { - const { state } = this.props.navigation; - const focusedRoute = state.routes[state.index]; - return route === focusedRoute; - }; - - _dangerouslyGetParent = () => { - return this.props.navigation; - }; - render() { - // Mutation in render 😩 - // The problem: - // - We don't want to re-render each screen every time the parent navigator changes - // - But we need to be able to access the parent navigator from callbacks - // - These functions should only be used within callbacks, but they are passed in props, - // which is what makes this awkward. What's a good way to pass in stuff that we don't - // want people to depend on in render? - let descriptors = { ...this.state.descriptors }; - Object.values(descriptors).forEach(descriptor => { - descriptor.navigation.isFocused = () => - this._isRouteFocused(descriptor.state); - descriptor.navigation.dangerouslyGetParent = this._dangerouslyGetParent; - }); - return ( ); } diff --git a/packages/react-navigation/src/routers/StackRouter.js b/packages/react-navigation/src/routers/StackRouter.js index bd480447..872642d7 100644 --- a/packages/react-navigation/src/routers/StackRouter.js +++ b/packages/react-navigation/src/routers/StackRouter.js @@ -149,6 +149,8 @@ export default (routeConfigs, stackConfig = {}) => { paths.sort((a, b) => b[1].priority - a[1].priority); return { + childRouters, + getComponentForState(state) { const activeChildRoute = state.routes[state.index]; const { routeName } = activeChildRoute; diff --git a/packages/react-navigation/src/routers/SwitchRouter.js b/packages/react-navigation/src/routers/SwitchRouter.js index cd375c1b..7005bde2 100644 --- a/packages/react-navigation/src/routers/SwitchRouter.js +++ b/packages/react-navigation/src/routers/SwitchRouter.js @@ -75,6 +75,8 @@ export default (routeConfigs, config = {}) => { } return { + childRouters, + getInitialState() { const routes = order.map(resetChildRoute); return {