Include a more stable way of detecting when withOnyx keys change

lib/withOnyx.js

@@ -10,6 +10,10 @@ import Onyx from './Onyx';
 import * as Str from './Str';
 import utils from './utils';
 
+// This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check
+// if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results.
+const mappingPropertiesToIgnoreChangesTo = ['initialValue'];
+
 /**
  * Returns the display name of a component
  *
@@ -94,27 +98,52 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) {
             }
 
             componentDidMount() {
+                const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState);
+
                 // Subscribe each of the state properties to the proper Onyx key
                 _.each(mapOnyxToState, (mapping, propertyName) => {
-                    this.connectMappingToOnyx(mapping, propertyName);
+                    if (_.includes(mappingPropertiesToIgnoreChangesTo, propertyName)) {
+                        return;
+                    }
+                    const key = Str.result(mapping.key, {...this.props, ...onyxDataFromState});
+                    this.connectMappingToOnyx(mapping, propertyName, key);
                 });
                 this.checkEvictableKeys();
             }
 
-            componentDidUpdate() {
-                // When the state is passed to the key functions with Str.result(), omit anything
-                // from state that was not part of the mapped keys.
+            componentDidUpdate(prevProps, prevState) {
+                // The whole purpose of this method is to check to see if a key that is subscribed to Onyx has changed, and then Onyx needs to be disconnected from the old
+                // key and connected to the new key.
+                // For example, a key could change if KeyB depends on data loading from Onyx for KeyA.
+                const isFirstTimeUpdatingAfterLoading = prevState.loading && !this.state.loading;
                 const onyxDataFromState = getOnyxDataFromState(this.state, mapOnyxToState);
+                const prevOnyxDataFromState = getOnyxDataFromState(prevState, mapOnyxToState);
+
+                // This ensures that only one property is reconnecting at a time, or else it can lead to race conditions and infinite rendering loops. No fun!
+                let isReconnectingToOnyx = false;
 
-                // If any of the mappings use data from the props, then when the props change, all the
-                // connections need to be reconnected with the new props
                 _.each(mapOnyxToState, (mapping, propName) => {
-                    const previousKey = mapping.previousKey;
+                    // Some properties can be ignored and also return early if the component is already reconnecting to Onyx
+                    if (_.includes(mappingPropertiesToIgnoreChangesTo, propName) || isReconnectingToOnyx) {
+                        return;
+                    }
+
+                    // The previous key comes from either:
+                    // 1) The initial key that was connected to (ie. set from `componentDidMount()`)
+                    // 2) The updated props which caused `componentDidUpdate()` to run
+                    // The first case cannot be used all the time because of race conditions where `componentDidUpdate()` can be triggered before connectingMappingToOnyx() is done
+                    // (eg. if a user switches chats really quickly). In this case, it's much more stable to always look at the changes to prevProp and prevState to derive the key.
+                    // The second case cannot be used all the time because the onyx data doesn't change the first time that `componentDidUpdate()` runs after loading. In this case,
+                    // the `mapping.previousKey` must be used for the comparison or else this logic never detects that onyx data could have changed during the loading process.
+                    const previousKey = isFirstTimeUpdatingAfterLoading
+                        ? mapping.previousKey
+                        : Str.result(mapping.key, {...prevProps, ...prevOnyxDataFromState});
                     const newKey = Str.result(mapping.key, {...this.props, ...onyxDataFromState});
                     if (previousKey !== newKey) {
+                        isReconnectingToOnyx = true;
                         Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey);
                         delete this.activeConnectionIDs[previousKey];
-                        this.connectMappingToOnyx(mapping, propName);
+                        this.connectMappingToOnyx(mapping, propName, newKey);
                     }
                 });
                 this.checkEvictableKeys();
@@ -254,16 +283,13 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) {
              * @param {string} statePropertyName the name of the state property that Onyx will add the data to
              * @param {boolean} [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the
              *  component
+             * @param {string} key to connect to Onyx with
              */
-            connectMappingToOnyx(mapping, statePropertyName) {
-                const key = Str.result(mapping.key, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)});
-
-                // Remember the previous key so that if it ever changes, the component will reconnect to Onyx
-                // in componentDidUpdate
-                if (statePropertyName !== 'initialValue' && mapOnyxToState[statePropertyName]) {
-                    // eslint-disable-next-line no-param-reassign
-                    mapOnyxToState[statePropertyName].previousKey = key;
-                }
+            connectMappingToOnyx(mapping, statePropertyName, key) {
+                // Remember what the previous key was so that key changes can be detected when data is being loaded from Onyx. This will allow
+                // dependent keys to finish loading their data.
+                // eslint-disable-next-line no-param-reassign
+                mapOnyxToState[statePropertyName].previousKey = key;
 
                 // eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs
                 this.activeConnectionIDs[key] = Onyx.connect({