OpenNeo/impress-2020
OpenNeo/impress-2020
1
0
Fork 0
Code Pull requests Activity

Fix infinite loop bug on initial outfit save

Browse source 
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.)
This commit is contained in:
Emi Matchu 2021-04-28 16:08:31 -07:00
parent 1d97988383
commit 995a2b8a3a
2 changed files with 22 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/app/WardrobePage/ItemsPanel.js
View file

@ -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

13
src/app/util.js
View file

@ -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;
}
/**