From 56e6f845da1b243020362f8c5c82b72e8cee554d Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 7 Sep 2023 09:20:15 +0200 Subject: [PATCH 01/14] Add initialValue, delayRendering and setStateProxy Linting on utils --- lib/Onyx.js | 65 +++++++++++++++++++------ lib/utils.js | 12 +++++ lib/withOnyx.js | 123 +++++++++++++++++++++++++++++++++--------------- 3 files changed, 146 insertions(+), 54 deletions(-) create mode 100644 lib/utils.js diff --git a/lib/Onyx.js b/lib/Onyx.js index 600ea1e73..dfaf267dc 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -9,6 +9,7 @@ import fastMerge from './fastMerge'; import * as PerformanceUtils from './metrics/PerformanceUtils'; import Storage from './storage'; import DevTools from './DevTools'; +import utils from './utils'; // Method constants const METHOD = { @@ -408,9 +409,13 @@ function keysChanged(collectionKey, partialCollection) { // If the subscriber has a selector, then the component's state must only be updated with the data // returned by the selector. if (subscriber.selector) { - subscriber.withOnyxInstance.setState((prevState) => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const previousData = prevState[subscriber.statePropertyName]; - const newData = reduceCollectionWithSelector(cachedCollection, subscriber.selector, subscriber.withOnyxInstance.state); + const newData = reduceCollectionWithSelector( + cachedCollection, + subscriber.selector, + subscriber.withOnyxInstance.state, + ); if (!deepEqual(previousData, newData)) { return { @@ -422,8 +427,10 @@ function keysChanged(collectionKey, partialCollection) { continue; } - subscriber.withOnyxInstance.setState((prevState) => { - const finalCollection = _.clone(prevState[subscriber.statePropertyName] || {}); + subscriber.withOnyxInstance.setStateProxy((prevState) => { + const finalCollection = _.clone( + prevState[subscriber.statePropertyName] || {}, + ); const dataKeys = _.keys(partialCollection); for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; @@ -451,11 +458,21 @@ function keysChanged(collectionKey, partialCollection) { // returned by the selector and the state should only change when the subset of data changes from what // it was previously. if (subscriber.selector) { - subscriber.withOnyxInstance.setState((prevState) => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; - const newData = getSubsetOfData(cachedCollection[subscriber.key], subscriber.selector, subscriber.withOnyxInstance.state); + const newData = getSubsetOfData( + cachedCollection[subscriber.key], + subscriber.selector, + subscriber.withOnyxInstance.state, + ); if (!deepEqual(prevData, newData)) { - PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); + PerformanceUtils.logSetStateCall( + subscriber, + prevData, + newData, + 'keysChanged', + collectionKey, + ); return { [subscriber.statePropertyName]: newData, }; @@ -466,9 +483,12 @@ function keysChanged(collectionKey, partialCollection) { continue; } - subscriber.withOnyxInstance.setState((prevState) => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const data = cachedCollection[subscriber.key]; const previousData = prevState[subscriber.statePropertyName]; + if (utils.areObjectsEmpty(data, previousData)) { + return null; + } if (data === previousData) { return null; } @@ -532,10 +552,14 @@ function keyChanged(key, data, canUpdateSubscriber) { // If the subscriber has a selector, then the consumer of this data must only be given the data // returned by the selector and only when the selected data has changed. if (subscriber.selector) { - subscriber.withOnyxInstance.setState((prevState) => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; const newData = { - [key]: getSubsetOfData(data, subscriber.selector, subscriber.withOnyxInstance.state), + [key]: getSubsetOfData( + data, + subscriber.selector, + subscriber.withOnyxInstance.state, + ), }; const prevDataWithNewData = { ...prevData, @@ -552,7 +576,7 @@ function keyChanged(key, data, canUpdateSubscriber) { continue; } - subscriber.withOnyxInstance.setState((prevState) => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const collection = prevState[subscriber.statePropertyName] || {}; const newCollection = { ...collection, @@ -569,9 +593,17 @@ function keyChanged(key, data, canUpdateSubscriber) { // If the subscriber has a selector, then the component's state must only be updated with the data // returned by the selector and only if the selected data has changed. if (subscriber.selector) { - subscriber.withOnyxInstance.setState((prevState) => { - const previousValue = getSubsetOfData(prevState[subscriber.statePropertyName], subscriber.selector, subscriber.withOnyxInstance.state); - const newValue = getSubsetOfData(data, subscriber.selector, subscriber.withOnyxInstance.state); + subscriber.withOnyxInstance.setStateProxy((prevState) => { + const previousValue = getSubsetOfData( + prevState[subscriber.statePropertyName], + subscriber.selector, + subscriber.withOnyxInstance.state, + ); + const newValue = getSubsetOfData( + data, + subscriber.selector, + subscriber.withOnyxInstance.state, + ); if (!deepEqual(previousValue, newValue)) { return { [subscriber.statePropertyName]: newValue, @@ -583,8 +615,11 @@ function keyChanged(key, data, canUpdateSubscriber) { } // If we did not match on a collection key then we just set the new data to the state property - subscriber.withOnyxInstance.setState((prevState) => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const previousData = prevState[subscriber.statePropertyName]; + if (utils.areObjectsEmpty(data, previousData)) { + return null; + } if (previousData === data) { return null; } diff --git a/lib/utils.js b/lib/utils.js new file mode 100644 index 000000000..a8ba0380d --- /dev/null +++ b/lib/utils.js @@ -0,0 +1,12 @@ +import * as _ from 'underscore'; + +function areObjectsEmpty(a, b) { + return ( + typeof a === 'object' + && typeof b === 'object' + && _.values(a).length === 0 + && _.values(b).length === 0 + ); +} + +export default {areObjectsEmpty}; diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 2eaba42f7..f09fbfeee 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -8,6 +8,7 @@ import React from 'react'; import _ from 'underscore'; import Onyx from './Onyx'; import * as Str from './Str'; +import utils from './utils'; /** * Returns the display name of a component @@ -19,7 +20,7 @@ function getDisplayName(component) { return component.displayName || component.name || 'Component'; } -export default function (mapOnyxToState) { +export default function (mapOnyxToState, options = {}) { // A list of keys that must be present in tempState before we can render the WrappedComponent const requiredKeysForInit = _.chain(mapOnyxToState) .omit(config => config.initWithStoredValues === false) @@ -28,37 +29,51 @@ export default function (mapOnyxToState) { return (WrappedComponent) => { const displayName = getDisplayName(WrappedComponent); class withOnyx extends React.Component { + pendingSetStates = []; + constructor(props) { super(props); - + this.delayUpdates = !!options.delayUpdates; this.setWithOnyxState = this.setWithOnyxState.bind(this); + this.flushPendingSetStates = this.flushPendingSetStates.bind(this); // This stores all the Onyx connection IDs to be used when the component unmounts so everything can be // disconnected. It is a key value store with the format {[mapping.key]: connectionID}. this.activeConnectionIDs = {}; - const cachedState = _.reduce(mapOnyxToState, (resultObj, mapping, propertyName) => { - const key = Str.result(mapping.key, props); - const value = Onyx.tryGetCachedValue(key, mapping); - - /** - * If we have a pending merge for a key it could mean that data is being set via Onyx.merge() and someone expects a component to have this data immediately. - * - * @example - * - * Onyx.merge('report_123', value); - * Navigation.navigate(route); // Where "route" expects the "value" to be available immediately once rendered. - * - * In reality, Onyx.merge() will only update the subscriber after all merges have been batched and the previous value is retrieved via a get() (returns a promise). - * So, we won't use the cache optimization here as it will lead us to arbitrarily defer various actions in the application code. - */ - if (value !== undefined && !Onyx.hasPendingMergeForKey(key)) { - // eslint-disable-next-line no-param-reassign - resultObj[propertyName] = value; - } + const cachedState = _.reduce( + mapOnyxToState, + (resultObj, mapping, propertyName) => { + const key = Str.result(mapping.key, props); + let value = Onyx.tryGetCachedValue(key, mapping); + if (!value && mapping.initialValue) { + value = mapping.initialValue; + } - return resultObj; - }, {}); + /** + * If we have a pending merge for a key it could mean that data is being set via Onyx.merge() and someone expects a component to have this data immediately. + * + * @example + * + * Onyx.merge('report_123', value); + * Navigation.navigate(route); // Where "route" expects the "value" to be available immediately once rendered. + * + * In reality, Onyx.merge() will only update the subscriber after all merges have been batched and the previous value is retrieved via a get() (returns a promise). + * So, we won't use the cache optimization here as it will lead us to arbitrarily defer various actions in the application code. + */ + if ( + (value !== undefined + && !Onyx.hasPendingMergeForKey(key)) + || mapping.allowStaleData + ) { + // eslint-disable-next-line no-param-reassign + resultObj[propertyName] = value; + } + + return resultObj; + }, + {}, + ); // If we have all the data we need, then we can render the component immediately cachedState.loading = _.size(cachedState) < requiredKeysForInit.length; @@ -84,7 +99,10 @@ export default function (mapOnyxToState) { const previousKey = Str.result(mapping.key, prevProps); const newKey = Str.result(mapping.key, this.props); if (previousKey !== newKey) { - Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); + Onyx.disconnect( + this.activeConnectionIDs[previousKey], + previousKey, + ); delete this.activeConnectionIDs[previousKey]; this.connectMappingToOnyx(mapping, propertyName); } @@ -101,6 +119,14 @@ export default function (mapOnyxToState) { }); } + setStateProxy(modifier) { + if (this.delayUpdates) { + this.pendingSetStates.push(modifier); + } else { + this.setState(modifier); + } + } + /** * This method is used externally by sendDataToConnection to prevent unnecessary renders while a component * still in a loading state. The temporary initial state is saved to the component instance and setState() @@ -116,31 +142,35 @@ export default function (mapOnyxToState) { // already loaded the values from cache. Thus we have to check whether the value has changed // before we set the state to prevent unnecessary renders. const prevValue = this.state[statePropertyName]; - if (!this.state.loading && prevValue === val) { + if ( + !this.state.loading + && (prevValue === val || utils.areObjectsEmpty(prevValue, val)) + ) { return; } if (!this.state.loading) { - this.setState({[statePropertyName]: val}); + this.setStateProxy({[statePropertyName]: val}); return; } this.tempState[statePropertyName] = val; // All state keys should exist and at least have a value of null - if (_.some(requiredKeysForInit, key => _.isUndefined(this.tempState[key]))) { + if ( + _.some(requiredKeysForInit, key => _.isUndefined(this.tempState[key])) + ) { return; } - // Leave untouched previous state to avoid data loss during pre-load updates. - // This handles case when setState was called before the setWithOnyxState. - // For example, when an Onyx property was updated by keyChanged before the call of the setWithOnyxState. - this.setState((prevState) => { - const remainingTempState = _.omit(this.tempState, _.keys(prevState)); + const stateUpdate = {...this.tempState, loading: false}; - return ({...remainingTempState, loading: false}); - }); + // The state is set here manually, instead of using setState because setState is an async operation, meaning it might execute on the next tick, + // or at a later point in the microtask queue. That can lead to a race condition where + // setWithOnyxState is called before the state is actually set. This results in unreliable behavior when checking the loading state and has been mainly observed on fabric. + this.state = stateUpdate; + this.setState(stateUpdate); // Trigger a render delete this.tempState; } @@ -162,11 +192,16 @@ export default function (mapOnyxToState) { const key = Str.result(mapping.key, this.props); if (!Onyx.isSafeEvictionKey(key)) { - throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); + throw new Error( + `canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`, + ); } if (canEvict) { - Onyx.removeFromEvictionBlockList(key, mapping.connectionID); + Onyx.removeFromEvictionBlockList( + key, + mapping.connectionID, + ); } else { Onyx.addToEvictionBlockList(key, mapping.connectionID); } @@ -196,7 +231,19 @@ export default function (mapOnyxToState) { }); } + flushPendingSetStates() { + this.delayUpdates = false; + + this.pendingSetStates.forEach((modifier) => { + this.setState(modifier); + }); + this.pendingSetStates = []; + } + render() { + // Remove any null values so that React replaces them with default props + const propsToPass = _.omit(this.props, value => _.isNull(value)); + if (this.state.loading) { return null; } @@ -206,12 +253,10 @@ export default function (mapOnyxToState) { let stateToPass = _.omit(this.state, 'loading'); stateToPass = _.omit(stateToPass, value => _.isNull(value)); - // Remove any null values so that React replaces them with default props - const propsToPass = _.omit(this.props, value => _.isNull(value)); - // Spreading props and state is necessary in an HOC where the data cannot be predicted return ( Date: Fri, 8 Sep 2023 17:01:48 +0200 Subject: [PATCH 02/14] Move fastMerge to utils, add documenation for initialValue and delayUpdates --- API.md | 1 + README.md | 30 ++++++++++++- lib/Onyx.js | 5 +-- lib/OnyxCache.js | 4 +- lib/storage/__mocks__/index.js | 4 +- lib/storage/providers/IDBKeyVal.js | 4 +- lib/utils.js | 71 ++++++++++++++++++++++++++++-- lib/withOnyx.js | 4 +- 8 files changed, 108 insertions(+), 15 deletions(-) diff --git a/API.md b/API.md index 3ac3a0ebb..367a4671b 100644 --- a/API.md +++ b/API.md @@ -131,6 +131,7 @@ Subscribes a react component's state directly to a store key | [mapping.initWithStoredValues] | Boolean | If set to false, then no data will be prefilled into the component | | [mapping.waitForCollectionCallback] | Boolean | If set to true, it will return the entire collection to the callback as a single object | | [mapping.selector] | String \| function | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. If the selector is a string, the selector is passed to lodashGet on the sourceData. If the selector is a function, the sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | +| [mapping.initialValue] | Any | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be passed as the initial hydrated value for the mapping. It will allow the component to render eagerly while data is being fetched from the DB. Note that it will not cause the component to have the `loading` prop set to true. | **Example** ```js diff --git a/README.md b/README.md index 4ce8a2c28..bd0984646 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,35 @@ export default withOnyx({ })(App); ``` -It is preferable to use the HOC over `Onyx.connect()` in React code as `withOnyx()` will delay the rendering of the wrapped component until all keys have been accessed and made available. +It is preferable to use the HOC over `Onyx.connect()` in React code as `withOnyx()` will delay the rendering of the wrapped component until all keys/entities have been fetched and passed to the component. This however, can really delay your application if many entities are connected to the same component, you can pass an `initialValue` to each key to allow Onyx to eagerly render your component with this value. + +```javascript +export default withOnyx({ + session: { + key: ONYXKEYS.SESSION, + initialValue: {} + }, +})(App); +``` + +Additionally, if your component has many keys/entities you might also trigger frequent re-renders on the initial mounting. You can workaround this by passing an additional object with the `delayUpdates` property set to true. Onyx will then put all the updates in a queue until you decide when then should be applied, the component will receive a function `markReadyForHydration`. A good place to call this function is on the `onLayout` method, which gets triggered after your component has been rendered. + +```javascript +const App = ({session, markReadyForHydration}) => ( + markReadyForHydration()}> + {session.token ? Logged in : Logged out } + +); + +export default withOnyx({ + session: { + key: ONYXKEYS.SESSION, + initialValue: {} + }, +}, { + delayUpdates: true +})(App); +``` ## Collections diff --git a/lib/Onyx.js b/lib/Onyx.js index dfaf267dc..516289d12 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -5,7 +5,6 @@ import * as Logger from './Logger'; import cache from './OnyxCache'; import * as Str from './Str'; import createDeferredTask from './createDeferredTask'; -import fastMerge from './fastMerge'; import * as PerformanceUtils from './metrics/PerformanceUtils'; import Storage from './storage'; import DevTools from './DevTools'; @@ -1083,7 +1082,7 @@ function applyMerge(existingValue, changes) { // Object values are merged one after the other // lodash adds a small overhead so we don't use it here // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - return _.reduce(changes, (modifiedData, change) => fastMerge(modifiedData, change), + return _.reduce(changes, (modifiedData, change) => utils.fastMerge(modifiedData, change), existingValue || {}); } @@ -1175,7 +1174,7 @@ function initializeWithDefaultKeyStates(enableDevTools = false) { .then((pairs) => { const asObject = _.object(pairs); - const merged = fastMerge(asObject, defaultKeyStates); + const merged = utils.fastMerge(asObject, defaultKeyStates); cache.merge(merged); _.each(merged, (val, key) => keyChanged(key, val)); if (enableDevTools) { diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index 1500e47ea..e7f6f58fb 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,6 +1,6 @@ import _ from 'underscore'; import {deepEqual} from 'fast-equals'; -import fastMerge from './fastMerge'; +import utils from './utils'; const isDefined = _.negate(_.isUndefined); @@ -119,7 +119,7 @@ class OnyxCache { // lodash adds a small overhead so we don't use it here // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - this.storageMap = Object.assign({}, fastMerge(this.storageMap, data)); + this.storageMap = Object.assign({}, utils.fastMerge(this.storageMap, data)); const storageKeys = this.getAllKeys(); const mergedKeys = _.keys(data); diff --git a/lib/storage/__mocks__/index.js b/lib/storage/__mocks__/index.js index 881028ad8..9f2282192 100644 --- a/lib/storage/__mocks__/index.js +++ b/lib/storage/__mocks__/index.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import fastMerge from '../../fastMerge'; +import utils from '../../utils'; let storageMapInternal = {}; @@ -27,7 +27,7 @@ const idbKeyvalMock = { _.forEach(pairs, ([key, value]) => { const existingValue = storageMapInternal[key]; const newValue = _.isObject(existingValue) - ? fastMerge(existingValue, value) : value; + ? utils.fastMerge(existingValue, value) : value; set(key, newValue); }); diff --git a/lib/storage/providers/IDBKeyVal.js b/lib/storage/providers/IDBKeyVal.js index f01718975..0dd2090e4 100644 --- a/lib/storage/providers/IDBKeyVal.js +++ b/lib/storage/providers/IDBKeyVal.js @@ -11,7 +11,7 @@ import { promisifyRequest, } from 'idb-keyval'; import _ from 'underscore'; -import fastMerge from '../../fastMerge'; +import utils from '../../utils'; // We don't want to initialize the store while the JS bundle loads as idb-keyval will try to use global.indexedDB // which might not be available in certain environments that load the bundle (e.g. electron main process). @@ -55,7 +55,7 @@ const provider = { return getValues.then((values) => { const upsertMany = _.map(pairs, ([key, value], index) => { const prev = values[index]; - const newValue = _.isObject(prev) ? fastMerge(prev, value) : value; + const newValue = _.isObject(prev) ? utils.fastMerge(prev, value) : value; return promisifyRequest(store.put(newValue, key)); }); return Promise.all(upsertMany); diff --git a/lib/utils.js b/lib/utils.js index a8ba0380d..db06ea7b8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -4,9 +4,74 @@ function areObjectsEmpty(a, b) { return ( typeof a === 'object' && typeof b === 'object' - && _.values(a).length === 0 - && _.values(b).length === 0 + && _.isEmpty(a) === 0 + && _.isEmpty(b) === 0 ); } -export default {areObjectsEmpty}; +// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 + +/** + * @param {mixed} val + * @returns {boolean} +*/ +function isMergeableObject(val) { + const nonNullObject = val != null ? typeof val === 'object' : false; + return (nonNullObject + && Object.prototype.toString.call(val) !== '[object RegExp]' + && Object.prototype.toString.call(val) !== '[object Date]'); +} + +/** +* @param {Object} target +* @param {Object} source +* @returns {Object} +*/ +function mergeObject(target, source) { + const destination = {}; + if (isMergeableObject(target)) { + // lodash adds a small overhead so we don't use it here + // eslint-disable-next-line rulesdir/prefer-underscore-method + const targetKeys = Object.keys(target); + for (let i = 0; i < targetKeys.length; ++i) { + const key = targetKeys[i]; + destination[key] = target[key]; + } + } + + // lodash adds a small overhead so we don't use it here + // eslint-disable-next-line rulesdir/prefer-underscore-method + const sourceKeys = Object.keys(source); + for (let i = 0; i < sourceKeys.length; ++i) { + const key = sourceKeys[i]; + if (source[key] === undefined) { + // eslint-disable-next-line no-continue + continue; + } + if (!isMergeableObject(source[key]) || !target[key]) { + destination[key] = source[key]; + } else { + // eslint-disable-next-line no-use-before-define + destination[key] = fastMerge(target[key], source[key]); + } + } + + return destination; +} + +/** +* @param {Object|Array} target +* @param {Object|Array} source +* @returns {Object|Array} +*/ +function fastMerge(target, source) { + // lodash adds a small overhead so we don't use it here + // eslint-disable-next-line rulesdir/prefer-underscore-method + const array = Array.isArray(source); + if (array) { + return source; + } + return mergeObject(target, source); +} + +export default {areObjectsEmpty, fastMerge}; diff --git a/lib/withOnyx.js b/lib/withOnyx.js index f09fbfeee..d8f146fd4 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -242,7 +242,7 @@ export default function (mapOnyxToState, options = {}) { render() { // Remove any null values so that React replaces them with default props - const propsToPass = _.omit(this.props, value => _.isNull(value)); + const propsToPass = _.omit(this.props, _.isNull); if (this.state.loading) { return null; @@ -251,7 +251,7 @@ export default function (mapOnyxToState, options = {}) { // Remove any internal state properties used by withOnyx // that should not be passed to a wrapped component let stateToPass = _.omit(this.state, 'loading'); - stateToPass = _.omit(stateToPass, value => _.isNull(value)); + stateToPass = _.omit(stateToPass, _.isNull); // Spreading props and state is necessary in an HOC where the data cannot be predicted return ( From 72d4a17fa4a2207af4774edd574de722b66963df Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Fri, 8 Sep 2023 17:13:57 +0200 Subject: [PATCH 03/14] Linting, revert of multi line breakdowns --- lib/Onyx.js | 42 +++++++----------------------------------- lib/withOnyx.js | 23 +++++------------------ 2 files changed, 12 insertions(+), 53 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 516289d12..d00e89585 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -410,11 +410,7 @@ function keysChanged(collectionKey, partialCollection) { if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const previousData = prevState[subscriber.statePropertyName]; - const newData = reduceCollectionWithSelector( - cachedCollection, - subscriber.selector, - subscriber.withOnyxInstance.state, - ); + const newData = reduceCollectionWithSelector(cachedCollection, subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(previousData, newData)) { return { @@ -427,9 +423,7 @@ function keysChanged(collectionKey, partialCollection) { } subscriber.withOnyxInstance.setStateProxy((prevState) => { - const finalCollection = _.clone( - prevState[subscriber.statePropertyName] || {}, - ); + const finalCollection = _.clone(prevState[subscriber.statePropertyName] || {}); const dataKeys = _.keys(partialCollection); for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; @@ -459,19 +453,9 @@ function keysChanged(collectionKey, partialCollection) { if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; - const newData = getSubsetOfData( - cachedCollection[subscriber.key], - subscriber.selector, - subscriber.withOnyxInstance.state, - ); + const newData = getSubsetOfData(cachedCollection[subscriber.key], subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(prevData, newData)) { - PerformanceUtils.logSetStateCall( - subscriber, - prevData, - newData, - 'keysChanged', - collectionKey, - ); + PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); return { [subscriber.statePropertyName]: newData, }; @@ -554,11 +538,7 @@ function keyChanged(key, data, canUpdateSubscriber) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; const newData = { - [key]: getSubsetOfData( - data, - subscriber.selector, - subscriber.withOnyxInstance.state, - ), + [key]: getSubsetOfData(data, subscriber.selector, subscriber.withOnyxInstance.state), }; const prevDataWithNewData = { ...prevData, @@ -593,16 +573,8 @@ function keyChanged(key, data, canUpdateSubscriber) { // returned by the selector and only if the selected data has changed. if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { - const previousValue = getSubsetOfData( - prevState[subscriber.statePropertyName], - subscriber.selector, - subscriber.withOnyxInstance.state, - ); - const newValue = getSubsetOfData( - data, - subscriber.selector, - subscriber.withOnyxInstance.state, - ); + const previousValue = getSubsetOfData(prevState[subscriber.statePropertyName], subscriber.selector, subscriber.withOnyxInstance.state); + const newValue = getSubsetOfData(data, subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(previousValue, newValue)) { return { [subscriber.statePropertyName]: newValue, diff --git a/lib/withOnyx.js b/lib/withOnyx.js index d8f146fd4..62ed7a213 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -99,10 +99,7 @@ export default function (mapOnyxToState, options = {}) { const previousKey = Str.result(mapping.key, prevProps); const newKey = Str.result(mapping.key, this.props); if (previousKey !== newKey) { - Onyx.disconnect( - this.activeConnectionIDs[previousKey], - previousKey, - ); + Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); delete this.activeConnectionIDs[previousKey]; this.connectMappingToOnyx(mapping, propertyName); } @@ -142,10 +139,7 @@ export default function (mapOnyxToState, options = {}) { // already loaded the values from cache. Thus we have to check whether the value has changed // before we set the state to prevent unnecessary renders. const prevValue = this.state[statePropertyName]; - if ( - !this.state.loading - && (prevValue === val || utils.areObjectsEmpty(prevValue, val)) - ) { + if (!this.state.loading && (prevValue === val || utils.areObjectsEmpty(prevValue, val))) { return; } @@ -157,9 +151,7 @@ export default function (mapOnyxToState, options = {}) { this.tempState[statePropertyName] = val; // All state keys should exist and at least have a value of null - if ( - _.some(requiredKeysForInit, key => _.isUndefined(this.tempState[key])) - ) { + if (_.some(requiredKeysForInit, key => _.isUndefined(this.tempState[key]))) { return; } @@ -192,16 +184,11 @@ export default function (mapOnyxToState, options = {}) { const key = Str.result(mapping.key, this.props); if (!Onyx.isSafeEvictionKey(key)) { - throw new Error( - `canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`, - ); + throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); } if (canEvict) { - Onyx.removeFromEvictionBlockList( - key, - mapping.connectionID, - ); + Onyx.removeFromEvictionBlockList(key, mapping.connectionID); } else { Onyx.addToEvictionBlockList(key, mapping.connectionID); } From 4396283003093aa3ea0e2a316151c9f6d22514ca Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Fri, 8 Sep 2023 17:14:44 +0200 Subject: [PATCH 04/14] Fix utils comparisson --- lib/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index db06ea7b8..da3aab7d8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -4,8 +4,8 @@ function areObjectsEmpty(a, b) { return ( typeof a === 'object' && typeof b === 'object' - && _.isEmpty(a) === 0 - && _.isEmpty(b) === 0 + && _.isEmpty(a) + && _.isEmpty(b) ); } From 3452854a582f81609b69d7db2949ecadb10a6ed1 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 12 Sep 2023 11:32:35 +0200 Subject: [PATCH 05/14] PR comments --- README.md | 4 ++-- lib/Onyx.js | 4 ++++ lib/withOnyx.js | 10 +++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index bd0984646..783b84887 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,7 @@ export default withOnyx({ })(App); ``` -It is preferable to use the HOC over `Onyx.connect()` in React code as `withOnyx()` will delay the rendering of the wrapped component until all keys/entities have been fetched and passed to the component. This however, can really delay your application if many entities are connected to the same component, you can pass an `initialValue` to each key to allow Onyx to eagerly render your component with this value. +While `Onyx.connect()` gives you more control on how your component reacts as data is fetched from disk, `withOnyx()` will delay the rendering of the wrapped component until all keys/entities have been fetched and passed to the component, this can be convenient for simple cases. This however, can really delay your application if many entities are connected to the same component, you can pass an `initialValue` to each key to allow Onyx to eagerly render your component with this value. ```javascript export default withOnyx({ @@ -146,7 +146,7 @@ export default withOnyx({ })(App); ``` -Additionally, if your component has many keys/entities you might also trigger frequent re-renders on the initial mounting. You can workaround this by passing an additional object with the `delayUpdates` property set to true. Onyx will then put all the updates in a queue until you decide when then should be applied, the component will receive a function `markReadyForHydration`. A good place to call this function is on the `onLayout` method, which gets triggered after your component has been rendered. +Additionally, if your component has many keys/entities when your component will mount but will receive many updates as data is fetched from DB and passed down to it, as every key that gets fetched will trigger a `setState` on the `withOnyx` HOC. This might cause re-renders on the initial mounting, preventing the component from mounting/rendering in reasonable time, making your app feel slow and even delaying animations. You can workaround this by passing an additional object with the `shouldDelayUpdates` property set to true. Onyx will then put all the updates in a queue until you decide when then should be applied, the component will receive a function `markReadyForHydration`. A good place to call this function is on the `onLayout` method, which gets triggered after your component has been rendered. ```javascript const App = ({session, markReadyForHydration}) => ( diff --git a/lib/Onyx.js b/lib/Onyx.js index d00e89585..c17457151 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -469,6 +469,8 @@ function keysChanged(collectionKey, partialCollection) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const data = cachedCollection[subscriber.key]; const previousData = prevState[subscriber.statePropertyName]; + + // Avoids triggering unnecessary re-renders when feeding empty objects if (utils.areObjectsEmpty(data, previousData)) { return null; } @@ -588,6 +590,8 @@ function keyChanged(key, data, canUpdateSubscriber) { // If we did not match on a collection key then we just set the new data to the state property subscriber.withOnyxInstance.setStateProxy((prevState) => { const previousData = prevState[subscriber.statePropertyName]; + + // Avoids triggering unnecessary re-renders when feeding empty objects if (utils.areObjectsEmpty(data, previousData)) { return null; } diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 62ed7a213..7cb5a0310 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -33,7 +33,7 @@ export default function (mapOnyxToState, options = {}) { constructor(props) { super(props); - this.delayUpdates = !!options.delayUpdates; + this.shouldDelayUpdates = Boolean(options.shouldDelayUpdates); this.setWithOnyxState = this.setWithOnyxState.bind(this); this.flushPendingSetStates = this.flushPendingSetStates.bind(this); @@ -117,7 +117,7 @@ export default function (mapOnyxToState, options = {}) { } setStateProxy(modifier) { - if (this.delayUpdates) { + if (this.shouldDelayUpdates) { this.pendingSetStates.push(modifier); } else { this.setState(modifier); @@ -219,7 +219,11 @@ export default function (mapOnyxToState, options = {}) { } flushPendingSetStates() { - this.delayUpdates = false; + if (!this.shouldDelayUpdates) { + return; + } + + this.shouldDelayUpdates = false; this.pendingSetStates.forEach((modifier) => { this.setState(modifier); From 95cd779d86ea52bb16b68743b2907f596db8d97a Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 12 Sep 2023 14:24:03 +0200 Subject: [PATCH 06/14] Update snippet on README --- README.md | 131 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 75 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 783b84887..9f9aa499b 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,18 @@ # `react-native-onyx` + Awesome persistent storage solution wrapped in a Pub/Sub library. # Features -- Onyx stores and retrieves data from persistent storage -- Data is stored as key/value pairs, where the value can be anything from a single piece of data to a complex object -- Collections of data are usually not stored as a single key (e.g. an array with multiple objects), but as individual keys+ID (e.g. `report_1234`, `report_4567`, etc.). Store collections as individual keys when a component will bind directly to one of those keys. For example: reports are stored as individual keys because `SidebarLink.js` binds to the individual report keys for each link. However, report actions are stored as an array of objects because nothing binds directly to a single report action. -- Onyx allows other code to subscribe to changes in data, and then publishes change events whenever data is changed -- Anything needing to read Onyx data needs to: +- Onyx stores and retrieves data from persistent storage +- Data is stored as key/value pairs, where the value can be anything from a single piece of data to a complex object +- Collections of data are usually not stored as a single key (e.g. an array with multiple objects), but as individual keys+ID (e.g. `report_1234`, `report_4567`, etc.). Store collections as individual keys when a component will bind directly to one of those keys. For example: reports are stored as individual keys because `SidebarLink.js` binds to the individual report keys for each link. However, report actions are stored as an array of objects because nothing binds directly to a single report action. +- Onyx allows other code to subscribe to changes in data, and then publishes change events whenever data is changed +- Anything needing to read Onyx data needs to: 1. Know what key the data is stored in (for web, you can find this by looking in the JS console > Application > local storage) 2. Subscribe to changes of the data for a particular key or set of keys. React components use `withOnyx()` and non-React libs use `Onyx.connect()`. 3. Get initialized with the current value of that key from persistent storage (Onyx does this by calling `setState()` or triggering the `callback` with the values currently on disk as part of the connection process) -- Subscribing to Onyx keys is done using a constant defined in `ONYXKEYS`. Each Onyx key represents either a collection of items or a specific entry in storage. For example, since all reports are stored as individual keys like `report_1234`, if code needs to know about all the reports (e.g. display a list of them in the nav menu), then it would subscribe to the key `ONYXKEYS.COLLECTION.REPORT`. +- Subscribing to Onyx keys is done using a constant defined in `ONYXKEYS`. Each Onyx key represents either a collection of items or a specific entry in storage. For example, since all reports are stored as individual keys like `report_1234`, if code needs to know about all the reports (e.g. display a list of them in the nav menu), then it would subscribe to the key `ONYXKEYS.COLLECTION.REPORT`. # Getting Started @@ -28,10 +29,10 @@ npm install react-native-onyx --save To initialize Onyx we call `Onyx.init()` with a configuration object. ```javascript -import Onyx from 'react-native-onyx'; +import Onyx from "react-native-onyx"; const ONYXKEYS = { - SESSION: 'session', + SESSION: "session", }; const config = { @@ -42,6 +43,7 @@ Onyx.init(config); ``` ### Usage in non react-native projects + Onyx can be used in non react-native projects, by leveraging the `browser` field in `package.json` Bundlers like Webpack respect that field and import code from the specified path We import Onyx the same way shown above - `import Onyx from 'react-native-onyx'` @@ -51,10 +53,9 @@ We import Onyx the same way shown above - `import Onyx from 'react-native-onyx'` To store some data we can use the `Onyx.set()` method. ```javascript -API.Authenticate(params) - .then((response) => { - Onyx.set(ONYXKEYS.SESSION, {token: response.token}); - }); +API.Authenticate(params).then((response) => { + Onyx.set(ONYXKEYS.SESSION, { token: response.token }); +}); ``` The data will then be cached and stored via [`AsyncStorage`](https://github.com/react-native-async-storage/async-storage). @@ -66,31 +67,31 @@ We can also use `Onyx.merge()` to merge new `Object` or `Array` data in with exi For `Array` the default behavior is to replace it fully, effectively making it equivalent to set: ```javascript -Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Joe']); // -> ['Joe'] -Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Jack']); // -> ['Joe', 'Jack'] +Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ["Joe"]); // -> ['Joe'] +Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ["Jack"]); // -> ['Joe', 'Jack'] ``` For `Object` values the default behavior uses `lodash/merge` under the hood to do a deep extend of the object. ```javascript -Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1} -Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'} +Onyx.merge(ONYXKEYS.POLICY, { id: 1 }); // -> {id: 1} +Onyx.merge(ONYXKEYS.POLICY, { name: "My Workspace" }); // -> {id: 1, name: 'My Workspace'} ``` Arrays inside objects will be replaced fully, same as arrays not inside objects: ```javascript -Onyx.merge(ONYXKEYS.POLICY, {employeeList: ['Joe', 'Jack']}); // -> {employeeList: ['Joe', 'Jack']} -Onyx.merge(ONYXKEYS.POLICY, {employeeList: ['Jack']}); // -> {employeeList: ['Jack']} +Onyx.merge(ONYXKEYS.POLICY, { employeeList: ["Joe", "Jack"] }); // -> {employeeList: ['Joe', 'Jack']} +Onyx.merge(ONYXKEYS.POLICY, { employeeList: ["Jack"] }); // -> {employeeList: ['Jack']} ``` ### Should I use `merge()` or `set()` or both? -- Use `merge()` when creating a new object -- Use `merge()` to merge partial data into an existing object -- Use `merge()` when storing simple values (`String`, `Boolean`, `Number`) -- Use `set()` when you need to delete an Onyx key completely from storage -- Use `set()` when you need to completely reset an object or array of data +- Use `merge()` when creating a new object +- Use `merge()` to merge partial data into an existing object +- Use `merge()` when storing simple values (`String`, `Boolean`, `Number`) +- Use `set()` when you need to delete an Onyx key completely from storage +- Use `set()` when you need to completely reset an object or array of data Consecutive calls to `Onyx.merge()` with the same key are batched in a stack and processed in the order that they were called. This helps avoid race conditions where one merge possibly finishes before another. However, it's important to note that calls to `Onyx.set()` are not batched together with calls to `Onyx.merge()`. For this reason, it is usually preferable to use one or the other, but not both. Onyx is a work-in-progress so always test code to make sure assumptions are correct! @@ -106,7 +107,7 @@ To set up a basic subscription for a given key use the `Onyx.connect()` method. let session; const connectionID = Onyx.connect({ key: ONYXKEYS.SESSION, - callback: (val) => session = val || {}, + callback: (val) => (session = val || {}), }); ``` @@ -119,12 +120,12 @@ Onyx.disconnect(connectionID); We can also access values inside React components via the `withOnyx()` [higher order component](https://reactjs.org/docs/higher-order-components.html). When the data changes the component will re-render. ```javascript -import React from 'react'; -import {withOnyx} from 'react-native-onyx'; +import React from "react"; +import { withOnyx } from "react-native-onyx"; -const App = ({session}) => ( +const App = ({ session }) => ( - {session.token ? Logged in : Logged out } + {session.token ? Logged in : Logged out} ); @@ -141,7 +142,7 @@ While `Onyx.connect()` gives you more control on how your component reacts as da export default withOnyx({ session: { key: ONYXKEYS.SESSION, - initialValue: {} + initialValue: {}, }, })(App); ``` @@ -149,20 +150,23 @@ export default withOnyx({ Additionally, if your component has many keys/entities when your component will mount but will receive many updates as data is fetched from DB and passed down to it, as every key that gets fetched will trigger a `setState` on the `withOnyx` HOC. This might cause re-renders on the initial mounting, preventing the component from mounting/rendering in reasonable time, making your app feel slow and even delaying animations. You can workaround this by passing an additional object with the `shouldDelayUpdates` property set to true. Onyx will then put all the updates in a queue until you decide when then should be applied, the component will receive a function `markReadyForHydration`. A good place to call this function is on the `onLayout` method, which gets triggered after your component has been rendered. ```javascript -const App = ({session, markReadyForHydration}) => ( +const App = ({ session, markReadyForHydration }) => ( markReadyForHydration()}> - {session.token ? Logged in : Logged out } + {session.token ? Logged in : Logged out} ); -export default withOnyx({ - session: { - key: ONYXKEYS.SESSION, - initialValue: {} +export default withOnyx( + { + session: { + key: ONYXKEYS.SESSION, + initialValue: {}, + }, }, -}, { - delayUpdates: true -})(App); + { + shouldDelayUpdates: true, + } +)(App); ``` ## Collections @@ -172,7 +176,7 @@ Collections allow keys with similar value types to be subscribed together by sub ```javascript const ONYXKEYS = { COLLECTION: { - REPORT: 'report_', + REPORT: "report_", }, }; ``` @@ -201,7 +205,7 @@ There are several ways to subscribe to these keys: ```javascript withOnyx({ - allReports: {key: ONYXKEYS.COLLECTION.REPORT}, + allReports: { key: ONYXKEYS.COLLECTION.REPORT }, })(MyComponent); ``` @@ -227,15 +231,21 @@ This final option forces `Onyx.connect()` to behave more like `withOnyx()` and o Be cautious when using collections as things can get out of hand if you have a subscriber hooked up to a collection key that has large numbers of individual keys. If this is the case, it is critical to use `mergeCollection()` over `merge()`. -Remember, `mergeCollection()` will notify a subscriber only *once* with the total collected values whereas each call to `merge()` would re-render a connected component *each time it is called*. Consider this example where `reports` is an array of reports that we want to index and save. +Remember, `mergeCollection()` will notify a subscriber only _once_ with the total collected values whereas each call to `merge()` would re-render a connected component _each time it is called_. Consider this example where `reports` is an array of reports that we want to index and save. ```js // Bad -_.each(reports, report => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report)); // -> A component using withOnyx() will have it's state updated with each iteration +_.each(reports, (report) => + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report) +); // -> A component using withOnyx() will have it's state updated with each iteration // Good const values = {}; -_.each(reports, report => values[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = report); +_.each( + reports, + (report) => + (values[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = report) +); Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, values); // -> A component using withOnyx() will only have it's state updated once ``` @@ -250,6 +260,7 @@ function signOut() { ``` ## Storage Providers + `Onyx.get`, `Onyx.set`, and the rest of the API accesses the underlying storage differently depending on the platform @@ -257,6 +268,7 @@ Under the hood storage access calls are delegated to a [`StorageProvider`](lib/s Some platforms (like web and desktop) might use the same storage provider If a platform needs to use a separate library (like using MMVK for react-native) it should be added in the following way: + 1. Create a `StorageProvider.js` at [lib/storage/providers](lib/storage/providers) Reference an existing [StorageProvider](lib/storage/providers/AsyncStorage.js) for the interface that has to be implemented 2. Update the factory at [lib/storage/index.web.js](lib/storage/index.web.js) and [lib/storage/index.native.js](lib/storage/index.native.js) to return the newly created Provider for the desired Platform(s) @@ -268,17 +280,20 @@ If a platform needs to use a separate library (like using MMVK for react-native) # Storage Eviction Different platforms come with varying storage capacities and Onyx has a way to gracefully fail when those storage limits are encountered. When Onyx fails to set or modify a key the following steps are taken: + 1. Onyx looks at a list of recently accessed keys (access is defined as subscribed to or modified) and locates the key that was least recently accessed 2. It then deletes this key and retries the original operation By default, Onyx will not evict anything from storage and will presume all keys are "unsafe" to remove unless explicitly told otherwise. **To flag a key as safe for removal:** -- Add the key to the `safeEvictionKeys` option in `Onyx.init(options)` -- Implement `canEvict` in the Onyx config for each component subscribing to a key -- The key will only be deleted when all subscribers return `true` for `canEvict` + +- Add the key to the `safeEvictionKeys` option in `Onyx.init(options)` +- Implement `canEvict` in the Onyx config for each component subscribing to a key +- The key will only be deleted when all subscribers return `true` for `canEvict` e.g. + ```js Onyx.init({ safeEvictionKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS], @@ -288,8 +303,9 @@ Onyx.init({ ```js export default withOnyx({ reportActions: { - key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}_`, - canEvict: props => !props.isActiveReport, + key: ({ reportID }) => + `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}_`, + canEvict: (props) => !props.isActiveReport, }, })(ReportActionsView); ``` @@ -309,14 +325,15 @@ Onyx.init({ At any point you can get the collected statistics using `Onyx.getMetrics()`. This will return an object containing `totalTime`, `averageTime` and `summaries`. `summaries` is a collection of statistics for each method it contains data about: - - method name - - total, max, min, average times for this method calls - - calls - a list of individual calls with each having: start time; end time; call duration; call arguments - - start/end times are relative to application launch time - 0.00 being exactly at launch + +- method name +- total, max, min, average times for this method calls +- calls - a list of individual calls with each having: start time; end time; call duration; call arguments + - start/end times are relative to application launch time - 0.00 being exactly at launch If you wish to reset the metrics and start over use `Onyx.resetMetrics()` -Finally, there's a `Onyx.printMetrics()` method which prints human statistics information on the dev console. You can use this method during debugging. For example add an `Onyx.printMetrics()` line somewhere in code or call it through the dev console. It supports 3 popular formats *MD* - human friendly markdown, *CSV* and *JSON*. The default is MD if you want to print another format call `Onyx.printMetrics({ format: 'csv' })` or +Finally, there's a `Onyx.printMetrics()` method which prints human statistics information on the dev console. You can use this method during debugging. For example add an `Onyx.printMetrics()` line somewhere in code or call it through the dev console. It supports 3 popular formats _MD_ - human friendly markdown, _CSV_ and _JSON_. The default is MD if you want to print another format call `Onyx.printMetrics({ format: 'csv' })` or `Onyx.printMetrics({ format: 'json' })`. Sample output of `Onyx.printMetrics()` @@ -367,12 +384,14 @@ Then, you can enable this type of debugging by passing `enableDevTools: true` to link a local version of Onyx during development To quickly test small changes you can directly go to `node_modules/react-native-onyx` in the parent project and: -- tweak original source if you're testing over a react-native project -- tweak `dist/web.development.js` for non react-native-projects + +- tweak original source if you're testing over a react-native project +- tweak `dist/web.development.js` for non react-native-projects To continuously work on Onyx we have to set up a task that copies content to parent project's `node_modules/react-native-onyx`: + 1. Work on Onyx feature or a fix 2. Save files 3. Optional: run `npm run build` (if you're working or want to test on a non react-native project) - - `npm link` would actually work outside of `react-native` and it can be used to link Onyx locally for a web only project + - `npm link` would actually work outside of `react-native` and it can be used to link Onyx locally for a web only project 4. Copy Onyx to consumer project's `node_modules/react-native-onyx` From 30cc1f356ae65cd3e972d8a605a8cea7dfd480e3 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Tue, 12 Sep 2023 14:24:19 +0200 Subject: [PATCH 07/14] Revert linting on README --- README.md | 131 +++++++++++++++++++++++------------------------------- 1 file changed, 56 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index 9f9aa499b..7967767f0 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,17 @@ # `react-native-onyx` - Awesome persistent storage solution wrapped in a Pub/Sub library. # Features -- Onyx stores and retrieves data from persistent storage -- Data is stored as key/value pairs, where the value can be anything from a single piece of data to a complex object -- Collections of data are usually not stored as a single key (e.g. an array with multiple objects), but as individual keys+ID (e.g. `report_1234`, `report_4567`, etc.). Store collections as individual keys when a component will bind directly to one of those keys. For example: reports are stored as individual keys because `SidebarLink.js` binds to the individual report keys for each link. However, report actions are stored as an array of objects because nothing binds directly to a single report action. -- Onyx allows other code to subscribe to changes in data, and then publishes change events whenever data is changed -- Anything needing to read Onyx data needs to: +- Onyx stores and retrieves data from persistent storage +- Data is stored as key/value pairs, where the value can be anything from a single piece of data to a complex object +- Collections of data are usually not stored as a single key (e.g. an array with multiple objects), but as individual keys+ID (e.g. `report_1234`, `report_4567`, etc.). Store collections as individual keys when a component will bind directly to one of those keys. For example: reports are stored as individual keys because `SidebarLink.js` binds to the individual report keys for each link. However, report actions are stored as an array of objects because nothing binds directly to a single report action. +- Onyx allows other code to subscribe to changes in data, and then publishes change events whenever data is changed +- Anything needing to read Onyx data needs to: 1. Know what key the data is stored in (for web, you can find this by looking in the JS console > Application > local storage) 2. Subscribe to changes of the data for a particular key or set of keys. React components use `withOnyx()` and non-React libs use `Onyx.connect()`. 3. Get initialized with the current value of that key from persistent storage (Onyx does this by calling `setState()` or triggering the `callback` with the values currently on disk as part of the connection process) -- Subscribing to Onyx keys is done using a constant defined in `ONYXKEYS`. Each Onyx key represents either a collection of items or a specific entry in storage. For example, since all reports are stored as individual keys like `report_1234`, if code needs to know about all the reports (e.g. display a list of them in the nav menu), then it would subscribe to the key `ONYXKEYS.COLLECTION.REPORT`. +- Subscribing to Onyx keys is done using a constant defined in `ONYXKEYS`. Each Onyx key represents either a collection of items or a specific entry in storage. For example, since all reports are stored as individual keys like `report_1234`, if code needs to know about all the reports (e.g. display a list of them in the nav menu), then it would subscribe to the key `ONYXKEYS.COLLECTION.REPORT`. # Getting Started @@ -29,10 +28,10 @@ npm install react-native-onyx --save To initialize Onyx we call `Onyx.init()` with a configuration object. ```javascript -import Onyx from "react-native-onyx"; +import Onyx from 'react-native-onyx'; const ONYXKEYS = { - SESSION: "session", + SESSION: 'session', }; const config = { @@ -43,7 +42,6 @@ Onyx.init(config); ``` ### Usage in non react-native projects - Onyx can be used in non react-native projects, by leveraging the `browser` field in `package.json` Bundlers like Webpack respect that field and import code from the specified path We import Onyx the same way shown above - `import Onyx from 'react-native-onyx'` @@ -53,9 +51,10 @@ We import Onyx the same way shown above - `import Onyx from 'react-native-onyx'` To store some data we can use the `Onyx.set()` method. ```javascript -API.Authenticate(params).then((response) => { - Onyx.set(ONYXKEYS.SESSION, { token: response.token }); -}); +API.Authenticate(params) + .then((response) => { + Onyx.set(ONYXKEYS.SESSION, {token: response.token}); + }); ``` The data will then be cached and stored via [`AsyncStorage`](https://github.com/react-native-async-storage/async-storage). @@ -67,31 +66,31 @@ We can also use `Onyx.merge()` to merge new `Object` or `Array` data in with exi For `Array` the default behavior is to replace it fully, effectively making it equivalent to set: ```javascript -Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ["Joe"]); // -> ['Joe'] -Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ["Jack"]); // -> ['Joe', 'Jack'] +Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Joe']); // -> ['Joe'] +Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Jack']); // -> ['Joe', 'Jack'] ``` For `Object` values the default behavior uses `lodash/merge` under the hood to do a deep extend of the object. ```javascript -Onyx.merge(ONYXKEYS.POLICY, { id: 1 }); // -> {id: 1} -Onyx.merge(ONYXKEYS.POLICY, { name: "My Workspace" }); // -> {id: 1, name: 'My Workspace'} +Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1} +Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'} ``` Arrays inside objects will be replaced fully, same as arrays not inside objects: ```javascript -Onyx.merge(ONYXKEYS.POLICY, { employeeList: ["Joe", "Jack"] }); // -> {employeeList: ['Joe', 'Jack']} -Onyx.merge(ONYXKEYS.POLICY, { employeeList: ["Jack"] }); // -> {employeeList: ['Jack']} +Onyx.merge(ONYXKEYS.POLICY, {employeeList: ['Joe', 'Jack']}); // -> {employeeList: ['Joe', 'Jack']} +Onyx.merge(ONYXKEYS.POLICY, {employeeList: ['Jack']}); // -> {employeeList: ['Jack']} ``` ### Should I use `merge()` or `set()` or both? -- Use `merge()` when creating a new object -- Use `merge()` to merge partial data into an existing object -- Use `merge()` when storing simple values (`String`, `Boolean`, `Number`) -- Use `set()` when you need to delete an Onyx key completely from storage -- Use `set()` when you need to completely reset an object or array of data +- Use `merge()` when creating a new object +- Use `merge()` to merge partial data into an existing object +- Use `merge()` when storing simple values (`String`, `Boolean`, `Number`) +- Use `set()` when you need to delete an Onyx key completely from storage +- Use `set()` when you need to completely reset an object or array of data Consecutive calls to `Onyx.merge()` with the same key are batched in a stack and processed in the order that they were called. This helps avoid race conditions where one merge possibly finishes before another. However, it's important to note that calls to `Onyx.set()` are not batched together with calls to `Onyx.merge()`. For this reason, it is usually preferable to use one or the other, but not both. Onyx is a work-in-progress so always test code to make sure assumptions are correct! @@ -107,7 +106,7 @@ To set up a basic subscription for a given key use the `Onyx.connect()` method. let session; const connectionID = Onyx.connect({ key: ONYXKEYS.SESSION, - callback: (val) => (session = val || {}), + callback: (val) => session = val || {}, }); ``` @@ -120,12 +119,12 @@ Onyx.disconnect(connectionID); We can also access values inside React components via the `withOnyx()` [higher order component](https://reactjs.org/docs/higher-order-components.html). When the data changes the component will re-render. ```javascript -import React from "react"; -import { withOnyx } from "react-native-onyx"; +import React from 'react'; +import {withOnyx} from 'react-native-onyx'; -const App = ({ session }) => ( +const App = ({session}) => ( - {session.token ? Logged in : Logged out} + {session.token ? Logged in : Logged out } ); @@ -142,7 +141,7 @@ While `Onyx.connect()` gives you more control on how your component reacts as da export default withOnyx({ session: { key: ONYXKEYS.SESSION, - initialValue: {}, + initialValue: {} }, })(App); ``` @@ -150,23 +149,20 @@ export default withOnyx({ Additionally, if your component has many keys/entities when your component will mount but will receive many updates as data is fetched from DB and passed down to it, as every key that gets fetched will trigger a `setState` on the `withOnyx` HOC. This might cause re-renders on the initial mounting, preventing the component from mounting/rendering in reasonable time, making your app feel slow and even delaying animations. You can workaround this by passing an additional object with the `shouldDelayUpdates` property set to true. Onyx will then put all the updates in a queue until you decide when then should be applied, the component will receive a function `markReadyForHydration`. A good place to call this function is on the `onLayout` method, which gets triggered after your component has been rendered. ```javascript -const App = ({ session, markReadyForHydration }) => ( +const App = ({session, markReadyForHydration}) => ( markReadyForHydration()}> - {session.token ? Logged in : Logged out} + {session.token ? Logged in : Logged out } ); -export default withOnyx( - { - session: { - key: ONYXKEYS.SESSION, - initialValue: {}, - }, +export default withOnyx({ + session: { + key: ONYXKEYS.SESSION, + initialValue: {} }, - { - shouldDelayUpdates: true, - } -)(App); +}, { + shouldDelayUpdates: true +})(App); ``` ## Collections @@ -176,7 +172,7 @@ Collections allow keys with similar value types to be subscribed together by sub ```javascript const ONYXKEYS = { COLLECTION: { - REPORT: "report_", + REPORT: 'report_', }, }; ``` @@ -205,7 +201,7 @@ There are several ways to subscribe to these keys: ```javascript withOnyx({ - allReports: { key: ONYXKEYS.COLLECTION.REPORT }, + allReports: {key: ONYXKEYS.COLLECTION.REPORT}, })(MyComponent); ``` @@ -231,21 +227,15 @@ This final option forces `Onyx.connect()` to behave more like `withOnyx()` and o Be cautious when using collections as things can get out of hand if you have a subscriber hooked up to a collection key that has large numbers of individual keys. If this is the case, it is critical to use `mergeCollection()` over `merge()`. -Remember, `mergeCollection()` will notify a subscriber only _once_ with the total collected values whereas each call to `merge()` would re-render a connected component _each time it is called_. Consider this example where `reports` is an array of reports that we want to index and save. +Remember, `mergeCollection()` will notify a subscriber only *once* with the total collected values whereas each call to `merge()` would re-render a connected component *each time it is called*. Consider this example where `reports` is an array of reports that we want to index and save. ```js // Bad -_.each(reports, (report) => - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report) -); // -> A component using withOnyx() will have it's state updated with each iteration +_.each(reports, report => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report)); // -> A component using withOnyx() will have it's state updated with each iteration // Good const values = {}; -_.each( - reports, - (report) => - (values[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = report) -); +_.each(reports, report => values[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = report); Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, values); // -> A component using withOnyx() will only have it's state updated once ``` @@ -260,7 +250,6 @@ function signOut() { ``` ## Storage Providers - `Onyx.get`, `Onyx.set`, and the rest of the API accesses the underlying storage differently depending on the platform @@ -268,7 +257,6 @@ Under the hood storage access calls are delegated to a [`StorageProvider`](lib/s Some platforms (like web and desktop) might use the same storage provider If a platform needs to use a separate library (like using MMVK for react-native) it should be added in the following way: - 1. Create a `StorageProvider.js` at [lib/storage/providers](lib/storage/providers) Reference an existing [StorageProvider](lib/storage/providers/AsyncStorage.js) for the interface that has to be implemented 2. Update the factory at [lib/storage/index.web.js](lib/storage/index.web.js) and [lib/storage/index.native.js](lib/storage/index.native.js) to return the newly created Provider for the desired Platform(s) @@ -280,20 +268,17 @@ If a platform needs to use a separate library (like using MMVK for react-native) # Storage Eviction Different platforms come with varying storage capacities and Onyx has a way to gracefully fail when those storage limits are encountered. When Onyx fails to set or modify a key the following steps are taken: - 1. Onyx looks at a list of recently accessed keys (access is defined as subscribed to or modified) and locates the key that was least recently accessed 2. It then deletes this key and retries the original operation By default, Onyx will not evict anything from storage and will presume all keys are "unsafe" to remove unless explicitly told otherwise. **To flag a key as safe for removal:** - -- Add the key to the `safeEvictionKeys` option in `Onyx.init(options)` -- Implement `canEvict` in the Onyx config for each component subscribing to a key -- The key will only be deleted when all subscribers return `true` for `canEvict` +- Add the key to the `safeEvictionKeys` option in `Onyx.init(options)` +- Implement `canEvict` in the Onyx config for each component subscribing to a key +- The key will only be deleted when all subscribers return `true` for `canEvict` e.g. - ```js Onyx.init({ safeEvictionKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS], @@ -303,9 +288,8 @@ Onyx.init({ ```js export default withOnyx({ reportActions: { - key: ({ reportID }) => - `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}_`, - canEvict: (props) => !props.isActiveReport, + key: ({reportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}_`, + canEvict: props => !props.isActiveReport, }, })(ReportActionsView); ``` @@ -325,15 +309,14 @@ Onyx.init({ At any point you can get the collected statistics using `Onyx.getMetrics()`. This will return an object containing `totalTime`, `averageTime` and `summaries`. `summaries` is a collection of statistics for each method it contains data about: - -- method name -- total, max, min, average times for this method calls -- calls - a list of individual calls with each having: start time; end time; call duration; call arguments - - start/end times are relative to application launch time - 0.00 being exactly at launch + - method name + - total, max, min, average times for this method calls + - calls - a list of individual calls with each having: start time; end time; call duration; call arguments + - start/end times are relative to application launch time - 0.00 being exactly at launch If you wish to reset the metrics and start over use `Onyx.resetMetrics()` -Finally, there's a `Onyx.printMetrics()` method which prints human statistics information on the dev console. You can use this method during debugging. For example add an `Onyx.printMetrics()` line somewhere in code or call it through the dev console. It supports 3 popular formats _MD_ - human friendly markdown, _CSV_ and _JSON_. The default is MD if you want to print another format call `Onyx.printMetrics({ format: 'csv' })` or +Finally, there's a `Onyx.printMetrics()` method which prints human statistics information on the dev console. You can use this method during debugging. For example add an `Onyx.printMetrics()` line somewhere in code or call it through the dev console. It supports 3 popular formats *MD* - human friendly markdown, *CSV* and *JSON*. The default is MD if you want to print another format call `Onyx.printMetrics({ format: 'csv' })` or `Onyx.printMetrics({ format: 'json' })`. Sample output of `Onyx.printMetrics()` @@ -384,14 +367,12 @@ Then, you can enable this type of debugging by passing `enableDevTools: true` to link a local version of Onyx during development To quickly test small changes you can directly go to `node_modules/react-native-onyx` in the parent project and: - -- tweak original source if you're testing over a react-native project -- tweak `dist/web.development.js` for non react-native-projects +- tweak original source if you're testing over a react-native project +- tweak `dist/web.development.js` for non react-native-projects To continuously work on Onyx we have to set up a task that copies content to parent project's `node_modules/react-native-onyx`: - 1. Work on Onyx feature or a fix 2. Save files 3. Optional: run `npm run build` (if you're working or want to test on a non react-native project) - - `npm link` would actually work outside of `react-native` and it can be used to link Onyx locally for a web only project + - `npm link` would actually work outside of `react-native` and it can be used to link Onyx locally for a web only project 4. Copy Onyx to consumer project's `node_modules/react-native-onyx` From c51841b7ad5e766556b2c048085122a946717155 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 13 Sep 2023 11:44:36 +0200 Subject: [PATCH 08/14] Work around race condition with initialValue on initial onyx hydration via HOC --- lib/withOnyx.js | 64 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 7cb5a0310..6c8f295c5 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -125,45 +125,73 @@ export default function (mapOnyxToState, options = {}) { } /** - * This method is used externally by sendDataToConnection to prevent unnecessary renders while a component - * still in a loading state. The temporary initial state is saved to the component instance and setState() + * This method is used by the internal raw Onyx `sendDataToConnection`, it is designed to prevent unnecessary renders while a component + * still in a "loading" (read "mounting") state. The temporary initial state is saved to the HOC instance and setState() * only called once all the necessary data has been collected. * + * There is however the possibility the component could have been updated by a call to setState() + * before the data was "initially" collected. A race condition. + * For example some update happened on some key, while onyx was still gathering the initial hydration data. + * This update is disptached directly to setStateProxy and therefore the component has the most up-to-date data + * + * This is a design flaw in Onyx itself as dispatching updates before initial hydration is not a correct event flow. + * We however need to workaround this issue in the HOC. The addition of initialValue makes things even more complex, + * since you cannot be really sure if the component has been updated before or after the initial hydration. Therefore if + * initialValue is there, we just check if the update is different than that and then try to handle it as best as we can. + * * @param {String} statePropertyName * @param {*} val */ setWithOnyxState(statePropertyName, val) { - // We might have loaded the values for the onyx keys/mappings already from the cache. - // In case we were able to load all the values upfront, the loading state will be false. - // However, Onyx.js will always call setWithOnyxState, as it doesn't know that this implementation - // already loaded the values from cache. Thus we have to check whether the value has changed - // before we set the state to prevent unnecessary renders. const prevValue = this.state[statePropertyName]; - if (!this.state.loading && (prevValue === val || utils.areObjectsEmpty(prevValue, val))) { - return; - } + // If the component is not loading (read "mounting"), then we can just update the state if (!this.state.loading) { + // Performance optimization, do not trigger update with same values + if (prevValue === val || utils.areObjectsEmpty(prevValue, val)) { + return; + } + this.setStateProxy({[statePropertyName]: val}); return; } this.tempState[statePropertyName] = val; - // All state keys should exist and at least have a value of null - if (_.some(requiredKeysForInit, key => _.isUndefined(this.tempState[key]))) { + // If some key does not have a value yet, do not update the state yet + const tempStateIsMissingKey = _.some(requiredKeysForInit, key => _.isUndefined(this.tempState[key])); + if (tempStateIsMissingKey) { return; } const stateUpdate = {...this.tempState, loading: false}; + delete this.tempState; - // The state is set here manually, instead of using setState because setState is an async operation, meaning it might execute on the next tick, - // or at a later point in the microtask queue. That can lead to a race condition where - // setWithOnyxState is called before the state is actually set. This results in unreliable behavior when checking the loading state and has been mainly observed on fabric. - this.state = stateUpdate; + // Full of hacky workarounds to prevent the race condition described above. + this.setState((prevState) => { + const finalState = _.reduce(stateUpdate, (result, value, key) => { + const initialValue = mapOnyxToState[key].initialValue; - this.setState(stateUpdate); // Trigger a render - delete this.tempState; + // If initialValue is there and the state contains something different it means + // an update has already been received and we can discard the value we are trying to hydrate + if (Boolean(initialValue) && Boolean(prevState[key]) && prevState[key] !== initialValue) { + // eslint-disable-next-line no-param-reassign + result[key] = prevState[key]; + + // if value is already there (without initial value) then we can discard the value we are trying to hydrate + } else if (prevState[key]) { + // eslint-disable-next-line no-param-reassign + result[key] = prevState[key]; + } else { + // eslint-disable-next-line no-param-reassign + result[key] = value; + } + return result; + }, {}); + + finalState.loading = false; + return finalState; + }); } /** From 8d5e11a48ba6565c0777223ba507bb5bd3e8091d Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Wed, 13 Sep 2023 12:27:29 +0200 Subject: [PATCH 09/14] Minor fixes with new merging algo --- lib/withOnyx.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 6c8f295c5..16f294284 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -164,22 +164,26 @@ export default function (mapOnyxToState, options = {}) { return; } - const stateUpdate = {...this.tempState, loading: false}; + const stateUpdate = {...this.tempState}; delete this.tempState; // Full of hacky workarounds to prevent the race condition described above. this.setState((prevState) => { const finalState = _.reduce(stateUpdate, (result, value, key) => { + if (key === 'loading') { + return result; + } + const initialValue = mapOnyxToState[key].initialValue; // If initialValue is there and the state contains something different it means // an update has already been received and we can discard the value we are trying to hydrate - if (Boolean(initialValue) && Boolean(prevState[key]) && prevState[key] !== initialValue) { + if (!_.isUndefined(initialValue) && !_.isUndefined(prevState[key]) && prevState[key] !== initialValue) { // eslint-disable-next-line no-param-reassign result[key] = prevState[key]; // if value is already there (without initial value) then we can discard the value we are trying to hydrate - } else if (prevState[key]) { + } else if (!_.isUndefined(prevState[key])) { // eslint-disable-next-line no-param-reassign result[key] = prevState[key]; } else { From d17c74f6a25ffa8999172beba97cdef2a987d164 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 14 Sep 2023 12:30:38 +0200 Subject: [PATCH 10/14] Add tests for initial value and shouldDelayUpdates --- lib/withOnyx.d.ts | 3 + tests/components/ViewWithCollections.js | 12 ++- tests/unit/withOnyxTest.js | 126 +++++++++++++++++++++--- 3 files changed, 122 insertions(+), 19 deletions(-) diff --git a/lib/withOnyx.d.ts b/lib/withOnyx.d.ts index cf377d304..a32f350d3 100644 --- a/lib/withOnyx.d.ts +++ b/lib/withOnyx.d.ts @@ -148,6 +148,9 @@ declare function withOnyx( | OnyxPropMapping | OnyxPropCollectionMapping; }, + options?: { + shouldDelayUpdates?: true; + } ): (component: React.ComponentType) => React.ComponentType>; export default withOnyx; diff --git a/tests/components/ViewWithCollections.js b/tests/components/ViewWithCollections.js index 876f3b2c5..ecf380efc 100644 --- a/tests/components/ViewWithCollections.js +++ b/tests/components/ViewWithCollections.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useImperativeHandle} from 'react'; import PropTypes from 'prop-types'; // eslint-disable-next-line no-restricted-imports import {View, Text} from 'react-native'; @@ -12,15 +12,21 @@ const propTypes = { ID: PropTypes.number, }), onRender: PropTypes.func, + markReadyForHydration: PropTypes.func, }; const defaultProps = { collections: {}, testObject: {isDefaultProp: true}, onRender: () => {}, + markReadyForHydration: () => {}, }; -function ViewWithCollections(props) { +const ViewWithCollections = React.forwardRef((props, ref) => { + useImperativeHandle(ref, () => ({ + markReadyForHydration: props.markReadyForHydration, + })); + props.onRender(props); return ( @@ -30,7 +36,7 @@ function ViewWithCollections(props) { ))} ); -} +}); ViewWithCollections.propTypes = propTypes; ViewWithCollections.defaultProps = defaultProps; diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index c0397cf13..7f86eeb4f 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -119,7 +119,8 @@ describe('withOnyx', () => { }, })(ViewWithCollections); const onRender = jest.fn(); - render(); + const markReadyForHydration = jest.fn(); + render(); return waitForPromisesToResolve() .then(() => Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: {list: [1, 2]}, @@ -130,7 +131,7 @@ describe('withOnyx', () => { .then(() => { expect(onRender).toHaveBeenCalledTimes(3); expect(onRender).toHaveBeenLastCalledWith({ - collections: {}, onRender, testObject: {isDefaultProp: true}, text: {list: [7]}, + collections: {}, markReadyForHydration, onRender, testObject: {isDefaultProp: true}, text: {list: [7]}, }); }); }); @@ -143,7 +144,8 @@ describe('withOnyx', () => { }, })(ViewWithCollections); const onRender = jest.fn(); - render(); + const markReadyForHydration = jest.fn(); + render(); return waitForPromisesToResolve() .then(() => { Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_4: {ID: 456}, test_5: {ID: 567}}); @@ -156,7 +158,7 @@ describe('withOnyx', () => { .then(() => { expect(onRender).toHaveBeenCalledTimes(3); expect(onRender).toHaveBeenLastCalledWith({ - collections: {}, onRender, testObject: {isDefaultProp: true}, text: {ID: 456, Name: 'Test4'}, + collections: {}, markReadyForHydration, onRender, testObject: {isDefaultProp: true}, text: {ID: 456, Name: 'Test4'}, }); }); }); @@ -213,6 +215,7 @@ describe('withOnyx', () => { it('should pass a prop from one connected component to another', () => { const collectionItemID = 1; const onRender = jest.fn(); + const markReadyForHydration = jest.fn(); Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {id: 1}}); Onyx.mergeCollection(ONYX_KEYS.COLLECTION.RELATED_KEY, {related_1: 'Test'}); return waitForPromisesToResolve() @@ -229,11 +232,11 @@ describe('withOnyx', () => { }, }), )(ViewWithCollections); - render(); + render(); }) .then(() => { expect(onRender).toHaveBeenLastCalledWith({ - collections: {}, onRender, testObject: {id: 1}, testThing: 'Test', + collections: {}, markReadyForHydration, onRender, testObject: {id: 1}, testThing: 'Test', }); }); }); @@ -242,6 +245,9 @@ describe('withOnyx', () => { const onRender1 = jest.fn(); const onRender2 = jest.fn(); const onRender3 = jest.fn(); + const markReadyForHydration1 = jest.fn(); + const markReadyForHydration2 = jest.fn(); + const markReadyForHydration3 = jest.fn(); // Given there is a collection with three simple items in it Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -258,21 +264,21 @@ describe('withOnyx', () => { key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, }, })(ViewWithCollections); - render(); + render(); const TestComponentWithOnyx2 = withOnyx({ testObject: { key: `${ONYX_KEYS.COLLECTION.TEST_KEY}2`, }, })(ViewWithCollections); - render(); + render(); const TestComponentWithOnyx3 = withOnyx({ testObject: { key: `${ONYX_KEYS.COLLECTION.TEST_KEY}3`, }, })(ViewWithCollections); - render(); + render(); }) // When a single item in the collection is updated with mergeCollection() @@ -285,19 +291,28 @@ describe('withOnyx', () => { // Note: each component is rendered twice. Once when it is initially rendered, and then again // when the collection is updated. That's why there are two checks here for each component. expect(onRender1).toHaveBeenCalledTimes(2); - expect(onRender1).toHaveBeenNthCalledWith(1, {collections: {}, onRender: onRender1, testObject: {ID: 1}}); - expect(onRender1).toHaveBeenNthCalledWith(2, {collections: {}, onRender: onRender1, testObject: {ID: 1, newProperty: 'yay'}}); + expect(onRender1).toHaveBeenNthCalledWith(1, { + collections: {}, markReadyForHydration: markReadyForHydration1, onRender: onRender1, testObject: {ID: 1}, + }); + expect(onRender1).toHaveBeenNthCalledWith(2, { + collections: {}, markReadyForHydration: markReadyForHydration1, onRender: onRender1, testObject: {ID: 1, newProperty: 'yay'}, + }); expect(onRender2).toHaveBeenCalledTimes(1); - expect(onRender2).toHaveBeenNthCalledWith(1, {collections: {}, onRender: onRender2, testObject: {ID: 2}}); + expect(onRender2).toHaveBeenNthCalledWith(1, { + collections: {}, markReadyForHydration: markReadyForHydration2, onRender: onRender2, testObject: {ID: 2}, + }); expect(onRender3).toHaveBeenCalledTimes(1); - expect(onRender3).toHaveBeenNthCalledWith(1, {collections: {}, onRender: onRender3, testObject: {ID: 3}}); + expect(onRender3).toHaveBeenNthCalledWith(1, { + collections: {}, markReadyForHydration: markReadyForHydration3, onRender: onRender3, testObject: {ID: 3}, + }); }); }); it('mergeCollection should merge previous props correctly to the new state', () => { const onRender = jest.fn(); + const markReadyForHydration = jest.fn(); // Given there is a collection with a simple item in it that has a `number` property set to 1 Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -312,7 +327,7 @@ describe('withOnyx', () => { key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, }, })(ViewWithCollections); - render(); + render(); }) // When the `number` property is updated using mergeCollection to be 2 @@ -324,8 +339,12 @@ describe('withOnyx', () => { // The first time it will render with number === 1 // The second time it will render with number === 2 expect(onRender).toHaveBeenCalledTimes(2); - expect(onRender).toHaveBeenNthCalledWith(1, {collections: {}, onRender, testObject: {ID: 1, number: 1}}); - expect(onRender).toHaveBeenNthCalledWith(2, {collections: {}, onRender, testObject: {ID: 1, number: 2}}); + expect(onRender).toHaveBeenNthCalledWith(1, { + collections: {}, markReadyForHydration, onRender, testObject: {ID: 1, number: 1}, + }); + expect(onRender).toHaveBeenNthCalledWith(2, { + collections: {}, markReadyForHydration, onRender, testObject: {ID: 1, number: 2}, + }); }); }); @@ -362,4 +381,79 @@ describe('withOnyx', () => { expect(onRender.mock.calls[1][0].simple).toBe('long_string'); }); }); + + it('initialValue should be feed into component', () => { + const onRender = jest.fn(); + const markReadyForHydration = 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, + initialValue: 'initialValue', + }, + })(ViewWithCollections); + render(); + }) + + // And we set the value to the same value it was before + // .then(() => Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'string')) + .then(() => { + // Then the component subscribed to the modified item should only render once + expect(onRender).toHaveBeenCalledTimes(1); + expect(onRender).toHaveBeenLastCalledWith({ + collections: {}, markReadyForHydration, onRender, testObject: {isDefaultProp: true}, simple: 'initialValue', + }); + }); + }); + + it('shouldDelayUpdates + initialValue does not feed data into component until marked ready', () => { + const onRender = jest.fn(); + const ref = React.createRef(); + + return waitForPromisesToResolve() + .then(() => { + // When a component subscribes to the simple key + const TestComponentWithOnyx = withOnyx({ + simple: { + key: ONYX_KEYS.SIMPLE_KEY, + initialValue: 'initialValue', + }, + }, + { + shouldDelayUpdates: true, + })(ViewWithCollections); + + render(); + }) + + // component mounted with the initial value, while updates are queueing + .then(() => Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'string')) + .then(() => { + expect(onRender).toHaveBeenCalledTimes(1); + expect(onRender.mock.calls[0][0].simple).toBe('initialValue'); + + // just to test we change the value + return Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'long_string'); + }) + .then(() => { + // Component still has not updated + expect(onRender).toHaveBeenCalledTimes(1); + + // We can now tell component to update + // console.log(componentInstance); + + ref.current.markReadyForHydration(); + }) + .then(() => { + // Component still has not updated + expect(onRender).toHaveBeenCalledTimes(4); + expect(onRender.mock.calls[3][0].simple).toBe('long_string'); + }); + }); }); From 1b92d7360119b124e0a49f4380edbb8a12f6ddeb Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Thu, 14 Sep 2023 12:37:33 +0200 Subject: [PATCH 11/14] Remove options object from withOnyx, replace with a single boolean flag for shouldDelayUpdates --- README.md | 5 ++--- lib/withOnyx.d.ts | 4 +--- lib/withOnyx.js | 4 ++-- tests/unit/withOnyxTest.js | 4 +--- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 2b9884aad..22a9b64e2 100644 --- a/README.md +++ b/README.md @@ -155,14 +155,13 @@ const App = ({session, markReadyForHydration}) => ( ); +// Second argument to funciton is `shouldDelayUpdates` export default withOnyx({ session: { key: ONYXKEYS.SESSION, initialValue: {} }, -}, { - shouldDelayUpdates: true -})(App); +}, true)(App); ``` ## Collections diff --git a/lib/withOnyx.d.ts b/lib/withOnyx.d.ts index a32f350d3..91838dfe1 100644 --- a/lib/withOnyx.d.ts +++ b/lib/withOnyx.d.ts @@ -148,9 +148,7 @@ declare function withOnyx( | OnyxPropMapping | OnyxPropCollectionMapping; }, - options?: { - shouldDelayUpdates?: true; - } + shouldDelayUpdates?: boolean, ): (component: React.ComponentType) => React.ComponentType>; export default withOnyx; diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 16f294284..976152848 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -20,7 +20,7 @@ function getDisplayName(component) { return component.displayName || component.name || 'Component'; } -export default function (mapOnyxToState, options = {}) { +export default function (mapOnyxToState, shouldDelayUpdates = false) { // A list of keys that must be present in tempState before we can render the WrappedComponent const requiredKeysForInit = _.chain(mapOnyxToState) .omit(config => config.initWithStoredValues === false) @@ -33,7 +33,7 @@ export default function (mapOnyxToState, options = {}) { constructor(props) { super(props); - this.shouldDelayUpdates = Boolean(options.shouldDelayUpdates); + this.shouldDelayUpdates = shouldDelayUpdates; this.setWithOnyxState = this.setWithOnyxState.bind(this); this.flushPendingSetStates = this.flushPendingSetStates.bind(this); diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 7f86eeb4f..b938c9992 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -425,9 +425,7 @@ describe('withOnyx', () => { initialValue: 'initialValue', }, }, - { - shouldDelayUpdates: true, - })(ViewWithCollections); + true)(ViewWithCollections); render(); }) From 9fcd0e53346b08a77614e905bd1b3e9b922d399e Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Fri, 15 Sep 2023 10:25:48 +0200 Subject: [PATCH 12/14] Update tests/unit/withOnyxTest.js Co-authored-by: Tim Golen --- tests/unit/withOnyxTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index b938c9992..efb0d25bc 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -382,7 +382,7 @@ describe('withOnyx', () => { }); }); - it('initialValue should be feed into component', () => { + it('initialValue should be fed into component', () => { const onRender = jest.fn(); const markReadyForHydration = jest.fn(); From eb78258065bebf7308e82c8801ff819efdde4861 Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Sat, 16 Sep 2023 09:51:21 +0200 Subject: [PATCH 13/14] Update PR --- API.md | 91 ++++++++++++++++++++++++++++++-------- lib/Onyx.js | 3 ++ tests/unit/withOnyxTest.js | 8 ---- 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/API.md b/API.md index 5db1cff93..7e45a2d1c 100644 --- a/API.md +++ b/API.md @@ -6,7 +6,7 @@
getSubsetOfData(sourceData, selector, [withOnyxInstanceState]) ⇒ Mixed
-

