Skip to content

Commit

Permalink
(chore) Code quality and tooling improvements (#59)
Browse files Browse the repository at this point in the history
* (chore) Code quality and tooling improvements

This PR includes a number of changes to improve code quality and tooling. These changes include:

- Adding ESLint plugins for testing (jest-dom, testing-library, playwright) and import rules for preventing duplicate imports.
- Adding the react-hooks ESLint plugin for enforcing rules of hooks and exhaustive-deps for React hooks.
- Resolving rules-of-hooks violations flagged by the react-hooks ESLint plugin.
- Adapting the Prettier configuration to align with similar configurations in O3.
- Adding emojis to steps in the GitHub Actions workflows.
- Improving the `react-i18next` test mock to properly handle interpolated values in translation strings.
- Annotating type-only imports with the `type` keyword.

* Review feedback - reorganize imports
  • Loading branch information
denniskigen authored Feb 10, 2025
1 parent c3b4dd0 commit 5e244cc
Show file tree
Hide file tree
Showing 49 changed files with 978 additions and 545 deletions.
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
src/**/*.spec.tsx
**/node_modules/**/*
__mocks__/*
29 changes: 23 additions & 6 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,25 @@
"env": {
"node": true
},
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "plugin:jest-dom/recommended"],
"overrides": [
{
"files": ["**/*.test.tsx"],
"extends": ["plugin:testing-library/react"]
},
{
"files": ["e2e/**/*.spec.ts"],
"extends": ["plugin:playwright/recommended"],
"rules": {
"testing-library/prefer-screen-queries": "off",
"react-hooks/rules-of-hooks": "off",
"react-hooks/exhaustive-deps": "off"
}
}
],
"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint", "react-hooks"],
"plugins": ["@typescript-eslint", "import", "jest-dom", "react-hooks", "testing-library"],
"rules": {
// Disabling these rules for now just to keep the diff small. I'll enable them one by one as we go.
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/ban-types": "off",
"@typescript-eslint/no-explicit-any": "off",
Expand All @@ -20,12 +34,12 @@
"fixStyle": "inline-type-imports"
}
],
"prefer-const": "off",
"import/no-duplicates": "error",
"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-unsafe-optional-chaining": "off",
"no-useless-escape": "off",
"no-restricted-imports": [
"error",
Expand All @@ -52,6 +66,9 @@
}
]
}
]
],
"prefer-const": "off",
"react-hooks/exhaustive-deps": "warn",
"react-hooks/rules-of-hooks": "error"
}
}
105 changes: 65 additions & 40 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
name: OpenMRS CI

env:
TURBO_API: 'http://127.0.0.1:9080'
TURBO_TOKEN: ${{ secrets.TURBO_SERVER_TOKEN }}
TURBO_TEAM: ${{ github.repository_owner }}

on:
push:
branches: [main]
Expand All @@ -14,42 +9,46 @@ on:
types:
- created

jobs:
env:
TURBO_API: 'http://127.0.0.1:9080'
TURBO_TOKEN: ${{ secrets.TURBO_SERVER_TOKEN }}
TURBO_TEAM: ${{ github.repository_owner }}

jobs:
build:
runs-on: ubuntu-latest
timeout-minutes: 15

steps:
- uses: actions/checkout@v4
- name: Use Node.js
- name: 🛠️ Setup Node.js
uses: actions/setup-node@v4
with:
node-version: "18"
node-version: '18'

- name: Cache dependencies
- name: 💾 Cache dependencies
id: cache
uses: actions/cache@v4
with:
path: '**/node_modules'
key: ${{ runner.os }}-${{ hashFiles('**/yarn.lock') }}

- name: Install dependencies
- name: 📦 Install dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: yarn install --immutable

- name: Setup a local cache server for Turborepo
- name: 🚀 Setup local cache server for Turborepo
uses: felixmosh/turborepo-gh-artifacts@v3
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
server-token: ${{ secrets.TURBO_SERVER_TOKEN }}
server-token: ${{ env.TURBO_TOKEN }}

- name: Run lint, typecheck and tests
- name: 🧪 Run tests, lint and type checks
run: yarn verify

- name: Run build
run: yarn turbo run build --color --concurrency=5
- name: 🏗️ Run build
run: yarn turbo run build

- name: Upload build artifacts
- name: 📤 Upload Artifacts
uses: actions/upload-artifact@v4
with:
name: packages
Expand All @@ -60,42 +59,49 @@ jobs:
pre_release:
runs-on: ubuntu-latest
needs: build

