Decouple Navigation from Redux (Part 1)

src/ZulipMobile.js

@@ -10,7 +10,9 @@ import CompatibilityChecker from './boot/CompatibilityChecker';
 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 NavigationService from './nav/NavigationService';
 
 import './i18n/locale';
 import { initializeSentry } from './sentry';
@@ -27,9 +29,11 @@ export default (): React$Node => (
         <AppDataFetcher>
           <TranslationProvider>
             <ThemeProvider>
-              <BackNavigationHandler>
-                <AppWithNavigation />
-              </BackNavigationHandler>
+              <InitialNavigationDispatcher>
+                <BackNavigationHandler>
+                  <AppWithNavigation ref={NavigationService.reduxContainerRef} />
+                </BackNavigationHandler>
+              </InitialNavigationDispatcher>
             </ThemeProvider>
           </TranslationProvider>
         </AppDataFetcher>

src/account/AccountPickScreen.js

@@ -32,7 +32,7 @@ class AccountPickScreen extends PureComponent<Props> {
         dispatch(accountSwitch(index));
       });
     } else {
-      dispatch(navigateToRealmScreen(realm));
+      dispatch(navigateToRealmScreen({ realm }));
     }
   };
 

src/account/accountActions.js

@@ -1,19 +1,25 @@
 /* @flow strict-local */
-import type { Action } from '../types';
+import type { Action, Dispatch, GetState } from '../types';
 import {
   ACCOUNT_SWITCH,
   REALM_ADD,
   ACCOUNT_REMOVE,
   LOGIN_SUCCESS,
   LOGOUT,
 } from '../actionConstants';
+import { resetToAccountPicker, resetToLoading } from '../nav/navActions';
 import type { ZulipVersion } from '../utils/zulipVersion';
 
