Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(fix) Fix react hooks exhaustive deps warnings #1380

Merged
merged 2 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint", "import", "jest-dom", "react-hooks", "testing-library"],
"rules": {
"import/no-duplicates": "error",
"react-hooks/rules-of-hooks": "error",
// Disabling these rules for now just to keep the diff small. We'll enable them one by one as we go.
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/ban-types": "off",
Expand All @@ -32,7 +30,7 @@
"fixStyle": "inline-type-imports"
}
],
"prefer-const": "off",
"import/no-duplicates": "error",
"no-console": [
"error",
{
Expand Down Expand Up @@ -69,7 +67,10 @@
}
]
}
]
],
"prefer-const": "off",
"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn"
},
"overrides": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export function useActiveVisits() {
if (data && data?.[pageNumber - 1]?.data?.links?.some((link) => link.rel === 'next')) {
setSize((currentSize) => currentSize + 1);
}
}, [data, pageNumber]);
}, [data, pageNumber, setSize]);

const mapVisitProperties = (visit: Visit): ActiveVisit => {
// create base object
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { showSnackbar, updateVisit, useVisit } from '@openmrs/esm-framework';
import { Button, ModalBody, ModalFooter, ModalHeader } from '@carbon/react';
import { showSnackbar, updateVisit, useVisit } from '@openmrs/esm-framework';
import { changeAppointmentStatus } from '../../patient-appointments/patient-appointments.resource';
import { useMutateAppointments } from '../../form/appointments-form.resource';

Expand Down Expand Up @@ -68,7 +68,7 @@ const EndAppointmentModal: React.FC<EndAppointmentModalProps> = ({ patientUuid,
.finally(() => {
closeModal();
});
}, [activeVisit, mutate, mutateAppointments, closeModal, patientUuid, appointmentUuid]);
}, [activeVisit, appointmentUuid, closeModal, mutate, mutateAppointments, t]);

return (
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const MonthlyWorkloadView: React.FC<MonthlyWorkloadViewProps> = ({ dateTime, eve
events?.find(
(event) => dayjs(event.appointmentDate)?.format('YYYY-MM-DD') === dayjs(dateTime)?.format('YYYY-MM-DD'),
),
[events],
[dateTime, events],
);

const visibleServices = useMemo(() => {
Expand All @@ -34,15 +34,15 @@ const MonthlyWorkloadView: React.FC<MonthlyWorkloadViewProps> = ({ dateTime, eve
return currentData.services.slice(0, layout === 'small-desktop' ? 2 : 4);
}
return [];
}, [currentData, showAllServices, layout, currentData]);
}, [currentData, showAllServices, layout]);

const hasHiddenServices = useMemo(() => {
if (currentData?.services) {
if (showAllServices) return false;
return layout === 'small-desktop' ? currentData.services.length > 2 : currentData.services.length > 4;
}
return false;
}, [layout, currentData, currentData]);
}, [currentData?.services, layout, showAllServices]);

const navigateToAppointmentsByDate = (serviceUuid: string) => {
navigate({ to: `${spaHomePage}/appointments/${dayjs(dateTime).format('YYYY-MM-DD')}/${serviceUuid}` });
Expand All @@ -62,7 +62,7 @@ const MonthlyWorkloadView: React.FC<MonthlyWorkloadViewProps> = ({ dateTime, eve
)}>
{isSameMonth(dateTime, dayjs(selectedDate)) && (
<p>
<div className={classNames(styles.totals)}>
<span className={classNames(styles.totals)}>
{currentData?.services ? (
<div role="button" tabIndex={0}>
<User size={16} />
Expand All @@ -72,7 +72,7 @@ const MonthlyWorkloadView: React.FC<MonthlyWorkloadViewProps> = ({ dateTime, eve
<div />
)}
<b className={styles.calendarDate}>{dateTime.format('D')}</b>
</div>
</span>
{currentData?.services && (
<div className={styles.currentData}>
{visibleServices.map(({ serviceName, serviceUuid, count }, i) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ const AppointmentsForm: React.FC<AppointmentsFormProps & DefaultWorkspaceProps>
},
});

useEffect(() => setValue('formIsRecurringAppointment', isRecurringAppointment), [isRecurringAppointment]);
useEffect(() => setValue('formIsRecurringAppointment', isRecurringAppointment), [isRecurringAppointment, setValue]);

// Retrive ref callback for appointmentDateTime (startDate & recurringPatternEndDate)
const {
Expand All @@ -277,7 +277,7 @@ const AppointmentsForm: React.FC<AppointmentsFormProps & DefaultWorkspaceProps>
return;
}
promptBeforeClosing(() => isDirty);
}, [isDirty, promptBeforeClosing, isSuccessful]);
}, [isDirty, promptBeforeClosing, isSuccessful, reset, closeWorkspace]);

const handleWorkloadDateChange = (date: Date) => {
const appointmentDate = getValues('appointmentDateTime');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import React, { useCallback, useContext } from 'react';
import { useTranslation } from 'react-i18next';

import { Layer, OverflowMenu, OverflowMenuItem } from '@carbon/react';
import { launchPatientWorkspace } from '@openmrs/esm-patient-common-lib';
import { launchWorkspace, showModal, useLayoutType } from '@openmrs/esm-framework';
import type { Appointment } from '../types';
import styles from './patient-appointments-action-menu.scss';

import PatientAppointmentContext, { PatientAppointmentContextTypes } from '../hooks/patientAppointmentContext';
import styles from './patient-appointments-action-menu.scss';

interface appointmentsActionMenuProps {
appointment: Appointment;
Expand All @@ -32,7 +30,7 @@ export const PatientAppointmentsActionMenu = ({ appointment, patientUuid }: appo
appointment,
});
}
}, [appointment, t]);
}, [appointment, patientAppointmentContext, t]);

