From 995a2b8a3a709503f79469d86104cf4bbf7888fe Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@gmail.com>
Date: Wed, 28 Apr 2021 16:08:31 -0700
Subject: [PATCH] Fix infinite loop bug on initial outfit save

Oops, the sequence here was:
1) Save a new outfit
2) The debounced outfit state still contains id=null, which doesn't match the saved outfit, which triggers an auto-save
3) And now again, the debounced outfit state contains the _previous_ saved outfit ID, but the saved outfit has a _new_ ID, so we save the _previous_ outfit again

and back and forth forever.

Right, ok, simple change: if the saved outfit ID changes, reset the debounced state immediately, so it can't even be out of sync in the first place! (I also considered checking it in the condition, but I didn't really understand what the timing properties of being out of sync due to debouncing would be, and it seemed to not represent the reality I want.)
---
 src/app/WardrobePage/ItemsPanel.js | 20 ++++++++++++++------
 src/app/util.js                    | 13 ++++++++-----
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/app/WardrobePage/ItemsPanel.js b/src/app/WardrobePage/ItemsPanel.js
index a89a93c..d9fbf00 100644
--- a/src/app/WardrobePage/ItemsPanel.js
+++ b/src/app/WardrobePage/ItemsPanel.js
@@ -367,11 +367,14 @@ function useOutfitSaving(outfitState, dispatchToOutfit) {
         // Also, send a `reset` action, to show whatever the server returned.
         // This is important for suffix changes to `name`, but can also be
         // relevant for graceful failure when a bug causes a change not to
-        // persist.
-        dispatchToOutfit({
-          type: "resetToSavedOutfitData",
-          savedOutfitData: outfit,
-        });
+        // persist. (But don't do it if it's not the current outfit anymore,
+        // we don't want laggy mutations to reset the outfit!)
+        if (outfit.id === outfitState.id) {
+          dispatchToOutfit({
+            type: "resetToSavedOutfitData",
+            savedOutfitData: outfit,
+          });
+        }
       },
     }
   );
@@ -423,7 +426,12 @@ function useOutfitSaving(outfitState, dispatchToOutfit) {
   // than the saved state.
   const debouncedOutfitState = useDebounce(
     outfitState.outfitStateWithoutExtras,
-    2000
+    2000,
+    {
+      // When the outfit ID changes, update the debounced state immediately!
+      forceReset: (debouncedOutfitState, newOutfitState) =>
+        debouncedOutfitState.id !== newOutfitState.id,
+    }
   );
   // HACK: This prevents us from auto-saving the outfit state that's still
   //       loading. I worry that this might not catch other loading scenarios
diff --git a/src/app/util.js b/src/app/util.js
index 1810467..cc1c4ce 100644
--- a/src/app/util.js
+++ b/src/app/util.js
@@ -164,7 +164,7 @@ export function safeImageUrl(urlString) {
 export function useDebounce(
   value,
   delay,
-  { waitForFirstPause = false, initialValue = null, forceReset = false } = {}
+  { waitForFirstPause = false, initialValue = null, forceReset = null } = {}
 ) {
   // State and setters for debounced value
   const [debouncedValue, setDebouncedValue] = React.useState(
@@ -188,14 +188,17 @@ export function useDebounce(
     [value, delay] // Only re-call effect if value or delay changes
   );
 
-  // The `forceReset` option sets the value immediately!
+  // The `forceReset` option helps us decide whether to set the value
+  // immediately! We'll update it in an effect for consistency and clarity, but
+  // also return it immediately rather than wait a tick.
+  const shouldForceReset = forceReset && forceReset(debouncedValue, value);
   React.useEffect(() => {
-    if (forceReset) {
+    if (shouldForceReset) {
       setDebouncedValue(value);
     }
-  }, [value, forceReset]);
+  }, [shouldForceReset, value]);
 
-  return debouncedValue;
+  return shouldForceReset ? value : debouncedValue;
 }
 
 /**