feat(core): Add comments for improvements

packages/core/src/SorobanReactProvider.tsx

@@ -78,12 +78,18 @@ export function SorobanReactProvider({
     disconnect: async () => {
       isConnectedRef.current = false;
       // TODO: Maybe reset address to undefined
+      // TODO: Handle other things here, such as perhaps resetting address to undefined.
     }
   });
 
   // Handle changes of address/network in "realtime"
   React.useEffect(() => {
     let timeoutId: NodeJS.Timer | null = null;
+
+    // If it turns out that requesting an update from Freighter is too taxing,
+    // then this could be increased. Humans perceive 100ms response times as instantaneous
+    // (source: https://www.pubnub.com/blog/how-fast-is-realtime-human-perception-and-technology/)
+    // but you also have to consider the re-render time of components.
     const freighterCheckIntervalMs = 200;
 
     async function checkForWalletChanges () {
@@ -93,8 +99,16 @@ export function SorobanReactProvider({
 
       try {
         let chain = networkToActiveChain(await mySorobanContext.activeConnector?.getNetworkDetails(), chains)
+
+        // NOTICE: If the user logs out from or uninstalls the Freighter extension while they are connected
+        // on this site, then a dialog will appear asking them to sign in again. We need a way to ask Freighter
+        // if there is _any_ connected user, without actually asking them to sign in. Unfortunately, that is not
+        // supported at this time; but it would be easy to submit a PR to the extension to add support for it.
         let address = await mySorobanContext.activeConnector?.getPublicKey();
 
+        // TODO: If you want to know when the user has disconnected, then you can set a timeout for getPublicKey.
+        // If it doesn't return in X milliseconds, you can be pretty confident that they aren't connected anymore.
+
         if (mySorobanContext.address !== address) {
           console.log("SorobanReactProvider: address changed from:", mySorobanContext.address," to: ", address);
           hasNoticedWalletUpdate = true;
@@ -113,6 +127,10 @@ export function SorobanReactProvider({
           mySorobanContext.connect();
         }
       } catch (error) {
+        // I would recommend keeping the try/catch so that any exceptions in this async function
+        // will get handled. Otherwise React could complain. I believe that eventually it may cause huge
+        // problems, but that might be a NodeJS specific approach to exceptions not handled in promises.
+
         console.error("SorobanReactProvider: error: ", error);
       } finally {
         if (!hasNoticedWalletUpdate) timeoutId = setTimeout(checkForWalletChanges, freighterCheckIntervalMs);
@@ -127,6 +145,13 @@ export function SorobanReactProvider({
   }, [mySorobanContext]);
 
   React.useEffect(() => {
+    // TODO: When the page loads and the user is not signed in, this gets called twice
+    // (due to the sorobanContext.activeWallet being seen as different by React), which causes
+    // the Freighter window to appear twice.
+    // I think an easy approach will be to use a ref in the connect function so that if it's already
+    // trying to connect from somewhere else, then it doesn't try again
+    // (since getPublicKey is what is causing the popup to appear)
+
     console.log("Something changing... in SorobanReactProvider.tsx")
     if (mySorobanContext.address) return;
     if (!mySorobanContext.activeConnector) return;