(fix) Prevent state updates on unmounted components (#446)

This PR fixes potential memory leaks by preventing state updates on unmounted components. It does so by adding cleanup functions to useEffect hook that fetch data. Per the React docs, if your effect fetches some data, the cleanup function should either abort the fetch or ignore its result. This PR adds AbortController to cancel network requests via openmrsFetch. It also uses the ignore flag pattern for custom promises (like loadDependencies) that don't directly use openmrsFetch.
openmrs · Dec 11, 2024 · 39c7051 · 39c7051
1 parent 4e34674
commit 39c7051
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 28 deletions.
diff --git a/src/components/inputs/ui-select-extended/ui-select-extended.component.tsx b/src/components/inputs/ui-select-extended/ui-select-extended.component.tsx
@@ -1,21 +1,21 @@
 import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { debounce } from 'lodash-es';
 import { ComboBox, DropdownSkeleton, Layer, InlineLoading } from '@carbon/react';
-import { isTrue } from '../../../utils/boolean-utils';
 import { useTranslation } from 'react-i18next';
-import { getRegisteredDataSource } from '../../../registry/registry';
+import { useWatch } from 'react-hook-form';
+import { type OpenmrsResource } from '@openmrs/esm-framework';
 import { getControlTemplate } from '../../../registry/inbuilt-components/control-templates';
-import { type DataSource, type FormFieldInputProps } from '../../../types';
+import { getRegisteredDataSource } from '../../../registry/registry';
 import { isEmpty } from '../../../validators/form-validator';
+import { isTrue } from '../../../utils/boolean-utils';
+import { isViewMode } from '../../../utils/common-utils';
 import { shouldUseInlineLayout } from '../../../utils/form-helper';
-import FieldValueView from '../../value/view/field-value-view.component';
-import styles from './ui-select-extended.scss';
+import { type DataSource, type FormFieldInputProps } from '../../../types';
 import { useFormProviderContext } from '../../../provider/form-provider';
-import FieldLabel from '../../field-label/field-label.component';
-import { useWatch } from 'react-hook-form';
 import useDataSourceDependentValue from '../../../hooks/useDataSourceDependentValue';
-import { isViewMode } from '../../../utils/common-utils';
-import { type OpenmrsResource } from '@openmrs/esm-framework';
+import FieldLabel from '../../field-label/field-label.component';
+import FieldValueView from '../../value/view/field-value-view.component';
+import styles from './ui-select-extended.scss';
 
 const UiSelectExtended: React.FC<FormFieldInputProps> = ({ field, errors, warnings, setFieldValue }) => {
   const { t } = useTranslation();
@@ -49,6 +49,7 @@ const UiSelectExtended: React.FC<FormFieldInputProps> = ({ field, errors, warnin
 
   const debouncedSearch = debounce((searchTerm: string, dataSource: DataSource<OpenmrsResource>) => {
     setIsSearching(true);
+
     dataSource
       .fetchData(searchTerm, config)
       .then((dataItems) => {
@@ -86,22 +87,33 @@ const UiSelectExtended: React.FC<FormFieldInputProps> = ({ field, errors, warnin
   }, [field.questionOptions?.datasource]);
 
   useEffect(() => {
+    let ignore = false;
+
     // If not searchable, preload the items
     if (dataSource && !isTrue(field.questionOptions.isSearchable)) {
       setItems([]);
       setIsLoading(true);
+
       dataSource
         .fetchData(null, { ...config, referencedValue: dataSourceDependentValue })
         .then((dataItems) => {
-          setItems(dataItems.map(dataSource.toUuidAndDisplay));
-          setIsLoading(false);
+          if (!ignore) {
+            setItems(dataItems.map(dataSource.toUuidAndDisplay));
+            setIsLoading(false);
+          }
         })
         .catch((err) => {
-          console.error(err);
-          setIsLoading(false);
-          setItems([]);
+          if (!ignore) {
+            console.error(err);
+            setIsLoading(false);
+            setItems([]);
+          }
         });
     }
+
+    return () => {
+      ignore = true;
+    };
   }, [dataSource, config, dataSourceDependentValue]);
 
   useEffect(() => {
@@ -111,19 +123,28 @@ const UiSelectExtended: React.FC<FormFieldInputProps> = ({ field, errors, warnin
   }, [dataSource, searchTerm, config]);
 
   useEffect(() => {
+    let ignore = false;
     if (value && !isDirty && dataSource && isSearchable && sessionMode !== 'enter' && !items.length) {
       // While in edit mode, search-based instances should fetch the initial item (previously selected value) to resolve its display property
       setIsLoading(true);
       try {
         dataSource.fetchSingleItem(value).then((item) => {
-          setItems([dataSource.toUuidAndDisplay(item)]);
-          setIsLoading(false);
+          if (!ignore) {
+            setItems([dataSource.toUuidAndDisplay(item)]);
+            setIsLoading(false);
+          }
         });
       } catch (error) {
-        console.error(error);
-        setIsLoading(false);
+        if (!ignore) {
+          console.error(error);
+          setIsLoading(false);
+        }
       }
     }
+
+    return () => {
+      ignore = true;
+    };
   }, [value, isDirty, sessionMode, dataSource, isSearchable, items]);
 
   if (isLoading) {

diff --git a/src/hooks/useEncounter.ts b/src/hooks/useEncounter.ts
@@ -11,8 +11,12 @@ export function useEncounter(formJson: FormSchema) {
   const [error, setError] = useState(null);
 
   useEffect(() => {
+    const abortController = new AbortController();
+
     if (!isEmpty(formJson.encounter) && isString(formJson.encounter)) {
-      openmrsFetch(`${restBaseUrl}/encounter/${formJson.encounter}?v=${encounterRepresentation}`)
+      openmrsFetch(`${restBaseUrl}/encounter/${formJson.encounter}?v=${encounterRepresentation}`, {
+        signal: abortController.signal,
+      })
         .then((response) => {
           setEncounter(response.data);
           setIsLoading(false);
@@ -26,6 +30,10 @@ export function useEncounter(formJson: FormSchema) {
     } else {
       setIsLoading(false);
     }
+
+    return () => {
+      abortController.abort();
+    };
   }, [formJson.encounter]);
 
   return { encounter: encounter, error, isLoading };

diff --git a/src/hooks/useFormJson.ts b/src/hooks/useFormJson.ts
@@ -11,6 +11,8 @@ export function useFormJson(formUuid: string, rawFormJson: any, encounterUuid: s
   const [error, setError] = useState(validateFormsArgs(formUuid, rawFormJson));
 
   useEffect(() => {
+    const abortController = new AbortController();
+
     const setFormJsonWithTranslations = (formJson: FormSchema) => {
       if (formJson?.translations) {
         const language = window.i18next.language;
@@ -24,9 +26,15 @@ export function useFormJson(formUuid: string, rawFormJson: any, encounterUuid: s
         setFormJsonWithTranslations({ ...formJson, encounter: encounterUuid });
       })
       .catch((error) => {
-        console.error(error);
-        setError(new Error('Error loading form JSON: ' + error.message));
+        if (error.name !== 'AbortError') {
+          console.error(error);
+          setError(new Error('Error loading form JSON: ' + error.message));
+        }
       });
+
+    return () => {
+      abortController.abort();
+    };
   }, [formSessionIntent, formUuid, rawFormJson, encounterUuid]);
 
   return {

diff --git a/src/hooks/usePatientPrograms.ts b/src/hooks/usePatientPrograms.ts
@@ -9,19 +9,29 @@ export const usePatientPrograms = (patientUuid: string, formJson: FormSchema) =>
   const [error, setError] = useState(null);
 
   useEffect(() => {
+    const abortController = new AbortController();
+
     if (formJson.meta?.programs?.hasProgramFields) {
-      openmrsFetch(`${restBaseUrl}/programenrollment?patient=${patientUuid}&v=${customRepresentation}`)
+      openmrsFetch(`${restBaseUrl}/programenrollment?patient=${patientUuid}&v=${customRepresentation}`, {
+        signal: abortController.signal,
+      })
         .then((response) => {
           setPatientPrograms(response.data.results.filter((enrollment) => enrollment.dateCompleted === null));
           setIsLoading(false);
         })
         .catch((error) => {
-          setError(error);
-          setIsLoading(false);
+          if (error.name !== 'AbortError') {
+            setError(error);
+            setIsLoading(false);
+          }
         });
     } else {
       setIsLoading(false);
     }
+
+    return () => {
+      abortController.abort();
+    };
   }, [formJson]);
 
   return {

diff --git a/src/hooks/useProcessorDependencies.ts b/src/hooks/useProcessorDependencies.ts
@@ -13,17 +13,27 @@ const useProcessorDependencies = (
   const { loadDependencies } = formProcessor;
 
   useEffect(() => {
+    let ignore = false;
+
     if (loadDependencies) {
       setIsLoading(true);
       loadDependencies(context, setContext)
-        .then((results) => {
-          setIsLoading(false);
+        .then(() => {
+          if (!ignore) {
+            setIsLoading(false);
+          }
         })
         .catch((error) => {
-          setError(error);
-          reportError(error, 'Encountered error while loading dependencies');
+          if (!ignore) {
+            setError(error);
+            reportError(error, 'Encountered error while loading dependencies');
+          }
         });
     }
+
+    return () => {
+      ignore = true;
+    };
   }, [loadDependencies]);
 
   return { isLoading, error };