-
-
Notifications
You must be signed in to change notification settings - Fork 673
Description
This would be another instance of #3949 and #3950.
Once something like #4201 lands, we'll have a roadmap for doing this. Done!
In addition to the performance benefit, this would be a big improvement to code quality. The FlagsState
type is the following:
export type FlagsState = {|
read: { [messageId: number]: boolean },
starred: { [messageId: number]: boolean },
collapsed: { [messageId: number]: boolean },
mentioned: { [messageId: number]: boolean },
wildcard_mentioned: { [messageId: number]: boolean },
summarize_in_home: { [messageId: number]: boolean },
summarize_in_stream: { [messageId: number]: boolean },
force_expand: { [messageId: number]: boolean },
force_collapse: { [messageId: number]: boolean },
has_alert_word: { [messageId: number]: boolean },
historical: { [messageId: number]: boolean },
is_me_message: { [messageId: number]: boolean },
|};
I'm not sure state.flags
itself should be made an Immutable.Map
because it has a certain set of named and non-optional properties (see FlagsState
above), but the values at those properties certainly look like good candidates for Immutable.Map
s. I'm not sure what we should land on for state.flags
, but I do think it should be something from Immutable
if we can.
In addition to the performance benefit, this will also be good for code quality and correctness. Currently, we have two instances of const newState = {};
in the flags reducer; that contradicts the FlagsState
type (its properties are not optional). Flow v0.113, which we get with RN v0.62 (#3782), will start to have an issue with this, indirectly. An obvious solution would be to initialize newState
to initialState
instead of {}
; the properties we care about updating will clobber the ones on initialState
, and it's much clearer that we won't end up with an incomplete state stored. But the implementation is such that initialState
itself—defined once, at the top of flagsReducer
—would get mutated. Which is no good. So I think it would be great to try to rework the logic with immutable data structures.