if: ${{ github.event_name == 'push' }}

steps:
- uses: actions/checkout@v4
- name: Use Node.js
- name: 🛠️ Setup Node.js
uses: actions/setup-node@v4
with:
node-version: "18"
registry-url: "https://registry.npmjs.org"
node-version: '18'

- name: Cache dependencies
- name: 💾 Cache dependencies
id: cache
uses: actions/cache@v4
with:
path: '**/node_modules'
key: ${{ runner.os }}-${{ hashFiles('**/yarn.lock') }}

- name: Install dependencies
- name: 📦 Install dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: yarn install --immutable

- name: Version
- name: 🚀 Setup local cache server for Turborepo
uses: felixmosh/turborepo-gh-artifacts@v3
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
server-token: ${{ env.TURBO_TOKEN }}

- name: 🏷️ Version
run: yarn workspaces foreach --worktree --topological --exclude @openmrs/esm-admin-tools version "$(node -e "console.log(require('semver').inc(require('./package.json').version, 'patch'))")-pre.${{ github.run_number }}"

- name: Build
run: yarn turbo run build --color --concurrency=5
- name: 🏗️ Build
run: yarn turbo run build

- run: git config user.email "info@openmrs.org" && git config user.name "OpenMRS CI"
- run: git add . && git commit -m "Prerelease version" --no-verify
- name: 🔧 Configure Git
run: git config user.email "info@openmrs.org" && git config user.name "OpenMRS CI"
- name: 💾 Commit changes
run: git add . && git commit -m "Prerelease version" --no-verify

- name: Pre-release
- name: 🚀 Pre-release
run: yarn config set npmAuthToken "${NODE_AUTH_TOKEN}" && yarn run ci:prepublish
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }}

- name: Upload Artifacts
- name: 📤 Upload Artifacts
uses: actions/upload-artifact@v4
with:
name: packages
Expand All @@ -105,44 +111,63 @@ jobs:

release:
runs-on: ubuntu-latest

needs: build

if: ${{ github.event_name == 'release' }}

steps:
- uses: actions/checkout@v4
- name: Use Node.js
- name: 📥 Download Artifacts
uses: actions/download-artifact@v4
- name: 🛠️ Use Node.js
uses: actions/setup-node@v4
with:
node-version: "18"
registry-url: "https://registry.npmjs.org"
node-version: '18'

- name: Cache dependencies
- name: 💾 Cache dependencies
id: cache
uses: actions/cache@v4
with:
path: '**/node_modules'
key: ${{ runner.os }}-${{ hashFiles('**/yarn.lock') }}

- name: Install dependencies
- name: 📦 Install dependencies
if: steps.cache.outputs.cache-hit != 'true'
run: yarn install --immutable

- name: Build
run: yarn turbo run build --color --concurrency=5
- name: 🚀 Setup local cache server for Turborepo
uses: felixmosh/turborepo-gh-artifacts@v3
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
server-token: ${{ env.TURBO_TOKEN }}

- run: yarn config set npmAuthToken "${NODE_AUTH_TOKEN}" && yarn run ci:publish
- name: 🏗️ Build
run: yarn turbo run build

- name: 🚀 Publish to NPM
run: yarn config set npmAuthToken "${NODE_AUTH_TOKEN}" && yarn run ci:publish
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }}

- name: 📤 Upload Artifacts
uses: actions/upload-artifact@v4
with:
name: dist
path: |
dist
overwrite: true

deploy_admin_tools:
runs-on: ubuntu-latest
needs: pre_release

if: ${{ github.event_name == 'push' }}

steps:
- name: Trigger RefApp Build
- name: 🚀 Trigger RefApp Build
uses: fjogeleit/http-request-action@v1
with:
url: https://ci.openmrs.org/rest/api/latest/queue/O3-BF
method: "POST"
method: 'POST'
customHeaders: '{ "Authorization": "Bearer ${{ secrets.BAMBOO_TOKEN }}" }'
5 changes: 2 additions & 3 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ jobs:
run-e2e-tests:
runs-on: ubuntu-latest
timeout-minutes: 15

steps:
- name: 📥 Checkout repo
uses: actions/checkout@v4
Expand Down Expand Up @@ -53,9 +52,9 @@ jobs:
~/.cache/ms-playwright
key: ${{ runner.os }}-playwright-${{ env.PLAYWRIGHT_VERSION }}

