(chore) Various tooling updates and tweaks #73

denniskigen · 2024-12-05T09:16:18Z

Requirements

This PR has a title that briefly describes the work done including a conventional commit type prefix and a Jira ticket number if applicable. See existing PR titles for inspiration.

For changes to apps

My work conforms to the OpenMRS 3.0 Styleguide and design documentation.

If applicable

My work includes tests or is validated by existing tests.

Summary

This PR introduces several improvements to our tooling setup:

Standardized ESLint Configuration: Aligns the ESLint configuration with the standards used in other O3 repositories for consistency.
Pre-commit Hook Enhancements: Separates linting and formatting tasks in the pre-commit hook to adhere to best practices, ensuring clearer and more efficient code checks.
Version Updates: Updates core tooling versions, including Turbo and Yarn, to leverage the latest features and improvements.
Refactoring: A significant portion of the code changes are due to refactoring necessary to address violations of React's rules of hooks, as identified by the ESLint React Hooks plugin.

Screenshots

Related Issue

Other

denniskigen · 2024-12-05T09:25:21Z

.eslintignore

@@ -1,2 +1 @@
-src/**/*.test.tsx


There's no reason why we shouldn't lint test files.

denniskigen · 2024-12-05T09:27:12Z

i18next-parser.config.js

  // Control the line ending. See options at https://github.com/ryanve/eol

-  locales: ["en", "am", "es", "fr", "km", "he"],


We only want to extract en translations and use those as the source of truth. We use the Transifex integration as the source of truth for non English locales.

denniskigen · 2024-12-05T09:27:28Z

jest.config.js

@@ -4,6 +4,7 @@
 const path = require('path');

 module.exports = {
+  clearMocks: true,


So mocks gets automatically cleared between test runs.

denniskigen · 2024-12-05T09:28:47Z

src/billing-form/visit-attributes/visit-attributes-form.component.tsx

-  };
+  }, [
+    getValues,
+    patientCatergory.insuranceScheme,


TODO: We need to fix this typo in a subsequent commit. It's coming from the config schema so we need to figure out what the blast radius of this change will be beforehand.

denniskigen · 2024-12-05T09:29:22Z

src/routes.json

-    {
-      "name": "require-billing-modal",
-      "component": "requirePaymentModal"
-    },


Modals should be registered under the modals key.

denniskigen · 2024-12-05T09:29:38Z

tsconfig.json

    "allowSyntheticDefaultImports": true,
+    "esModuleInterop": true,
    "jsx": "react",
+    "lib": [
+      "dom",
+      "dom.iterable",
+      "esnext"
+    ],
+    "module": "esnext",
    "moduleResolution": "node",
-    "module": "ESNext",
    "noEmit": true,
-    "lib": ["es2022", "dom", "dom.iterable"],
+    "resolveJsonModule": true,
+    "skipLibCheck": true,
+    "target": "esnext"


Just reorganized for sanity.

NethmiRodrigo · 2024-12-06T06:35:02Z

.eslintrc

  },
-  "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "prettier"],
+  "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],


Shouldn't we add plugin:jest-dom/recommended and plugin:testing-library/react?

We could. Only reason I didn't is because I want to keep the diff small

NethmiRodrigo · 2024-12-06T06:37:03Z

.eslintrc

+    "project": true,
+    "tsconfigRootDir": "__dirname"
+  },
+  "plugins": ["@typescript-eslint", "import", "react-hooks"],


I don't know if its a standard or if we should do it but we have testing-library added as a plugin in patient-chart and form-builder but not core

Shall we add the import plugin as well, to flag duplicate imports?

It's already in there

NethmiRodrigo · 2024-12-06T06:40:51Z

src/bill-item-actions/edit-bill-item.component.tsx

 import { getBillableServiceUuid } from '../invoice/payments/utils';
+import { type LineItem, type MappedBill } from '../types';


Suggested change

import { type LineItem, type MappedBill } from '../types';

import type { LineItem, MappedBill } from '../types';

NethmiRodrigo · 2024-12-06T06:42:02Z

src/bill-item-actions/edit-bill-item.test.tsx

 import { showSnackbar } from '@openmrs/esm-framework';
-import { type LineItem, type MappedBill } from '../types';
+import { type MappedBill } from '../types';


Suggested change

import { type MappedBill } from '../types';

import type { MappedBill } from '../types';

NethmiRodrigo · 2024-12-06T06:43:07Z

src/billable-services/billable-services.component.tsx

  Tile,
 } from '@carbon/react';
 import { ArrowRight } from '@carbon/react/icons';
 import { useLayoutType, isDesktop, useConfig, usePagination, ErrorState, navigate } from '@openmrs/esm-framework';
 import { EmptyState } from '@openmrs/esm-patient-common-lib';
+import { type BillableService } from '../types/index';


Suggested change

import { type BillableService } from '../types/index';

import type { BillableService } from '../types/index';

NethmiRodrigo · 2024-12-06T06:44:35Z

src/billing-form/billing-form.component.tsx

 import { apiBasePath } from '../constants';
-import isEmpty from 'lodash-es/isEmpty';
+import { convertToCurrency } from '../helpers';
+import { type BillabeItem } from '../types';


Suggested change

import { type BillabeItem } from '../types';

import type { BillabeItem } from '../types';

NethmiRodrigo · 2024-12-06T06:46:05Z

src/helpers/functions.ts

@@ -1,6 +1,4 @@
 import { type Payment, type LineItem } from '../types';


Suggested change

import { type Payment, type LineItem } from '../types';

import type { Payment, LineItem } from '../types';

Please feel free to ignore the type import suggestions, we can always do them later on if this needs to go in asap!

NethmiRodrigo · 2024-12-06T06:47:48Z

src/invoice/invoice-table.test.tsx

-import { showModal, useConfig, useDebounce } from '@openmrs/esm-framework';
-import { useTranslation } from 'react-i18next';
-import { MappedBill } from '../types';
+import { type MappedBill } from '../types';


Suggested change

import { type MappedBill } from '../types';

import type { MappedBill } from '../types';

NethmiRodrigo · 2024-12-06T06:48:04Z

src/invoice/payments/payment-history/payment-history.test.tsx

 import { useConfig } from '@openmrs/esm-framework';
-import { MappedBill } from '../../../types';
+import PaymentHistory from './payment-history.component';
+import { type MappedBill } from '../../../types';


Suggested change

import { type MappedBill } from '../../../types';

import type { MappedBill } from '../../../types';

NethmiRodrigo

Thanks @denniskigen! LGTM! Left some comments of a couple of nitpicks and questions I had about the imports in .eslintrc

samuelmale

LGTM!

samuelmale · 2024-12-06T07:01:20Z

.eslintrc

@@ -1,17 +1,39 @@
 {
  "env": {
-    "node": true,
-    "browser": true


Wouldn't this disable global browser-specific variables like console, window etc.?

Don't think so, no. We don't have that elsewhere AFAICT.

denniskigen · 2024-12-06T08:27:23Z

Thanks for the reviews

denniskigen added 2 commits December 5, 2024 11:25

Tooling updates and tweaks

7cf4f75

Fix lint errors and some other niggles

f6a6326

denniskigen requested review from NethmiRodrigo, samuelmale and ibacher December 5, 2024 09:17

denniskigen commented Dec 5, 2024

View reviewed changes

denniskigen requested a review from vasharma05 December 5, 2024 09:29

NethmiRodrigo reviewed Dec 6, 2024

View reviewed changes

NethmiRodrigo approved these changes Dec 6, 2024

View reviewed changes

samuelmale approved these changes Dec 6, 2024

View reviewed changes

denniskigen merged commit ab6f516 into main Dec 6, 2024
5 checks passed

denniskigen deleted the chore/tooling branch December 6, 2024 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(chore) Various tooling updates and tweaks #73

(chore) Various tooling updates and tweaks #73

denniskigen commented Dec 5, 2024 •

edited

Loading

denniskigen Dec 5, 2024

denniskigen Dec 5, 2024

denniskigen Dec 5, 2024

denniskigen Dec 5, 2024

denniskigen Dec 5, 2024

denniskigen Dec 5, 2024

NethmiRodrigo Dec 6, 2024

denniskigen Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

denniskigen Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo Dec 6, 2024

NethmiRodrigo left a comment

samuelmale left a comment

samuelmale Dec 6, 2024

denniskigen Dec 6, 2024

denniskigen commented Dec 6, 2024

		// Control the line ending. See options at https://github.com/ryanve/eol

		locales: ["en", "am", "es", "fr", "km", "he"],

		import { getBillableServiceUuid } from '../invoice/payments/utils';
		import { type LineItem, type MappedBill } from '../types';

	import { type LineItem, type MappedBill } from '../types';
	import type { LineItem, MappedBill } from '../types';

	import { type MappedBill } from '../types';
	import type { MappedBill } from '../types';

	import { type BillableService } from '../types/index';
	import type { BillableService } from '../types/index';

	import { type BillabeItem } from '../types';
	import type { BillabeItem } from '../types';

		@@ -1,6 +1,4 @@
		import { type Payment, type LineItem } from '../types';

	import { type Payment, type LineItem } from '../types';
	import type { Payment, LineItem } from '../types';

	import { type MappedBill } from '../../../types';
	import type { MappedBill } from '../../../types';

		@@ -1,2 +1 @@
		src/*/.test.tsx

(chore) Various tooling updates and tweaks #73

(chore) Various tooling updates and tweaks #73

Conversation

denniskigen commented Dec 5, 2024 • edited Loading

Requirements

For changes to apps

If applicable

Summary

Screenshots

Related Issue

Other

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NethmiRodrigo left a comment

Choose a reason for hiding this comment

samuelmale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denniskigen commented Dec 6, 2024

denniskigen commented Dec 5, 2024 •

edited

Loading