From dab9f5ee597a2dc61ab50ab238c5e0db0c96ac6b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 28 Sep 2020 15:41:21 -0700 Subject: [PATCH] nav: Stop using react-navigation-redux-helpers. We've long used Redux to manage our navigation state, with a helper package called react-navigation-redux-helpers. React Navigation has recommended against this for quite some time. In this commit: - Stop using r-n-r-h's `createReduxContainer`, and use React Navigation's own `createAppContainer` instead. - Rename `AppWithNavigation` (arguably better named `ReduxContainer` before this commit) to `AppContainer` - Rename `NavigationService.reduxContainerRef` to `NavigationService.appContainerRef` and change some of `NavigationService`'s implementation details to handle the new shape. - Remove use of `createReactNavigationReduxMiddleware`. - Remove `state.nav`. - Remove `navReducer`. - Adjust types as necessary. - Remove 'nav' from `discardKeys` in src/boot/store.js Fixes: #3804 --- src/ZulipMobile.js | 9 ++++++-- src/boot/reducers.js | 2 -- src/boot/store.js | 7 +----- src/nav/AppContainer.js | 7 ++++++ src/nav/AppWithNavigation.js | 16 -------------- src/nav/NavigationService.js | 41 ++++++++++++------------------------ src/nav/navReducer.js | 37 -------------------------------- src/reduxTypes.js | 8 ------- 8 files changed, 28 insertions(+), 99 deletions(-) create mode 100644 src/nav/AppContainer.js delete mode 100644 src/nav/AppWithNavigation.js delete mode 100644 src/nav/navReducer.js diff --git a/src/ZulipMobile.js b/src/ZulipMobile.js index 5bb53d63d35..e510a5b6093 100644 --- a/src/ZulipMobile.js +++ b/src/ZulipMobile.js @@ -11,7 +11,7 @@ import AppEventHandlers from './boot/AppEventHandlers'; import AppDataFetcher from './boot/AppDataFetcher'; import BackNavigationHandler from './nav/BackNavigationHandler'; import InitialNavigationDispatcher from './nav/InitialNavigationDispatcher'; -import AppWithNavigation from './nav/AppWithNavigation'; +import AppContainer from './nav/AppContainer'; import NavigationService from './nav/NavigationService'; import './i18n/locale'; @@ -31,7 +31,12 @@ export default (): React$Node => ( - + diff --git a/src/boot/reducers.js b/src/boot/reducers.js index a112ef84259..cf9e95dc606 100644 --- a/src/boot/reducers.js +++ b/src/boot/reducers.js @@ -17,7 +17,6 @@ import flags from '../chat/flagsReducer'; import narrows from '../chat/narrowsReducer'; import messages from '../message/messagesReducer'; import mute from '../mute/muteReducer'; -import nav from '../nav/navReducer'; import outbox from '../outbox/outboxReducer'; import presence from '../presence/presenceReducer'; import realm from '../realm/realmReducer'; @@ -46,7 +45,6 @@ const reducers = { messages, narrows, mute, - nav, outbox, presence, realm, diff --git a/src/boot/store.js b/src/boot/store.js index 91d04f1e8ac..f3691147bb0 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -4,7 +4,6 @@ import type { Store } from 'redux'; import thunkMiddleware from 'redux-thunk'; import { createLogger } from 'redux-logger'; import createActionBuffer from 'redux-action-buffer'; -import { createReactNavigationReduxMiddleware } from 'react-navigation-redux-helpers'; import Immutable from 'immutable'; import * as Serialize from 'remotedev-serialize'; import { persistStore, autoRehydrate } from '../third/redux-persist'; @@ -29,7 +28,7 @@ import createMigration from '../redux-persist-migrate/index'; // prettier-ignore export const discardKeys: Array<$Keys> = [ 'alertWords', 'caughtUp', 'fetching', - 'nav', 'presence', 'session', 'topics', 'typing', 'userStatus', + 'presence', 'session', 'topics', 'typing', 'userStatus', ]; /** @@ -209,10 +208,6 @@ const migrations: { [string]: (GlobalState) => GlobalState } = { */ function listMiddleware() { const result = [ - // Allow us to cause navigation by dispatching Redux actions. - // See docs: https://github.com/react-navigation/redux-helpers - createReactNavigationReduxMiddleware((state: GlobalState) => state.nav, 'root'), - // Delay ("buffer") actions until a REHYDRATE action comes through. // After dispatching the latter, this will go back and dispatch // all the buffered actions. See docs: diff --git a/src/nav/AppContainer.js b/src/nav/AppContainer.js new file mode 100644 index 00000000000..08a6492d895 --- /dev/null +++ b/src/nav/AppContainer.js @@ -0,0 +1,7 @@ +/* @flow strict-local */ +import { createAppContainer } from 'react-navigation'; +import type { NavigationState } from 'react-navigation'; + +import AppNavigator from './AppNavigator'; + +export default createAppContainer(AppNavigator); diff --git a/src/nav/AppWithNavigation.js b/src/nav/AppWithNavigation.js deleted file mode 100644 index abf650f5e9e..00000000000 --- a/src/nav/AppWithNavigation.js +++ /dev/null @@ -1,16 +0,0 @@ -/* @flow strict-local */ - -import { createReduxContainer } from 'react-navigation-redux-helpers'; - -import { connect } from 'react-redux'; -import AppNavigator from './AppNavigator'; - -// $FlowFixMe - should use a type-checked `connect` -export default connect( - state => ({ - state: state.nav, - }), - null, - null, - { forwardRef: true }, -)((createReduxContainer(AppNavigator, 'root'): React$ComponentType<{||}>)); diff --git a/src/nav/NavigationService.js b/src/nav/NavigationService.js index bc20cf6a2b2..9577ea0728a 100644 --- a/src/nav/NavigationService.js +++ b/src/nav/NavigationService.js @@ -2,47 +2,32 @@ import React from 'react'; import type { NavigationAction, - NavigationNavigatorProps, NavigationState, - NavigationDispatch, - SupportedThemes, + NavigationContainer, + NavigationContainerProps, } from 'react-navigation'; -type ReduxContainerProps = { - ...$Diff, { navigation: mixed }>, - state: NavigationState, - dispatch: NavigationDispatch, - theme: SupportedThemes | 'no-preference', -}; - -// Should mimic return type of react-navigation-redux-helpers's -// `createReduxContainer`. -type ReduxContainer = React$Component; - -// TODO: This will eventually be a ref to the component instance -// created by React Navigation's `createAppContainer`, not -// react-navigation-redux-helpers's `createReduxContainer`. -const reduxContainerRef = React.createRef(); +const appContainerRef = React.createRef< + NavigationContainer>, +>(); const getState = (): NavigationState => { - if (reduxContainerRef.current === null) { - throw new Error('Tried to use NavigationService before reduxContainerRef was set.'); + if (appContainerRef.current === null) { + throw new Error('Tried to use NavigationService before appContainerRef was set.'); } return ( - reduxContainerRef.current - // $FlowFixMe - how to tell Flow about this method? - .getCurrentNavigation().state + // $FlowFixMe - how to tell Flow about `.state`? + appContainerRef.current.state.nav ); }; const dispatch = (navigationAction: NavigationAction): boolean => { - if (reduxContainerRef.current === null) { - throw new Error('Tried to use NavigationService before reduxContainerRef was set.'); + if (appContainerRef.current === null) { + throw new Error('Tried to use NavigationService before appContainerRef was set.'); } return ( - reduxContainerRef.current + appContainerRef.current // $FlowFixMe - how to tell Flow about this method? - .getCurrentNavigation() .dispatch(navigationAction) ); }; @@ -50,5 +35,5 @@ const dispatch = (navigationAction: NavigationAction): boolean => { export default { getState, dispatch, - reduxContainerRef, + appContainerRef, }; diff --git a/src/nav/navReducer.js b/src/nav/navReducer.js deleted file mode 100644 index 07eee0d51b4..00000000000 --- a/src/nav/navReducer.js +++ /dev/null @@ -1,37 +0,0 @@ -/* @flow strict-local */ -import type { NavigationAction } from 'react-navigation'; - -import type { NavigationState, Action } from '../types'; -import AppNavigator from './AppNavigator'; - -/** - * Get the initial state for the given route. - * - * Private; exported only for tests. - */ -export const getStateForRoute = (route: string): NavigationState => { - const action = AppNavigator.router.getActionForPathAndParams(route); - if (!action) { - // The argument should be a constant string that is a genuine nav route; - // so this condition can only happen if we've gotten that wrong. - throw new Error(`bad route: ${route}`); - } - const state = AppNavigator.router.getStateForAction(action); - if (!state) { - throw new Error(`bad route at getStateForAction: ${route}`); - } - return state; -}; - -export const initialState = getStateForRoute('loading'); - -export default (state: NavigationState = initialState, action: Action): NavigationState => { - switch (action.type) { - default: { - // The `react-navigation` libdef says this only takes a NavigationAction, - // but docs say pass arbitrary action. $FlowFixMe - const action1: NavigationAction = action; - return AppNavigator.router.getStateForAction(action1, state) || state; - } - } -}; diff --git a/src/reduxTypes.js b/src/reduxTypes.js index 58f088ebbac..a88b3738c14 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -196,13 +196,6 @@ export type NavigationRouteState = { }, }; -export type NavigationState = {| - index: number, - isTransitioning: boolean, - key: string, - routes: NavigationRouteState[], -|}; - export type OutboxState = Outbox[]; /** @@ -358,7 +351,6 @@ export type GlobalState = {| migrations: MigrationsState, mute: MuteState, narrows: NarrowsState, - nav: NavigationState, outbox: OutboxState, presence: PresenceState, realm: RealmState,