- name: 🎭 Install Playwright browsers
- name: 🎭 Install Playwright Browsers
run: npx playwright install chromium --with-deps
if: steps.playwright-cache.outputs.cache-hit != 'true'
run: yarn playwright install --with-deps

- name: 🚀 Setup local cache server for Turborepo
uses: felixmosh/turborepo-gh-artifacts@v3
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/size.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ jobs:
- name: 📏 Compute bundle size differences
uses: preactjs/compressed-size-action@v2
with:
repo-token: "${{ secrets.GITHUB_TOKEN }}"
repo-token: '${{ secrets.GITHUB_TOKEN }}'
minimum-change-threshold: 10000 # 10 KB
build-script: "turbo run build"
build-script: 'turbo run build'
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ dist/
node_modules/
**/*.md
**/*.json
.eslintrc
14 changes: 7 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

## What is this?

openmrs-esm-admin-tools provides microfrontends for various administrative functions and administrative modules in the OpenMRS ecosystem. It provides tools to allow admin users to effectively manage their 3.x installation without needing to drop to the v1 console.
Admin Tools provides microfrontends for various administrative functions and administrative modules in the OpenMRS ecosystem. It provides tools to allow admin users to effectively manage their 3.x installation without needing to drop to the v1 console.

## How do I configure this module?

Please see the [O3 Docs config guide](https://o3-docs.openmrs.org/docs/configure-o3/overview#configuring-individual-frontend-modules) for information about configuring modules.

## Setup
Expand All @@ -18,15 +18,15 @@ See the guidance in the [Developer Documentation](https://o3-docs.openmrs.org/do

This repository uses Yarn.

To start the dev server for a specific package, run
To start the dev server for a specific package, run:

```bash
yarn start --sources 'packages/esm-<package>-app'
```

This will start a dev server for that package.

To start a dev server running all the packages, run
To start a dev server running all the packages, run:

```bash
yarn start-all
Expand Down Expand Up @@ -86,9 +86,9 @@ To set up environment variables for the project, follow these steps:

1. Create a copy of the .env.example file by running the following command:

```bash
cp example.env .env
```
```bash
cp example.env .env
```

2. Open the newly created .env file in the root of the project.

Expand Down
15 changes: 11 additions & 4 deletions __mocks__/react-i18next.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,23 @@ const renderNodes = (reactNodes) => {
});
};

const useMock = [(k) => k, {}];
useMock.t = (k, o) => (o && o.defaultValue) || (typeof o === 'string' ? o : k);
const useMock = [(key) => key, {}];
useMock.t = (key, defaultValue, options = {}) => {
let translatedString = defaultValue || key;
Object.entries(options).forEach(([k, v]) => {
translatedString = translatedString.replace(new RegExp(`{{${k}}}`, 'g'), v);
});

return translatedString;
};

useMock.i18n = { language: 'en_US' };

module.exports = {
// this mock makes sure any components using the translate HoC receive the t function as a prop
Trans: ({ children }) => renderNodes(children),
Trans: ({ children }) => (Array.isArray(children) ? renderNodes(children) : renderNodes([children])),
Translation: ({ children }) => children((k) => k, { i18n: {} }),
useTranslation: () => useMock,

// mock if needed
I18nextProvider: reactI18next.I18nextProvider,
initReactI18next: reactI18next.initReactI18next,
Expand Down
2 changes: 2 additions & 0 deletions e2e/fixtures/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { type APIRequestContext, type PlaywrightWorkerArgs, type WorkerFixture }
* });
* ```
*/
/* eslint-disable react-hooks/rules-of-hooks */
export const api: WorkerFixture<APIRequestContext, PlaywrightWorkerArgs> = async ({ playwright }, use) => {
const ctx = await playwright.request.newContext({
baseURL: `${process.env.E2E_BASE_URL}/ws/rest/v1/`,
Expand All @@ -24,3 +25,4 @@ export const api: WorkerFixture<APIRequestContext, PlaywrightWorkerArgs> = async

await use(ctx);
};
/* eslint-enable react-hooks/rules-of-hooks */
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @type {import('jest').Config} */
const path = require('path');

/** @type {import('jest').Config} */
module.exports = {
clearMocks: true,
transform: {
Expand Down
Loading

0 comments on commit 5e244cc

Please sign in to comment.