diff --git a/lib/Onyx.js b/lib/Onyx.js index f992b57b..014e1565 100644 --- a/lib/Onyx.js +++ b/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); } diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 7641d75c..834e1ad9 100644 --- a/lib/withOnyx.js +++ b/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}); - } } /** diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 6053bac4..509cb5ff 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -68,9 +68,13 @@ describe('withOnyx', () => { })(ViewWithCollections); const onRender = jest.fn(); render(); - - 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(); - - 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(); - 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'});