From 39c7051b64b40e6d3d52b5bdf7bc24fdf41ab1c4 Mon Sep 17 00:00:00 2001 From: Dennis Kigen Date: Wed, 11 Dec 2024 18:19:39 +0300 Subject: [PATCH] (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. --- .../ui-select-extended.component.tsx | 57 +++++++++++++------ src/hooks/useEncounter.ts | 10 +++- src/hooks/useFormJson.ts | 12 +++- src/hooks/usePatientPrograms.ts | 16 +++++- src/hooks/useProcessorDependencies.ts | 18 ++++-- 5 files changed, 85 insertions(+), 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 index bd8df9e65..a7b721f29 100644 --- 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 = ({ field, errors, warnings, setFieldValue }) => { const { t } = useTranslation(); @@ -49,6 +49,7 @@ const UiSelectExtended: React.FC = ({ field, errors, warnin const debouncedSearch = debounce((searchTerm: string, dataSource: DataSource) => { setIsSearching(true); + dataSource .fetchData(searchTerm, config) .then((dataItems) => { @@ -86,22 +87,33 @@ const UiSelectExtended: React.FC = ({ 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 = ({ 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 index c030283a0..0ec89b2c1 100644 --- 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 index 3b995de09..e52b1134b 100644 --- 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 index 45ce5dbe3..6386e55bd 100644 --- 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 index 78848b90b..c31f68d7a 100644 --- 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 };