Merge pull request #188 from Expensify/marcaaron-whyDidOnyxUpdate

Add logs to show why Onyx data is changing. Prevent updates when simple values change.

README.md

@@ -319,6 +319,9 @@ Sample output of `Onyx.printMetrics()`
 |    1.17sec |   2.20sec |   1.03sec | currentURL, /            |
 ```
 
+# Debug mode
+
+It can be useful to log why Onyx is calling `setState()` on a particular React component so that we can understand which key changed, what changed about the value, and the connected component that ultimately rendered as a result. When used correctly this can help isolate problem areas and unnecessary renders in the code. To enable this feature, pass `debugSetState: true` to the config and grep JS console logs for `[Onyx-Debug]`.
 
 # Development
 

lib/Onyx.js

@@ -7,6 +7,7 @@ import * as Logger from './Logger';
 import cache from './OnyxCache';
 import createDeferredTask from './createDeferredTask';
 import fastMerge from './fastMerge';
+import * as PerformanceUtils from './metrics/PerformanceUtils';
 
 // Keeps track of the last connectionID that was used so we can keep incrementing it
 let lastConnectionID = 0;
@@ -323,12 +324,13 @@ function keysChanged(collectionKey, partialCollection) {
             if (isSubscribedToCollectionKey) {
                 subscriber.withOnyxInstance.setState((prevState) => {
                     const finalCollection = _.clone(prevState[subscriber.statePropertyName] || {});
-
                     const dataKeys = _.keys(partialCollection);
                     for (let j = 0; j < dataKeys.length; j++) {
                         const dataKey = dataKeys[j];
                         finalCollection[dataKey] = cachedCollection[dataKey];
                     }
+
+                    PerformanceUtils.logSetStateCall(subscriber, prevState[subscriber.statePropertyName], finalCollection, 'keysChanged', collectionKey);
                     return {
                         [subscriber.statePropertyName]: finalCollection,
                     };
@@ -345,8 +347,17 @@ function keysChanged(collectionKey, partialCollection) {
                     continue;
                 }
 
-                subscriber.withOnyxInstance.setState({
-                    [subscriber.statePropertyName]: cachedCollection[subscriber.key],
+                subscriber.withOnyxInstance.setState((prevState) => {
+                    const data = cachedCollection[subscriber.key];
+                    const previousData = prevState[subscriber.statePropertyName];
+                    if (data === previousData) {
+                        return null;
+                    }
+
+                    PerformanceUtils.logSetStateCall(subscriber, previousData, data, 'keysChanged', collectionKey);
+                    return {
+                        [subscriber.statePropertyName]: data,
+                    };
                 });
             }
         }
@@ -397,19 +408,29 @@ function keyChanged(key, data) {
             if (isCollectionKey(subscriber.key)) {
                 subscriber.withOnyxInstance.setState((prevState) => {
                     const collection = prevState[subscriber.statePropertyName] || {};
+                    const newCollection = {
+                        ...collection,
+                        [key]: data,
+                    };
+                    PerformanceUtils.logSetStateCall(subscriber, collection, newCollection, 'keyChanged', key);
                     return {
-                        [subscriber.statePropertyName]: {
-                            ...collection,
-                            [key]: data,
-                        },
+                        [subscriber.statePropertyName]: newCollection,
                     };
                 });
                 continue;
             }
 
             // If we did not match on a collection key then we just set the new data to the state property
-            subscriber.withOnyxInstance.setState({
-                [subscriber.statePropertyName]: data,
+            subscriber.withOnyxInstance.setState((prevState) => {
+                const previousData = prevState[subscriber.statePropertyName];
+                if (previousData === data) {
+                    return null;
+                }
+
+                PerformanceUtils.logSetStateCall(subscriber, previousData, data, 'keyChanged', key);
+                return {
+                    [subscriber.statePropertyName]: data,
+                };
             });
             continue;
         }
@@ -439,6 +460,7 @@ function sendDataToConnection(config, val, matchedKey) {
     }
 
     if (config.withOnyxInstance) {
+        PerformanceUtils.logSetStateCall(config, null, val, 'sendDataToConnection');
         config.withOnyxInstance.setWithOnyxState(config.statePropertyName, val);
     } else if (_.isFunction(config.callback)) {
         config.callback(val, matchedKey);
@@ -988,6 +1010,7 @@ function update(data) {
  * of Onyx running in different tabs/windows. Defaults to true for platforms that support local storage (web/desktop)
  * @param {String[]} [option.keysToDisableSyncEvents=[]] Contains keys for which
  * we want to disable sync event across tabs.
+ * @param {Boolean} [options.debugSetState] Enables debugging setState() calls to connected components.
  * @example
  * Onyx.init({
  *     keys: ONYXKEYS,
@@ -1004,13 +1027,18 @@ function init({
     captureMetrics = false,
     shouldSyncMultipleInstances = Boolean(global.localStorage),
     keysToDisableSyncEvents = [],
+    debugSetState = false,
 } = {}) {
     if (captureMetrics) {
         // The code here is only bundled and applied when the captureMetrics is set
         // eslint-disable-next-line no-use-before-define
         applyDecorators();
     }
 
+    if (debugSetState) {
+        PerformanceUtils.setShouldDebugSetState(true);
+    }
+
     if (maxCachedKeysCount > 0) {
         cache.setRecentKeysLimit(maxCachedKeysCount);
     }

lib/metrics/PerformanceUtils.js

@@ -0,0 +1,68 @@
+import lodashTransform from 'lodash/transform';
+import _ from 'underscore';
+
+let debugSetState = false;
+
+/**
+ * @param {Boolean} debug
+ */
+function setShouldDebugSetState(debug) {
+    debugSetState = debug;
+}
+
+/**
+ * Deep diff between two objects. Useful for figuring out what changed about an object from one render to the next so
+ * that state and props updates can be optimized.
+ *
+ * @param  {Object} object
+ * @param  {Object} base
+ * @return {Object}
+ */
+function diffObject(object, base) {
+    function changes(obj, comparisonObject) {
+        return lodashTransform(obj, (result, value, key) => {
+            if (_.isEqual(value, comparisonObject[key])) {
+                return;
+            }
+
+            // eslint-disable-next-line no-param-reassign
+            result[key] = (_.isObject(value) && _.isObject(comparisonObject[key]))
+                ? changes(value, comparisonObject[key])
+                : value;
+        });
+    }
+    return changes(object, base);
+}
+
+/**
+ * Provide insights into why a setState() call occurred by diffing the before and after values.
+ *
+ * @param {Object} mapping
+ * @param {*} previousValue
+ * @param {*} newValue
+ * @param {String} caller
+ * @param {String} [keyThatChanged]
+ */
+function logSetStateCall(mapping, previousValue, newValue, caller, keyThatChanged) {
+    if (!debugSetState) {
+        return;
+    }
+
+    const logParams = {};
+    if (keyThatChanged) {
+        logParams.keyThatChanged = keyThatChanged;
+    }
+    if (_.isObject(newValue) && _.isObject(previousValue)) {
+        logParams.difference = diffObject(previousValue, newValue);
+    } else {
+        logParams.previousValue = previousValue;
+        logParams.newValue = newValue;
+    }
+
+    console.debug(`[Onyx-Debug] ${mapping.displayName} setState() called. Subscribed to key '${mapping.key}' (${caller})`, logParams);
+}
+
+export {
+    logSetStateCall,
+    setShouldDebugSetState,
+};

lib/withOnyx.js

@@ -25,8 +25,8 @@ export default function (mapOnyxToState) {
         .omit(config => config.initWithStoredValues === false)
         .keys()
         .value();
-
     return (WrappedComponent) => {
+        const displayName = getDisplayName(WrappedComponent);
         class withOnyx extends React.Component {
             constructor(props) {
                 super(props);
@@ -152,6 +152,7 @@ export default function (mapOnyxToState) {
                     key,
                     statePropertyName,
                     withOnyxInstance: this,
+                    displayName,
                 });
             }
 
@@ -190,7 +191,7 @@ export default function (mapOnyxToState) {
         withOnyx.defaultProps = {
             forwardedRef: undefined,
         };
-        withOnyx.displayName = `withOnyx(${getDisplayName(WrappedComponent)})`;
+        withOnyx.displayName = `withOnyx(${displayName})`;
         return React.forwardRef((props, ref) => {
             const Component = withOnyx;
             // eslint-disable-next-line react/jsx-props-no-spreading

tests/unit/withOnyxTest.js

@@ -12,6 +12,7 @@ const ONYX_KEYS = {
         TEST_KEY: 'test_',
         RELATED_KEY: 'related_',
     },
+    SIMPLE_KEY: 'simple',
 };
 
 Onyx.init({
@@ -328,4 +329,43 @@ describe('withOnyx', () => {
                 expect(onRender.mock.calls[1][0].testObject).toStrictEqual({ID: 1, number: 2});
             });
     });
+
+    it('merge with a simple value should not update a connected component unless the data has changed', () => {
+        const onRender = jest.fn();
+
+        // Given there is a simple key that is not an array or object value
+        Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'string');
+
+        return waitForPromisesToResolve()
+            .then(() => {
+                // When a component subscribes to the simple key
+                const TestComponentWithOnyx = withOnyx({
+                    simple: {
+                        key: ONYX_KEYS.SIMPLE_KEY,
+                    },
+                })(ViewWithCollections);
+                render(<TestComponentWithOnyx onRender={onRender} />);
+
+                return waitForPromisesToResolve();
+            })
+            .then(() => {
+                // And we set the value to the same value it was before
+                Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'string');
+                return waitForPromisesToResolve();
+            })
+            .then(() => {
+                // THEN the component subscribed to the modified item should only render once
+                expect(onRender).toHaveBeenCalledTimes(1);
+                expect(onRender.mock.calls[0][0].simple).toBe('string');
+
+                // If we change the value to something new
+                Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'long_string');
+                return waitForPromisesToResolve();
+            })
+            .then(() => {
+                // THEN the component subscribed to the modified item should only render once
+                expect(onRender).toHaveBeenCalledTimes(2);
+                expect(onRender.mock.calls[1][0].simple).toBe('long_string');
+            });
+    });
 });