Skip to content
4 changes: 2 additions & 2 deletions src/account/AccountPickScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { TranslationContext } from '../boot/TranslationProvider';
import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import { useSelector, useGlobalDispatch } from '../react-redux';
import { useGlobalSelector, useGlobalDispatch } from '../react-redux';
import { getAccountStatuses } from '../selectors';
import { Centerer, ZulipButton, Logo, Screen, ViewPlaceholder } from '../common';
import AccountList from './AccountList';
Expand All @@ -29,7 +29,7 @@ type Props = $ReadOnly<{|

export default function AccountPickScreen(props: Props): Node {
const { navigation } = props;
const accounts = useSelector(getAccountStatuses);
const accounts = useGlobalSelector(getAccountStatuses);
const dispatch = useGlobalDispatch();
const _ = useContext(TranslationContext);

Expand Down
4 changes: 2 additions & 2 deletions src/boot/HideIfNotHydrated.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* @flow strict-local */
import React from 'react';
import type { Node } from 'react';
import { useSelector } from 'react-redux';

import { useGlobalSelector } from '../react-redux';
import { getIsHydrated } from '../selectors';

type Props = $ReadOnly<{|
Expand All @@ -12,7 +12,7 @@ type Props = $ReadOnly<{|
|}>;

export default function HideIfNotHydrated(props: Props): Node {
const isHydrated = useSelector(getIsHydrated);
const isHydrated = useGlobalSelector(getIsHydrated);

const { children, PlaceholderComponent } = props;

Expand Down
4 changes: 2 additions & 2 deletions src/boot/ThemeProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import React from 'react';
import type { Node } from 'react';

import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { getGlobalSettings } from '../directSelectors';
import { themeData, ThemeContext } from '../styles/theme';
import { ZulipStatusBar } from '../common';
Expand All @@ -14,7 +14,7 @@ type Props = $ReadOnly<{|

export default function ThemeProvider(props: Props): Node {
const { children } = props;
const theme = useSelector(state => getGlobalSettings(state).theme);
const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
return (
<ThemeContext.Provider value={themeData[theme]}>
<ZulipStatusBar />
Expand Down
4 changes: 2 additions & 2 deletions src/boot/TranslationProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { IntlProvider, IntlContext } from 'react-intl';
import type { IntlShape } from 'react-intl';

import type { GetText } from '../types';
import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { getGlobalSettings } from '../selectors';
import messages from '../i18n/messages';

Expand Down Expand Up @@ -78,7 +78,7 @@ type Props = $ReadOnly<{|

export default function TranslationProvider(props: Props): Node {
const { children } = props;
const language = useSelector(state => getGlobalSettings(state).language);
const language = useGlobalSelector(state => getGlobalSettings(state).language);

return (
<IntlProvider locale={language} textComponent={Text} messages={messages[language]}>
Expand Down
4 changes: 2 additions & 2 deletions src/common/OfflineNotice.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import NetInfo from '@react-native-community/netinfo';
import * as logging from '../utils/logging';
import { createStyleSheet, HALF_COLOR } from '../styles';
import { useHasStayedTrueForMs } from '../reactUtils';
import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { getGlobalSession } from '../selectors';
import Label from './Label';

Expand Down Expand Up @@ -37,7 +37,7 @@ type Props = $ReadOnly<{||}>;
* Shows nothing if the Internet is reachable.
*/
export default function OfflineNotice(props: Props): Node {
const isOnline = useSelector(state => getGlobalSession(state).isOnline);
const isOnline = useGlobalSelector(state => getGlobalSession(state).isOnline);

const shouldShowUncertaintyNotice = useHasStayedTrueForMs(
// See note in `SessionState` for what this means.
Expand Down
4 changes: 2 additions & 2 deletions src/common/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Node } from 'react';
import { View } from 'react-native';

import { ThemeContext, createStyleSheet } from '../styles';
import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { getGlobalSettings } from '../directSelectors';

const styles = createStyleSheet({
Expand Down Expand Up @@ -46,7 +46,7 @@ type Props = $ReadOnly<{|
export default function Popup(props: Props): Node {
const themeContext = useContext(ThemeContext);
// TODO(color/theme): find a cleaner way to express this
const isDarkTheme = useSelector(state => getGlobalSettings(state).theme !== 'default');
const isDarkTheme = useGlobalSelector(state => getGlobalSettings(state).theme !== 'default');
return (
<View style={[{ backgroundColor: themeContext.backgroundColor }, styles.popup]}>
<View style={isDarkTheme && styles.overlay}>{props.children}</View>
Expand Down
4 changes: 2 additions & 2 deletions src/common/ServerCompatBanner.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React from 'react';
import type { Node } from 'react';

import ZulipBanner from './ZulipBanner';
import { useSelector, useDispatch } from '../react-redux';
import { useSelector, useGlobalSelector, useDispatch } from '../react-redux';
import { getIdentity, getServerVersion } from '../account/accountsSelectors';
import { getIsAdmin, getSession, getGlobalSettings } from '../directSelectors';
import { dismissCompatNotice } from '../session/sessionActions';
Expand All @@ -27,7 +27,7 @@ export default function ServerCompatBanner(props: Props): Node {
const zulipVersion = useSelector(getServerVersion);
const realm = useSelector(state => getIdentity(state).realm);
const isAdmin = useSelector(getIsAdmin);
const settings = useSelector(getGlobalSettings);
const settings = useGlobalSelector(getGlobalSettings);

let visible = false;
let text = '';
Expand Down
6 changes: 3 additions & 3 deletions src/common/ZulipStatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Platform, StatusBar } from 'react-native';
import Color from 'color';

import type { ThemeName } from '../types';
import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { foregroundColorFromBackground } from '../utils/color';
import { getGlobalSession, getGlobalSettings } from '../selectors';

Expand Down Expand Up @@ -45,8 +45,8 @@ type Props = $ReadOnly<{|
*/
export default function ZulipStatusBar(props: Props): Node {
const { hidden = false } = props;
const theme = useSelector(state => getGlobalSettings(state).theme);
const orientation = useSelector(state => getGlobalSession(state).orientation);
const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
const orientation = useGlobalSelector(state => getGlobalSession(state).orientation);
const backgroundColor = props.backgroundColor;
const statusBarColor = getStatusBarColor(backgroundColor, theme);
return (
Expand Down
2 changes: 1 addition & 1 deletion src/compose/MentionWarnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import React, { useState, useCallback, useContext, forwardRef, useImperativeHandle } from 'react';
import type { AbstractComponent, Node } from 'react';
import { useSelector } from 'react-redux';

import type { Stream, Narrow, UserOrBot, Subscription, UserId } from '../types';
import { useSelector } from '../react-redux';
import { TranslationContext } from '../boot/TranslationProvider';
import { getAllUsersById, getAuth } from '../selectors';
import { isPmNarrow } from '../utils/narrow';
Expand Down
4 changes: 2 additions & 2 deletions src/lightbox/Lightbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { useActionSheet } from '@expo/react-native-action-sheet';

import * as NavigationService from '../nav/NavigationService';
import type { Message } from '../types';
import { useSelector } from '../react-redux';
import { useGlobalSelector, useSelector } from '../react-redux';
import type { ShowActionSheetWithOptions } from '../action-sheets';
import { getAuth, getGlobalSession } from '../selectors';
import { getResource } from '../utils/url';
Expand Down Expand Up @@ -69,7 +69,7 @@ export default function Lightbox(props: Props): Node {

// Since we're using `Dimensions.get` (below), we'll want a rerender
// when the orientation changes. No need to store the value.
useSelector(state => getGlobalSession(state).orientation);
useGlobalSelector(state => getGlobalSession(state).orientation);

const { width: windowWidth, height: windowHeight } = Dimensions.get('window');

Expand Down
6 changes: 3 additions & 3 deletions src/nav/AppNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '@react-navigation/stack';

import type { RouteParamsOf } from '../react-navigation';
import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { getHasAuth, getAccounts } from '../selectors';
import getInitialRouteInfo from './getInitialRouteInfo';
import type { GlobalParamList } from './globalTypes';
Expand Down Expand Up @@ -89,8 +89,8 @@ const Stack = createStackNavigator<GlobalParamList, AppNavigatorParamList, AppNa
type Props = $ReadOnly<{||}>;

export default function AppNavigator(props: Props): Node {
const hasAuth = useSelector(getHasAuth);
const accounts = useSelector(getAccounts);
const hasAuth = useGlobalSelector(getHasAuth);
const accounts = useGlobalSelector(getAccounts);

const { initialRouteName, initialRouteParams } = getInitialRouteInfo({
hasAuth,
Expand Down
4 changes: 2 additions & 2 deletions src/nav/ZulipNavigationContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { useContext, useEffect } from 'react';
import type { Node } from 'react';
import { NavigationContainer, DefaultTheme, DarkTheme } from '@react-navigation/native';

import { useSelector } from '../react-redux';
import { useGlobalSelector } from '../react-redux';
import { ThemeContext } from '../styles';
import * as NavigationService from './NavigationService';
import { getGlobalSettings } from '../selectors';
Expand All @@ -22,7 +22,7 @@ type Props = $ReadOnly<{||}>;
* and `initialRouteParams` which we get from data in Redux.
*/
export default function ZulipAppContainer(props: Props): Node {
const themeName = useSelector(state => getGlobalSettings(state).theme);
const themeName = useGlobalSelector(state => getGlobalSettings(state).theme);

useEffect(
() =>
Expand Down
32 changes: 29 additions & 3 deletions src/notification/notificationActions.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
/* @flow strict-local */
import { Platform } from 'react-native';

import type { Account, Dispatch, Identity, Action, ThunkAction, GlobalThunkAction } from '../types';
import type {
Account,
Dispatch,
GlobalDispatch,
Identity,
Action,
ThunkAction,
GlobalThunkAction,
} from '../types';
import * as api from '../api';
import {
getNotificationToken,
Expand Down Expand Up @@ -58,12 +66,30 @@ export const narrowToNotification = (data: ?Notification): GlobalThunkAction<voi
getOwnUserId(state),
);
if (narrow) {
dispatch(doNarrow(narrow));
// We have a GlobalDispatch, because this is a global thunk action --
// at the top of the function, we didn't yet know which account was
// intended and had to work that out. But now we know we're working on
// the active account, and want to dispatch a per-account action there.
// For the present, we just use the fact that our GlobalDispatch value
// is the same function as we use for Dispatch.
// TODO(#5006): perhaps have an extra `activeAccountDispatch: Dispatch`?
(dispatch: $FlowFixMe)(doNarrow(narrow));
}
};

/** Tell the given server about this device token, if it doesn't already know. */
const sendPushToken = async (dispatch: Dispatch, account: Account | void, pushToken: string) => {
const sendPushToken = async (
// Why `Dispatch | GlobalDispatch`? Well, this function is per-account...
// but whereas virtually all our other per-account code is implicitly
// about the active account, this is about a specific account it's
// explicitly passed. That makes it equally legitimate to call from
// per-account or global code, and we do both.
// TODO(#5006): Once we have per-account states for all accounts, make
// this an ordinary per-account action.
dispatch: Dispatch | GlobalDispatch,
account: Account | void,
pushToken: string,
) => {
if (!account || account.apiKey === '') {
// We've logged out of the account and/or forgotten it. Shrug.
return;
Expand Down
4 changes: 2 additions & 2 deletions src/presence/PresenceHeartbeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import { PureComponent } from 'react';
import type { ComponentType } from 'react';
import { AppState } from 'react-native';
import type { Dispatch } from '../types';

import { assumeSecretlyGlobalState, type Dispatch } from '../reduxTypes';
import { connect } from '../react-redux';
import { getHasAuth } from '../account/accountsSelectors';
import { reportPresence } from '../actions';
Expand Down Expand Up @@ -76,7 +76,7 @@ class PresenceHeartbeatInner extends PureComponent<Props> {
/** (NB this is a per-account component.) */
// TODO(#5005): either make one of these per account, or make it act on all accounts
const PresenceHeartbeat: ComponentType<OuterProps> = connect(state => ({
hasAuth: getHasAuth(state),
hasAuth: getHasAuth(assumeSecretlyGlobalState(state)), // a job for withHaveServerDataGate?
}))(PresenceHeartbeatInner);

export default PresenceHeartbeat;
12 changes: 9 additions & 3 deletions src/react-redux.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
useDispatch as useDispatchInner,
} from 'react-redux';

import type { GlobalState, Dispatch, GlobalDispatch } from './types';
import type { PerAccountState, GlobalState, Dispatch, GlobalDispatch } from './types';
import type { BoundedDiff } from './generics';

/* eslint-disable flowtype/generic-spacing */
Expand Down Expand Up @@ -69,8 +69,7 @@ export type OwnProps<C, -SP, -D> = BoundedDiff<
*/
// prettier-ignore
export function connect<SP, P, C: ComponentType<P>>(
// TODO(#5006): should be PerAccountState
mapStateToProps?: (GlobalState, OwnProps<C,
mapStateToProps?: (PerAccountState, OwnProps<C,
// Error "property `foo` is missing"? Add to inner component's props.
SP, Dispatch>) => SP,
): C => ComponentType<$ReadOnly<OwnProps<C, SP, Dispatch>>> {
Expand Down Expand Up @@ -101,6 +100,13 @@ export function connectGlobal<SP, P, C: ComponentType<P>>(
* function effectively gets no type-checking of anything it does with it.
*/
export function useSelector<SS>(
selector: (state: PerAccountState) => SS,
equalityFn?: (a: SS, b: SS) => boolean,
): SS {
return useSelectorInner<PerAccountState, SS>(selector, equalityFn);
}

export function useGlobalSelector<SS>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redux types: Split useSelector vs. new useGlobalSelector

A search for from 'react-redux' helps find two cases where the useSelector we're using is the one straight from react-redux, not our wrapper here that now uses PerAccountState.

After changing those two import { useSelector } from 'react-redux';s to instead say from '../react-redux', it looks like the useSelector in HideIfNotHydrated wants to be a useGlobalSelector.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good catch, thanks!

It looks like what happened in those cases is:

  • One was a change written before we had the type-wrapper, and we didn't notice that point when rebasing and merging.
  • The other was originally an equally mistaken direct import of connect. We didn't notice in code review when merging, and didn't happen to notice later when converting to Hooks.

It'd be nice to have a lint rule against these imports, because it's an easy thing to do by accident (especially for contributors new to our codebase). I'll take a quick look to see if there's a real easy way to do that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selector: (state: GlobalState) => SS,
equalityFn?: (a: SS, b: SS) => boolean,
): SS {
Expand Down
24 changes: 9 additions & 15 deletions src/reduxTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,23 +530,17 @@ export interface GlobalDispatch {
}

/** A global thunk action returning T. */
export type GlobalThunkAction<T> = (
GlobalDispatch,
() => GlobalState,
// These extras are meant for a per-account thunk action; when writing a
// global thunk action, everything they provide is redundant with the
// GlobalState provided by the previous argument. But passing them here
// allows a ThunkAction to be used as a GlobalThunkAction. For #5006
// we'll want to disallow that, but it's convenient for the present.
ThunkExtras,
) => T;
// This might take some extras later (e.g., to do something per-account on a
// specific account), but for now it needs none.
export type GlobalThunkAction<T> = (GlobalDispatch, () => GlobalState) => T;

/* eslint-disable no-unused-expressions */
// For now, it'll smooth our migration to let a GlobalDispatch be seamlessly
// usable as a plain Dispatch, and a ThunkAction as a GlobalThunkAction.
(d: GlobalDispatch): Dispatch => d; // TODO(#5006)
<T>(a: ThunkAction<T>): GlobalThunkAction<T> => a; // TODO(#5006)
// But we don't allow the reverse.
// The two pairs of dispatch/thunk-action types aren't interchangeable,
// in either direction.
// $FlowExpectedError[incompatible-return]
(d: GlobalDispatch): Dispatch => d;
// $FlowExpectedError[incompatible-return]
<T>(a: ThunkAction<T>): GlobalThunkAction<T> => a;
// $FlowExpectedError[incompatible-return]
(d: Dispatch): GlobalDispatch => d;
// $FlowExpectedError[incompatible-exact]
Expand Down
4 changes: 2 additions & 2 deletions src/settings/LanguageScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Node } from 'react';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import { useSelector, useDispatch } from '../react-redux';
import { useGlobalSelector, useDispatch } from '../react-redux';
import { Screen } from '../common';
import LanguagePicker from './LanguagePicker';
import { getGlobalSettings } from '../selectors';
Expand All @@ -18,7 +18,7 @@ type Props = $ReadOnly<{|

export default function LanguageScreen(props: Props): Node {
const dispatch = useDispatch();
const language = useSelector(state => getGlobalSettings(state).language);
const language = useGlobalSelector(state => getGlobalSettings(state).language);

const [filter, setFilter] = useState<string>('');

Expand Down
8 changes: 4 additions & 4 deletions src/settings/SettingsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { Node } from 'react';
import type { RouteProp } from '../react-navigation';
import type { MainTabsNavigationProp } from '../main/MainTabsScreen';
import * as NavigationService from '../nav/NavigationService';
import { useSelector, useDispatch } from '../react-redux';
import { useGlobalSelector, useDispatch } from '../react-redux';
import { getGlobalSettings } from '../selectors';
import { NestedNavRow, SwitchRow, Screen } from '../common';
import {
Expand All @@ -30,9 +30,9 @@ type Props = $ReadOnly<{|
|}>;

export default function SettingsScreen(props: Props): Node {
const theme = useSelector(state => getGlobalSettings(state).theme);
const browser = useSelector(state => getGlobalSettings(state).browser);
const doNotMarkMessagesAsRead = useSelector(
const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
const browser = useGlobalSelector(state => getGlobalSettings(state).browser);
const doNotMarkMessagesAsRead = useGlobalSelector(
state => getGlobalSettings(state).doNotMarkMessagesAsRead,
);
const dispatch = useDispatch();
Expand Down
Loading