Fix buggy async callbacks (#360)

City-of-Helsinki · Sep 16, 2024 · 3844b48 · 3844b48
1 parent 955f34d
commit 3844b48
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 11 deletions.
diff --git a/src/components/opening-period-accordion/OpeningPeriodAccordion.tsx b/src/components/opening-period-accordion/OpeningPeriodAccordion.tsx
@@ -250,11 +250,11 @@ const OpeningPeriodAccordion = ({
           </button>
         </div>
         <ConfirmationModal
-          onConfirm={async (): Promise<void> => {
+          onConfirm={() => {
             if (onDelete) {
               setDeleting(true);
               try {
-                await onDelete();
+                onDelete();
                 setDeleting(false);
                 toast.success({
                   label: t('ResourcePage.Notifications.PeriodRemoveSuccess', {

diff --git a/src/pages/ResourceBatchUpdatePage.tsx b/src/pages/ResourceBatchUpdatePage.tsx
@@ -264,6 +264,8 @@ const ResourceBatchUpdatePage = ({
         const targetResourceIDs = targetResourcesString.split(',');
 
         const handleApiResponse = async (resources: Resource[]) => {
+          if (!isMounted) return;
+
           // for some reason origins are not part of Resource type, this need to be fixed in API
           const resourcesWithOrigins = resources as ResourceWithOrigins[];
           const targetResources = targetResourceIDs
@@ -293,10 +295,8 @@ const ResourceBatchUpdatePage = ({
             key: targetResourcesStorageKey,
             value: newData,
           });
-          if (isMounted) {
-            setTargetResourceData(newData);
-            setLoading(false);
-          }
+          setTargetResourceData(newData);
+          setLoading(false);
         };
 
         // fetch target resource data from api
@@ -337,10 +337,10 @@ const ResourceBatchUpdatePage = ({
     api
       .getResource(mainResourceId)
       .then(async (r: Resource) => {
-        if (isMounted) {
-          setResource(r);
-          setLoading(false);
-        }
+        if (!isMounted) return;
+
+        setResource(r);
+        setLoading(false);
       })
       .catch((e: Error) => {
         setLoading(false);

diff --git a/src/pages/ResourcePage.tsx b/src/pages/ResourcePage.tsx
@@ -123,12 +123,16 @@ const ResourcePage = ({
     }
   }, [language, resource, targetResourcesString]);
 
-  useEffect((): void => {
+  useEffect(() => {
+    let isMounted = true;
+
     // UseEffect's callbacks are synchronous to prevent a race condition.
     // We can not use an async function as an useEffect's callback because it would return Promise<void>
     api
       .getResource(mainResourceId)
       .then(async (r: Resource) => {
+        if (!isMounted) return;
+
         setResource(r);
         const resourceHasChildren = r.children.length > 0;
         const resourceHasParents = r.parents.length > 0;
@@ -149,6 +153,10 @@ const ResourcePage = ({
         setError(e);
         setLoading(false);
       });
+
+    return () => {
+      isMounted = false;
+    };
   }, [mainResourceId]);
 
   const gotoBatchUpdatePage = (): void => {