Skip to content

Use Immutable for state.flags. #4252

@chrisbobbe

Description

@chrisbobbe

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.Maps. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions