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

(chore) Various tooling updates and tweaks #73

Merged
merged 2 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 0 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
src/**/*.test.tsx
**/*.d.ts
57 changes: 33 additions & 24 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -1,17 +1,39 @@
{
"env": {
"node": true,
"browser": true
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint"],
"ignorePatterns": ["**/*.test.tsx"],
"parserOptions": {
"project": true,
"tsconfigRootDir": "__dirname"
},
"plugins": ["@typescript-eslint", "import", "react-hooks"],

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

It's already in there

"root": true,
"rules": {
// The following rules need `noImplicitAny` to be set to `true` in our tsconfig. They are too restrictive for now, but should be reconsidered in future
"@typescript-eslint/ban-types": "off",
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-floating-promises": "off",
"@typescript-eslint/no-unsafe-argument": "off",
"@typescript-eslint/no-unsafe-assignment": "off",
"@typescript-eslint/no-unsafe-call": "off",
"@typescript-eslint/no-unsafe-member-access": "off",
"@typescript-eslint/no-unsafe-return": "off",
"@typescript-eslint/no-unused-vars": "off",
"@typescript-eslint/unbound-method": "off",
"@typescript-eslint/consistent-type-definitions": "off",
"@typescript-eslint/consistent-type-exports": "error",
// Use `import type` instead of `import` for type imports
"@typescript-eslint/consistent-type-imports": [
"error",
{
"fixStyle": "inline-type-imports"
}
],
"import/no-duplicates": "error",
"no-console": ["error", { "allow": ["warn", "error"] }],
"no-explicit-any": "off",
"no-extra-boolean-cast": "off",
"no-prototype-builtins": "off",
"no-restricted-imports": [
"error",
{
Expand All @@ -26,6 +48,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 @@ -37,21 +60,7 @@
]
}
],
"no-unsafe-optional-chaining": "off",
"no-useless-escape": "off",
"prefer-const": "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"
}
],
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-unused-vars": "off",
"@typescript-eslint/no-var-requires": "off",
"@typescript-eslint/triple-slash-reference": "off"
"react-hooks/exhaustive-deps": "warn",
"react-hooks/rules-of-hooks": "error"
}
}
2 changes: 1 addition & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@

set -e # die on error