Uses a selector string or function to return a simplified version of sourceData

+

Uses a selector function to return a simplified version of sourceData

reduceCollectionWithSelector(collection, selector, [withOnyxInstanceState]) ⇒ Object

Takes a collection of items (eg. {testKey_1:{a:'a'}, testKey_2:{b:'b'}}) @@ -15,6 +15,10 @@ The resulting collection will only contain items that are returned by the select

isCollectionMemberKey(collectionKey, key) ⇒ Boolean
+
tryGetCachedValue(key, mapping) ⇒ Mixed
+

Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. +If the requested key is a collection, it will return an object with all the collection members.

+
connect(mapping) ⇒ Number

Subscribes a react component's state directly to a store key

@@ -32,18 +36,21 @@ behavior just yet.

so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the subscriber callbacks receive the data in a different format than they normally expect and it breaks code.

+
broadcastUpdate(key, value, hasChanged, method)
+

Notifys subscribers and writes current value to cache

+
+
hasPendingMergeForKey(key) ⇒ Boolean
+
set(key, value) ⇒ Promise

Write a value to our store with the given key

multiSet(data) ⇒ Promise

Sets multiple keys and values

-
merge(key, value) ⇒ Promise
+
merge(key, changes) ⇒ Promise

Merge a new value into an existing value at a key.

