Skip to content

Commit 2700b78

Browse files
committed
flags [nfc]: Fix some read-only vs. read-write object types in reducer
We've been breaching here the read-only markers on the properties of FlagsState and of its per-flag subtrees. The reason Flow hasn't complained seems to be related to the use of computed properties here; when we make these `flag` variables have a union type that enumerates the actual expected flags, rather than just `string`, it would start complaining. Get ahead of that by fixing the issue. As a tiny bonus, we make fewer lookups into these `newState` objects. (Though this code could use some deeper data structures / algorithms improvements than that. Some other time.)
1 parent 629a55b commit 2700b78

File tree

1 file changed

+11
-9
lines changed

1 file changed

+11
-9
lines changed

src/chat/flagsReducer.js

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* @flow strict-local */
22
import invariant from 'invariant';
33

4+
import type { ReadWrite } from '../generics';
45
import type { PerAccountApplicableAction, FlagsState, Message } from '../types';
56
import {
67
REGISTER_COMPLETE,
@@ -13,6 +14,9 @@ import {
1314
} from '../actionConstants';
1415
import { deeperMerge } from '../utils/misc';
1516

17+
type ReadWriteFlagsState = $Rest<ReadWrite<$ObjMap<FlagsState, <V>(V) => ReadWrite<V>>>, { ... }>;
18+
type ReadWritePerFlagState = $Values<ReadWriteFlagsState>;
19+
1620
const initialState = {
1721
read: {},
1822
starred: {},
@@ -40,14 +44,14 @@ const addFlagsForMessages = (
4044
/* $FlowFixMe[incompatible-exact] - We should ignore flags from the server
4145
that we don't already know about. After all, we can't have any code
4246
intending to do anything with them. */
43-
const newState: FlagsState = {};
47+
const newState: ReadWriteFlagsState = {};
4448

4549
flags.forEach(flag => {
46-
newState[flag] = { ...(state[flag] || {}) };
47-
50+
const perFlag: ReadWritePerFlagState = { ...(state[flag] || {}) };
4851
messages.forEach(message => {
49-
newState[flag][message] = true;
52+
perFlag[message] = true;
5053
});
54+
newState[flag] = perFlag;
5155
});
5256

5357
return {
@@ -84,14 +88,12 @@ const processFlagsForMessages = (
8488
/* $FlowFixMe[incompatible-exact] - We should ignore flags from the server
8589
that we don't already know about. After all, we can't have any code
8690
intending to do anything with them. */
87-
const newState: FlagsState = {};
91+
const newState: ReadWriteFlagsState = {};
8892
messages.forEach(msg => {
8993
(msg.flags || []).forEach(flag => {
9094
if (!state[flag] || !state[flag][msg.id]) {
91-
if (!newState[flag]) {
92-
newState[flag] = {};
93-
}
94-
newState[flag][msg.id] = true;
95+
const perFlag: ReadWritePerFlagState = newState[flag] || (newState[flag] = {});
96+
perFlag[msg.id] = true;
9597
stateChanged = true;
9698
}
9799
});

0 commit comments

Comments
 (0)