yarn pretty-quick --staged
npx lint-staged
yarn turbo run extract-translations
2 changes: 1 addition & 1 deletion .turbo.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "https://turbo.build/schema.json",
"pipeline": {
"tasks": {
"build": {
"outputs": ["dist/**"]
},
Expand Down
893 changes: 0 additions & 893 deletions .yarn/releases/yarn-4.1.1.cjs

This file was deleted.

934 changes: 934 additions & 0 deletions .yarn/releases/yarn-4.5.3.cjs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion .yarnrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ plugins:
- path: .yarn/plugins/@yarnpkg/plugin-outdated.cjs
spec: "https://mskelton.dev/yarn-outdated/v3"

yarnPath: .yarn/releases/yarn-4.1.1.cjs
yarnPath: .yarn/releases/yarn-4.5.3.cjs
38 changes: 19 additions & 19 deletions i18next-parser.config.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
module.exports = {
contextSeparator: "_",
contextSeparator: '_',
// Key separator used in your translation keys

createOldCatalogs: false,
// Save the \_old files

defaultNamespace: "translations",
defaultNamespace: 'translations',
// Default namespace used in your i18next config

defaultValue: "",
defaultValue: '',
// Default value to give to empty keys
// You may also specify a function accepting the locale, namespace, and key as arguments

Expand All @@ -18,43 +18,43 @@ module.exports = {
keepRemoved: false,
// Keep keys from the catalog that are no longer in code

keySeparator: ".",
keySeparator: '.',
// Key separator used in your translation keys
// If you want to use plain english keys, separators such as `.` and `:` will conflict. You might want to set `keySeparator: false` and `namespaceSeparator: false`. That way, `t('Status: Loading...')` will not think that there are a namespace and three separator dots for instance.

// see below for more details
lexers: {
hbs: ["HandlebarsLexer"],
handlebars: ["HandlebarsLexer"],
hbs: ['HandlebarsLexer'],
handlebars: ['HandlebarsLexer'],

htm: ["HTMLLexer"],
html: ["HTMLLexer"],
htm: ['HTMLLexer'],
html: ['HTMLLexer'],

mjs: ["JavascriptLexer"],
js: ["JavascriptLexer"], // if you're writing jsx inside .js files, change this to JsxLexer
ts: ["JavascriptLexer"],
jsx: ["JsxLexer"],
tsx: ["JsxLexer"],
mjs: ['JavascriptLexer'],
js: ['JavascriptLexer'], // if you're writing jsx inside .js files, change this to JsxLexer
ts: ['JavascriptLexer'],
jsx: ['JsxLexer'],
tsx: ['JsxLexer'],

default: ["JavascriptLexer"],
default: ['JavascriptLexer'],
},

lineEnding: "auto",
lineEnding: 'auto',
// Control the line ending. See options at https://github.com/ryanve/eol

locales: ["en", "am", "es", "fr", "km", "he"],
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

locales: ['en'],
// An array of the locales in your applications

namespaceSeparator: ":",
namespaceSeparator: ':',
// Namespace separator used in your translation keys
// If you want to use plain english keys, separators such as `.` and `:` will conflict. You might want to set `keySeparator: false` and `namespaceSeparator: false`. That way, `t('Status: Loading...')` will not think that there are a namespace and three separator dots for instance.

output: "$NAMESPACE/$LOCALE.json",
output: '$NAMESPACE/$LOCALE.json',
// Supports $LOCALE and $NAMESPACE injection
// Supports JSON (.json) and YAML (.yml) file formats
// Where to write the locale files relative to process.cwd()

pluralSeparator: "_",
pluralSeparator: '_',
// Plural separator used in your translation keys
// If you want to use plain english keys, separators such as `_` might conflict. You might want to set `pluralSeparator` to a different string that does not occur in your keys.

Expand Down
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
const path = require('path');

module.exports = {
clearMocks: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

So mocks gets automatically cleared between test runs.

collectCoverageFrom: [
'**/src/**/*.component.tsx',
'!**/node_modules/**',
Expand Down
22 changes: 10 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@openmrs/esm-billing-app",
"version": "1.0.0",
"description": "Billing frontend module for use in O3",
"description": "O3 frontend module for handling billing concerns in healthcare settings",
"browser": "dist/openmrs-esm-billing-app.js",
"main": "src/index.ts",
"source": true,
Expand All @@ -11,15 +11,12 @@
"start": "openmrs develop",
"analyze": "webpack --mode=production --env.analyze=true",
"build": "webpack --mode production",
"ci:prepublish": "lerna publish from-package --no-git-reset --yes --dist-tag next",
"ci:publish": "lerna publish from-package --yes --no-git-reset --yes",
"coverage": "yarn test --coverage",
"debug": "npm run serve",
"extract-translations": "i18next 'src/**/*.component.tsx' 'src/index.ts' i18next-parser.config.js",
"extract-translations": "i18next 'src/**/*.component.tsx' 'src/index.ts' --config ./i18next-parser.config.js",
"lint": "eslint src --ext ts,tsx --max-warnings=0",
"postinstall": "husky install",
"prettier": "prettier --config prettier.config.js --write \"src/**/*.{ts,tsx,css,scss}\" \"e2e/**/*.ts\"",
"release": "lerna version --no-git-tag-version",
"serve": "webpack serve --mode=development",
"test-e2e": "playwright test",
"test": "cross-env TZ=UTC jest --config jest.config.js --verbose false --passWithNoTests",
Expand Down Expand Up @@ -88,21 +85,20 @@
"dayjs": "^1.11.10",
"dotenv": "^16.4.1",
"eslint": "^8.56.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-import": "^2.31.0",
"eslint-plugin-react-hooks": "^5.0.0",
"husky": "^9.0.6",
"i18next": "^23.7.20",
"i18next-parser": "^8.12.0",
"identity-obj-proxy": "^3.0.0",
"jest": "^29.7.0",
"jest-cli": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"lerna": "^8.0.2",
"lint-staged": "^15.2.10",
"lodash": "^4.17.21",
"openmrs": "next",
"pinst": "^3.0.0",
"prettier": "^3.2.4",
"pretty-quick": "^4.0.0",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-i18next": "^14.0.1",
Expand All @@ -111,13 +107,15 @@
"sass": "^1.70.0",
"swc-loader": "^0.2.3",
"swr": "^2.2.4",
"turbo": "^1.13.0",
"turbo": "^2.3.3",
"typescript": "^4.9.5",
"webpack": "^5.97.0",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^4.15.1"
},
"lint-staged": {
"*.{js,jsx,ts,tsx}": "eslint --cache --fix"
"*.{ts,tsx}": "eslint --cache --fix --max-warnings 0",
"*.{css,scss,ts,tsx}": "prettier --write --list-different"
},
"packageManager": "yarn@4.1.1"
"packageManager": "yarn@4.5.3"
}
4 changes: 2 additions & 2 deletions src/bill-history/bill-history.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@use '@carbon/layout';
@use '@carbon/type';
@use '~@openmrs/esm-styleguide/src/vars' as *;
@use '@openmrs/esm-styleguide/src/vars' as *;

.container {
margin: layout.$spacing-07 0;
Expand Down Expand Up @@ -163,4 +163,4 @@

.tableCells {
white-space: wrap !important;
}
}
31 changes: 19 additions & 12 deletions src/bill-item-actions/edit-bill-item.component.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import React, { useEffect, useMemo, useState } from 'react';
import { Button, ModalBody, ModalFooter, ModalHeader, Form, InlineLoading } from '@carbon/react';
import {
Button,
Column,
Form,
InlineLoading,
InlineNotification,
ModalBody,
ModalFooter,
ModalHeader,
NumberInput,
TextInput,
} from '@carbon/react';
import { useTranslation } from 'react-i18next';
import { showSnackbar } from '@openmrs/esm-framework';
import { Controller, type FieldErrors, useForm } from 'react-hook-form';
import { mutate } from 'swr';
import { z } from 'zod';
import { zodResolver } from '@hookform/resolvers/zod';
import { type LineItem, type MappedBill } from '../types';
import styles from './bill-item-actions.scss';
import { updateBillItems } from '../billing.resource';
import { mutate } from 'swr';
import { showSnackbar } from '@openmrs/esm-framework';
import { apiBasePath } from '../constants';
import { Column } from '@carbon/react';
import { InlineNotification } from '@carbon/react';
import { getBillableServiceUuid } from '../invoice/payments/utils';
import { type LineItem, type MappedBill } from '../types';

Choose a reason for hiding this comment

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

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

import { updateBillItems } from '../billing.resource';
import { useBillableServices } from '../billable-services/billable-service.resource';
import { NumberInput } from '@carbon/react';
import { TextInput } from '@carbon/react';
import styles from './bill-item-actions.scss';

interface BillLineItemProps {
bill: MappedBill;
Expand All @@ -25,17 +32,17 @@ interface BillLineItemProps {

const ChangeStatus: React.FC<BillLineItemProps> = ({ bill, item, closeModal }) => {
const { t } = useTranslation();
const { billableServices } = useBillableServices();
const [showErrorNotification, setShowErrorNotification] = useState(false);
const [total, setTotal] = useState(0);
const { billableServices } = useBillableServices();

const schema = useMemo(
() =>
z.object({
quantity: z.string({ required_error: t('quantityRequired', 'Quantity is required') }),
price: z.string({ required_error: t('priceIsRequired', 'Price is required') }),
}),
[],
[t],
);

type BillLineItemForm = z.infer<typeof schema>;
Expand Down
6 changes: 3 additions & 3 deletions src/bill-item-actions/edit-bill-item.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React from 'react';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import ChangeStatus from './edit-bill-item.component';
import { updateBillItems } from '../billing.resource';
import { showSnackbar } from '@openmrs/esm-framework';
import { type LineItem, type MappedBill } from '../types';
import { type MappedBill } from '../types';

Choose a reason for hiding this comment

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

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

import { updateBillItems } from '../billing.resource';
import ChangeStatus from './edit-bill-item.component';

// Mock external dependencies
jest.mock('../billing.resource', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import {
StructuredListWrapper,
} from '@carbon/react';
import { useTranslation } from 'react-i18next';
import { useConfig } from '@openmrs/esm-framework';
import { convertToCurrency } from '../../helpers';
import { type MappedBill, type LineItem } from '../../types';
import BillWaiverForm from './bill-waiver-form.component';
import styles from './bill-waiver.scss';
import { useConfig } from '@openmrs/esm-framework';

const PatientBillsSelections: React.FC<{ bills: MappedBill; setPatientUuid: (patientUuid) => void }> = ({
bills,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import { Form, Stack, FormGroup, Layer, Button, NumberInput } from '@carbon/reac
import { TaskAdd } from '@carbon/react/icons';
import { mutate } from 'swr';
import { useTranslation } from 'react-i18next';
import { restBaseUrl, showSnackbar, useConfig } from '@openmrs/esm-framework';
import { showSnackbar, useConfig } from '@openmrs/esm-framework';
import { createBillWaiverPayload } from './utils';
import { convertToCurrency } from '../../helpers';
import { processBillPayment } from '../../billing.resource';
import { useBillableItems } from '../../billing-form/billing-form.resource';
import type { LineItem, MappedBill } from '../../types';
import styles from './bill-waiver-form.scss';
import { apiBasePath } from '../../constants';
import styles from './bill-waiver-form.scss';

type BillWaiverFormProps = {
bill: MappedBill;
Expand Down
8 changes: 4 additions & 4 deletions src/billable-services/bill-waiver/bill-waiver.component.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import React, { useState } from 'react';
import { ExtensionSlot, UserHasAccess } from '@openmrs/esm-framework';
import PatientBills from './patient-bills.component';
import { useBills } from '../../billing.resource';
import PatientBills from './patient-bills.component';
import styles from './bill-waiver.scss';

type BillWaiverProps = {};

const BillWaiver: React.FC<BillWaiverProps> = () => {
const BillWaiver: React.FC = () => {
const [patientUuid, setPatientUuid] = useState<string>('');
const { bills } = useBills(patientUuid);

const filterBills = bills.filter((bill) => bill?.status !== 'PAID' && patientUuid === bill.patientUuid) ?? [];

return (
<UserHasAccess privilege="coreapps.systemAdministration">
<div className={styles.billWaiverContainer}>
Expand Down
Loading
Loading