Skip to content

Commit

Permalink
(chore) Add ESLint Rules for React-Hooks (#1862)
Browse files Browse the repository at this point in the history
* Updated Husky files (pre-commit, pre-push) and package.json for lint-staged integration to improve code quality checks.

* Changes for ESLint rules

* Changes for ESLint rules

* Contd Changes for ESLint react-hooks/exhaustive-deps

* Contd Changes for ESLint react-hooks/exhaustive-deps

* Changes based on PR review

* Contd changes based on PR review

* Contd changes based on PR review
  • Loading branch information
nravilla authored Jun 15, 2024
1 parent d6ac6b2 commit 98ee403
Show file tree
Hide file tree
Showing 36 changed files with 438 additions and 237 deletions.
39 changes: 35 additions & 4 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,14 +1,38 @@
{
"env": {
"node": true
},
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "prettier"],
"parser": "@typescript-eslint/parser",
"plugins": [
"@typescript-eslint"
],
"extends": ["ts-react-important-stuff", "plugin:prettier/recommended"],
"plugins": ["@typescript-eslint", "react-hooks", "prettier"],
"rules": {
"react-hooks/exhaustive-deps": "warn",
"react-hooks/rules-of-hooks": "error",
// Disabling these rules for now just to keep the diff small. I'll enable them in a future PR that fixes lint issues.
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-unused-vars": "off",
"@typescript-eslint/no-var-requires": "off",
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/ban-types": "off",
// Use `import type` instead of `import` for type imports https://typescript-eslint.io/blog/consistent-type-imports-and-exports-why-and-how
"@typescript-eslint/consistent-type-imports": [
"error",
{
"fixStyle": "inline-type-imports"
}
],
"prefer-const": "off",
"no-console": ["error", { "allow": ["warn", "error"] }],
"no-unsafe-optional-chaining": "off",
"no-explicit-any": "off",
"no-extra-boolean-cast": "off",
"no-prototype-builtins": "off",
"no-useless-escape": "off",
"no-restricted-imports": [
"error",
{
"paths": [
// These two rules ensure that we're importing lodash and lodash-es correctly. Not doing so can bloat our bundle size significantly.
{
"name": "lodash",
"message": "Import specific methods from `lodash`. e.g. `import map from 'lodash/map'`"
Expand All @@ -18,6 +42,7 @@
"importNames": ["default"],
"message": "Import specific methods from `lodash-es`. e.g. `import { map } from 'lodash-es'`"
},
// These two rules ensure that we're importing Carbon components and icons from the correct packages (after v10). May be removed in the future.
{
"name": "carbon-components-react",
"message": "Import from `@carbon/react` directly. e.g. `import { Toggle } from '@carbon/react'`"
Expand All @@ -28,6 +53,12 @@
}
]
}
],
"prettier/prettier": [
"error",
{
"endOfLine": "auto"
}
]
}
}
3 changes: 3 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ set -e # die on error
# yarn prettier // commented because it causes alot of unncecessary changes that are not related to the commit
# yarn turbo run extract-translations // commented because it creates new translations files that are not related to the commit
# yarn pretty-quick --staged

npx lint-staged

4 changes: 4 additions & 0 deletions .husky/pre-push
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

#not sure if its needed for Mac, but its needed for Windows
exec < /dev/tty

set -e # die on error
yarn verify
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@
"webpack-cli": "^4.10.0",
"webpack-dev-server": "^4.8.1"
},
"lint-staged": {
"packages/**/src/**/*.{ts,tsx}": "eslint --cache --fix --max-warnings 0",
"*.{css,scss,ts,tsx}": "prettier --write --list-different"
},
"resolutions": {
"@types/react": "^18.0.14",
"@types/react-dom": "^18.0.5"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useEffect, useState } from 'react';
import styles from '../../styleguide/tiles.scss';
import { SkeletonText, Tile, Column } from '@carbon/react';
import { LazyCell } from '../lazy-cell/lazy-cell.component';
import { OpenmrsEncounter } from '@openmrs/openmrs-form-engine-lib';
import { type OpenmrsEncounter } from '@openmrs/openmrs-form-engine-lib';
import { fetchLatestEncountersOfTypes } from './helpers';

export interface SummaryCardProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { basePath } from '../../constants';
import styles from './cohort-patient-list.scss';
import { useTranslation } from 'react-i18next';
import { useFormsJson } from '../../hooks/useFormsJson';
import { columns, consolidatatePatientMeta, filterPatientsByName, PatientListColumn } from './helpers';
import { columns, consolidatatePatientMeta, filterPatientsByName, type PatientListColumn } from './helpers';

