redux: Provide the global state to each sub-reducer

src/__tests__/lib/exampleData.js

@@ -453,15 +453,75 @@ export const makeOutboxMessage = (data: $Shape<$Diff<Outbox, {| id: mixed |}>>):
 
 const privateReduxStore = createStore(rootReducer);
 
-/** The global Redux state, at its initial value. */
+/**
+ * The global Redux state, at its initial value.
+ *
+ * See `plusReduxState` for a version of the state that incorporates
+ * `selfUser` and other standard example data.
+ */
 export const baseReduxState: GlobalState = deepFreeze(privateReduxStore.getState());
 
+/**
+ * A global Redux state, with `baseReduxState` plus the given data.
+ *
+ * See `reduxStatePlus` for a version that automatically includes `selfUser`
+ * and other standard example data.
+ */
 export const reduxState = (extra?: $Rest<GlobalState, {}>): GlobalState =>
   deepFreeze({
     ...baseReduxState,
     ...extra,
   });
 
+/**
+ * The global Redux state, reflecting standard example data like `selfUser`.
+ *
+ * This approximates what the state might look like at a time when the app
+ * is showing its main UI: so when the user has logged into some account and
+ * we have our usual server data for it.
+ *
+ * In particular:
+ *  * The self-user is `selfUser`.
+ *  * Users `otherUser` and `thirdUser` also exist.
+ *
+ * More generally, each object in the Zulip app model -- a user, a stream,
+ * etc. -- that this module exports as a constant value (rather than only as
+ * a function to make a value) will typically appear here.
+ *
+ * That set will grow over time, so a test should never rely on
+ * `plusReduxState` containing only the things it currently contains.  For
+ * example, it should not assume there are only three users, even if that
+ * happens to be true now.  If the test needs a user (or stream, etc.) that
+ * isn't in this state, it should create the user privately for itself, with
+ * a helper like `makeUser`.
+ *
+ * On the other hand, a test *can* rely on an item being here if it
+ * currently is here.  So for example a test which uses `plusReduxState` can
+ * assume it contains `otherUser`; it need not, and should not bother to,
+ * add `otherUser` to the state.
+ *
+ * See `baseReduxState` for a minimal version of the state.
+ */
+export const plusReduxState: GlobalState = reduxState({
+  // TODO add .accounts, reflecting selfAuth, zulipVersion, zulipFeatureLevel
+  realm: { ...baseReduxState.realm, user_id: selfUser.user_id, email: selfUser.email },
+  // TODO add crossRealmBot
+  users: [selfUser, otherUser, thirdUser],
+  // TODO add stream and subscription
+});
+
+/**
+ * A global Redux state, adding the given data to `plusReduxState`.
+ *
+ * This automatically includes standard example data like `selfUser` and
+ * `otherUser`, so that there's no need to add those explicitly.  See
+ * `plusReduxState` for details on what's included.
+ *
+ * See `reduxState` for a version starting from a minimal state.
+ */
+export const reduxStatePlus = (extra?: $Rest<GlobalState, {}>): GlobalState =>
+  deepFreeze({ ...plusReduxState, ...extra });
+
 export const realmState = (extra?: $Rest<RealmState, {}>): RealmState =>
   deepFreeze({
     ...baseReduxState.realm,

src/boot/__tests__/reducers-test.js

@@ -1,6 +1,7 @@
 /* @flow strict-local */
-import reducers, { ALL_KEYS } from '../reducers';
+import reducers from '../reducers';
 import { discardKeys, storeKeys, cacheKeys } from '../store';
+import * as eg from '../../__tests__/lib/exampleData';
 
 describe('reducers', () => {
   test('reducers return the default states on unknown action', () => {
@@ -10,7 +11,7 @@ describe('reducers', () => {
 
   test('every reducer is listed in config as "discard", "store" or "cache"', () => {
     const configKeys = [...discardKeys, ...storeKeys, ...cacheKeys];
-    expect(configKeys).toHaveLength(ALL_KEYS.length);
-    expect(configKeys).toSatisfyAll(key => ALL_KEYS.includes(key));
+    const reducerKeys = Object.keys(eg.baseReduxState);
+    expect(configKeys.sort()).toEqual(reducerKeys.sort());
   });
 });

src/boot/reducers.js

@@ -1,10 +1,7 @@
 /* @flow strict-local */
-import { combineReducers } from 'redux';
-import type { CombinedReducer } from 'redux';
 import { enableBatching } from 'redux-batched-actions';
 
 import config from '../config';
-import { logSlowReducers } from '../utils/redux';
 import { NULL_OBJECT } from '../nullObjects';
 import type { Action, GlobalState, MigrationsState } from '../types';
 
@@ -31,38 +28,90 @@ import { reducer as unread } from '../unread/unreadModel';
 import userGroups from '../user-groups/userGroupsReducer';
 import userStatus from '../user-status/userStatusReducer';
 import users from '../users/usersReducer';
+import timing from '../utils/timing';
 
-const reducers = {
-  migrations: (state: MigrationsState = NULL_OBJECT) => state,
-  accounts,
-  alertWords,
-  caughtUp,
-  drafts,
-  fetching,
-  flags,
-  messages,
-  narrows,
-  mute,
-  outbox,
-  pmConversations,
-  presence,
-  realm,
-  session,
-  settings,
-  streams,
-  subscriptions,
-  topics,
-  typing,
-  unread,
-  userGroups,
-  userStatus,
-  users,
-};
+const migrations = (state: MigrationsState = NULL_OBJECT): MigrationsState => state;
+
+const { enableReduxSlowReducerWarnings, slowReducersThreshold } = config;
+
+function maybeLogSlowReducer(action, key, startMs, endMs) {
+  if (endMs - startMs >= slowReducersThreshold) {
+    timing.add({ text: `${action.type} @ ${key}`, startMs, endMs });
+  }
+}
+
+function applyReducer<Key: $Keys<GlobalState>, State>(
+  key: Key,
+  reducer: (void | State, Action, GlobalState) => State,
+  state: void | State,
+  action: Action,
+  globalState: void | GlobalState,
+): State {
+  let startMs = undefined;
+  if (enableReduxSlowReducerWarnings) {
+    startMs = Date.now();
+  }
+
+  /* $FlowFixMe - We make a small lie about the type, pretending that
+     globalState is not void.
+
+     This is OK because it's only ever void at the initialization action,
+     and no reducer should do anything there other than return its initial
+     state, so in particular no reducer should even look at globalState.
 
-export const ALL_KEYS: string[] = Object.keys(reducers);
+     Then on the other hand it's helpful because we want each reducer that
+     ever does use the globalState parameter to require it -- so that Flow
+     can help us be sure to pass it at the reducer's many other call sites,
+     in tests.  That means it has to be `globalState: GlobalState`, not
+     `globalState : void | GlobalState`.
+   */
+  const castGlobalState: GlobalState = globalState;
 
-const combinedReducer: CombinedReducer<GlobalState, Action> = combineReducers(
-  config.enableReduxSlowReducerWarnings ? logSlowReducers(reducers) : reducers,
-);
+  const nextState = reducer(state, action, castGlobalState);
+
+  if (startMs !== undefined) {
+    const endMs = Date.now();
+    maybeLogSlowReducer(action, key, startMs, endMs);
+  }
+
+  return nextState;
+}
+
+// Based on Redux upstream's combineReducers.
+const combinedReducer = (state: void | GlobalState, action: Action): GlobalState => {
+  // prettier-ignore
+  const nextState = {
+    migrations: applyReducer('migrations', migrations, state?.migrations, action, state),
+    accounts: applyReducer('accounts', accounts, state?.accounts, action, state),
+    alertWords: applyReducer('alertWords', alertWords, state?.alertWords, action, state),
+    caughtUp: applyReducer('caughtUp', caughtUp, state?.caughtUp, action, state),
+    drafts: applyReducer('drafts', drafts, state?.drafts, action, state),
+    fetching: applyReducer('fetching', fetching, state?.fetching, action, state),
+    flags: applyReducer('flags', flags, state?.flags, action, state),
+    messages: applyReducer('messages', messages, state?.messages, action, state),
+    narrows: applyReducer('narrows', narrows, state?.narrows, action, state),
+    mute: applyReducer('mute', mute, state?.mute, action, state),
+    outbox: applyReducer('outbox', outbox, state?.outbox, action, state),
+    pmConversations: applyReducer('pmConversations', pmConversations, state?.pmConversations, action, state),
+    presence: applyReducer('presence', presence, state?.presence, action, state),
+    realm: applyReducer('realm', realm, state?.realm, action, state),
+    session: applyReducer('session', session, state?.session, action, state),
+    settings: applyReducer('settings', settings, state?.settings, action, state),
+    streams: applyReducer('streams', streams, state?.streams, action, state),
+    subscriptions: applyReducer('subscriptions', subscriptions, state?.subscriptions, action, state),
+    topics: applyReducer('topics', topics, state?.topics, action, state),
+    typing: applyReducer('typing', typing, state?.typing, action, state),
+    unread: applyReducer('unread', unread, state?.unread, action, state),
+    userGroups: applyReducer('userGroups', userGroups, state?.userGroups, action, state),
+    userStatus: applyReducer('userStatus', userStatus, state?.userStatus, action, state),
+    users: applyReducer('users', users, state?.users, action, state),
+  };
+
+  if (state && Object.keys(nextState).every(key => nextState[key] === state[key])) {
+    return state;
+  }
+
+  return nextState;
+};
 
 export default enableBatching(combinedReducer);

src/topics/__tests__/topicsSelectors-test.js

@@ -94,7 +94,7 @@ describe('getTopicsForStream', () => {
   test('Return list of topic object with isMuted, unreadCount, topic name and max id in it.', () => {
     const stream = { ...eg.makeStream({ name: 'stream 1' }), stream_id: 1 };
 
-    const state = eg.reduxState({
+    const state = eg.reduxStatePlus({
       streams: [stream],
       topics: {
         [stream.stream_id]: [
@@ -113,8 +113,8 @@ describe('getTopicsForStream', () => {
         eg.streamMessage({ stream_id: 1, subject: 'topic 4', id: 7 }),
         eg.streamMessage({ stream_id: 1, subject: 'topic 4', id: 8 }),
       ].reduce(
-        (st, message) => unreadReducer(st, mkMessageAction(message)),
-        eg.baseReduxState.unread,
+        (st, message) => unreadReducer(st, mkMessageAction(message), eg.plusReduxState),
+        eg.plusReduxState.unread,
       ),
     });
     const expected = [

src/unread/__tests__/unread-testlib.js

@@ -4,7 +4,11 @@ import type { Message } from '../../types';
 import { reducer } from '../unreadModel';
 import * as eg from '../../__tests__/lib/exampleData';
 
-export const initialState = reducer(undefined, ({ type: eg.randString() }: $FlowFixMe));
+export const initialState = reducer(
+  undefined,
+  ({ type: eg.randString() }: $FlowFixMe),
+  eg.baseReduxState,
+);
 
 export const mkMessageAction = (message: Message) => ({
   ...eg.eventNewMessageActionBase,
@@ -19,6 +23,16 @@ const [user0, user1, user2, user3, user4, user5] = [0, 1, 2, 3, 4, 5].map(user_i
 );
 
 export const selectorBaseState = (() => {
+  // We take user1 to be self.
+  // It might be convenient to convert this to the standard eg.selfUser,
+  // and use eg.reduxStatePlus.  Until then, this just minimizes how much
+  // we've had to do in order to adapt our pre-existing tests.
+  const globalState = eg.reduxState({
+    realm: { ...eg.baseReduxState.realm, user_id: user1.user_id },
+    users: [user0, user1, user2, user3, user4, user5],
+    streams: [stream0, stream2],
+  });
+
   let state = initialState;
   for (const message of [
     eg.streamMessage({ stream_id: 0, subject: 'a topic', id: 1, flags: ['mentioned'] }),
@@ -28,7 +42,6 @@ export const selectorBaseState = (() => {
     eg.streamMessage({ stream_id: 0, subject: 'another topic', id: 5 }),
     eg.streamMessage({ stream_id: 2, subject: 'some other topic', id: 6 }),
     eg.streamMessage({ stream_id: 2, subject: 'some other topic', id: 7 }),
-    // We take user1 to be self.
     eg.pmMessageFromTo(user0, [user1], { id: 11 }),
     eg.pmMessageFromTo(user0, [user1], { id: 12 }),
     eg.pmMessageFromTo(user2, [user1], { id: 13 }),
@@ -40,7 +53,7 @@ export const selectorBaseState = (() => {
     eg.pmMessageFromTo(user4, [user1, user5], { id: 24 }),
     eg.pmMessageFromTo(user4, [user1, user5], { id: 25 }),
   ]) {
-    state = reducer(state, mkMessageAction(message));
+    state = reducer(state, mkMessageAction(message), globalState);
   }
   return state;
 })();