Make Desktop pass React's strict mode

[rafeca]

Motivation

The benefits of making a React app run in strict mode are well described in the official docs, but here's a summary of the most important benefits for us:

• Stop using the deprecated lifecycle methods that throw a warning on latest React versions.
• Make Desktop ready for concurrent mode once React ships it. This will allow us to use Suspense
• It's a good opportunity to clean up some components logic where the state handling logic has become very complicated or fragile.
• Fix some potential subtle issues due to the inconsistency on behaviour of the deprecated lifecycle methods (I don't think this is a big factor, since it's possible that some of these issues have already been worked around or don't affect users at all).

Steps involved

There are 2 main steps required for the migration:

1. Update our components code to not throw errors in strict mode.
2. Update third party dependencies to versions that do not throw errors in strict mode.

The second point may involve more investigation/uncertain, since it's unclear the total number of dependencies to upgrade and the amount of work needed to upgrade them.

So far I have identified the following third party libraries that need to get updated:

• react-transition-group
• react-css-transition-replace

Migration tips and tricks

These are some trickt to help us get rid of the deprecated React methods:

componentWillReceiveProps

• When used to calculate some state with the main purpose of caching a calculated value, it's usually safe to create a memoized function with memoizeOne and call it on every render. You can notice this one when the same logic is applied on the constructor/componentWillMount and these are the only places where state is updated (example).
• In rare situations, the calculation of the value may depend on previous state/data (example). In these cases keeping the local state may be required, so getDerivedStateFromProps can be used.
  • Note that having local state that gets updated based on props in most of the cases is an anti-pattern, but refactoring current components to avoid this could be specially tricky.
• In some situations, componentWillReceiveProps will be used for side-effects (example). In these situations componentDidUpdate should be used since getDerivedStateFromProps has to be a pure function.

(see notes on React docs).

componentWillUpdate

• In most of the situations, the same rules that apply to componentWillReceiveProps can be applied to componentWillUpdate.

(see notes on React docs).

componentWillMount

• In a lot of cases, componentWillMount is used in conjunction with componentWillUpdate or componentWillReceiveProps, so it's better to handle these first since that will remove a bunch of componentWillMount calls.
• In some other situations, componentWillMount contains side-effects (example) or async code (example). In these cases the side-effects should be moved to a componentDidMount call.
• It's quite common in our codebase to use componentWillMount to generate unique ids for the HTML nodes and store them in state (example). This generation has side effects (since it allocates the ids) so it needs to be moved to componentDidMount. Note that this means that the render method will then be able to handle undefined ids (since componentDidUpdate runs after first render).

(see notes on React docs).

findDOMNode

• It's usually safe to switch these to React refs, by using React.createRef().

(see notes on React docs).