diff --git a/src/diagnostics/TimingScreen.js b/src/diagnostics/TimingScreen.js index f4b0fb48ccb..b50dad19f43 100644 --- a/src/diagnostics/TimingScreen.js +++ b/src/diagnostics/TimingScreen.js @@ -1,5 +1,5 @@ /* @flow strict-local */ -import React, { PureComponent } from 'react'; +import React from 'react'; import { FlatList } from 'react-native'; import type { RouteProp } from '../react-navigation'; @@ -13,18 +13,16 @@ type Props = $ReadOnly<{| route: RouteProp<'timing', void>, |}>; -export default class TimingScreen extends PureComponent { - render() { - return ( - - index.toString()} - renderItem={({ item }) => ( - - )} - /> - - ); - } +export default function TimingScreen(props: Props) { + return ( + + index.toString()} + renderItem={({ item }) => ( + + )} + /> + + ); } diff --git a/src/main/MainTabsScreen.js b/src/main/MainTabsScreen.js index 2e1c1f6938f..1257acf28e9 100644 --- a/src/main/MainTabsScreen.js +++ b/src/main/MainTabsScreen.js @@ -8,7 +8,6 @@ import { import { useSafeAreaInsets } from 'react-native-safe-area-context'; import type { RouteProp, RouteParamsOf } from '../react-navigation'; -import type { Dispatch } from '../types'; import type { AppNavigationProp } from '../nav/AppNavigator'; import type { GlobalParamList } from '../nav/globalTypes'; import { bottomTabNavigatorConfig } from '../styles/tabs'; @@ -20,10 +19,7 @@ import { IconInbox, IconSettings, IconStream } from '../common/Icons'; import { OwnAvatar, OfflineNotice, ZulipStatusBar } from '../common'; import IconUnreadConversations from '../nav/IconUnreadConversations'; import ProfileScreen from '../account-info/ProfileScreen'; -import { connect } from '../react-redux'; -import { getHaveServerData } from '../selectors'; import styles, { ThemeContext } from '../styles'; -import FullScreenLoading from '../common/FullScreenLoading'; export type MainTabsNavigatorParamList = {| home: RouteParamsOf, @@ -43,35 +39,16 @@ const Tab = createBottomTabNavigator< MainTabsNavigationProp<>, >(); -type SelectorProps = $ReadOnly<{| - haveServerData: boolean, -|}>; - type Props = $ReadOnly<{| navigation: AppNavigationProp<'main-tabs'>, route: RouteProp<'main-tabs', void>, - - dispatch: Dispatch, - ...SelectorProps, |}>; -function MainTabsScreen(props: Props) { +export default function MainTabsScreen(props: Props) { const { backgroundColor } = useContext(ThemeContext); - const { haveServerData } = props; const insets = useSafeAreaInsets(); - if (!haveServerData) { - // Show a full-screen loading indicator while waiting for the - // initial fetch to complete, if we don't have potentially stale - // data to show instead. Also show it for the duration of the nav - // transition just after the user logs out (see our #4275). - // - // And avoid rendering any of our main UI, to maintain the - // guarantee that it can all rely on server data existing. - return ; - } - return ( @@ -128,22 +105,3 @@ function MainTabsScreen(props: Props) { ); } - -// `connect` does something useful for us that `useSelector` doesn't -// do: it interposes a new `ReactReduxContext.Provider` component, -// which proxies subscriptions so that the descendant components only -// rerender if this one continues to say their subtree should be kept -// around. See -// https://github.com/zulip/zulip-mobile/pull/4454#discussion_r578140524 -// and some discussion around -// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/converting.20to.20Hooks/near/1111970 -// where we describe some limits of our understanding. -// -// We found these things out while investigating an annoying crash: we -// found that `mapStateToProps` on a descendant of `MainTabsScreen` -// was running -- and throwing an uncaught error -- on logout, and -// `MainTabsScreen`'s early return on `!haveServerData` wasn't -// preventing that from happening. -export default connect(state => ({ - haveServerData: getHaveServerData(state), -}))(MainTabsScreen); diff --git a/src/nav/AppNavigator.js b/src/nav/AppNavigator.js index 50f0f311ccf..d95354eeccc 100644 --- a/src/nav/AppNavigator.js +++ b/src/nav/AppNavigator.js @@ -42,6 +42,7 @@ import EmojiPickerScreen from '../emoji/EmojiPickerScreen'; import LegalScreen from '../settings/LegalScreen'; import UserStatusScreen from '../user-status/UserStatusScreen'; import SharingScreen from '../sharing/SharingScreen'; +import withHaveServerDataGate from '../withHaveServerDataGate'; export type AppNavigatorParamList = {| 'account-pick': RouteParamsOf, @@ -104,39 +105,55 @@ export default function AppNavigator(props: Props) { }), }} > + {/* These screens expect server data in order to function normally. */} + + + + + + + + + + + + + + + + + + + + + + + + + + {/* These screens do not expect server data in order to function + normally. */} - - - - - - - - - - - - - - - - - - - - - - - - ); diff --git a/src/withHaveServerDataGate.js b/src/withHaveServerDataGate.js new file mode 100644 index 00000000000..48733f27d76 --- /dev/null +++ b/src/withHaveServerDataGate.js @@ -0,0 +1,63 @@ +/* @flow strict-local */ +import React, { type ComponentType, type ElementConfig } from 'react'; + +import { connect } from './react-redux'; +import type { Dispatch } from './types'; +import { getHaveServerData } from './selectors'; +import FullScreenLoading from './common/FullScreenLoading'; + +/** + * A HOC for any server-data-dependent screen component that might be + * mounted when we don't have server data. + * + * Prevents rerendering of the component's subtree unless we have + * server data. + * + * The implementation uses props named `dispatch` and `haveServerData`; the + * inner component shouldn't try to accept props with those names, and the + * caller shouldn't try to pass them in. + */ +// It sure seems like Flow should catch the `dispatch` / `haveServerData` +// thing and reflect it in the types; it's not clear why it doesn't. +export default function withHaveServerDataGate>>( + Comp: C, +): ComponentType<$Exact>> { + // `connect` does something useful for us that `useSelector` doesn't + // do: it interposes a new `ReactReduxContext.Provider` component, + // which proxies subscriptions so that the descendant components only + // rerender if this one continues to say their subtree should be kept + // around. See + // https://github.com/zulip/zulip-mobile/pull/4454#discussion_r578140524 + // and some discussion around + // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/converting.20to.20Hooks/near/1111970 + // where we describe some limits of our understanding. + // + // We found these things out while investigating an annoying crash: we + // found that `mapStateToProps` on a descendant of `MainTabsScreen` + // was running -- and throwing an uncaught error -- on logout, and + // `MainTabsScreen`'s early return on `!haveServerData` wasn't + // preventing that from happening. + return connect(state => ({ haveServerData: getHaveServerData(state) }))( + ({ + dispatch, + haveServerData, + ...props + }: {| + dispatch: Dispatch, + haveServerData: boolean, + ...$Exact

, + |}) => + haveServerData ? ( + + ) : ( + // Show a full-screen loading indicator while waiting for the + // initial fetch to complete, if we don't have potentially stale + // data to show instead. Also show it for the duration of the nav + // transition just after the user logs out (see our #4275). + // + // And avoid rendering any of our main UI, to maintain the + // guarantee that it can all rely on server data existing. + + ), + ); +}