Merge pull request #97 from Expensify/marcaaron-setInitialState

Call setState once when handling initial withOnyx data to reduce JS scripting time

lib/Onyx.js

@@ -328,9 +328,7 @@ function keyChanged(key, data, hasNewValue = true) {
  */
 function sendDataToConnection(config, val, key) {
     if (config.withOnyxInstance) {
-        config.withOnyxInstance.setState({
-            [config.statePropertyName]: val,
-        });
+        config.withOnyxInstance.setInitialState(config.statePropertyName, val);
     } else if (_.isFunction(config.callback)) {
         config.callback(val, key);
     }

lib/withOnyx.js

@@ -20,15 +20,26 @@ function getDisplayName(component) {
 }
 
 export default function (mapOnyxToState) {
+    // 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)
+        .keys()
+        .value();
+
     return (WrappedComponent) => {
         class withOnyx extends React.Component {
             constructor(props) {
                 super(props);
 
+                this.setInitialState = this.setInitialState.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 = {};
 
+                // Object holding the temporary initial state for the component while we load the various Onyx keys
+                this.tempState = {};
+
                 this.state = {
                     loading: true,
                 };
@@ -39,7 +50,7 @@ export default function (mapOnyxToState) {
                 _.each(mapOnyxToState, (mapping, propertyName) => {
                     this.connectMappingToOnyx(mapping, propertyName);
                 });
-                this.checkAndUpdateLoading();
+                this.checkEvictableKeys();
             }
 
             componentDidUpdate(prevProps) {
@@ -55,7 +66,7 @@ export default function (mapOnyxToState) {
                         this.connectMappingToOnyx(mapping, propertyName);
                     }
                 });
-                this.checkAndUpdateLoading();
+                this.checkEvictableKeys();
             }
 
             componentWillUnmount() {
@@ -67,12 +78,37 @@ export default function (mapOnyxToState) {
                 });
             }
 
+            /**
+             * 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()
+             * only called once all the necessary data has been collected.
+             *
+             * @param {String} statePropertyName
+             * @param {*} val
+             */
+            setInitialState(statePropertyName, val) {
+                if (!this.state.loading) {
+                    console.error('withOnyx.setInitialState() called after loading: false');
+                    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]))) {
+                    return;
+                }
+
+                this.setState({...this.tempState, loading: false});
+                delete this.tempState;
+            }
+
             /**
              * Makes sure each Onyx key we requested has been set to state with a value of some kind.
              * We are doing this so that the wrapped component will only render when all the data
              * it needs is available to it.
              */
-            checkAndUpdateLoading() {
+            checkEvictableKeys() {
                 // We will add this key to our list of recently accessed keys
                 // if the canEvict function returns true. This is necessary criteria
                 // we MUST use to specify if a key can be removed or not.
@@ -95,24 +131,6 @@ export default function (mapOnyxToState) {
                         Onyx.addToEvictionBlockList(key, mapping.connectionID);
                     }
                 });
-
-                if (!this.state.loading) {
-                    return;
-                }
-
-                // Filter all keys by those which we do want to init with stored values
-                // since keys that are configured to not init with stored values will
-                // never appear on state when the component mounts - only after they update
-                // organically.
-                const requiredKeysForInit = _.chain(mapOnyxToState)
-                    .omit(config => config.initWithStoredValues === false)
-                    .keys()
-                    .value();
-
-                // All state keys should exist and at least have a value of null
-                if (_.every(requiredKeysForInit, key => !_.isUndefined(this.state[key]))) {
-                    this.setState({loading: false});
-                }
             }
 
             /**

tests/unit/withOnyxTest.js

@@ -68,9 +68,13 @@ describe('withOnyx', () => {
         })(ViewWithCollections);
         const onRender = jest.fn();
         render(<TestComponentWithOnyx onRender={onRender} />);
-
-        Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, test_2: {ID: 234}, test_3: {ID: 345}});
         return waitForPromisesToResolve()
+            .then(() => {
+                Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+                    test_1: {ID: 123}, test_2: {ID: 234}, test_3: {ID: 345},
+                });
+                return waitForPromisesToResolve();
+            })
             .then(() => {
                 expect(onRender.mock.calls.length).toBe(2);
             });
@@ -85,9 +89,13 @@ describe('withOnyx', () => {
         })(ViewWithCollections);
         const onRender = jest.fn();
         render(<TestComponentWithOnyx onRender={onRender} />);
-
-        Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, test_2: {ID: 234}, test_3: {ID: 345}});
         return waitForPromisesToResolve()
+            .then(() => {
+                Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+                    test_1: {ID: 123}, test_2: {ID: 234}, test_3: {ID: 345},
+                });
+                return waitForPromisesToResolve();
+            })
             .then(() => {
                 expect(onRender.mock.calls.length).toBe(2);
             });
@@ -102,13 +110,16 @@ describe('withOnyx', () => {
         })(ViewWithCollections);
         const onRender = jest.fn();
         render(<TestComponentWithOnyx onRender={onRender} />);
-        Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_4: {ID: 456}, test_5: {ID: 567}});
-        Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
-            test_4: {Name: 'Test4'},
-            test_5: {Name: 'Test5'},
-            test_6: {ID: 678, Name: 'Test6'},
-        });
         return waitForPromisesToResolve()
+            .then(() => {
+                Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_4: {ID: 456}, test_5: {ID: 567}});
+                Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+                    test_4: {Name: 'Test4'},
+                    test_5: {Name: 'Test5'},
+                    test_6: {ID: 678, Name: 'Test6'},
+                });
+                return waitForPromisesToResolve();
+            })
             .then(() => {
                 expect(onRender.mock.calls.length).toBe(3);
                 expect(onRender.mock.instances[2].text).toEqual({ID: 456, Name: 'Test4'});