interface CohortPatientListProps {
cohortId: string;
Expand Down Expand Up @@ -172,6 +172,8 @@ export const CohortPatientList: React.FC<CohortPatientListProps> = ({
launchableForm,
addPatientToListOptions,
t,
viewTptPatientProgramSummary,
viewPatientProgramSummary,
]);

useEffect(() => {
Expand All @@ -190,7 +192,7 @@ export const CohortPatientList: React.FC<CohortPatientListProps> = ({
);
}
setPatientsCount(allPatients.length);
}, [hasLoadedPatients]);
}, [allPatients, associatedEncounterType, hasLoadedPatients]);

useEffect(() => {
const fetchHivResults = excludeColumns ? !excludeColumns.includes('hivResult') : true;
Expand Down Expand Up @@ -289,7 +291,6 @@ export const CohortPatientList: React.FC<CohortPatientListProps> = ({
autoFocus: true,
};
}, [
loadedExtraEncounters,
searchTerm,
filteredResults,
paginatedPatients,
Expand All @@ -303,7 +304,7 @@ export const CohortPatientList: React.FC<CohortPatientListProps> = ({

useEffect(() => {
setCounter(counter + 1);
}, [state]);
}, [counter, state]);

useEffect(() => {
if (allPatients.length && extraAssociatedEncounterTypes && !loadedExtraEncounters) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const LaunchableFormMenuItem = ({
} else {
setIsLoading(false);
}
}, []);
}, [continueEncounterActionText, encounterType, encounterUuid, launchableForm.editLatestEncounter, patientUuid]);

return (
<>
Expand Down Expand Up @@ -95,7 +95,7 @@ export const ViewSummaryMenuItem = ({ patientUuid, ViewSummary, encounterType })
} else {
setIsLoading(false);
}
}, []);
}, [ViewSummary.editLatestEncounter, encounterType, encounterUuid, patientUuid, viewSummaryActionText]);

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import {
MenuItem,
} from '@carbon/react';
import { Add } from '@carbon/react/icons';
import { FormSchema } from '@openmrs/openmrs-form-engine-lib';
import { type FormSchema } from '@openmrs/openmrs-form-engine-lib';
import { deleteEncounter, launchEncounterForm } from './helpers';
import { useEncounterRows } from '../../hooks/useEncounterRows';
import { OpenmrsEncounter } from '../../api/types';
import { type OpenmrsEncounter } from '../../api/types';
import { useFormsJson } from '../../hooks/useFormsJson';
import { usePatientDeathStatus } from '../../hooks/usePatientDeathStatus';
import { mutate } from 'swr';
Expand Down Expand Up @@ -327,7 +327,18 @@ export const EncounterList: React.FC<EncounterListProps> = ({
);
}
return null;
}, [forms, hideFormLauncher, isDead, displayText, moduleName, workspaceWindowSize, onFormSave, patientUuid]);
}, [
forms,
hideFormLauncher,
isDead,
displayText,
moduleName,
onFormSave,
workspaceWindowSize,
patientUuid,
t,
formsJson,
]);

if (isLoading === true || isLoadingForms === true || isLoadingFormsJson === true) {
return <DataTableSkeleton rowCount={5} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const MultipleEncounterList: React.FC<MultipleEncounterListProps> = ({
setAllRows(rows);
updateTable(rows, 0, pageSize);
}
}, [baseEncounterType, encountersMap, columns]);
}, [baseEncounterType, encountersMap, columns, pageSize]);

const updateTable = (fullDataSet, start, itemCount) => {
let currentRows = [];
Expand All @@ -150,7 +150,7 @@ export const MultipleEncounterList: React.FC<MultipleEncounterListProps> = ({

useEffect(() => {
loadRows(encounterTypeUuids);
}, [counter]);
}, [counter, encounterTypeUuids, loadRows]);

return (
<>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { showToast, usePatient } from '@openmrs/esm-framework';
import { ListItem, Modal, RadioButton, RadioButtonGroup, SkeletonText, UnorderedList } from '@carbon/react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import ReactDOM from 'react-dom';
import { useTranslation } from 'react-i18next';
import { addPatientToCohort, evictCohortMembership, getCohorts, getPatientListsForPatient } from '../../api/api';

Expand Down Expand Up @@ -79,7 +78,7 @@ export const AddPatientToListModal: React.FC<{
setIsLoading(false);
},
);
}, [cohortType]);
}, [cohortType, excludeCohorts, patientUuid]);