-export const accountSwitch = (index: number): Action => ({
+const accountSwitchPlain = (index: number): Action => ({
   type: ACCOUNT_SWITCH,
   index,
 });
 
+export const accountSwitch = (index: number) => (dispatch: Dispatch, getState: GetState) => {
+  dispatch(resetToLoading());
+  dispatch(accountSwitchPlain(index));
+};
+
 export const realmAdd = (
   realm: URL,
   zulipFeatureLevel: number,
@@ -30,13 +36,26 @@ export const removeAccount = (index: number): Action => ({
   index,
 });
 
-export const loginSuccess = (realm: URL, email: string, apiKey: string): Action => ({
+const loginSuccessPlain = (realm: URL, email: string, apiKey: string): Action => ({
   type: LOGIN_SUCCESS,
   realm,
   email,
   apiKey,
 });
 
-export const logout = (): Action => ({
+export const loginSuccess = (realm: URL, email: string, apiKey: string) => (
+  dispatch: Dispatch,
+  getState: GetState,
+) => {
+  dispatch(resetToLoading());
+  dispatch(loginSuccessPlain(realm, email, apiKey));
+};
+
+const logoutPlain = (): Action => ({
   type: LOGOUT,
 });
+
+export const logout = () => async (dispatch: Dispatch, getState: GetState) => {
+  dispatch(resetToAccountPicker());
+  dispatch(logoutPlain());
+};

src/boot/store.js

@@ -17,7 +17,6 @@ import { REHYDRATE } from '../actionConstants';
 import rootReducer from './reducers';
 import ZulipAsyncStorage from './ZulipAsyncStorage';
 import createMigration from '../redux-persist-migrate/index';
-import { getNav } from '../nav/navSelectors';
 
 // AsyncStorage.clear(); // use to reset storage during development
 
@@ -212,7 +211,7 @@ function listMiddleware() {
   const result = [
     // Allow us to cause navigation by dispatching Redux actions.
     // See docs: https://github.com/react-navigation/redux-helpers
-    createReactNavigationReduxMiddleware(getNav, 'root'),
+    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

src/events/doEventActionSideEffects.js

@@ -28,7 +28,9 @@ const messageEvent = (state: GlobalState, message: Message): void => {
   }
 
   const activeAccount = getActiveAccount(state);
-  const { narrow } = getChatScreenParams(state);
+  // Assume (unchecked) that `narrow` is `Narrow` if present
+  // $FlowFixMe
+  const narrow: Narrow | void = getChatScreenParams().narrow;
   const isUserInSameNarrow =
     activeAccount
     && narrow !== undefined // chat screen is not at top

src/message/__tests__/fetchActions-test.js

@@ -16,7 +16,6 @@ import { FIRST_UNREAD_ANCHOR } from '../../anchor';
 import type { Message } from '../../api/modelTypes';
 import type { ServerMessage } from '../../api/messages/getMessages';
 import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow';
-import { navStateWithNarrow } from '../../utils/testHelpers';
 import * as eg from '../../__tests__/lib/exampleData';
 
 const mockStore = configureStore([thunk]);
@@ -41,7 +40,6 @@ describe('fetchActions', () => {
             older: true,
           },
         },
-        ...navStateWithNarrow(HOME_NARROW),
         narrows: {
           [streamNarrowStr]: [],
         },
@@ -64,7 +62,6 @@ describe('fetchActions', () => {
             older: false,
           },
         },
-        ...navStateWithNarrow(HOME_NARROW),
         narrows: {
           [streamNarrowStr]: [1],
         },
@@ -116,7 +113,6 @@ describe('fetchActions', () => {
     const message1 = eg.streamMessage({ id: 1 });
 
     const baseState = eg.reduxState({
-      ...navStateWithNarrow(HOME_NARROW),
       accounts: [eg.makeAccount()],
       narrows: {
         [streamNarrowStr]: [message1.id],
@@ -295,7 +291,6 @@ describe('fetchActions', () => {
     test('when messages to be fetched both before and after anchor, numBefore and numAfter are greater than zero', async () => {
       const store = mockStore<GlobalState, Action>(
         eg.reduxState({
-          ...navStateWithNarrow(HOME_NARROW),
           accounts: [eg.selfAccount],
           narrows: {
             [streamNarrowStr]: [1],
@@ -322,7 +317,6 @@ describe('fetchActions', () => {
     test('when no messages to be fetched before the anchor, numBefore is not greater than zero', async () => {
       const store = mockStore<GlobalState, Action>(
         eg.reduxState({
-          ...navStateWithNarrow(HOME_NARROW),
           accounts: [eg.selfAccount],
           narrows: {
             [streamNarrowStr]: [1],
@@ -348,7 +342,6 @@ describe('fetchActions', () => {
     test('when no messages to be fetched after the anchor, numAfter is not greater than zero', async () => {
       const store = mockStore<GlobalState, Action>(
         eg.reduxState({
-          ...navStateWithNarrow(HOME_NARROW),
           accounts: [eg.selfAccount],
           narrows: {
             [streamNarrowStr]: [1],
@@ -377,7 +370,6 @@ describe('fetchActions', () => {
     const message2 = eg.streamMessage({ id: 2 });
 
     const baseState = eg.reduxState({
-      ...navStateWithNarrow(HOME_NARROW),
       accounts: [eg.selfAccount],
       narrows: {
         ...eg.baseReduxState.narrows,
@@ -471,7 +463,6 @@ describe('fetchActions', () => {
     const message2 = eg.streamMessage({ id: 2 });
 
     const baseState = eg.reduxState({
-      ...navStateWithNarrow(HOME_NARROW),
       accounts: [eg.selfAccount],
       narrows: {
         ...eg.baseReduxState.narrows,

src/message/__tests__/messageActions-test.js

@@ -3,8 +3,7 @@ import configureStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
 
 import { doNarrow } from '../messagesActions';
-import { streamNarrow, HOME_NARROW } from '../../utils/narrow';
-import { navStateWithNarrow } from '../../utils/testHelpers';
+import { streamNarrow } from '../../utils/narrow';
 import * as eg from '../../__tests__/lib/exampleData';
 import type { GlobalState } from '../../reduxTypes';
 import type { Action } from '../../actionTypes';
@@ -20,7 +19,6 @@ describe('messageActions', () => {
         eg.reduxState({
           accounts: [eg.selfAccount],
           session: { ...eg.baseReduxState.session, isHydrated: true },
-          ...navStateWithNarrow(HOME_NARROW),
           streams: [eg.makeStream({ name: 'some stream' })],
         }),
       );

src/message/fetchActions.js

@@ -10,6 +10,7 @@ import type {
 } from '../types';
 import type { InitialData } from '../api/initialDataTypes';
 import * as api from '../api';
+import { resetToMainTabs } from '../actions';
 import { isClientError } from '../api/apiErrors';
 import {
   getAuth,
@@ -18,6 +19,7 @@ import {
   getLastMessageId,
   getCaughtUpForNarrow,
   getFetchingForNarrow,
+  getNavigationRoutes,
 } from '../selectors';
 import config from '../config';
 import {
@@ -156,10 +158,26 @@ const initialFetchStart = (): Action => ({
   type: INITIAL_FETCH_START,
 });
 
-const initialFetchComplete = (): Action => ({
+const initialFetchCompletePlain = (): Action => ({
   type: INITIAL_FETCH_COMPLETE,
 });
 
+export const initialFetchComplete = () => async (dispatch: Dispatch, getState: GetState) => {
+  if (!getNavigationRoutes().some(navigationRoute => navigationRoute.routeName === 'main')) {
+    // If we're anywhere in the normal UI of the app, then remain
+    // where we are. Only reset the nav state if we're elsewhere,
+    // and in that case, go to the main screen.
+    //
+    // TODO: "elsewhere" is probably just a way of saying "on the
+    // loading screen", but we're not sure. We could adjust the
+    // conditional accordingly, if we found out we're not depending on
+    // the more general condition; see
+    //   https://github.com/zulip/zulip-mobile/pull/4274#discussion_r505941875
+    dispatch(resetToMainTabs());
+  }
+  dispatch(initialFetchCompletePlain());
+};
+
 /** Private; exported only for tests. */
 export const isFetchNeededAtAnchor = (
   state: GlobalState,

src/nav/AppNavigator.js

@@ -94,7 +94,7 @@ export default createStackNavigator(
         ios: TransitionPresets.DefaultTransition,
       }),
     },
-    initialRouteName: 'main',
+    initialRouteName: 'loading',
     headerMode: 'none',
   },
 );

src/nav/AppWithNavigation.js

@@ -2,10 +2,15 @@
 
 import { createReduxContainer } from 'react-navigation-redux-helpers';
 
-import { connect } from '../react-redux';
-import { getNav } from '../selectors';
+import { connect } from 'react-redux';
 import AppNavigator from './AppNavigator';
 
-export default connect(state => ({
-  state: getNav(state),
-}))(createReduxContainer(AppNavigator, 'root'));
+// $FlowFixMe - should use a type-checked `connect`
+export default connect(
+  state => ({
+    state: state.nav,
+  }),
+  null,
+  null,
+  { forwardRef: true },
+)((createReduxContainer(AppNavigator, 'root'): React$ComponentType<{||}>));

src/nav/BackNavigationHandler.js

@@ -6,7 +6,6 @@ import { BackHandler } from 'react-native';
 
 import type { Dispatch } from '../types';
 import { connect } from '../react-redux';
-import { getCanGoBack } from '../selectors';
 import { navigateBack } from '../actions';
 
 type Props = $ReadOnly<{|
@@ -38,5 +37,5 @@ class BackNavigationHandler extends PureComponent<Props> {
 }
 
 export default connect(state => ({
-  canGoBack: getCanGoBack(state),
+  canGoBack: state.nav.index > 0,
 }))(BackNavigationHandler);

src/nav/InitialNavigationDispatcher.js

@@ -0,0 +1,95 @@
+/* @flow strict-local */
+import type { Node as React$Node } from 'react';
+import { PureComponent } from 'react';
+
+import type { Dispatch, Account } from '../types';
+import { resetToAccountPicker, resetToRealmScreen, resetToMainTabs } from '../actions';
+import { connect } from '../react-redux';
+import { getIsHydrated, hasAuth as getHasAuth, getHaveServerData } from '../selectors';
+
+type SelectorProps = $ReadOnly<{|
+  isHydrated: boolean,
+  hasAuth: boolean,
+  accounts: Account[],
+  haveServerData: boolean,
+|}>;
+
+type Props = $ReadOnly<{|
+  children: React$Node,
+
+  dispatch: Dispatch,
+  ...SelectorProps,
+|}>;
+
+class InitialNavigationDispatcher extends PureComponent<Props> {
+  componentDidMount() {
+    if (this.props.isHydrated) {
+      this.doInitialNavigation();
+    }
+  }
+
+  componentDidUpdate(prevProps) {
+    if (!prevProps.isHydrated && this.props.isHydrated) {
+      this.doInitialNavigation();
+    }
+  }
+
+  /**
+   * Data has been loaded, so open the app to the right screen.
+   *
+   * Not to be called before the REHYDRATE action, and not to be
+   * called more than once.
+   */
+  doInitialNavigation = () => {
+    const { hasAuth, accounts, haveServerData, dispatch } = this.props;
+
+    // If the active account is not logged in, bring the user as close
+    // as we can to AuthScreen, the place where they can log in.
+    if (!hasAuth) {
+      if (accounts.length > 1) {
+        // We can't guess which account, of multiple, the user wants
+        // to use. Let them pick one.
+        dispatch(resetToAccountPicker());
+        return;
+      } else if (accounts.length === 1) {
+        // We already know the realm, so give that to the realm
+        // screen. If that screen finds that the realm is valid, it'll
+        // send the user along to AuthScreen for that realm right
+        // away. If this means you're on the AuthScreen when you don't
+        // want to be (i.e., you want to choose a different realm),
+        // you can always go back to RealmScreen.
+        dispatch(resetToRealmScreen({ initial: true, realm: accounts[0].realm }));
+        return;
+      } else {
+        // Just go to the realm screen and have the user type out the
+        // realm.
+        dispatch(resetToRealmScreen({ initial: true }));
+        return;
+      }
+    }
+
+    // If there's an active, logged-in account but no server data, then behave
+    // like `ACCOUNT_SWITCH`: show loading screen.  Crucially, `sessionReducer`
+    // will have set `needsInitialFetch`, too, so we really will be loading.
+    if (!haveServerData) {
+      // We're already on the loading screen -- see `initialRouteName`
+      // in `AppNavigator`.
+      return;
+    }
+
+    // Great: we have an active, logged-in account, and server data for it.
+    // Show the main UI.
+    dispatch(resetToMainTabs());
+  };
+
+  render() {
+    return this.props.children;
+  }
+}
+
+export default connect(state => ({
+  hasAuth: getHasAuth(state),
+  accounts: state.accounts,
+  haveServerData: getHaveServerData(state),
+  isHydrated: getIsHydrated(state),
+}))(InitialNavigationDispatcher);