Skip to content

Commit

Permalink
redux types: Mark most selectors as per-account vs. global
Browse files Browse the repository at this point in the history
After this change, the bulk of our selectors are correctly marked
as per-account, taking a PerAccountState -- which is most of
them -- or global, taking a GlobalState.

The major class of exceptions is the `state.accounts` selectors
found in `accountsSelectors`, as discussed in a previous commit.

A more boring class of exceptions is the many small anonymous
selectors (mostly as callbacks to `useSelector`) that get bits of
the settings and session data.  Many of those are really
per-account, but for now we leave them all marked as global.

At this stage none of our thunk actions or React components get
converted.  That means they're all still acting on GlobalState; which
is why it's very convenient that (for the next while) we've set up
PerAccountState so that it's a supertype of GlobalState, and a value
of the latter can be freely implicitly used as the former.

(Of all the Redux-related names we'll be splitting, this one is
unusual in requiring renames through the per-account majority of the
app code.  Most will be like Selector in that the existing name is
short and generic and we'll keep that for the per-account majority
to use.  GlobalState is the main exception, because the existing
name is explicitly "global".)

This doesn't yet have any real effect, because PerAccountState is
for the moment just an alias of GlobalState.  The real test that we've
assigned each of these to the proper column will be that Flow passes
when we make those two types distinct, coming soon.
  • Loading branch information
gnprice committed Sep 22, 2021
1 parent b33f278 commit 59d8813
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 60 deletions.
6 changes: 3 additions & 3 deletions src/caughtup/caughtUpSelectors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* @flow strict-local */
import type { CaughtUp, CaughtUpState, GlobalState, Narrow } from '../types';
import type { CaughtUp, CaughtUpState, PerAccountState, Narrow } from '../types';
import { NULL_OBJECT } from '../nullObjects';
import { keyFromNarrow } from '../utils/narrow';

Expand All @@ -9,7 +9,7 @@ export const DEFAULT_CAUGHTUP: CaughtUp = {
newer: false,
};

export const getCaughtUp = (state: GlobalState): CaughtUpState => state.caughtUp || NULL_OBJECT;
export const getCaughtUp = (state: PerAccountState): CaughtUpState => state.caughtUp || NULL_OBJECT;

export const getCaughtUpForNarrow = (state: GlobalState, narrow: Narrow): CaughtUp =>
export const getCaughtUpForNarrow = (state: PerAccountState, narrow: Narrow): CaughtUp =>
getCaughtUp(state)[keyFromNarrow(narrow)] || DEFAULT_CAUGHTUP;
4 changes: 2 additions & 2 deletions src/chat/fetchingSelectors.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/* @flow strict-local */
import type { Fetching, GlobalState, Narrow } from '../types';
import type { Fetching, PerAccountState, Narrow } from '../types';
import { getFetching } from '../directSelectors';
import { keyFromNarrow } from '../utils/narrow';

/** The value implicitly represented by a missing entry in FetchingState. */
export const DEFAULT_FETCHING = { older: false, newer: false };

export const getFetchingForNarrow = (state: GlobalState, narrow: Narrow): Fetching =>
export const getFetchingForNarrow = (state: PerAccountState, narrow: Narrow): Fetching =>
getFetching(state)[keyFromNarrow(narrow)] || DEFAULT_FETCHING;
8 changes: 4 additions & 4 deletions src/chat/narrowsSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import isEqual from 'lodash.isequal';
import { createSelector } from 'reselect';

import type {
GlobalState,
PerAccountState,
Message,
Narrow,
Outbox,
Expand Down Expand Up @@ -61,7 +61,7 @@ export const outboxMessagesForNarrow: Selector<$ReadOnlyArray<Outbox>, Narrow> =
);

export const getFetchedMessageIdsForNarrow = (
state: GlobalState,
state: PerAccountState,
narrow: Narrow,
): $ReadOnlyArray<number> => getAllNarrows(state).get(keyFromNarrow(narrow)) || NULL_ARRAY;

Expand Down Expand Up @@ -181,12 +181,12 @@ export const getShownMessagesForNarrow: Selector<$ReadOnlyArray<Message | Outbox
}),
);

