From c43eef89ad03105afb11dcecd290bd74abc9b11e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 11 Sep 2020 17:34:21 -0700 Subject: [PATCH 01/10] Dont render a component unless we have attempted to load props --- src/components/withIon.js | 36 ++++++++++++++++++++++++++++++------ src/lib/Ion.js | 15 ++++++--------- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index c8528dae7c78e..63b592083800f 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -33,6 +33,10 @@ export default function (mapIonToState) { // this.props. These are stored differently because anytime the props change, the component has to be // reconnected to Ion with the new props. this.activeConnectionIDsWithPropsData = {}; + + this.state = { + loading: true, + }; } componentDidMount() { @@ -63,6 +67,12 @@ export default function (mapIonToState) { _.each(this.activeConnectionIDsWithPropsData, Ion.disconnect); } + propsDidLoad() { + if (_.size(this.actionConnectionIDs) + _.size(this.activeConnectionIDsWithPropsData) === _.size(mapIonToState)) { + this.setState({loading: false}); + } + } + /** * Takes a single mapping and binds the state of the component to the store * @@ -86,8 +96,6 @@ export default function (mapIonToState) { withIonInstance: this, }; - let connectionID; - // Connect to Ion and keep track of the connectionID if (mapping.pathForProps) { // If there is a path for props data, then the data needs to be pulled out of props and parsed @@ -98,15 +106,31 @@ export default function (mapIonToState) { // Store the connectionID with a key that is unique to the data coming from the props which allows // it to be easily reconnected to when the props change - connectionID = Ion.connect(ionConnectionConfig); - this.activeConnectionIDsWithPropsData[mapping.pathForProps] = connectionID; + Ion.connect(ionConnectionConfig) + .then(connectionID => { + this.activeConnectionIDsWithPropsData[mapping.pathForProps] = connectionID; + this.propsDidLoad(); + }); } else { - connectionID = Ion.connect(ionConnectionConfig); - this.actionConnectionIDs[connectionID] = connectionID; + Ion.connect(ionConnectionConfig) + .then(connectionID => { + this.actionConnectionIDs[connectionID] = connectionID; + this.propsDidLoad(); + }); } } render() { + if (this.state.loading) { + return null; + } + + const stateToPass = { + ...this.state, + }; + + delete stateToPass.loading; + // Spreading props and state is necessary in an HOC where the data cannot be predicted return ( { // Find all the keys matched by the config regex const matchingKeys = _.filter(keys, config.regex.test.bind(config.regex)); @@ -139,20 +139,17 @@ function connect(mapping) { // React components or callbacks should only have a single data change published // to them. if (config.indexBy) { - Promise.all(_.map(matchingKeys, key => get(key))) + return Promise.all(_.map(matchingKeys, key => get(key))) .then(values => _.reduce(values, (finalObject, value) => ({ ...finalObject, [value[config.indexBy]]: value, }), {})) .then(val => sendDataToConnection(config, val)); - } else { - _.each(matchingKeys, (key) => { - get(key).then(val => sendDataToConnection(config, val, key)); - }); } - }); - return connectionID; + return Promise.all(_.map(matchingKeys, key => get(key).then(val => sendDataToConnection(config, val, key)))); + }) + .then(() => connectionID); } /** From eee4a9e398c93d4f9f4cb968a6332b55b848281e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 11 Sep 2020 18:07:23 -0700 Subject: [PATCH 02/10] fix style --- src/components/withIon.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index 63b592083800f..fd6e0a4e60680 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -67,10 +67,22 @@ export default function (mapIonToState) { _.each(this.activeConnectionIDsWithPropsData, Ion.disconnect); } - propsDidLoad() { - if (_.size(this.actionConnectionIDs) + _.size(this.activeConnectionIDsWithPropsData) === _.size(mapIonToState)) { - this.setState({loading: false}); + /** + * Compares the total number of connections with the total + * number of Ion keys mapped to state. When we have them all + * we are ready to render the component. + */ + checkAndUpdateLoading() { + const totalConnectionSize = _.size({ + ...this.actionConnectionIDs, + ...this.activeConnectionIDsWithPropsData + }); + + if (totalConnectionSize !== _.size(mapIonToState)) { + return; } + + this.setState({loading: false}); } /** @@ -107,15 +119,15 @@ export default function (mapIonToState) { // Store the connectionID with a key that is unique to the data coming from the props which allows // it to be easily reconnected to when the props change Ion.connect(ionConnectionConfig) - .then(connectionID => { + .then((connectionID) => { this.activeConnectionIDsWithPropsData[mapping.pathForProps] = connectionID; - this.propsDidLoad(); + this.checkAndUpdateLoading(); }); } else { Ion.connect(ionConnectionConfig) - .then(connectionID => { + .then((connectionID) => { this.actionConnectionIDs[connectionID] = connectionID; - this.propsDidLoad(); + this.checkAndUpdateLoading(); }); } } From 7c38c895c5628143fb9250cbf5b6dca9ddeedc3e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 14:21:07 -0700 Subject: [PATCH 03/10] Do not make get a promise + add check to componentDidUpdate --- src/components/withIon.js | 39 +++++++++++---------------------------- src/lib/Ion.js | 16 ++++++++++------ 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index fd6e0a4e60680..bcf7a2d7e324c 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -59,6 +59,11 @@ export default function (mapIonToState) { } } }); + + if (this.state.loading && _.every(_.keys(mapIonToState), (key) => this.state[key])) { + // Make sure each Ion key we requested has been set to state with a value of some kind + this.setState({loading: false}); + } } componentWillUnmount() { @@ -67,24 +72,6 @@ export default function (mapIonToState) { _.each(this.activeConnectionIDsWithPropsData, Ion.disconnect); } - /** - * Compares the total number of connections with the total - * number of Ion keys mapped to state. When we have them all - * we are ready to render the component. - */ - checkAndUpdateLoading() { - const totalConnectionSize = _.size({ - ...this.actionConnectionIDs, - ...this.activeConnectionIDsWithPropsData - }); - - if (totalConnectionSize !== _.size(mapIonToState)) { - return; - } - - this.setState({loading: false}); - } - /** * Takes a single mapping and binds the state of the component to the store * @@ -108,6 +95,8 @@ export default function (mapIonToState) { withIonInstance: this, }; + let connectionID; + // Connect to Ion and keep track of the connectionID if (mapping.pathForProps) { // If there is a path for props data, then the data needs to be pulled out of props and parsed @@ -118,17 +107,11 @@ export default function (mapIonToState) { // Store the connectionID with a key that is unique to the data coming from the props which allows // it to be easily reconnected to when the props change - Ion.connect(ionConnectionConfig) - .then((connectionID) => { - this.activeConnectionIDsWithPropsData[mapping.pathForProps] = connectionID; - this.checkAndUpdateLoading(); - }); + connectionID = Ion.connect(ionConnectionConfig); + this.activeConnectionIDsWithPropsData[mapping.pathForProps] = connectionID; } else { - Ion.connect(ionConnectionConfig) - .then((connectionID) => { - this.actionConnectionIDs[connectionID] = connectionID; - this.checkAndUpdateLoading(); - }); + connectionID = Ion.connect(ionConnectionConfig); + this.actionConnectionIDs[connectionID] = connectionID; } } diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 8e2f92bd12e92..1cb08bc108d0f 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -117,11 +117,11 @@ function connect(mapping) { callbackToStateMapping[connectionID] = config; if (mapping.initWithStoredValues === false) { - return Promise.resolve(connectionID); + connectionID; } // Get all the data from Ion to initialize the connection with - return AsyncStorage.getAllKeys() + AsyncStorage.getAllKeys() .then((keys) => { // Find all the keys matched by the config regex const matchingKeys = _.filter(keys, config.regex.test.bind(config.regex)); @@ -139,17 +139,21 @@ function connect(mapping) { // React components or callbacks should only have a single data change published // to them. if (config.indexBy) { - return Promise.all(_.map(matchingKeys, key => get(key))) + Promise.all(_.map(matchingKeys, key => get(key))) .then(values => _.reduce(values, (finalObject, value) => ({ ...finalObject, [value[config.indexBy]]: value, }), {})) .then(val => sendDataToConnection(config, val)); + return; } - return Promise.all(_.map(matchingKeys, key => get(key).then(val => sendDataToConnection(config, val, key)))); - }) - .then(() => connectionID); + _.each(matchingKeys, (key) => { + get(key).then(val => sendDataToConnection(config, val, key)); + }); + }); + + return connectionID; } /** From f8c01e7bd228fbbd11758f07625f2f04836c67a8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 14:22:03 -0700 Subject: [PATCH 04/10] revert some other things --- src/lib/Ion.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 1cb08bc108d0f..38abbc1f19442 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -117,7 +117,7 @@ function connect(mapping) { callbackToStateMapping[connectionID] = config; if (mapping.initWithStoredValues === false) { - connectionID; + return connectionID; } // Get all the data from Ion to initialize the connection with @@ -146,11 +146,11 @@ function connect(mapping) { }), {})) .then(val => sendDataToConnection(config, val)); return; + } else { + _.each(matchingKeys, (key) => { + get(key).then(val => sendDataToConnection(config, val, key)); + }); } - - _.each(matchingKeys, (key) => { - get(key).then(val => sendDataToConnection(config, val, key)); - }); }); return connectionID; From 89e0099e34cabc677fef8cfcb53ce0c8864f071c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 14:23:56 -0700 Subject: [PATCH 05/10] use omit --- src/components/withIon.js | 10 ++++------ src/lib/Ion.js | 1 - 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index bcf7a2d7e324c..f61085bce7adf 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -120,11 +120,9 @@ export default function (mapIonToState) { return null; } - const stateToPass = { - ...this.state, - }; - - delete stateToPass.loading; + // Remove any internal state properties used by withIon + // that should not be passed to a wrapped component. + const stateToPass = _.omit(this.state, 'loading'); // Spreading props and state is necessary in an HOC where the data cannot be predicted return ( @@ -132,7 +130,7 @@ export default function (mapIonToState) { // eslint-disable-next-line react/jsx-props-no-spreading {...this.props} // eslint-disable-next-line react/jsx-props-no-spreading - {...this.state} + {...stateToPass} /> ); } diff --git a/src/lib/Ion.js b/src/lib/Ion.js index 38abbc1f19442..898c6047e42c1 100644 --- a/src/lib/Ion.js +++ b/src/lib/Ion.js @@ -145,7 +145,6 @@ function connect(mapping) { [value[config.indexBy]]: value, }), {})) .then(val => sendDataToConnection(config, val)); - return; } else { _.each(matchingKeys, (key) => { get(key).then(val => sendDataToConnection(config, val, key)); From 7dbce07d0da7975b864c91eb3127da3e2180a838 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 14:26:13 -0700 Subject: [PATCH 06/10] add comment --- src/components/withIon.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index f61085bce7adf..db53df49d8908 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -60,8 +60,10 @@ export default function (mapIonToState) { } }); - if (this.state.loading && _.every(_.keys(mapIonToState), (key) => this.state[key])) { - // Make sure each Ion key we requested has been set to state with a value of some kind + // Make sure each Ion 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. + if (this.state.loading && _.every(_.keys(mapIonToState), key => this.state[key])) { this.setState({loading: false}); } } From 1c092ca29def581983624238361f7a626a42e534 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 15:06:01 -0700 Subject: [PATCH 07/10] Fix the update method --- src/components/withIon.js | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index db53df49d8908..a74a248d4b198 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -44,6 +44,7 @@ export default function (mapIonToState) { _.each(mapIonToState, (mapping, propertyName) => { this.connectMappingToIon(mapping, propertyName, this.wrappedComponent); }); + this.checkAndUpdateLoading(); } componentDidUpdate(prevProps) { @@ -59,13 +60,7 @@ export default function (mapIonToState) { } } }); - - // Make sure each Ion 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. - if (this.state.loading && _.every(_.keys(mapIonToState), key => this.state[key])) { - this.setState({loading: false}); - } + this.checkAndUpdateLoading(); } componentWillUnmount() { @@ -74,6 +69,29 @@ export default function (mapIonToState) { _.each(this.activeConnectionIDsWithPropsData, Ion.disconnect); } + /** + * Makes sure each Ion 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() { + if (this.state.loading) { + const requiredKeysForInit = _.chain(mapIonToState) + .reduce((prev, curr, key) => { + if (curr.initWithStoredValues === false) { + return prev; + } + return ({...prev, [key]: curr}); + }, {}) + .keys() + .value(); + + if (_.every(requiredKeysForInit, key => !_.isUndefined(this.state[key]))) { + this.setState({loading: false}); + } + } + } + /** * Takes a single mapping and binds the state of the component to the store * From 976d5a5a5463fb1d6a097a80913d7134e60f9621 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 15:08:56 -0700 Subject: [PATCH 08/10] add comments --- src/components/withIon.js | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index a74a248d4b198..9b7e7a4054e24 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -75,20 +75,27 @@ export default function (mapIonToState) { * it needs is available to it. */ checkAndUpdateLoading() { - if (this.state.loading) { - const requiredKeysForInit = _.chain(mapIonToState) - .reduce((prev, curr, key) => { - if (curr.initWithStoredValues === false) { - return prev; - } - return ({...prev, [key]: curr}); - }, {}) - .keys() - .value(); - - if (_.every(requiredKeysForInit, key => !_.isUndefined(this.state[key]))) { - this.setState({loading: false}); - } + if (!this.state.loading) { + return; + } + + // Filter all keys by those which we do want to init with stored values + // since key's 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(mapIonToState) + .reduce((prev, curr, key) => { + if (curr.initWithStoredValues === false) { + return prev; + } + return ({...prev, [key]: curr}); + }, {}) + .keys() + .value(); + + // All state keys should must exist and should at least be null + if (_.every(requiredKeysForInit, key => !_.isUndefined(this.state[key]))) { + this.setState({loading: false}); } } From 6183981f523da4b2396abe23e199d35aec1308ed Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 15:11:01 -0700 Subject: [PATCH 09/10] fix bad english --- src/components/withIon.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index 9b7e7a4054e24..9b5c1dace0ad5 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -80,7 +80,7 @@ export default function (mapIonToState) { } // Filter all keys by those which we do want to init with stored values - // since key's that are configured to not init with stored values will + // 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(mapIonToState) @@ -93,7 +93,7 @@ export default function (mapIonToState) { .keys() .value(); - // All state keys should must exist and should at least be null + // 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}); } @@ -148,7 +148,7 @@ export default function (mapIonToState) { } // Remove any internal state properties used by withIon - // that should not be passed to a wrapped component. + // that should not be passed to a wrapped component const stateToPass = _.omit(this.state, 'loading'); // Spreading props and state is necessary in an HOC where the data cannot be predicted From 42f3747d8de9e868cf21eabd56f82797da3d8eba Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 14 Sep 2020 16:45:36 -0700 Subject: [PATCH 10/10] Use chirags trick --- src/components/withIon.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/components/withIon.js b/src/components/withIon.js index 9b5c1dace0ad5..6c83857d0ca1d 100644 --- a/src/components/withIon.js +++ b/src/components/withIon.js @@ -84,12 +84,7 @@ export default function (mapIonToState) { // never appear on state when the component mounts - only after they update // organically. const requiredKeysForInit = _.chain(mapIonToState) - .reduce((prev, curr, key) => { - if (curr.initWithStoredValues === false) { - return prev; - } - return ({...prev, [key]: curr}); - }, {}) + .omit(config => config.initWithStoredValues === false) .keys() .value();