Store state.messages as an Immutable.Map.

src/__tests__/lib/exampleData.js

@@ -1,6 +1,7 @@
 /* @flow strict-local */
 import deepFreeze from 'deep-freeze';
 import { createStore } from 'redux';
+import Immutable from 'immutable';
 
 import type {
   CrossRealmBot,
@@ -27,7 +28,6 @@ import {
 import rootReducer from '../../boot/reducers';
 import { authOfAccount } from '../../account/accountMisc';
 import { HOME_NARROW } from '../../utils/narrow';
-import { objectFromEntries } from '../../jsBackport';
 
 /* ========================================================================
  * Utilities
@@ -391,7 +391,7 @@ export const streamMessage = (args?: {|
 
 /** Construct a MessagesState from a list of messages. */
 export const makeMessagesState = (messages: Message[]): MessagesState =>
-  objectFromEntries(messages.map(m => [m.id, m]));
+  Immutable.Map(messages.map(m => [m.id, m]));
 
 /* ========================================================================
  * Outbox messages

src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap

@@ -1,5 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Stringify emptyMap 1`] = `"{\\"data\\":{},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;
+
 exports[`Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.example.org/avatar/1\\",\\"__serializedType__\\":\\"FallbackAvatarURL\\"}"`;
 
 exports[`Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`;
@@ -8,6 +10,8 @@ exports[`Stringify list 1`] = `"{\\"data\\":[1,2,\\"a\\",null],\\"__serializedTy
 
 exports[`Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;
 
+exports[`Stringify mapNumKeys 1`] = `"{\\"data\\":{\\"1\\":1,\\"2\\":2,\\"3\\":3,\\"4\\":4},\\"__serializedType__\\":\\"ImmutableMapNumKeys\\"}"`;
+
 exports[`Stringify mapWithTypeKey 1`] = `"{\\"data\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;
 
 exports[`Stringify plainObjectWithTypeKey 1`] = `"{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}}"`;

src/boot/__tests__/replaceRevive-test.js

@@ -19,6 +19,8 @@ const data = {
       [SERIALIZED_TYPE_FIELD_NAME]: { c: [3] },
     },
   }),
+  mapNumKeys: Immutable.Map([[1, 1], [2, 2], [3, 3], [4, 4]]),
+  emptyMap: Immutable.Map([]),
   zulipVersion: new ZulipVersion('3.0.0'),
   url: new URL('https://chat.zulip.org'),
   gravatarURL: GravatarURL.validateAndConstructInstance({ email: eg.selfUser.email }),

src/boot/replaceRevive.js

@@ -75,8 +75,24 @@ function replacer(key, value) {
       };
     case (Immutable.List.prototype: $FlowFixMe):
       return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableList' };
-    case (Immutable.Map.prototype: $FlowFixMe):
-      return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' };
+    case (Immutable.Map.prototype: $FlowFixMe): {
+      const firstKey = origValue.keySeq().first();
+      return {
+        data: value,
+        // We assume that any `Immutable.Map` will have
+        //   - all string keys,
+        //   - all numeric keys, or
+        //   - no keys (be empty).
+        //
+        // We store string-keyed maps with `ImmutableMap`,
+        // number-keyed maps with `ImmutableMapNumKeys`, and empty
+        // maps with either one of those (chosen arbitrarily) because
+        // the reviver will give the same output for both of them
+        // (i.e., an empty `Immutable.Map`).
+        [SERIALIZED_TYPE_FIELD_NAME]:
+          firstKey && typeof firstKey === 'number' ? 'ImmutableMapNumKeys' : 'ImmutableMap',
+      };
+    }
     default: {
       // If the identity of the first item in the prototype chain
       // isn't good enough as a distinguishing mark, we can put some
@@ -138,6 +154,9 @@ function reviver(key, value) {
         return Immutable.List(data);
       case 'ImmutableMap':
         return Immutable.Map(data);
+      case 'ImmutableMapNumKeys': {
+        return Immutable.Map(Object.keys(data).map(k => [Number.parseInt(k, 10), data[k]]));
+      }
       case 'Object':
         return {
           ...data,

src/boot/store.js

@@ -248,6 +248,9 @@ const migrations: { [string]: (GlobalState) => GlobalState } = {
     ),
   }),
 
+  // Convert `messages` from object-as-map to `Immutable.Map`.
+  '23': dropCache,
+
   // TIP: When adding a migration, consider just using `dropCache`.
 };
 

src/chat/__tests__/narrowsReducer-test.js

@@ -518,13 +518,11 @@ describe('narrowsReducer', () => {
   });
 
   describe('EVENT_UPDATE_MESSAGE_FLAGS', () => {
-    const allMessages = {
-      // Flow doesn't like number literals as keys...but it also wants
-      // them to be numbers. See https://github.com/facebook/flow/issues/380.
-      [1]: eg.streamMessage({ id: 1 }) /* eslint-disable-line no-useless-computed-key */,
-      [2]: eg.streamMessage({ id: 2 }) /* eslint-disable-line no-useless-computed-key */,
-      [4]: eg.streamMessage({ id: 4 }) /* eslint-disable-line no-useless-computed-key */,
-    };
+    const allMessages = eg.makeMessagesState([
+      eg.streamMessage({ id: 1 }),
+      eg.streamMessage({ id: 2 }),
+      eg.streamMessage({ id: 4 }),
+    ]);
 
     test('Do nothing if the is:starred narrow has not been fetched', () => {
       const initialState = Immutable.Map({ [HOME_NARROW_STR]: [1, 2] });

src/chat/__tests__/narrowsSelectors-test.js

@@ -23,11 +23,7 @@ import * as eg from '../../__tests__/lib/exampleData';
 
 describe('getMessagesForNarrow', () => {
   const message = eg.streamMessage({ id: 123 });
-  const messages = {
-    // Flow doesn't like number literals as keys...but it also wants
-    // them to be numbers.
-    [123]: message /* eslint-disable-line no-useless-computed-key */,
-  };
+  const messages = eg.makeMessagesState([message]);
   const outboxMessage = eg.makeOutboxMessage({});
 
   test('if no outbox messages returns messages with no change', () => {
@@ -43,7 +39,7 @@ describe('getMessagesForNarrow', () => {
 
     const result = getMessagesForNarrow(state, HOME_NARROW);
 
-    expect(result).toEqual([state.messages[123]]);
+    expect(result).toEqual([state.messages.get(123)]);
   });
 
   test('combine messages and outbox in same narrow', () => {
@@ -78,7 +74,7 @@ describe('getMessagesForNarrow', () => {
 
     const result = getMessagesForNarrow(state, HOME_NARROW);
 
-    expect(result).toEqual([state.messages[123]]);
+    expect(result).toEqual([state.messages.get(123)]);
   });
 
   test('do not combine messages and outbox in different narrow', () => {
@@ -137,7 +133,7 @@ describe('getLastMessageId', () => {
       narrows: Immutable.Map({
         [HOME_NARROW_STR]: [],
       }),
-      messages: {},
+      messages: eg.makeMessagesState([]),
       outbox: [],
     });
 

src/chat/narrowsSelectors.js

@@ -30,6 +30,7 @@ import {
 } from '../utils/narrow';
 import { shouldBeMuted } from '../utils/message';
 import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects';
+import * as logging from '../utils/logging';
 
 export const outboxMessagesForNarrow: Selector<Outbox[], Narrow> = createSelector(
   (state, narrow) => narrow,
@@ -61,7 +62,16 @@ export const getFetchedMessageIdsForNarrow = (state: GlobalState, narrow: Narrow
 const getFetchedMessagesForNarrow: Selector<Message[], Narrow> = createSelector(
   getFetchedMessageIdsForNarrow,
   state => getMessages(state),
-  (messageIds, messages) => messageIds.map(id => messages[id]),
+  (messageIds, messages) =>
+    messageIds.map(id => {
+      const message = messages.get(id);
+      if (!message) {
+        const msg = 'getFetchedMessagesForNarrow: message with id is missing in getMessages(state)';
+        logging.error(msg, { id });
+        throw new Error(msg);
+      }
+      return message;
+    }),
 );
 
 // Prettier mishandles this Flow syntax.