-

The types of values that can be merged are Object and Array. To set another type of value use Onyx.set(). Merge -behavior uses lodash/merge under the hood for Object and simple concatenation for Array. However, it's important -to note that if you have an array value property on an Object that the default behavior of lodash/merge is not to -concatenate. See here: https://github.com/lodash/lodash/issues/2872

+

The types of values that can be merged are Object and Array. To set another type of value use Onyx.set(). +Values of type Object get merged with the old value, whilst for Array's we simply replace the current value with the new one.

Calls to Onyx.merge() are batched so that any calls performed in a single tick will stack in a queue and get applied in the order they were called. Note: Onyx.set() calls do not work this way so use caution when mixing Onyx.merge() and Onyx.set().

@@ -70,6 +77,9 @@ value will be saved to storage after the default value.

update(data) ⇒ Promise

Insert API responses and lifecycle data into Onyx

+
setMemoryOnlyKeys(keyList)
+

When set these keys will not be persisted to storage

+
init([options])

Initialize the store with actions and listening for storage events

@@ -78,15 +88,15 @@ value will be saved to storage after the default value.

## getSubsetOfData(sourceData, selector, [withOnyxInstanceState]) ⇒ Mixed -Uses a selector string or function to return a simplified version of sourceData +Uses a selector function to return a simplified version of sourceData **Kind**: global function | Param | Type | Description | | --- | --- | --- | | sourceData | Mixed | | -| selector | String \| function | | -| [withOnyxInstanceState] | Object | If it's a string, the selector is passed to lodashGet on the sourceData If it's a function, it is passed the sourceData and it should return the simplified data | +| selector | function | Function that takes sourceData and returns a simplified version of it | +| [withOnyxInstanceState] | Object | | @@ -113,6 +123,19 @@ The resulting collection will only contain items that are returned by the select | collectionKey | String | | key | String | + + +## tryGetCachedValue(key, mapping) ⇒ Mixed +Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. +If the requested key is a collection, it will return an object with all the collection members. + +**Kind**: global function + +| Param | Type | +| --- | --- | +| key | String | +| mapping | Object | + ## connect(mapping) ⇒ Number @@ -130,8 +153,8 @@ Subscribes a react component's state directly to a store key | [mapping.callback] | function | a method that will be called with changed data This is used by any non-React code to connect to Onyx | | [mapping.initWithStoredValues] | Boolean | If set to false, then no data will be prefilled into the component | | [mapping.waitForCollectionCallback] | Boolean | If set to true, it will return the entire collection to the callback as a single object | -| [mapping.selector] | String \| function | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. If the selector is a string, the selector is passed to lodashGet on the sourceData. If the selector is a function, the sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | -| [mapping.initialValue] | Any | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be passed as the initial hydrated value for the mapping. It will allow the component to render eagerly while data is being fetched from the DB. Note that it will not cause the component to have the `loading` prop set to true. | +| [mapping.selector] | function | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | +| [mapping.initialValue] | String \| Number \| Boolean \| Object | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. Note that it will not cause the component to have the loading prop set to true. | | **Example** ```js @@ -190,6 +213,29 @@ subscriber callbacks receive the data in a different format than they normally e | key | String | | value | \* | + + +## broadcastUpdate(key, value, hasChanged, method) +Notifys subscribers and writes current value to cache + +**Kind**: global function + +| Param | Type | +| --- | --- | +| key | String | +| value | \* | +| hasChanged | Boolean | +| method | String | + + + +## hasPendingMergeForKey(key) ⇒ Boolean +**Kind**: global function + +| Param | Type | +| --- | --- | +| key | String | + ## set(key, value) ⇒ Promise @@ -219,13 +265,11 @@ Onyx.multiSet({'key1': 'a', 'key2': 'b'}); ``` -## merge(key, value) ⇒ Promise +## merge(key, changes) ⇒ Promise Merge a new value into an existing value at a key. -The types of values that can be merged are `Object` and `Array`. To set another type of value use `Onyx.set()`. Merge -behavior uses lodash/merge under the hood for `Object` and simple concatenation for `Array`. However, it's important -to note that if you have an array value property on an `Object` that the default behavior of lodash/merge is not to -concatenate. See here: https://github.com/lodash/lodash/issues/2872 +The types of values that can be merged are `Object` and `Array`. To set another type of value use `Onyx.set()`. +Values of type `Object` get merged with the old value, whilst for `Array`'s we simply replace the current value with the new one. Calls to `Onyx.merge()` are batched so that any calls performed in a single tick will stack in a queue and get applied in the order they were called. Note: `Onyx.set()` calls do not work this way so use caution when mixing @@ -236,7 +280,7 @@ applied in the order they were called. Note: `Onyx.set()` calls do not work this | Param | Type | Description | | --- | --- | --- | | key | String | ONYXKEYS key | -| value | Object \| Array | Object or Array value to merge | +| changes | Object \| Array | Object or Array value to merge | **Example** ```js @@ -301,7 +345,18 @@ Insert API responses and lifecycle data into Onyx | Param | Type | Description | | --- | --- | --- | -| data | Array | An array of objects with shape {onyxMethod: oneOf('set', 'merge', 'mergeCollection'), key: string, value: *} | +| data | Array | An array of objects with shape {onyxMethod: oneOf('set', 'merge', 'mergeCollection', 'multiSet', 'clear'), key: string, value: *} | + + + +## setMemoryOnlyKeys(keyList) +When set these keys will not be persisted to storage + +**Kind**: global function + +| Param | Type | +| --- | --- | +| keyList | Array.<string> | diff --git a/lib/Onyx.js b/lib/Onyx.js index 4cc60c594..e5a556d00 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -716,6 +716,9 @@ function getCollectionDataAndSendAsObject(matchingKeys, mapping) { * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). + * @param {String | Number | Boolean | Object} [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). + * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. + * Note that it will not cause the component to have the loading prop set to true. | * @returns {Number} an ID to use when calling disconnect */ function connect(mapping) { diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index efb0d25bc..a5b0b33d7 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -386,9 +386,6 @@ describe('withOnyx', () => { const onRender = jest.fn(); const markReadyForHydration = 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 @@ -400,9 +397,6 @@ describe('withOnyx', () => { })(ViewWithCollections); render(); }) - - // And we set the value to the same value it was before - // .then(() => Onyx.merge(ONYX_KEYS.SIMPLE_KEY, 'string')) .then(() => { // Then the component subscribed to the modified item should only render once expect(onRender).toHaveBeenCalledTimes(1); @@ -444,8 +438,6 @@ describe('withOnyx', () => { expect(onRender).toHaveBeenCalledTimes(1); // We can now tell component to update - // console.log(componentInstance); - ref.current.markReadyForHydration(); }) .then(() => { From d689261234c6b25ccff8f8a760fa0f993107432e Mon Sep 17 00:00:00 2001 From: Oscar Franco Date: Mon, 18 Sep 2023 11:32:17 +0200 Subject: [PATCH 14/14] Fix fast merge function --- lib/utils.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 0dd151a67..140057125 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -67,8 +67,7 @@ function mergeObject(target, source) { function fastMerge(target, source) { // lodash adds a small overhead so we don't use it here // eslint-disable-next-line rulesdir/prefer-underscore-method - const array = Array.isArray(source); - if (array) { + if (_.isArray(source) || _.isNull(source) || _.isUndefined(source)) { return source; } return mergeObject(target, source);