Use a simple subscription ID

Fixes #11
Expensify · Aug 9, 2020 · 67ee483 · 67ee483
1 parent 2f8016b
commit 67ee483
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 13 deletions.
diff --git a/src/components/WithStoreSubscribeToState.js b/src/components/WithStoreSubscribeToState.js
@@ -12,7 +12,7 @@ export default function (mapStoreToStates) {
         constructor(props) {
             super(props);
 
-            this.subscribedEventGuids = [];
+            this.subscriptionIDs = [];
 
             // Initialize the state with each of our property names
             this.state = _.reduce(_.keys(mapStoreToStates), (finalResult, propertyName) => ({
@@ -22,14 +22,16 @@ export default function (mapStoreToStates) {
         }
 
         componentDidMount() {
-            this.subscribedEventGuids = _.reduce(mapStoreToStates, (finalResult, mapStoreToState, propertyName) => {
+            // Subscribe each of the state properties to the proper store key
+            this.subscriptionIDs = _.reduce(mapStoreToStates, (finalResult, mapStoreToState, propertyName) => {
                 const {key, path} = mapStoreToState;
                 return [
                     ...finalResult,
                     Store.subscribeToState(key, propertyName, path, null, this.wrappedComponent),
                 ];
             }, []);
 
+            // Call any loaders that will fill the store with their initial data
             _.each(mapStoreToStates, (mapStoreToState) => {
                 if (mapStoreToState.loader) {
                     mapStoreToState.loader(...mapStoreToState.loaderParams || []);
@@ -38,10 +40,11 @@ export default function (mapStoreToStates) {
         }
 
         componentWillUnmount() {
-            _.each(this.subscribedEventGuids, Store.unsubscribeFromState);
+            _.each(this.subscriptionIDs, Store.unsubscribeFromState);
         }
 
         render() {
+            // Spreading props and state is necessary in an HOC where the data cannot be predicted
             // eslint-disable-next-line react/jsx-props-no-spreading
             return <WrappedComponent {...this.props} {...this.state} ref={el => this.wrappedComponent = el} />;
         }

diff --git a/src/store/Store.js b/src/store/Store.js
@@ -6,6 +6,9 @@ import Guid from '../lib/Guid';
 // Holds all of the callbacks that have registered for a specific key pattern
 const callbackMapping = {};
 
+// Keeps track of the last subscription ID that was used
+let lastSubscriptionID = 0;
+
 /**
  * Initialize the store with actions and listening for storage events
  */
@@ -47,9 +50,7 @@ function subscribe(keyPattern, cb) {
     callbackMapping[keyPattern].push(cb);
 }
 
-/**
- * Holds a mapping of all the react components that want their state subscribed to a store key
- */
+// Holds a mapping of all the react components that want their state subscribed to a store key
 const callbackToStateMapping = {};
 
 /**
@@ -63,27 +64,27 @@ const callbackToStateMapping = {};
  * @returns {string} a guid to use when unsubscribing
  */
 function subscribeToState(keyPattern, statePropertyName, path, defaultValue, reactComponent) {
-    const guid = Guid();
-    callbackToStateMapping[guid] = {
+    const subscriptionID = lastSubscriptionID++;
+    callbackToStateMapping[subscriptionID] = {
         keyPattern,
         statePropertyName,
         path,
         reactComponent,
         defaultValue,
     };
-    return guid;
+    return subscriptionID;
 }
 
 /**
  * Remove the listener for a react component
  *
- * @param {string} guid
+ * @param {string} subscriptionID
  */
-function unsubscribeFromState(guid) {
-    if (!callbackToStateMapping[guid]) {
+function unsubscribeFromState(subscriptionID) {
+    if (!callbackToStateMapping[subscriptionID]) {
         return;
     }
-    delete callbackToStateMapping[guid];
+    delete callbackToStateMapping[subscriptionID];
 }
 
 /**