const launchCancelAppointmentDialog = () => {
const dispose = showModal('patient-appointment-cancel-confirmation-dialog', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export function useBedsGroupedByLocation() {
isValidatingBedsGroupedByLocation: isValidating,
mutateBedsGroupedByLocation: mutate,
}),
[result, error, isLoading, isValidating, mutate],
[error, isLoading, isLoadingAdmissionLocations, isValidating, mutate, result],
);

return results;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const AddPatient: React.FC<AddPatientProps> = ({ closeModal, patientUuid }) => {
const key = `${restBaseUrl}/cohortm/cohortmember?patient=${patientUuid}&v=custom:(uuid,patient:ref,cohort:(uuid,name,startDate,endDate))`;

return mutate((k) => typeof k === 'string' && k === key);
}, []);
}, [patientUuid]);

const handleSubmit = useCallback(() => {
Promise.all(
Expand Down Expand Up @@ -67,7 +67,7 @@ const AddPatient: React.FC<AddPatientProps> = ({ closeModal, patientUuid }) => {
});
}),
).finally(closeModal);
}, [data, selected, closeModal, t, patientUuid]);
}, [selected, closeModal, data, mutateCohortMembers, t]);

const searchResults = useMemo(() => {
if (!data) {
Expand All @@ -88,7 +88,7 @@ const AddPatient: React.FC<AddPatientProps> = ({ closeModal, patientUuid }) => {
if (currentPage !== 1) {
goTo(1);
}
}, [searchValue]);
}, [currentPage, goTo, searchValue]);

return (
<div className={styles.modalContent}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,19 +269,22 @@ function useStarredLists() {
setStarredLists(starredPatientLists.split(','));
}, [currentUser?.userProperties?.starredPatientLists, setStarredLists]);

