Skip to content

Commit

Permalink
nav: Stop using react-navigation-redux-helpers.
Browse files Browse the repository at this point in the history
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. Leave a $FlowFixMe that's been a bit mystifying, but is
    probably caused by an oddness in the libdef.

- Remove use of `createReactNavigationReduxMiddleware`.

- Remove `state.nav`.

  - Remove `navReducer`.

  - Adjust types as necessary.

  - Remove 'nav' from `discardKeys` in src/boot/store.js

Fixes: #3804
  • Loading branch information
chrisbobbe committed Nov 3, 2020
1 parent 16528a2 commit 28c403c
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 105 deletions.
4 changes: 2 additions & 2 deletions src/ZulipMobile.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,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 { initializeSentry } from './sentry';
Expand All @@ -29,7 +29,7 @@ export default (): React$Node => (
<ThemeProvider>
<InitialNavigationDispatcher>
<BackNavigationHandler>
<AppWithNavigation ref={NavigationService.reduxContainerRef} />
<AppContainer ref={NavigationService.appContainerRef} />
</BackNavigationHandler>
</InitialNavigationDispatcher>
</ThemeProvider>
Expand Down
2 changes: 0 additions & 2 deletions src/boot/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -46,7 +45,6 @@ const reducers = {
messages,
narrows,
mute,
nav,
outbox,
presence,
realm,
Expand Down
7 changes: 1 addition & 6 deletions src/boot/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -29,7 +28,7 @@ import createMigration from '../redux-persist-migrate/index';
// prettier-ignore
export const discardKeys: Array<$Keys<GlobalState>> = [
'alertWords', 'caughtUp', 'fetching',
'nav', 'presence', 'session', 'topics', 'typing', 'userStatus',
'presence', 'session', 'topics', 'typing', 'userStatus',
];

/**
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions src/nav/AppContainer.js
Original file line number Diff line number Diff line change
@@ -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<NavigationState, { ... }>(AppNavigator);
16 changes: 0 additions & 16 deletions src/nav/AppWithNavigation.js

This file was deleted.

53 changes: 19 additions & 34 deletions src/nav/NavigationService.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,53 +2,38 @@
import React from 'react';
import type {
NavigationAction,
NavigationNavigatorProps,
NavigationState,
NavigationDispatch,
SupportedThemes,
NavigationContainer,
NavigationContainerProps,
} from 'react-navigation';

type ReduxContainerProps = {
...$Diff<NavigationNavigatorProps<{ ... }, NavigationState>, { 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<ReduxContainerProps>;

// 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<ReduxContainer>();
/* prettier-ignore */
const appContainerRef = React.createRef<
React$ElementRef<
NavigationContainer<
NavigationState,
{ ... },
NavigationContainerProps<{ ... }, NavigationState>>>
>();

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
);
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/decouple.20nav.20from.20redux.20%28.23M3804%29/near/1056167
// $FlowFixMe
return 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
// $FlowFixMe - how to tell Flow about this method?
.getCurrentNavigation()
.dispatch(navigationAction)
);
return appContainerRef.current.dispatch(navigationAction);
};

export default {
getState,
dispatch,
reduxContainerRef,
appContainerRef,
};
37 changes: 0 additions & 37 deletions src/nav/navReducer.js

This file was deleted.

8 changes: 0 additions & 8 deletions src/reduxTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,6 @@ export type NavigationRouteState = {
},
};

export type NavigationState = {|
index: number,
isTransitioning: boolean,
key: string,
routes: NavigationRouteState[],
|};

export type OutboxState = Outbox[];

/**
Expand Down Expand Up @@ -360,7 +353,6 @@ export type GlobalState = {|
migrations: MigrationsState,
mute: MuteState,
narrows: NarrowsState,
nav: NavigationState,
outbox: OutboxState,
presence: PresenceState,
realm: RealmState,
Expand Down

0 comments on commit 28c403c

Please sign in to comment.