diff --git a/README.md b/README.md index a5fa61da..a72f8150 100644 --- a/README.md +++ b/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 diff --git a/lib/Onyx.js b/lib/Onyx.js index 1d6778c7..65c4bf71 100644 --- a/lib/Onyx.js +++ b/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,6 +1027,7 @@ 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 @@ -1011,6 +1035,10 @@ function init({ applyDecorators(); } + if (debugSetState) { + PerformanceUtils.setShouldDebugSetState(true); + } + if (maxCachedKeysCount > 0) { cache.setRecentKeysLimit(maxCachedKeysCount); } diff --git a/lib/metrics/PerformanceUtils.js b/lib/metrics/PerformanceUtils.js new file mode 100644 index 00000000..ff2c801a --- /dev/null +++ b/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, +}; diff --git a/lib/withOnyx.js b/lib/withOnyx.js index aaefce79..f8154d74 100644 --- a/lib/withOnyx.js +++ b/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 diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 94efa1f0..ea5a5b53 100644 --- a/tests/unit/withOnyxTest.js +++ b/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(); + + 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'); + }); + }); });