const updateUserProperties = (newStarredLists: Array<string>) => {
const starredPatientLists = newStarredLists.join(',');
const userProperties = { ...(currentUser?.userProperties ?? {}), starredPatientLists };
const updateUserProperties = useCallback(
(newStarredLists: Array<string>) => {
const starredPatientLists = newStarredLists.join(',');
const userProperties = { ...(currentUser?.userProperties ?? {}), starredPatientLists };

starPatientList(currentUser?.uuid, userProperties).catch(() => {
setInitialStarredLists();
showSnackbar({
subtitle: t('starringPatientListFailed', 'Marking patient lists starred / unstarred failed'),
kind: 'error',
title: 'Failed to update patient lists',
starPatientList(currentUser?.uuid, userProperties).catch(() => {
setInitialStarredLists();
showSnackbar({
subtitle: t('starringPatientListFailed', 'Marking patient lists starred / unstarred failed'),
kind: 'error',
title: 'Failed to update patient lists',
});
});
});
};
},
[currentUser?.userProperties, currentUser?.uuid, setInitialStarredLists, t],
);

/**
* Handles toggling the starred list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const ChangeStatus: React.FC<ChangeStatusDialogProps> = ({ queueEntry, closeModa
service: z.string({ required_error: t('serviceIsRequired', 'Service is required') }),
status: z.string({ required_error: t('statusIsRequired', 'Status is required') }),
}),
[],
[t],
);

type ChangeStatusForm = z.infer<typeof schema>;
Expand Down
10 changes: 5 additions & 5 deletions packages/esm-service-queues-app/src/hooks/useQueueEntries.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { type FetchResponse, openmrsFetch, restBaseUrl } from '@openmrs/esm-framework';
import { type QueueEntry, type QueueEntrySearchCriteria } from '../types';
import useSWR from 'swr';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useSWRConfig } from 'swr/_internal';
import isEqual from 'lodash-es/isEqual';
import useSWR from 'swr';
import { useSWRConfig } from 'swr/_internal';
import { type FetchResponse, openmrsFetch, restBaseUrl } from '@openmrs/esm-framework';
import { type QueueEntry, type QueueEntrySearchCriteria } from '../types';

type QueueEntryResponse = FetchResponse<{
results: Array<QueueEntry>;
Expand Down Expand Up @@ -111,7 +111,7 @@ export function useQueueEntries(searchCriteria?: QueueEntrySearchCriteria, rep:
}
refetchAllData(rep, searchCriteria);
}
}, [searchCriteria, currentSearchCriteria, setCurrentSearchCriteria, currentRep, rep]);
}, [currentRep, currentSearchCriteria, refetchAllData, rep, searchCriteria, setCurrentSearchCriteria]);

const { data: pageData, isValidating, error: pageError } = useSWR<QueueEntryResponse, Error>(pageUrl, openmrsFetch);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@
const currentQueueLocationName = useSelectedQueueLocationName();
const currentQueueLocationUuid = useSelectedQueueLocationUuid();

const handleQueueLocationChange = useCallback(({ selectedItem }) => {
if (selectedItem.id === 'all') {
updateSelectedQueueLocationUuid(null);
updateSelectedQueueLocationName(null);
} else {
updateSelectedQueueLocationUuid(selectedItem.id);
updateSelectedQueueLocationName(selectedItem.name);
updateSelectedService(null, t('all', 'All'));
}
}, []);
const handleQueueLocationChange = useCallback(
({ selectedItem }) => {
if (selectedItem.id === 'all') {
updateSelectedQueueLocationUuid(null);
updateSelectedQueueLocationName(null);
} else {
updateSelectedQueueLocationUuid(selectedItem.id);
updateSelectedQueueLocationName(selectedItem.name);
updateSelectedService(null, t('all', 'All'));
}
},
[t],
);

useEffect(() => {
if (!isLoading && !error && !currentQueueLocationUuid) {
Expand All @@ -55,13 +58,17 @@
});
}
}
}, [

Check warning on line 61 in packages/esm-service-queues-app/src/patient-queue-header/patient-queue-header.component.tsx

View workflow job for this annotation

GitHub Actions / build

React Hook useEffect has duplicate dependencies: 'handleQueueLocationChange' and 'userSession?.sessionLocation?.display'. Either omit them or remove the dependency array
queueLocations,
currentQueueLocationName,
currentQueueLocationUuid,
isLoading,
error,
handleQueueLocationChange,
isLoading,
queueLocations,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate dependencies warning here.

userSession?.sessionLocation?.display,
userSession?.sessionLocation?.uuid,
handleQueueLocationChange,
userSession?.sessionLocation?.display,
]);

return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback, useEffect, useState } from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import dayjs from 'dayjs';
import head from 'lodash-es/head';
import { useTranslation } from 'react-i18next';
Expand Down Expand Up @@ -59,7 +59,7 @@ const ScheduledVisitsForVisitType: React.FC<{
const { mutateQueueEntries } = useMutateQueueEntries();
const [isSubmitting, setIsSubmitting] = useState(false);
const timeFormat = new Date().getHours() >= 12 ? 'PM' : 'AM';
const visitDate = new Date();
const visitDate = useMemo(() => new Date(), []);
const visitTime = dayjs(new Date()).format('hh:mm');
const [appointment, setAppointment] = useState<Appointment>();
const [patientId, setPatientId] = useState('');
Expand Down Expand Up @@ -159,23 +159,21 @@ const ScheduledVisitsForVisitType: React.FC<{
}
},
[
visitTime,
timeFormat,
allVisitTypes,
patientId,
visitDate,
userLocation,
queues,
config.concepts.defaultStatusConceptUuid,
config.concepts.defaultPriorityConceptUuid,
currentVisit,
t,
priorities,
appointment,
selectedQueueLocation,
visitQueueNumberAttributeUuid,
closeWorkspace,
currentVisit,
defaultStatus,
mutateQueueEntries,
patientId,
selectedQueueLocation,
service,
t,
timeFormat,
userLocation,
visitDate,
visitQueueNumberAttributeUuid,
visitTime,
],
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import React, { useEffect, useState } from 'react';
import { Button, DataTableSkeleton } from '@carbon/react';
import isNil from 'lodash-es/isNil';
import { useTranslation } from 'react-i18next';
import { Button, DataTableSkeleton } from '@carbon/react';
import {
type DefaultWorkspaceProps,
ArrowLeftIcon,
ErrorState,
getPatientName,
PatientBannerContactDetails,
PatientBannerPatientInfo,
PatientBannerToggleContactDetailsButton,
PatientPhoto,
type DefaultWorkspaceProps,
usePatient,
useVisit,
} from '@openmrs/esm-framework';
Expand Down Expand Up @@ -69,7 +69,7 @@ const PatientSearch: React.FC<PatientSearchProps> = ({
if (searchType === SearchTypes.SCHEDULED_VISITS && appointments && !hasAppointments) {
setSearchType(SearchTypes.VISIT_FORM);
}
}, [hasAppointments, appointments]);
}, [appointments, hasAppointments, searchType]);

useEffect(() => {
if (searchType === SearchTypes.SEARCH_RESULTS) {
Expand Down
Loading
Loading