const availableLists = useMemo(() => {
const controls = cohorts.map((cohort, index) => (
Expand Down Expand Up @@ -150,7 +149,7 @@ export const AddPatientToListModal: React.FC<{
}
});
}
}, [selectedList, patientUuid, close, currentMemberships]);
}, [selectedList, currentMemberships, t, close, patientUuid]);
return (
<>
<Modal
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { type CSSProperties, HTMLAttributes, useCallback, useId, useMemo, useState } from 'react';
import React, { useId, useMemo, useState } from 'react';
import type { CSSProperties, HTMLAttributes } from 'react';
import fuzzy from 'fuzzy';
import { useTranslation } from 'react-i18next';
import {
Button,
DataTable,
DataTableRow,
type DataTableRow,
DataTableSkeleton,
InlineLoading,
Layer,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { DashboardGroupExtension } from '@openmrs/esm-patient-common-lib';
import React from 'react';
import PatientExtensionRenderer from '../components/extension-conditional-renderer/patient-based-extension-renderer';
import { DashboardLinkConfig } from '../types';
import { type DashboardLinkConfig } from '../types';
import { DashboardExtension } from './DashboardExtension';
import { BrowserRouter } from 'react-router-dom';

Expand Down
2 changes: 1 addition & 1 deletion packages/esm-commons-lib/src/hooks/useLastEncounter.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { openmrsFetch } from '@openmrs/esm-framework';
import { OpenmrsEncounter } from '../api/types';
import { type OpenmrsEncounter } from '../api/types';
import { encounterRepresentation } from '../constants';
import useSWR from 'swr';

Expand Down
2 changes: 1 addition & 1 deletion packages/esm-commons-lib/src/hooks/usePatientList.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useEffect, useMemo, useState } from 'react';
import useSWRImmutable from 'swr';
import { FhirPatientResponse } from '../api/types';
import { type FhirPatientResponse } from '../api/types';
import { ConfigurableLink, openmrsFetch } from '@openmrs/esm-framework';
import dayjs from 'dayjs';
import capitalize from 'lodash/capitalize';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function CovidHomePatientTabs() {
header: t('vaccine', 'Vaccine'),
getValue: ({ latestEncounter }) => {
const obs = findObs(latestEncounter, config.obsConcepts.covidVaccineAdministeredConcept_UUID);
if (typeof obs !== undefined && obs) {
if (typeof obs !== 'undefined' && obs) {
if (typeof obs.value === 'object') {
const vaccineNAME =
obs.value.names?.find((conceptName) => conceptName.conceptNameType === 'SHORT')?.name ||
Expand All @@ -137,7 +137,24 @@ function CovidHomePatientTabs() {
],
},
],
[],
[
config.cohorts.clientsAssessedForCovid,
config.cohorts.covidClientsWithPendingLabResults,
config.obsConcepts.covidCaseAssessmentEncType,
config.obsConcepts.covidLabTestEncType,
config.obsConcepts.covidOutcome,
config.obsConcepts.covidTestType,
config.obsConcepts.covidVaccinatedClients,
config.obsConcepts.covidVaccinationDose_UUID,
config.obsConcepts.covidVaccinationEncType,
config.obsConcepts.covidVaccineAdministeredConcept_UUID,
config.obsConcepts.covidVaccineConcept_UUID,
config.obsConcepts.dateSpecimenCollected,
config.obsConcepts.pcrTestResult,
config.obsConcepts.rapidTestResult,
config.obsConcepts.returnVisitDateConcept,
t,
],
);
return <OHRIPatientListTabs patientListConfigs={tabsConfigs} moduleName={moduleName} />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ function CovidSummaryTiles() {
getReportingCohort(config.cohorts.covidOutcomesCohortUUID).then((data) => {
setPeopleWithCovidOutcome(data.members.length);
});
}, []);
}, [
config.cohorts.covid19PositiveClients,
config.cohorts.covidOutcomesCohortUUID,
config.cohorts.covidVaccinatedClients,
]);
const tiles = useMemo(
() => [
{
Expand Down Expand Up @@ -50,7 +54,7 @@ function CovidSummaryTiles() {
value: PeopleWithCovidOutcome,
},
],
[],
[PeopleWithCovidOutcome, activeClientsCount, covid19PositiveClientsCount, covidVaccinatedClientsCount, t],
);
return <OHRIProgrammeSummaryTiles tiles={tiles} />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ export const Outcomes: React.FC<{}> = () => {
},
},
],
[],
[
config.obsConcepts.covidEncounterDateTime_UUID,
config.obsConcepts.covidOutcome,
config.obsConcepts.covidOutcomeUUID,
config.obsConcepts.covidPresentSymptonsConcept_UUID,
t,
],
);

useEffect(() => {
Expand All @@ -88,7 +94,7 @@ export const Outcomes: React.FC<{}> = () => {
setTotalPatientCount(response.length);
setIsLoading(false);
});
}, [pageSize, currentPage]);
}, [pageSize, currentPage, config.cohorts.covidOutcomesCohortUUID]);

useEffect(() => {
attach('outcomes-table-slot', 'patient-table');
Expand Down Expand Up @@ -133,12 +139,12 @@ export const Outcomes: React.FC<{}> = () => {
isLoading,
autoFocus: true,
}),
[searchTerm, filteredResults, patients, handleSearch, pagination, isLoading],
[searchTerm, filteredResults, patients, columns, t, handleSearch, pagination, isLoading],
);

useEffect(() => {
setCounter(counter + 1);
}, [state]);
}, [counter, state]);

return (
<div style={{ width: '100%', marginBottom: '2rem' }}>
Expand Down
Loading

0 comments on commit 98ee403

Please sign in to comment.