export const getFirstMessageId = (state: GlobalState, narrow: Narrow): number | void => {
export const getFirstMessageId = (state: PerAccountState, narrow: Narrow): number | void => {
const ids = getFetchedMessageIdsForNarrow(state, narrow);
return ids.length > 0 ? ids[0] : undefined;
};

export const getLastMessageId = (state: GlobalState, narrow: Narrow): number | void => {
export const getLastMessageId = (state: PerAccountState, narrow: Narrow): number | void => {
const ids = getFetchedMessageIdsForNarrow(state, narrow);
return ids.length > 0 ? ids[ids.length - 1] : undefined;
};
Expand Down
50 changes: 26 additions & 24 deletions src/directSelectors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* @flow strict-local */
import type {
PerAccountState,
GlobalState,
AccountsState,
SubscriptionsState,
Expand Down Expand Up @@ -36,66 +37,67 @@ export const getIsOnline = (state: GlobalState): boolean | null => state.session
export const getDebug = (state: GlobalState): Debug => state.session.debug;
export const getIsHydrated = (state: GlobalState): boolean => state.session.isHydrated;

export const getCanCreateStreams = (state: GlobalState): boolean => state.realm.canCreateStreams;
export const getCanCreateStreams = (state: PerAccountState): boolean =>
state.realm.canCreateStreams;

export const getDrafts = (state: GlobalState): DraftsState => state.drafts;
export const getDrafts = (state: PerAccountState): DraftsState => state.drafts;

export const getLoading = (state: GlobalState): boolean => state.session.loading;
export const getLoading = (state: PerAccountState): boolean => state.session.loading;

export const getMessages = (state: GlobalState): MessagesState => state.messages;
export const getMessages = (state: PerAccountState): MessagesState => state.messages;

export const getMute = (state: GlobalState): MuteState => state.mute;
export const getMute = (state: PerAccountState): MuteState => state.mute;

export const getMutedUsers = (state: GlobalState): MutedUsersState => state.mutedUsers;
export const getMutedUsers = (state: PerAccountState): MutedUsersState => state.mutedUsers;

export const getTyping = (state: GlobalState): TypingState => state.typing;
export const getTyping = (state: PerAccountState): TypingState => state.typing;

export const getTopics = (state: GlobalState): TopicsState => state.topics;
export const getTopics = (state: PerAccountState): TopicsState => state.topics;

export const getUserGroups = (state: GlobalState): UserGroupsState => state.userGroups;
export const getUserGroups = (state: PerAccountState): UserGroupsState => state.userGroups;

export const getUserStatus = (state: GlobalState): UserStatusState => state.userStatus;
export const getUserStatus = (state: PerAccountState): UserStatusState => state.userStatus;

/**
* WARNING: despite the name, only (a) `is_active` users (b) excluding cross-realm bots.
*
* See `getAllUsers`.
*/
export const getUsers = (state: GlobalState): UsersState => state.users;
export const getUsers = (state: PerAccountState): UsersState => state.users;

export const getFetching = (state: GlobalState): FetchingState => state.fetching;
export const getFetching = (state: PerAccountState): FetchingState => state.fetching;

export const getFlags = (state: GlobalState): FlagsState => state.flags;
export const getFlags = (state: PerAccountState): FlagsState => state.flags;

export const getAllNarrows = (state: GlobalState): NarrowsState => state.narrows;
export const getAllNarrows = (state: PerAccountState): NarrowsState => state.narrows;

export const getSettings = (state: GlobalState): SettingsState => state.settings;

export const getSubscriptions = (state: GlobalState): SubscriptionsState => state.subscriptions;
export const getSubscriptions = (state: PerAccountState): SubscriptionsState => state.subscriptions;

/**
* All streams in the current realm.
*
* This is rarely the right selector to use: consider `getStreamForId`
* or `getStreamsById` instead.
*/
export const getStreams = (state: GlobalState): StreamsState => state.streams;
export const getStreams = (state: PerAccountState): StreamsState => state.streams;

export const getPresence = (state: GlobalState): PresenceState => state.presence;
export const getPresence = (state: PerAccountState): PresenceState => state.presence;

export const getOutbox = (state: GlobalState): OutboxState => state.outbox;
export const getOutbox = (state: PerAccountState): OutboxState => state.outbox;

export const getRealm = (state: GlobalState): RealmState => state.realm;
export const getRealm = (state: PerAccountState): RealmState => state.realm;

export const getCrossRealmBots = (state: GlobalState): $ReadOnlyArray<CrossRealmBot> =>
export const getCrossRealmBots = (state: PerAccountState): $ReadOnlyArray<CrossRealmBot> =>
state.realm.crossRealmBots;

export const getRawRealmEmoji = (state: GlobalState): RealmEmojiById => state.realm.emoji;
export const getRawRealmEmoji = (state: PerAccountState): RealmEmojiById => state.realm.emoji;

export const getNonActiveUsers = (state: GlobalState): $ReadOnlyArray<User> =>
export const getNonActiveUsers = (state: PerAccountState): $ReadOnlyArray<User> =>
state.realm.nonActiveUsers;

export const getIsAdmin = (state: GlobalState): boolean => state.realm.isAdmin;
export const getIsAdmin = (state: PerAccountState): boolean => state.realm.isAdmin;

export const getVideoChatProvider = (state: GlobalState): VideoChatProvider | null =>
export const getVideoChatProvider = (state: PerAccountState): VideoChatProvider | null =>
state.realm.videoChatProvider;
4 changes: 2 additions & 2 deletions src/drafts/draftsSelectors.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* @flow strict-local */
import type { Narrow, GlobalState } from '../types';
import type { Narrow, PerAccountState } from '../types';
import { keyFromNarrow } from '../utils/narrow';

export const getDraftForNarrow = (state: GlobalState, narrow: Narrow): string =>
export const getDraftForNarrow = (state: PerAccountState, narrow: Narrow): string =>
state.drafts[keyFromNarrow(narrow)] || '';
4 changes: 2 additions & 2 deletions src/pm-conversations/pmConversationsSelectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import invariant from 'invariant';
import { createSelector } from 'reselect';

import type { GlobalState, Message, PmConversationData, Selector } from '../types';
import type { PerAccountState, Message, PmConversationData, Selector } from '../types';
import { assumeSecretlyGlobalState } from '../reduxTypes';
import { getPrivateMessages } from '../message/messageSelectors';
import { getAllUsersById, getOwnUserId } from '../users/userSelectors';
Expand Down Expand Up @@ -129,7 +129,7 @@ const getServerIsOld: Selector<boolean> = createSelector(
/**
* The most recent PM conversations, with unread count and latest message ID.
*/
export const getRecentConversations = (state: GlobalState): PmConversationData[] =>
export const getRecentConversations = (state: PerAccountState): PmConversationData[] =>
getServerIsOld(state) ? getRecentConversationsLegacy(state) : getRecentConversationsModern(state);

export const getUnreadConversations: Selector<
Expand Down
6 changes: 3 additions & 3 deletions src/subscriptions/subscriptionSelectors.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* @flow strict-local */
import { createSelector } from 'reselect';

import type { GlobalState, Narrow, Selector, Stream, Subscription } from '../types';
import type { PerAccountState, Narrow, Selector, Stream, Subscription } from '../types';
import { isStreamOrTopicNarrow, streamNameOfNarrow } from '../utils/narrow';
import { getSubscriptions, getStreams } from '../directSelectors';

Expand Down Expand Up @@ -70,7 +70,7 @@ export const getSubscribedStreams: Selector<Subscription[]> = createSelector(
* See `getStreamsById` for use when a stream might not exist, or when
* multiple streams are relevant.
*/
export const getStreamForId = (state: GlobalState, streamId: number): Stream => {
export const getStreamForId = (state: PerAccountState, streamId: number): Stream => {
const stream = getStreamsById(state).get(streamId);
if (!stream) {
throw new Error(`getStreamForId: missing stream: id ${streamId}`);
Expand All @@ -97,7 +97,7 @@ export const getIsActiveStreamAnnouncementOnly: Selector<boolean, Narrow> = crea
*
* Gives undefined for narrows that are not stream or topic narrows.
*/
export const getStreamColorForNarrow = (state: GlobalState, narrow: Narrow): string | void => {
export const getStreamColorForNarrow = (state: PerAccountState, narrow: Narrow): string | void => {
if (!isStreamOrTopicNarrow(narrow)) {
return undefined;
}
Expand Down
19 changes: 11 additions & 8 deletions src/unread/unreadModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {
UnreadHuddlesState,
UnreadMentionsState,
} from './unreadModelTypes';
import type { GlobalState } from '../reduxTypes';
import type { PerAccountState } from '../reduxTypes';
import unreadPmsReducer from './unreadPmsReducer';
import unreadHuddlesReducer from './unreadHuddlesReducer';
import unreadMentionsReducer from './unreadMentionsReducer';
Expand All @@ -32,15 +32,18 @@ import {
//

/** The unread-messages state as a whole. */
export const getUnread = (state: GlobalState): UnreadState => state.unread;
export const getUnread = (state: PerAccountState): UnreadState => state.unread;

export const getUnreadStreams = (state: GlobalState): UnreadStreamsState => state.unread.streams;
export const getUnreadStreams = (state: PerAccountState): UnreadStreamsState =>
state.unread.streams;

export const getUnreadPms = (state: GlobalState): UnreadPmsState => state.unread.pms;
export const getUnreadPms = (state: PerAccountState): UnreadPmsState => state.unread.pms;

export const getUnreadHuddles = (state: GlobalState): UnreadHuddlesState => state.unread.huddles;
export const getUnreadHuddles = (state: PerAccountState): UnreadHuddlesState =>
state.unread.huddles;

export const getUnreadMentions = (state: GlobalState): UnreadMentionsState => state.unread.mentions;
export const getUnreadMentions = (state: PerAccountState): UnreadMentionsState =>
state.unread.mentions;

//
//
Expand Down Expand Up @@ -116,7 +119,7 @@ function deleteMessages(
function streamsReducer(
state: UnreadStreamsState = initialStreamsState,
action: Action,
globalState: GlobalState,
globalState: PerAccountState,
): UnreadStreamsState {
switch (action.type) {
case LOGOUT:
Expand Down Expand Up @@ -208,7 +211,7 @@ function streamsReducer(
export const reducer = (
state: void | UnreadState,
action: Action,
globalState: GlobalState,
globalState: PerAccountState,
): UnreadState => {
const nextState = {
streams: streamsReducer(state?.streams, action, globalState),
Expand Down
10 changes: 5 additions & 5 deletions src/user-status/userStatusSelectors.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* @flow strict-local */
import type { GlobalState, Selector, UserId, UserStatus } from '../types';
import type { PerAccountState, Selector, UserId, UserStatus } from '../types';
import { getUserStatus } from '../directSelectors';
import { getOwnUserId } from '../users/userSelectors';

/**
* Extract the user status object for the logged in user.
* If no away status and status text have been set we do not have any data thus `undefined`.
*/
export const getSelfUserStatus: Selector<?UserStatus> = (state: GlobalState) => {
export const getSelfUserStatus: Selector<?UserStatus> = (state: PerAccountState) => {
const userStatus = getUserStatus(state);
return userStatus[getOwnUserId(state)];
};
Expand All @@ -17,7 +17,7 @@ export const getSelfUserStatus: Selector<?UserStatus> = (state: GlobalState) =>
* It is `true` if explicitly set to that value.
* If no value is set we consider it `false`.
*/
export const getSelfUserAwayStatus = (state: GlobalState): boolean => {
export const getSelfUserAwayStatus = (state: PerAccountState): boolean => {
const selfUserStatus = getSelfUserStatus(state);
return !!(selfUserStatus && selfUserStatus.away);
};
Expand All @@ -27,7 +27,7 @@ export const getSelfUserAwayStatus = (state: GlobalState): boolean => {
* If it is set we get as result that value.
* If no value is set we get a valid but empty string.
*/
export const getSelfUserStatusText = (state: GlobalState): string => {
export const getSelfUserStatusText = (state: PerAccountState): string => {
const selfUserStatus = getSelfUserStatus(state);
return (selfUserStatus && selfUserStatus.status_text) || '';
};
Expand All @@ -36,7 +36,7 @@ export const getSelfUserStatusText = (state: GlobalState): string => {
* Returns the `status text` value of the user with the given userId.
* We return `undefined` if no value is set.
*/
export const getUserStatusTextForUser = (state: GlobalState, userId: UserId): string | void => {
export const getUserStatusTextForUser = (state: PerAccountState, userId: UserId): string | void => {
const userStatus = getUserStatus(state);
return userStatus[userId] && userStatus[userId].status_text;
};
14 changes: 7 additions & 7 deletions src/users/userSelectors.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* @flow strict-local */
import { createSelector } from 'reselect';

import type { GlobalState, UserOrBot, Selector, User, UserId } from '../types';
import type { GlobalState, PerAccountState, UserOrBot, Selector, User, UserId } from '../types';
import { getUsers, getCrossRealmBots, getNonActiveUsers } from '../directSelectors';
import { getHasAuth, tryGetActiveAccount } from '../account/accountsSelectors';

Expand Down Expand Up @@ -82,7 +82,7 @@ export const getSortedUsers: Selector<User[]> = createSelector(getUsers, users =
*/
// Not currently used, but should replace uses of `getOwnEmail` (e.g. inside
// `getOwnUser`). See #3764.
export const getOwnUserId = (state: GlobalState): UserId => {
export const getOwnUserId = (state: PerAccountState): UserId => {
const { user_id } = state.realm;
if (user_id === undefined) {
throw new Error('No server data found');
Expand All @@ -97,7 +97,7 @@ export const getOwnUserId = (state: GlobalState): UserId => {
*
* Prefer using `getOwnUserId` or `getOwnUser`; see #3764.
*/
export const getOwnEmail = (state: GlobalState): string => {
export const getOwnEmail = (state: PerAccountState): string => {
const { email } = state.realm;
if (email === undefined) {
throw new Error('No server data found');
Expand All @@ -116,7 +116,7 @@ export const getOwnEmail = (state: GlobalState): string => {
*
* See also `getOwnUserId` and `getOwnEmail`.
*/
export const getOwnUser = (state: GlobalState): User => {
export const getOwnUser = (state: PerAccountState): User => {
const ownUser = getUsersById(state).get(getOwnUserId(state));
if (ownUser === undefined) {
throw new Error('Have ownUserId, but not found in user data');
Expand All @@ -134,7 +134,7 @@ export const getOwnUser = (state: GlobalState): User => {
* throwing if none. That makes it a bit simpler to use in contexts where
* we assume the relevant user must exist, which is true of most of the app.
*/
export const tryGetUserForId = (state: GlobalState, userId: UserId): UserOrBot | null =>
export const tryGetUserForId = (state: PerAccountState, userId: UserId): UserOrBot | null =>
getAllUsersById(state).get(userId) ?? null;

/**
Expand All @@ -147,7 +147,7 @@ export const tryGetUserForId = (state: GlobalState, userId: UserId): UserOrBot |
*
* See `tryGetUserForId` for a non-throwing version.
*/
export const getUserForId = (state: GlobalState, userId: UserId): UserOrBot => {
export const getUserForId = (state: PerAccountState, userId: UserId): UserOrBot => {
const user = tryGetUserForId(state, userId);
if (!user) {
throw new Error(`getUserForId: missing user: id ${userId}`);
Expand Down Expand Up @@ -185,7 +185,7 @@ const getActiveUsersById: Selector<Map<UserId, UserOrBot>> = createSelector(
*/
// To understand this implementation, see the comment about `is_active` in
// the `User` type definition.
export const getUserIsActive = (state: GlobalState, userId: UserId): boolean =>
export const getUserIsActive = (state: PerAccountState, userId: UserId): boolean =>
!!getActiveUsersById(state).get(userId);

/**
Expand Down

0 comments on commit 59d8813

Please sign in to comment.