diff --git a/.gitignore b/.gitignore index facb70d..15f7c32 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,6 @@ node_modules .claude/settings.local.json + +# Eval-time scratch artifacts (real app clones, scaffold output, agent transcripts) -- +# regenerable, not part of any shipped skill. +src/skills/*-workspace/ diff --git a/CLAUDE.md b/CLAUDE.md index 2809185..a487c5f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,6 +1,6 @@ # ai-devtools -A pnpm monorepo for publishing AI development tools under the `@dhis2` scope. +A monorepo for publishing AI development tools under the `@dhis2` scope. ## Structure diff --git a/README.md b/README.md index 43bc722..1401ea3 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,97 @@ npx skills add dhis2/ai-devtools --skill dhis2-apps npx skills add https://github.com/dhis2/ai-devtools/tree/main/src/skills/dhis2-apps ``` +## `@dhis2/skill-dhis2-apps` + +An AI skill for building production-quality custom DHIS2 web applications. It guides an AI agent through the full development lifecycle — scaffolding, data fetching, UI, testing, and App Hub compliance — using the DHIS2 App Platform, `@dhis2/ui`, and the DHIS2 Web API. + +The skill reads `@dhis2/api-types` OpenAPI specs and platform library source directly from `node_modules` before writing any code, so it never guesses at API shapes or component props. It cross-checks endpoints across bundled version specs (v40–v43) and surfaces differences to the developer before implementation. + +### How it works + +```mermaid +flowchart TD + U([User request]) --> S[SKILL.md] + + S --> Q{New or existing\nproject?} + + Q -->|Neither d2.config.js\nnor app-runtime found| BOOT[bootstrapping.md] + Q -->|Existing project| RT[Routing table] + + BOOT --> RT + + RT --> RUN[running-your-app.md] + RT --> DF[data-fetching.md] + RT --> UI[ui-patterns.md] + RT --> RO[routing.md] + RT --> TY[types.md] + RT --> TE[testing.md] + + DF -->|version differences\nacross DHIS2 releases| TY + + UI --> UIF[ui-patterns/forms.md] + UI --> UIT[ui-patterns/tables.md] + UI --> UIS[ui-patterns/sidebar.md] + UI --> UIW[ui-patterns/widget.md] + UI --> UID[ui-patterns/dashboards.md] + + RO --> UIS + UIW --> UID + + S --> RULES["Rules (always active)\n─────────────────────\nReact 18 only · @dhis2/ui only\nRead source before writing code\ni18n via @dhis2/d2-i18n · displayName\nCSS Modules + design tokens\nVerify after each turn"] +``` + +### Install + +```sh +npx skills add dhis2/ai-devtools --skill dhis2-apps +``` + +## `@dhis2/skill-modernise-apps` + +An AI skill for bringing an existing DHIS2 app's tooling up to date. It covers six things an +app can need any combination of, or none: + +- **Yarn → pnpm.** Bumps `@dhis2/cli-app-scripts` to a pnpm-capable version, adds a + `pnpm-workspace.yaml` with the hoist patterns `@dhis2/app-shell` needs, converts the + lockfile, fixes the phantom-dependency imports that Yarn 1's flat hoisting used to paper + over, and updates git hooks and CI to call `pnpm` instead of `yarn`. +- **`@dhis2/cli-style` → shared configs.** Replaces the `d2-style` CLI with the shared + `@dhis2/config-eslint`/`@dhis2/config-prettier`/`@dhis2/config-stylelint`/`@dhis2/config-lslint`/`@dhis2/config-commitlint` + packages (whichever tools the app already had configured) and migrates git hooks from the + old `.hooks/` + `d2-style` setup to native `husky`/`lint-staged` — the same setup a + freshly scaffolded app already uses by default. +- **Platform library bumps.** Updates `@dhis2/ui`, `@dhis2/app-runtime`, and + `@dhis2/d2-i18n` — non-breaking (current major) by default for each, with an explicit + choice offered whenever `latest` would cross a major version. Also bumps + `@dhis2/multi-calendar-dates` to its latest version when the app uses it, walking through + the real breaking change in `getNowInCalendar` (Temporal types and time-of-day data + dropped from its return value) with a before/after regression test rather than treating it + as routine. +- **README refresh.** A pnpm badge alongside existing badges, an app description pulled from + the App Hub when the app is published there, and the scaffold's verbose "Available + Scripts" section collapsed into a concise "Get Started". +- **CI modernisation.** Replaces bespoke GitHub Actions workflows with the shared + `dhis2/workflows-platform` reusable workflows wherever one exists, and bumps outdated + action versions and the Node version in whatever stays custom. +- **App Hub continuous delivery.** Wires the app to `dhis2/workflows-platform`'s shared + `release.yml` workflow so releases publish to apps.dhis2.org automatically on every + conventional-commit push, instead of the manual version-bump-and-draft-release flow — + offered when it's missing rather than added unprompted, since it needs an App Hub app ID + and API key from the user. + +Each task ends with an install/build/lint pass to catch anything the change broke, plus an +optional sanity check that starts the app against a real DHIS2 server and confirms the UI +still renders after logging in. + +### Install + +```sh +npx skills add dhis2/ai-devtools --skill modernise-apps +``` + +--- + ## Contributing ```sh @@ -28,3 +119,28 @@ pnpm changeset # record a changeset before opening a PR ``` See [CLAUDE.md](./CLAUDE.md) for repo structure and release flow details. + +### Testing a skill locally, on another project + +`npx skills add` accepts a local filesystem path, not just a GitHub org — no need to push +your changes anywhere first: + +```sh +cd /path/to/some-other-project +npx skills add /path/to/ai-devtools --skill modernise-apps -y +``` + +This copies the skill into that project's `.agents/skills//` (with a `.claude/skills/` +symlink for Claude Code to discover it) and records the source path in `skills-lock.json`. +It's a one-time snapshot, not a live link — re-run the same command (or `npx skills update`) +after editing the skill to pick up changes. Add `-g`/`--global` instead to install it for +every local project under this machine's Claude Code profile, rather than just one. + +Once installed, open a Claude Code session in that project and either describe a matching +task naturally (the skill's description should trigger it) or invoke it explicitly with +`/`. + +For fast iteration while actively editing a skill, skip installing entirely and just point a +session at the file directly — always reflects your latest edits, but doesn't exercise +auto-triggering: _"Read and follow `/path/to/ai-devtools/src/skills//SKILL.md` to do +the task."_ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 087bf6d..fea3856 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -37,6 +37,8 @@ importers: src/skills/example-skill: {} + src/skills/modernise-apps: {} + packages: '@babel/code-frame@7.29.7': diff --git a/src/skills/modernise-apps/SKILL.md b/src/skills/modernise-apps/SKILL.md new file mode 100644 index 0000000..c19836b --- /dev/null +++ b/src/skills/modernise-apps/SKILL.md @@ -0,0 +1,173 @@ +--- +name: modernise-apps +description: > + Guide for modernising existing DHIS2 applications. Use this skill whenever the user + wants to migrate, modernise, or upgrade an existing DHIS2 app to pnpm, move off yarn + or yarn 1, update @dhis2/cli-app-scripts to a version that supports pnpm, or mentions + pnpm-workspace.yaml, the packageManager field, or corepack, or wants to update a + README's badges, app description, or Available Scripts/Get Started section, in the + context of an existing DHIS2 app. Also use it when the user wants to move an app off @dhis2/cli-style + or d2-style onto the shared @dhis2/config-eslint / @dhis2/config-prettier / + @dhis2/config-stylelint / @dhis2/config-lslint / @dhis2/config-commitlint packages, or + mentions eslint.config.mjs, flat config, shared lint config, stylelint, ls-lint, + commitlint, migrating husky/git hooks, or bumping @dhis2/ui, @dhis2/app-runtime, + @dhis2/d2-i18n, or @dhis2/multi-calendar-dates to a newer version, in the + context of an existing DHIS2 app. Also use it when the user wants to modernise an + existing app's CI/GitHub Actions pipeline, move it onto the dhis2/workflows-platform + reusable workflows, bump outdated GitHub Actions, or update the Node version used in CI. + Also use it when the user wants to set up continuous delivery to the DHIS2 App Hub, + automate publishing an app to apps.dhis2.org, or mentions App Hub release automation, + d2-app-scripts publish, semantic-release, or the App Hub API key/app ID. + This skill is for modernising an already-existing app — for scaffolding a brand new app, + use the dhis2-apps skill instead. +--- + +# Modernising DHIS2 Apps + +You are helping a developer bring an existing DHIS2 app up to date. "Modernising" currently +covers five independent tasks — an app can need any combination of them, or none: + +1. **Migrating the package manager** from Yarn (typically Yarn 1) to pnpm. + `@dhis2/cli-app-scripts` has supported pnpm since `12.7.0`, and this skill captures the + concrete steps and gotchas involved — most of which come from Yarn 1's flat dependency + hoisting silently covering up missing dependencies that pnpm's strict resolution exposes. + This task also includes a README refresh: a pnpm badge, an app description pulled from + the App Hub (apps.dhis2.org) if the app is published there, and collapsing the scaffold's + verbose "Available Scripts" section into a concise "Get Started" one. +2. **Migrating off `@dhis2/cli-style`** (the `d2-style` CLI) onto the shared + `@dhis2/config-*` packages — `config-eslint`/`config-prettier` always, plus + `config-stylelint`/`config-lslint` for whichever of those the app already had configured + — with plain `eslint`/`prettier`/`stylelint`/`ls-lint` and native `husky`/`lint-staged`, + the same setup a fresh `pnpm create @dhis2/app` scaffold already uses by default. +3. **Bumping platform libraries** — `@dhis2/ui`, `@dhis2/app-runtime`, and `@dhis2/d2-i18n` + (non-breaking by default, with an explicit choice offered if `latest` would cross a major + version for any of them), and `@dhis2/multi-calendar-dates` if used — its `3.x` (`3.0.0`, + now `latest`) is a real breaking change: `getNowInCalendar` + dropped Temporal types and time-of-day data from its return value entirely. +4. **Modernising the CI pipeline** — replacing bespoke GitHub Actions workflows with the + shared `dhis2/workflows-platform` reusable workflows wherever one exists, and bumping any + outdated action version or Node version left in whatever stays custom. +5. **Setting up continuous delivery to the App Hub** — wiring the app to + `dhis2/workflows-platform`'s `release.yml` reusable workflow so releases publish to + apps.dhis2.org automatically on every conventional-commit push to the default branch, + instead of the manual per-release `yarn version` + draft-GitHub-Release flow. Needs an App + Hub app ID and API key from the user — **offer this task when it's missing rather than + adding it unprompted**, since it changes how releases happen going forward. + +## First: confirm this is an existing DHIS2 app, and which task(s) apply + +| Check | How | Result | +| ------------------------------------------------------------------------------------------------------ | ------------------------------------------------ | -------------------------------------------------------------------------------------------------- | +| `d2.config.js` exists and `@dhis2/cli-app-scripts` is in `package.json` | Glob + read `package.json` | **DHIS2 app** — continue | +| Neither found | — | **Not a DHIS2 app** — this skill doesn't apply; see `dhis2-apps` if the user wants to scaffold one | +| `yarn.lock` exists and `pnpm-lock.yaml` does not | `ls` | **Not yet migrated to pnpm** — proceed with `references/pnpm-migration.md` if that's what's needed | +| `pnpm-lock.yaml` already exists | `ls` | **Already migrated to pnpm** — nothing to do for that task | +| `@dhis2/cli-style` is in `package.json` | Read `package.json` | **Still on `d2-style`** — proceed with `references/style-configs-migration.md` if that's needed | +| `@dhis2/ui`/`app-runtime`/`d2-i18n`/`multi-calendar-dates` is present and behind `latest` | Read `package.json` + `npm view dist-tags` | **Platform library behind** — proceed with `references/platform-library-bumps.md` if that's needed | +| `.github/workflows/*.yml` has bespoke jobs, not just `uses: dhis2/workflows-platform/...` | Read `.github/workflows/*.yml` | **CI not fully modernised** — proceed with `references/ci-migration.md` if that's needed | +| No workflow calls `.../release.yml` with App Hub publish on, and `d2.config.js`'s `type` isn't `'lib'` | Read `.github/workflows/*.yml` + `d2.config.js` | **No App Hub CD** — offer `references/apphub-release.md`; don't set it up unprompted (see Rules) | + +## What does the user need? + +| Scenario | References (read in order) | +| ----------------------------------------------------------------------------------------------------- | ----------------------------------------------- | +| Migrate an app from yarn to pnpm | `references/pnpm-migration.md` | +| Refresh a migrated app's README (pnpm badge, App Hub description, concise "Get Started") | `references/pnpm-migration.md` (Step 8) | +| Sanity-check a migrated app renders correctly against a real server (opt-in) | `references/pnpm-migration.md` (Step 11) | +| Move off `@dhis2/cli-style`/`d2-style` onto shared lint/format configs | `references/style-configs-migration.md` | +| Bump `@dhis2/ui`/`@dhis2/app-runtime`/`@dhis2/d2-i18n` to the latest version | `references/platform-library-bumps.md` (Step 1) | +| Bump `@dhis2/multi-calendar-dates` to the latest version | `references/platform-library-bumps.md` (Step 2) | +| Modernise CI: adopt `dhis2/workflows-platform` reusable workflows, bump outdated actions/Node version | `references/ci-migration.md` | +| Set up continuous delivery to the App Hub | `references/apphub-release.md` | + +More modernisation tasks (dependency upgrades, router migrations, etc.) may be added here +in the future — this table is deliberately structured to grow. + +## Rules + +- **Never delete `yarn.lock` before `pnpm import` has run successfully.** `pnpm import` + reads `yarn.lock` to seed `pnpm-lock.yaml` — deleting it first throws away the information + needed for a clean conversion. +- **Always bump `@dhis2/cli-app-scripts` to `>=12.7.0`** (check `npm view @dhis2/cli-app-scripts version` for the current latest) before attempting the migration — pnpm support does not exist in older versions. +- **Always install and build/test after migrating, and fix every resolution error before + considering the migration done.** Don't leave `pnpm-workspace.yaml` hoist patterns + incomplete just because `pnpm install` succeeded — module resolution errors often only + surface at build or test time. +- **`dhis2/workflows-platform`'s reusable workflow refs are not interchangeable — pick the + one that matches this app's pnpm _and_ `cli-style` status, not just `@v1` by default.** + `@v1` is yarn-only and `lint`/`lint-commits`/`lint-pr-title` require `@dhis2/cli-style`; + the `pnpm` branch is pnpm-native but those same jobs still require `cli-style`; `@v2` is + pnpm-native and drops that requirement. See `references/ci-migration.md` Step 3. +- **The Step 11 sanity e2e check (`references/pnpm-migration.md`) is optional and opt-in.** + Only run it if the user asks for extra confidence before deploying, or explicitly requests + it — never run it automatically as part of a routine migration. It needs outbound network + access to a real DHIS2 server, downloads a browser binary on first use, and takes real + wall-clock time, none of which are appropriate defaults for every migration. +- **When migrating off `@dhis2/cli-style`, only migrate the tools the app actually had + configured.** `d2-style` makes eslint/prettier/stylelint/ls-lint each independently + opt-in — check for `.stylelintrc.js`/`.ls-lint.yml` before adding `@dhis2/config-stylelint` + or `@dhis2/config-lslint`; don't introduce a tool the app never used. +- **Remove `@dhis2/cli-style` last, once every config it was proxying has a real + replacement** — not as the first step. It stays installed while eslint/prettier/ + stylelint/ls-lint/commitlint are migrated one at a time so each can be verified against a + still-working baseline, and only comes out once nothing references it (see + `references/style-configs-migration.md` Step 10). Don't leave it listed but unused once + removed — the reference PR (`route-manager-app#35`) missed this; don't repeat that + oversight. +- **The `@dhis2/ui`/`@dhis2/app-runtime`/`@dhis2/d2-i18n` bump defaults to non-breaking** + (current major only) for each library independently. If `latest` would cross a major for + any of them, ask the user whether they want that library's latest non-breaking version or + its true latest — don't silently pick one. If you can't ask (unattended), take the + non-breaking default and say so in the summary. If a same-major bump surfaces new type or + lint errors, that's a real regression to investigate via the changelog, not something to + silence. +- **`@dhis2/multi-calendar-dates`'s `3.x` is a real breaking change — don't treat it like + the routine non-breaking bumps above.** + `getNowInCalendar` no longer returns anything Temporal-shaped, and — easy to miss — no + longer carries time-of-day at all, only `{ year, month, day, eraYear? }`. This fails + silently (wrong output, not an error) if a call site reads `.hour`/`.minute`/`.second` off + the result. +- **Before bumping `@dhis2/multi-calendar-dates`, write a before/after regression test + covering every direct import from the library** — `getNowInCalendar`, the + period-calculation helpers (`getFixedPeriodByDate`, `getAdjacentFixedPeriods`, etc. — + `#102` rewrote these too, not just `getNowInCalendar`), and `useDatePicker` if the app + uses it directly. The exclusion is `@dhis2/ui`'s own bundled `Calendar`/`DatePicker` + components (a separate internal copy this app doesn't control), not this library's own + hook — check the import, not the visible UI. Cover `gregory`, `ethiopian`, and `nepali` + for each; run the test against the currently-installed version first to confirm it's a + real baseline, then bump and re-run it unmodified. Add it as a real committed test if the + app has a test runner; otherwise use a temporary standalone script (a headless-hook test + via `renderHook` for `useDatePicker` if used) and delete it once confirmed safe. Don't rely + on `pnpm lint`/a generic `pnpm test` pass alone as proof this is safe. See + `references/platform-library-bumps.md` Step 2. +- **When matching an app to its App Hub listing, match on `d2.config.js`'s `id` first, not + `title`.** The App Hub display name doesn't always match the repo/title name (e.g. + `aggregate-data-entry-app`'s `title` is `Data Entry`, listed on the App Hub as just "Data + Entry"), but `id` is the same UUID on both sides. Don't fabricate a description if the app + genuinely isn't listed there. +- **Offer App Hub CD setup when it's missing, don't add it unprompted** — unlike the other + four tasks, this one changes how releases happen going forward (automatic, on every + conventional-commit push to the default branch) and needs two things only the user can + provide: the App Hub app ID (ask for it directly) and a `DHIS2_BOT_APPHUB_TOKEN` API key + (**never ask the user for the token value itself — ask them to add the secret in GitHub + themselves, then confirm it exists via `gh secret list`, which never exposes the value**). + Never invent or guess either. See `references/apphub-release.md`. +- **Use the shared `release.yml` reusable workflow for App Hub delivery, not the bespoke + `apphub-release.yml` + `d2-app-scripts publish` workflow from the public App Hub guide.** + The shared workflow already exists or is used across the platform and drives versioning + from conventional commits via `dhis2/action-semantic-release`, rather than a manual + `yarn version --patch` per release. See `references/apphub-release.md`. + +## Troubleshooting + +| Symptom | Fix | +| ------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Build/runtime error resolving a package that isn't declared as a direct dependency | It was likely phantom-hoisted by yarn 1. Add it explicitly to `dependencies`/`devDependencies`, or add it to `publicHoistPattern` in `pnpm-workspace.yaml` if `@dhis2/app-shell` needs it directly. | +| Import from a scoped subpackage fails (e.g. `@dhis2-ui/checkbox`, `@dhis2/ui-forms`) | These have been consolidated into `@dhis2/ui`. Update the import rather than trying to hoist the old package. | +| pnpm warns about ignored build scripts for a dependency | Add the package to `onlyBuiltDependencies` (to allow it) or `ignoredBuiltDependencies` (to silence it) in `pnpm-workspace.yaml`, depending on whether the app actually needs that build step. | +| ESLint errors "Cannot find config @dhis2/config-eslint" after moving off `d2-style` | Either it wasn't installed, or the old `.eslintrc.js` still exists alongside the new flat `eslint.config.mjs` and ESLint is confused about which to use — delete the old one. See `references/style-configs-migration.md`. | +| `stylelint`/`ls-lint` fails after moving off `d2-style` | The app never had `stylelint`/`@ls-lint/ls-lint` as a direct dependency before — `cli-style` bundled and ran its own copies. Add the real tool as a devDependency, not just its `@dhis2/config-*` package. See `references/style-configs-migration.md` Step 2. | +| A `dhis2/workflows-platform` job fails with "Cannot find module '@dhis2/cli-style'" | That job (`lint.yml`, `lint-commits.yml`, or `lint-pr-title.yml`) requires `cli-style`. See `references/ci-migration.md` Step 3. | +| A CI workflow silently never runs after switching to a reusable workflow | The `uses:` reference (org/repo/path/ref) is wrong — GitHub doesn't report this as a failure, the workflow just never appears in the Actions tab. See `references/ci-migration.md`. | +| A date/time string looks subtly wrong (not an error) after bumping `@dhis2/multi-calendar-dates` | A `getNowInCalendar` call site is reading `.hour`/`.minute`/`.second` off the new plain object, which no longer has them. See `references/platform-library-bumps.md` Step 2. | +| App Hub release job fails with `EMISSINGAPPHUBID`/`EMISSINGMINDHIS2VERSION`/`EMISSINGTOKEN` | `d2.config.js`'s `id`/`minDHIS2Version` or the `DHIS2_BOT_APPHUB_TOKEN` secret is missing, or the release job wasn't given `secrets: inherit`. See `references/apphub-release.md` Steps 2, 4-5. | diff --git a/src/skills/modernise-apps/evals/evals.json b/src/skills/modernise-apps/evals/evals.json new file mode 100644 index 0000000..ce924c9 --- /dev/null +++ b/src/skills/modernise-apps/evals/evals.json @@ -0,0 +1,205 @@ +{ + "skill_name": "modernise-apps", + "evals": [ + { + "id": 0, + "name": "scaffold-js-migration", + "prompt": "I scaffolded this app with `pnpm create @dhis2/app` but picked yarn as the package manager before we standardized on pnpm across the team. Can you migrate it over to pnpm like our other apps?", + "expected_output": "A fresh, unmodified `pnpm create @dhis2/app --typescript=false --packageManager yarn` scaffold (JS, basic template). @dhis2/cli-app-scripts already ships at 12.10.3 in this scaffold, so the version-bump step is a no-op the agent should recognize rather than force. Needs: pnpm-workspace.yaml with the app-shell hoist patterns, pnpm import + removal of yarn.lock, packageManager field, and a successful pnpm install + build. No CI workflows or git hooks exist in a fresh scaffold, so those steps are naturally no-ops here.", + "files": ["fixtures/scaffold-js/"], + "assertions": [ + "yarn.lock has been deleted from the app", + "pnpm-lock.yaml exists in the app", + "pnpm-workspace.yaml exists with a publicHoistPattern that covers @dhis2/*", + "package.json has a packageManager field set to a pnpm version", + "@dhis2/cli-app-scripts is left at (or bumped to) >=12.7.0 -- it already ships at 12.10.3 in this scaffold", + "pnpm install completes successfully", + "pnpm build completes successfully" + ] + }, + { + "id": 1, + "name": "scaffold-ts-migration", + "prompt": "Same as our JS apps -- I scaffolded this one with `pnpm create @dhis2/app` using TypeScript, but yarn ended up as the package manager. Can you get it onto pnpm like the rest of our apps?", + "expected_output": "A fresh, unmodified `pnpm create @dhis2/app --typescript=true --packageManager yarn` scaffold (TS, basic template). Same expectations as scaffold-js-migration, plus tsconfig.json/TypeScript setup should be left intact and a type-check should still pass.", + "files": ["fixtures/scaffold-ts/"], + "assertions": [ + "yarn.lock has been deleted from the app", + "pnpm-lock.yaml exists in the app", + "pnpm-workspace.yaml exists with a publicHoistPattern that covers @dhis2/*", + "package.json has a packageManager field set to a pnpm version", + "@dhis2/cli-app-scripts is left at (or bumped to) >=12.7.0 -- it already ships at 12.10.3 in this scaffold", + "pnpm install completes successfully", + "pnpm build completes successfully", + "tsconfig.json and the TypeScript setup were left intact" + ] + }, + { + "id": 2, + "name": "real-aggregate-data-entry-app", + "prompt": "Please migrate this app from yarn to pnpm, the same way our other apps have been doing it.", + "expected_output": "A real, unmigrated snapshot of dhis2/aggregate-data-entry-app@master (as of this eval's creation). @dhis2/cli-app-scripts here is already ^12.11.0, so that step is already done -- the agent should notice and not redundantly 'bump' it. Needs: pnpm-workspace.yaml with the app-shell hoist patterns, pnpm import + removal of yarn.lock, packageManager field, fixing any @dhis2-ui/* or @dhis2/ui-forms imports found under src/ to use @dhis2/ui instead, updating .hooks/* and .github/workflows/*.yml from yarn to pnpm (pointing any dhis2/workflows-platform reusable workflow refs at the @pnpm branch), and a successful pnpm install + build. There is a real open PR (dhis2/aggregate-data-entry-app#481, branch upgrade/pnpm) that did this exact migration -- useful as a sanity-check reference for what changes were actually needed, though it may not be the final/merged state.", + "files": ["fixtures/real-aggregate-data-entry-app/"], + "assertions": [ + "yarn.lock has been deleted from the app", + "pnpm-lock.yaml exists in the app", + "pnpm-workspace.yaml exists with a publicHoistPattern that covers @dhis2/* and other app-shell-relevant packages", + "package.json has a packageManager field set to a pnpm version", + "@dhis2/cli-app-scripts was left at (or bumped to) >=12.7.0 without being redundantly re-bumped or downgraded", + ".hooks/pre-commit and .hooks/commit-msg were updated to invoke pnpm instead of yarn", + "every .github/workflows/*.yml file that referenced yarn was updated to reference pnpm, and any dhis2/workflows-platform reusable workflow refs point at @pnpm", + "any @dhis2-ui/* or @dhis2/ui-forms imports found under src/ were updated to import from @dhis2/ui", + "pnpm install completes without unresolved dependency/module errors", + "pnpm build completes successfully" + ] + }, + { + "id": 3, + "name": "real-route-manager-app", + "prompt": "Can you get this app moved over from yarn to pnpm? Want to keep it consistent with the rest of our apps.", + "expected_output": "A real, unmigrated snapshot of dhis2/route-manager-app@main (as of this eval's creation) -- a TypeScript DHIS2 app. @dhis2/cli-app-scripts here is still ^12.3.0, so it genuinely needs bumping. Same migration steps as the other evals, including pointing any dhis2/workflows-platform reusable workflow refs at @pnpm. TypeScript should not need any special handling beyond what the skill already covers. pnpm install and build should succeed afterwards.", + "files": ["fixtures/real-route-manager-app/"], + "assertions": [ + "yarn.lock has been deleted from the app", + "pnpm-lock.yaml exists in the app", + "pnpm-workspace.yaml exists with an appropriate publicHoistPattern covering @dhis2/*", + "package.json has a packageManager field set to a pnpm version", + "@dhis2/cli-app-scripts was bumped from ^12.3.0 to >=12.7.0", + ".hooks and .github/workflows/*.yml were updated from yarn to pnpm, and any dhis2/workflows-platform reusable workflow refs point at @pnpm", + "pnpm install completes without unresolved dependency/module errors", + "pnpm build completes successfully", + "tsconfig.json and the app's TypeScript setup were left intact" + ] + }, + { + "id": 4, + "name": "style-configs-route-manager-app", + "prompt": "Can you get this app off @dhis2/cli-style and onto our shared configs, like we've been doing with our other apps?", + "expected_output": "A real snapshot of dhis2/route-manager-app (TypeScript) still on @dhis2/cli-style (`lint: yarn tsc && d2-style check`, `@dhis2/cli-style: ^10.7.6`) with a CI job at `lint: uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1`. No .hooks/.husky directory exists in this fixture, so the hook-migration part of Step 8 is a no-op -- the agent should recognize that rather than inventing hooks that weren't there. This fixture has NO .stylelintrc.js or .ls-lint.yml -- only eslint/prettier were ever configured, so the agent should recognize that and not introduce stylelint/ls-lint tooling the app never had. `@dhis2/ui` is at `^10.1.11`, well behind current -- since the skill bundles a non-breaking @dhis2/ui bump into this task, that should happen even though the prompt doesn't ask for it by name. Needs: @dhis2/cli-style removed, @dhis2/config-eslint + @dhis2/config-prettier + @eslint/compat added, eslint.config.mjs + .prettierrc.mjs added, .eslintrc.js/.prettierrc.js removed, lint/format scripts updated, the reusable lint.yml@v1 CI job folded into an inline lint step, @dhis2/ui bumped within its current major, and pnpm/yarn lint passing afterwards.", + "files": ["fixtures/real-route-manager-app/"], + "assertions": [ + "@dhis2/cli-style is removed from package.json entirely", + "eslint.config.mjs exists and imports from @dhis2/config-eslint", + ".prettierrc.mjs exists and imports from @dhis2/config-prettier", + "the old .eslintrc.js and .prettierrc.js are gone", + "package.json's lint script no longer calls d2-style (uses eslint and prettier directly)", + "the separate reusable lint.yml@v1 CI job is gone and lint now runs as an inline step in the remaining job", + "eslint runs successfully against the app with no config-resolution errors", + "prettier -c . runs successfully with no config-resolution errors", + "no stylelint or ls-lint config was introduced -- this app never had either configured under cli-style", + "@dhis2/ui was bumped to the latest version within its current major (was ^10.1.11)" + ] + }, + { + "id": 5, + "name": "style-configs-aggregate-data-entry-app", + "prompt": "Can you move this app off @dhis2/cli-style onto our shared configs? Same as the other apps.", + "expected_output": "A real snapshot of dhis2/aggregate-data-entry-app still on @dhis2/cli-style (`lint: d2-style check`, `@dhis2/cli-style: ^10.7.9`), with `.hooks/pre-commit` calling `d2-style check --staged` AND `.hooks/commit-msg` calling `d2-style check commit` directly (not just delegating to CI) -- this is the harder case the route-manager-app fixture doesn't cover, since the commit-msg hook needs a real commitlint replacement, not just deletion, per Step 9. This fixture ALSO has a real `.stylelintrc.js` (proxying `@dhis2/cli-style`'s `config.stylelint`) and a real, customized `.ls-lint.yml` (per-extension casing rules, not just the generic base `.dir: kebab-case`) -- the agent needs to notice both are configured, migrate them to @dhis2/config-stylelint/@dhis2/config-lslint, add `stylelint`/`@ls-lint/ls-lint` as real devDependencies (cli-style bundled its own copies of both, so the app never had them directly), and preserve the custom ls-lint rules rather than blowing them away with a blind copy of the generic shared base file. Also has a `lint-commits`/`lint-pr-title` pair pointed at `dhis2/workflows-platform`'s reusable workflows -- these are NOT unrelated to this migration despite the name, since both resolve their commitlint config via `require('@dhis2/cli-style').config.commitlint)` internally and will break once cli-style is removed; they need a custom commitlint step instead. Also has a `lint: uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1` job (should be folded into an inline step, it runs `d2-style check` internally too), and `@dhis2/ui` at `^10.16.1` that should get a non-breaking bump.", + "files": ["fixtures/real-aggregate-data-entry-app/"], + "assertions": [ + "@dhis2/cli-style is removed from package.json entirely", + "eslint.config.mjs exists and imports from @dhis2/config-eslint", + ".prettierrc.mjs exists and imports from @dhis2/config-prettier", + "the old .eslintrc.js and .prettierrc.js are gone", + "package.json's lint script no longer calls d2-style", + ".hooks/ is gone and .husky/pre-commit exists calling lint-staged", + ".husky/commit-msg exists and calls commitlint (not d2-style) since the original .hooks/commit-msg called d2-style check commit directly", + "the lint-commits/lint-pr-title reusable workflow jobs are replaced with a commitlint-based step (both require @dhis2/cli-style internally and would otherwise break)", + "the separate reusable lint.yml@v1 CI job is gone and lint now runs as an inline step", + "eslint runs successfully against the app with no config-resolution errors", + ".stylelintrc.* no longer proxies cli-style and imports @dhis2/config-stylelint instead", + ".ls-lint.yml is migrated to @dhis2/config-lslint's base, with this app's custom per-extension casing rules preserved rather than dropped", + "stylelint and @ls-lint/ls-lint were added as real devDependencies, not just their @dhis2/config-* packages", + "@dhis2/ui was bumped to the latest version within its current major (was ^10.16.1)" + ] + }, + { + "id": 6, + "name": "combined-migration-route-manager-app", + "prompt": "Can you fully modernize this app? We want it moved over to pnpm and off @dhis2/cli-style onto our shared configs -- same as what we've been doing with our other apps.", + "expected_output": "A real snapshot of dhis2/route-manager-app still on yarn AND @dhis2/cli-style -- tests that both migrations, done together in one request, land on one coherent end-state rather than the agent doing the pnpm-shaped edit to a file (e.g. CI workflow, git hooks) and then redoing it for the style-configs migration, or leaving a half-pnpm/half-yarn or half-cli-style/half-shared-config hybrid anywhere. No .hooks/.husky directory exists in this fixture, and it has no .stylelintrc.js/.ls-lint.yml either -- only eslint/prettier were ever configured, so the agent shouldn't introduce stylelint/ls-lint tooling the app never had. `@dhis2/ui` is at `^10.1.11` and should get a non-breaking bump even though the prompt doesn't name it explicitly. Needs: everything from both pnpm-migration.md and style-configs-migration.md, with CI and any hooks reflecting the combined final state, not two sequential unreconciled passes.", + "files": ["fixtures/real-route-manager-app/"], + "assertions": [ + "yarn.lock is gone and pnpm-lock.yaml exists", + "pnpm-workspace.yaml exists with a publicHoistPattern covering @dhis2/*", + "package.json has a packageManager field set to a pnpm version", + "@dhis2/cli-app-scripts was bumped to >=12.7.0", + "@dhis2/cli-style is removed from package.json", + "eslint.config.mjs exists and imports @dhis2/config-eslint", + ".prettierrc.mjs exists and imports @dhis2/config-prettier", + "the old .eslintrc.js/.prettierrc.js are gone", + "package.json's lint script uses eslint/prettier directly, calls neither d2-style nor yarn", + "the CI workflow reflects one coherent end-state: pnpm-based install/build, the reusable lint.yml@v1 job folded into an inline step, no leftover yarn or d2-style references anywhere", + "pnpm install completes without unresolved dependency errors", + "eslint runs with no config-resolution errors", + "no stylelint or ls-lint config was introduced -- this app never had either configured under cli-style", + "@dhis2/ui was bumped to the latest version within its current major (was ^10.1.11)" + ] + }, + { + "id": 7, + "name": "combined-migration-aggregate-data-entry-app", + "prompt": "Can you fully modernize this app? We want it moved over to pnpm and off @dhis2/cli-style onto our shared configs -- same as what we've been doing with our other apps.", + "expected_output": "A real snapshot of dhis2/aggregate-data-entry-app still on yarn AND @dhis2/cli-style, with real .hooks/pre-commit and .hooks/commit-msg (commit-msg calls d2-style check commit directly) -- the harder combined case, since the pnpm migration's hook step (swap yarn->pnpm in .hooks/*) and the style-configs migration's hook step (replace .hooks/ with .husky/ + lint-staged + commitlint) both touch the same two files. Also has a real `.stylelintrc.js` (proxying cli-style) and a customized `.ls-lint.yml` (per-extension casing rules) that both need migrating to @dhis2/config-stylelint/@dhis2/config-lslint, with stylelint/@ls-lint/ls-lint added as real devDependencies and the custom ls-lint rules preserved. Also has a `lint-commits`/`lint-pr-title` pair pointed at dhis2/workflows-platform's reusable workflows, which resolve their commitlint config via `require('@dhis2/cli-style').config.commitlint)` internally and will break once cli-style is removed -- needs a custom commitlint step instead, not just repointing to @pnpm (the @pnpm branch has the same cli-style dependency) -- plus a non-breaking `@dhis2/ui` bump (currently ^10.16.1). Tests that the agent reconciles all of this into one final coherent state rather than doing a wasted intermediate pnpm-flavored edit to .hooks/ that then gets thrown away, or dropping the style-configs pieces because the request also mentioned pnpm.", + "files": ["fixtures/real-aggregate-data-entry-app/"], + "assertions": [ + "yarn.lock is gone and pnpm-lock.yaml exists", + "pnpm-workspace.yaml exists with a publicHoistPattern covering @dhis2/*", + "package.json has a packageManager field set to a pnpm version", + "@dhis2/cli-app-scripts left at (or bumped to) >=12.7.0 without being redundantly downgraded", + "@dhis2/cli-style is removed from package.json", + "eslint.config.mjs exists and imports @dhis2/config-eslint", + ".prettierrc.mjs exists and imports @dhis2/config-prettier", + "the old .eslintrc.js/.prettierrc.js are gone", + ".hooks/ is gone (not left as a half-migrated pnpm-flavored version) and .husky/pre-commit exists calling lint-staged", + ".husky/commit-msg exists and calls commitlint, not d2-style", + "the lint-commits/lint-pr-title reusable workflow jobs are replaced with a commitlint-based step (both require @dhis2/cli-style internally and would otherwise break)", + "the reusable lint.yml@v1 CI job is gone and lint runs as an inline pnpm step", + "any @dhis2-ui/* or @dhis2/ui-forms imports found under src/ were updated to import from @dhis2/ui", + "pnpm install completes without unresolved dependency errors", + "eslint runs with no config-resolution errors", + ".stylelintrc.* no longer proxies cli-style and imports @dhis2/config-stylelint instead", + ".ls-lint.yml is migrated to @dhis2/config-lslint's base, with this app's custom per-extension casing rules preserved", + "stylelint and @ls-lint/ls-lint were added as real devDependencies, not just their @dhis2/config-* packages", + "@dhis2/ui was bumped to the latest version within its current major (was ^10.16.1)" + ] + }, + { + "id": 8, + "name": "ci-migration-app-management-app", + "prompt": "Can you modernize this app's CI pipeline? Move it onto our shared GitHub Actions workflows wherever possible, and bump anything outdated.", + "expected_output": "A real snapshot of dhis2/app-management-app's CI as it was just before dhis2/app-management-app#554 (the real PR that did this exact migration) -- entirely bespoke workflows, none of them using dhis2/workflows-platform yet: comment-and-close.yml uses the third-party vardevs/candc@v1 action directly, dhis2-verify-app.yml hand-rolls build/lint/test/release jobs on actions/checkout@v2 + actions/setup-node@v1 + Node 16 + the unmaintained c-hive/gha-yarn-cache action, dhis2-verify-commits.yml hand-rolls PR-title and commit-message linting (also via ancient actions, also resolving its commitlint config through @dhis2/cli-style, which stays installed and in-scope here since this eval is CI-only), and dhis2-preview-pr.yml hand-rolls a Netlify PR-preview deploy. Needs: comment-and-close.yml, lint, lint-commits, test, release, and lint-pr-title all replaced with dhis2/workflows-platform reusable-workflow wrappers (composed the way #554 did -- e.g. one test-and-release.yml calling lint-commits.yml/lint.yml/test.yml/release.yml, a separate lint-pr-title.yml), the three old bespoke files deleted, and no outdated actions/checkout, actions/setup-node, Node version, or c-hive/gha-yarn-cache left in whatever custom step (if any) remains.", + "files": ["fixtures/real-app-management-app/"], + "assertions": [ + "the old bespoke actions (c-hive/gha-yarn-cache, vardevs/candc, JulienKode/pull-request-name-linter-action, dhis2/action-semantic-release, dhis2/deploy-build, nwtgck/actions-netlify) are gone from every workflow, whether by deleting the file or rewriting it in place", + "comment-and-close.yml points at the reusable comment-and-close.yml workflow instead of vardevs/candc@v1", + "lint, lint-commits, test, release, and lint-pr-title are all composed from dhis2/workflows-platform reusable-workflow wrappers", + "no remaining custom step uses actions/checkout below v4, actions/setup-node below v4, a Node version below 20, or c-hive/gha-yarn-cache" + ] + }, + { + "id": 9, + "name": "ci-migration-synthetic-app", + "prompt": "Can you modernize this app's CI pipeline? Move it onto our shared GitHub Actions workflows wherever possible, and bump anything outdated.", + "expected_output": "A synthetic (not-real) DHIS2 app, 'field-visit-tracker-app', whose repo does not exist on GitHub -- deliberately, so an agent with network access can't shortcut by finding a real repo that's already completed this exact migration and copying its answer (that's what happened with the real-app-management-app fixture once network access was allowed -- see iteration-5b's benchmark notes). Its .github/workflows/ content is the same genuinely-shared old-DHIS2-CI-template pattern real apps of that era used (comment-and-close.yml on vardevs/candc@v1, dhis2-verify-app.yml hand-rolling build/lint/test/release on actions/checkout@v2 + setup-node@v1 + Node 16 + c-hive/gha-yarn-cache, dhis2-verify-commits.yml resolving commitlint config through @dhis2/cli-style on ancient actions, dhis2-preview-pr.yml hand-rolling a Netlify PR preview) -- so the fix itself is identical to eval 8, but this fixture can't be 'solved' by fetching a real answer key, only by actually knowing (from the skill, or independently discovering) dhis2/workflows-platform's real reusable-workflow catalog. Use this fixture instead of/alongside real-app-management-app whenever comparing with-skill vs. baseline WITH network access enabled for both agents.", + "files": ["fixtures/synthetic-ci-app/"], + "assertions": [ + "the old bespoke actions (c-hive/gha-yarn-cache, vardevs/candc, JulienKode/pull-request-name-linter-action, dhis2/action-semantic-release, dhis2/deploy-build, nwtgck/actions-netlify) are gone from every workflow, whether by deleting the file or rewriting it in place", + "comment-and-close.yml points at the reusable comment-and-close.yml workflow instead of vardevs/candc@v1", + "lint, lint-commits, test, release, and lint-pr-title are all composed from dhis2/workflows-platform reusable-workflow wrappers", + "no remaining custom step uses actions/checkout below v4, actions/setup-node below v4, a Node version below 20, or c-hive/gha-yarn-cache" + ] + }, + { + "id": 10, + "name": "multi-calendar-dates-bump-aggregate-data-entry-app", + "prompt": "Can you update @dhis2/multi-calendar-dates to the latest version in this app?", + "expected_output": "A real snapshot of dhis2/aggregate-data-entry-app on @dhis2/multi-calendar-dates ^2.1.2 -- the latest major (3.x, now published as latest) is a genuine breaking change (dhis2/multi-calendar-dates#102): getNowInCalendar now returns a plain { year, month, day, eraYear? } object instead of a Temporal.ZonedDateTime, dropping time-of-day entirely. This app's src/shared/date/get-now-in-calendar.js wraps it in getNowInCalendarString, whose stringifyDate helper reads .hour/.minute/.second off the result when `long: true` is passed (used by src/shared/date/date-utils.js and src/shared/locked-status/use-check-lock-status.js) -- naively bumping without fixing this produces garbled output like '2024-06-15Tundefined:undefined:undefined', not an error. The app's own pre-existing jest suite (src/shared/date/get-now-in-calendar.test.js) hard-asserts exact HH:MM:SS strings, including a timezone-correction case for Africa/Kigali (UTC+2) -- this is the actual acceptance test, verified for real against the live package: the correct fix (deriving time-of-day via Intl.DateTimeFormat with the timezone param, not a bare `new Date()` which would only give local time and silently drop the timezone correction) passes all 4 tests; the naive bump fails 3 of 4.", + "files": ["fixtures/real-aggregate-data-entry-app/"], + "assertions": [ + "@dhis2/multi-calendar-dates is bumped to the 3.x line", + "src/shared/date/get-now-in-calendar.test.js still passes in full, including the Africa/Kigali timezone-correction case -- not just that eslint/tsc are silent" + ] + } + ] +} diff --git a/src/skills/modernise-apps/package.json b/src/skills/modernise-apps/package.json new file mode 100644 index 0000000..ca3784b --- /dev/null +++ b/src/skills/modernise-apps/package.json @@ -0,0 +1,12 @@ +{ + "name": "@dhis2/skill-modernise-apps", + "version": "0.0.1", + "description": "AI skill for modernising DHIS2 apps (pnpm migration and beyond)", + "license": "BSD-3-Clause", + "publishConfig": { + "access": "public" + }, + "exports": { + ".": "./SKILL.md" + } +} diff --git a/src/skills/modernise-apps/references/apphub-release.md b/src/skills/modernise-apps/references/apphub-release.md new file mode 100644 index 0000000..05a118b --- /dev/null +++ b/src/skills/modernise-apps/references/apphub-release.md @@ -0,0 +1,167 @@ +# Setting Up Continuous Delivery to the App Hub + +This is the recipe for wiring an app up to publish itself to the [DHIS2 App +Hub](https://apps.dhis2.org) automatically, instead of the manual +`yarn version --patch` → push tag → draft GitHub Release flow described in +[the App Hub publishing guide](https://developers.dhis2.org/docs/guides/publish-apphub). +**Use the shared `dhis2/workflows-platform` `release.yml` reusable workflow for this, not +the bespoke `apphub-release.yml` + `d2-app-scripts publish` workflow the guide shows** — the +shared workflow already exists, is what current DHIS2 apps actually use, and drives +versioning from conventional commits (via `dhis2/action-semantic-release`) rather than a +manual version bump per release. This is independent of the other modernisation tasks — an +app can need this in any combination with pnpm/style-configs/platform-library/CI work. + +Reference: [`dhis2/workflows-platform`'s `release.yml`](https://github.com/dhis2/workflows-platform/blob/v2/.github/workflows/release.yml) +and [`dhis2/action-semantic-release`](https://github.com/dhis2/action-semantic-release) (the +action it calls — see `custom/semantic-release-apphub.js` for exactly what it checks before +publishing). + +--- + +## Step 1: Check whether this is even needed + +**Offer this, don't silently add it** — it changes how releases happen (automatic, driven by +commit messages, on every push to the default branch) and needs a secret only a human with +App Hub publish rights can create. Check first whether it's already set up: + +- Does any `.github/workflows/*.yml` already call + `dhis2/workflows-platform/.github/workflows/release.yml@` (any ref)? If so, and + `publish_apphub` isn't explicitly set to `false`, App Hub delivery is already live — + nothing to do. +- Does a bespoke workflow already run `d2-app-scripts publish` (the manual pattern from the + guide)? That's a working, if higher-maintenance, alternative — ask whether the user wants + to replace it with the shared workflow or leave it, don't assume. +- Is `d2.config.js`'s `type` set to `'lib'`? App Hub publish is a no-op for libraries — the + `semantic-release-apphub` plugin itself skips it silently. Don't set this up for a library. + +If none of the above rule it out, this app genuinely has no App Hub CD yet — offer to set it +up rather than doing it unprompted. + +## Step 2: Get the App Hub app ID — ask the user, don't guess + +`d2.config.js` needs an `id` field: the app's App Hub UUID (e.g. +`b783bf32-0cc2-4d31-aadc-22d4c4807c30`). **There is no way to derive or look this up +unilaterally — always ask the user for it.** Two cases: + +- **The app is already listed on the App Hub** (published before, just never wired up to + CD). The user finds it by navigating to their app's page on apps.dhis2.org and copying the + UUID out of the URL. +- **The app has never been listed on the App Hub at all.** The one-time initial submission + and DHIS2 Core Team review is a manual step this skill can't perform — the user needs to + submit it once through the App Hub UI to get an ID before this automation has anything to + attach to. Tell them this rather than inventing a placeholder ID. + +Add (or update) in `d2.config.js`: + +```javascript +const config = { + id: '', + // ...existing fields +} +``` + +Also confirm `minDHIS2Version` is set — most existing apps already have it (it's used at +runtime by the App Management app, not just App Hub publishing), but the +`semantic-release-apphub` plugin hard-fails the release if it's missing, so check rather than +assume: + +```bash +node -e "console.log(require('./d2.config.js').minDHIS2Version)" +``` + +## Step 3: Confirm commit-message linting is in place + +Versioning is driven entirely by [conventional commits](https://www.conventionalcommits.org/) +via `@semantic-release/commit-analyzer` — `fix:`/`feat:`/`BREAKING CHANGE:` commits produce a +patch/minor/major release respectively, anything else produces no release at all. If this +repo doesn't already enforce conventional commits (a `lint-commits`/`lint-pr-title` job, or +local commitlint hook), releases will be unpredictable or silently never trigger. Set that up +first if it's missing — see `references/style-configs-migration.md` Step 9 (or, if the app +is still on `cli-style`, its existing `lint-commits`/`lint-pr-title` jobs already cover this). + +## Step 4: Get the `DHIS2_BOT_APPHUB_TOKEN` secret added — never ask for the token itself + +The workflow needs an App Hub API key in this repo's `DHIS2_BOT_APPHUB_TOKEN` secret +(Settings → Secrets and variables → Actions). **Never ask the user to paste or share the +token value** — a secret typed into a chat with an agent defeats the point of it being a +secret. Instead: + +1. Tell the user what's needed and where it comes from: a human with publish rights on the + App Hub account this app releases under signs into apps.dhis2.org, profile icon → "Your + API Keys" → generate one (it's shown once, so it must be saved immediately). +2. Ask them to add it themselves, directly in GitHub — repo Settings → Secrets and variables + → Actions → New repository secret, named exactly `DHIS2_BOT_APPHUB_TOKEN`. **This isn't + something this skill can do on the user's behalf** even if given the value; don't offer to + run `gh secret set` with a value the user pastes. +3. Once they say it's done, confirm it by checking the secret _exists_ — + `gh secret list` lists names and last-updated dates only, never values — and treat that as + the confirmation, not a value they report back to you. + +Don't proceed to Step 5/6 until this confirms as set. + +## Step 5: Wire the reusable release workflow + +Add (or extend an existing combined workflow file with) a `release` job pointed at the ref +this app already uses for other `dhis2/workflows-platform` jobs (see +`references/ci-migration.md` Step 3 for picking `@v1`/`pnpm`/`@v2`) — gated on lint/test +passing, and on the default branch only: + +```yaml +# .github/workflows/release.yml (or folded into an existing combined workflow) +name: release + +on: + push: + branches: [main] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: false + +jobs: + lint-commits: + uses: dhis2/workflows-platform/.github/workflows/lint-commits.yml@v2 + lint: + uses: dhis2/workflows-platform/.github/workflows/lint.yml@v2 + test: + uses: dhis2/workflows-platform/.github/workflows/test.yml@v2 + release: + needs: [lint-commits, lint, test] + uses: dhis2/workflows-platform/.github/workflows/release.yml@v2 + with: + publish_apphub: true + apphub_channel: stable # or 'development'/'canary' if the user wants a pre-release channel + secrets: inherit +``` + +`secrets: inherit` matters here beyond just the two secrets `release.yml` declares +(`DHIS2_BOT_GITHUB_TOKEN`, `DHIS2_BOT_APPHUB_TOKEN`) — the job also references +`DHIS2_BOT_SSH_SIGNING_KEY`/`DHIS2_BOT_SSH_SIGNING_PASSPHRASE` for signed release commits, +which aren't declared as required inputs but still need to flow through. Passing secrets +individually instead of `inherit` will silently break commit signing. + +If a bespoke `release`/`publish` job already exists (e.g. an npm-only `release.yml@` +wrapper from `references/ci-migration.md` Step 2 without App Hub wired up), extend that same +job with `publish_apphub: true` rather than adding a second, competing release job. + +## Step 6: Verify + +This can't be verified with a normal CI dry run — a real release actually publishes to the +App Hub and creates a GitHub release. Confirm instead by: + +- Checking the workflow YAML is valid and the `uses:` ref resolves (a typo here fails + silently — see `references/ci-migration.md`'s troubleshooting entry on this). +- Confirming `DHIS2_BOT_APPHUB_TOKEN` is present in repo secrets (Step 4) — you can check it + exists via `gh secret list`, but never its value. +- Telling the user the next `fix:`/`feat:` commit merged to the default branch will trigger a + real release, and to watch that first run in the Actions tab rather than assuming success. + +## Troubleshooting + +| Symptom | Fix | +| ------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Release job fails with `EMISSINGAPPHUBID` / `'id' field missing from d2.config.js` | Step 2 wasn't completed, or the ID was added somewhere other than `d2.config.js`'s top-level `id` field. | +| Release job fails with `EMISSINGMINDHIS2VERSION` | `minDHIS2Version` is missing from `d2.config.js` — see Step 2. | +| Release job fails with `EMISSINGTOKEN` / `D2_APP_HUB_TOKEN missing from environment` | `DHIS2_BOT_APPHUB_TOKEN` isn't set in repo secrets, or the job wasn't given `secrets: inherit` — see Steps 4-5. | +| Pushed `fix:`/`feat:` commits to the default branch but no release happens | Check `lint-commits` is passing (a non-conventional commit message produces no release, not an error) and that the `release` job's `needs:` aren't failing upstream. | +| App Hub publish is silently skipped | `d2.config.js`'s `type` is `'lib'` — the `semantic-release-apphub` plugin intentionally no-ops for libraries. This is expected, not a bug, if the app is genuinely a library. | diff --git a/src/skills/modernise-apps/references/ci-migration.md b/src/skills/modernise-apps/references/ci-migration.md new file mode 100644 index 0000000..e0cb8bd --- /dev/null +++ b/src/skills/modernise-apps/references/ci-migration.md @@ -0,0 +1,155 @@ +# Modernising a DHIS2 App's CI Pipeline + +This is the step-by-step recipe for moving an app's GitHub Actions workflows off bespoke, +hand-rolled jobs and onto the shared `dhis2/workflows-platform` reusable workflows wherever +one exists, bumping any action version and Node version that remains in a custom step. This +is independent of the pnpm and style-configs migrations (`references/pnpm-migration.md`, +`references/style-configs-migration.md`) — an app can need any combination of the three, in +any order. If doing more than one, land on one coherent CI end-state, not sequential passes +that half-undo each other. + +Reference migrations: [`dhis2/app-management-app#554`](https://github.com/dhis2/app-management-app/pull/554) +(the actual migration — deletes three bespoke workflows and replaces them with reusable-workflow +wrappers) and [`dhis2/app-management-app#561`](https://github.com/dhis2/app-management-app/pull/561) +(a same-week follow-up fixing a `github.ref` typo introduced by #554 — read it too, it's a +realistic mistake to avoid: `contains(fromJSON('[...]'), github.ref)` needs the list of _refs_ +spelled correctly, `refs/heads/main` not `ref/heads/main`, and the arguments to `contains()` +in the right order). + +--- + +## Step 1: Inventory the current workflows + +List everything under `.github/workflows/*.yml` and classify each job: + +- Already `uses: dhis2/workflows-platform/.github/workflows/.yml@` — done, skip it + (but still check the `@ref` — see Step 3). +- A bespoke/inline job doing something a reusable workflow also covers (build, lint, test, + release/publish, commit-message or PR-title linting, PR preview deploy, production deploy, + issue triage-and-close) — a migration candidate for Step 2. +- A bespoke job doing something genuinely app-specific — leave it, but still bump its actions + and Node version (Step 4). + +## Step 2: Replace bespoke jobs with reusable workflows + +`dhis2/workflows-platform` currently publishes: `test.yml`, `lint.yml`, `lint-commits.yml`, +`lint-pr-title.yml`, `release.yml`, `deploy-pr.yml`, `deploy-production.yml`, +`deploy-branch.yml`, `comment-and-close.yml`, `e2e.yml`/`legacy-e2e.yml`, and +`generate-and-upload-bom.yml`. Compose the ones the app needs the same way +`app-management-app#554` did — one small wrapper file per concern rather than one big custom +workflow: + +```yaml +# .github/workflows/test-and-release.yml +name: test-and-release + +on: push + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ !contains(fromJSON('["refs/heads/master", "refs/heads/main"]'), github.ref) }} + +jobs: + lint-commits: + uses: dhis2/workflows-platform/.github/workflows/lint-commits.yml@v1 + lint: + uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1 + test: + uses: dhis2/workflows-platform/.github/workflows/test.yml@v1 + release: + needs: [lint-commits, lint, test] + uses: dhis2/workflows-platform/.github/workflows/release.yml@v1 + if: '!github.event.push.repository.fork' + secrets: inherit +``` + +```yaml +# .github/workflows/lint-pr-title.yml +name: lint-pr-title + +on: + pull_request: + types: ['opened', 'edited', 'reopened', 'synchronize'] + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref }} + cancel-in-progress: true + +jobs: + lint-pr-title: + uses: dhis2/workflows-platform/.github/workflows/lint-pr-title.yml@v1 +``` + +Delete the bespoke workflow file(s) each wrapper replaces once the wrapper is in place — +don't leave both around (the real `#554` migration deleted `dhis2-verify-app.yml`, +`dhis2-verify-commits.yml`, and `dhis2-preview-pr.yml` entirely). + +## Step 3: Pick the right ref for a pnpm app + +The `@v1` refs above match `#554`'s real migration (a yarn-based, still-on-`cli-style` app at +the time). If this app is still on yarn and still on `cli-style`, `@v1` is what you want too +and there's nothing further to decide here. The two cases worth pausing on are pnpm apps: + +| App is on... | Use | Why | +| --------------------------------- | ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| pnpm, still on `@dhis2/cli-style` | the `pnpm` branch | pnpm-native, but `lint.yml`/`lint-commits.yml`/`lint-pr-title.yml` still resolve config through `cli-style` — the right fit if the app has done the pnpm migration but not the style-configs one. | +| pnpm, off `@dhis2/cli-style` | `@v2` | pnpm-native, and those same three jobs resolve config via the app's own `commitlint.config.mjs`/`pnpm lint` script instead — no `cli-style` involved at all. | + +If neither fits (e.g. this app needs a job those refs don't cover), fall back to a small +custom step rather than adopting one that's wrong for it: + +```yaml +lint-commits: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: { fetch-depth: 0 } + - uses: pnpm/action-setup@v4 # omit if the app is still on yarn + - uses: actions/setup-node@v4 + with: + node-version: 20 + cache: pnpm # or yarn + - run: pnpm install --frozen-lockfile + - uses: wagoid/commitlint-github-action@v5 + with: + configFile: commitlint.config.mjs +``` + +`lint.yml`'s job is moot anyway once `cli-style` is gone (whether via a `cli-style`-independent +ref or a custom step) — `references/style-configs-migration.md` Step 11 already folds linting +into an inline `pnpm lint`/`yarn lint` step in the `test` job rather than using a separate +`lint` job at all, reusable or not. + +## Step 4: Bump remaining custom actions and the Node version + +For whatever's left as a genuinely bespoke job (Step 1's third bucket), or any reusable-workflow +wrapper's own surrounding boilerplate: + +- Bump `actions/checkout` to `@v4`, `actions/setup-node` to `@v4`, and any other + `actions/*` action to its current major (`gh api repos/actions//releases/latest --jq .tag_name` + per action, or check the marketplace listing) — outdated majors on `actions/*` are a common + source of silent Node 16 deprecation warnings and, eventually, hard failures as GitHub + retires old runners. +- Set `node-version: 20` (or later — check what the current reusable workflows use, since + they're the reference for what DHIS2 CI currently standardizes on: + `gh api repos/dhis2/workflows-platform/contents/.github/workflows/test.yml --jq '.content' | base64 -d`). + Don't leave a bespoke job on Node 16 or 18 while adopting reusable workflows that run 20 — + that's a worse, more confusing hybrid than either extreme. +- Third-party (non-`dhis2/*`, non-`actions/*`) actions — bump those too if a newer major + exists (`nwtgck/actions-netlify`, `wagoid/commitlint-github-action`, etc.) unless the app + has a specific reason to pin. + +## Step 5: Verify + +Push to a branch and confirm every workflow actually triggers and passes — a `uses:` typo in +a reusable workflow reference (wrong org, wrong ref, wrong file name) fails silently as "workflow +file not found" rather than a normal job failure, so don't just eyeball the YAML. + +## Troubleshooting + +| Symptom | Fix | +| -------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| A reusable-workflow job fails with "Cannot find module '@dhis2/cli-style'" | The ref this job is pinned to (`@v1` and the `pnpm` branch, as of this writing) still resolves its config through `cli-style` internally — see Step 3's table for which ref to use instead, or the custom-step fallback. | +| `contains(fromJSON('[...]'), github.ref)` never matches the default branch | Check argument order (`contains(list, value)`, not `contains(value, list)`) and that every ref string is spelled `refs/heads/` in full — see `app-management-app#561`. | +| A workflow silently never runs | The `uses:` reference is wrong (org/repo/path/ref) — GitHub doesn't surface this as a run failure, the workflow just never appears in the Actions tab. Double-check the exact path against `workflows-platform`. | +| `yarn: command not found` (or similar) inside a reusable workflow job | This job's ref is still yarn-based (`@v1`, as of this writing) but the app is on pnpm — pick a pnpm-native ref instead (`pnpm` branch or `@v2` depending on whether `cli-style` is also gone, see Step 3's table). | diff --git a/src/skills/modernise-apps/references/platform-library-bumps.md b/src/skills/modernise-apps/references/platform-library-bumps.md new file mode 100644 index 0000000..1b981f9 --- /dev/null +++ b/src/skills/modernise-apps/references/platform-library-bumps.md @@ -0,0 +1,279 @@ +# Bumping DHIS2 Platform Libraries + +This is the recipe for updating an app's `@dhis2/ui`, `@dhis2/app-runtime`, `@dhis2/d2-i18n`, +and `@dhis2/multi-calendar-dates` dependencies. This is independent of the pnpm, style-configs, +and CI migrations (`references/pnpm-migration.md`, `references/style-configs-migration.md`, +`references/ci-migration.md`) — an app can need any combination, in any order. + +--- + +## Step 1: Bump `@dhis2/ui`, `@dhis2/app-runtime`, and `@dhis2/d2-i18n` + +Check each one independently; they don't necessarily move majors at the same time: + +```bash +node -e " +const p = require('./package.json') +for (const n of ['@dhis2/ui', '@dhis2/app-runtime', '@dhis2/d2-i18n']) + console.log(n, p.dependencies?.[n] ?? p.devDependencies?.[n] ?? '(not present)') +" +npm view @dhis2/ui dist-tags +npm view @dhis2/app-runtime dist-tags +npm view @dhis2/d2-i18n dist-tags +``` + +For each library that's present, compare its current major against `latest`'s major: + +- **Same major → just bump to `latest`.** This is the common case and needs no further + decision — update `package.json` (keep whatever range style — `^`, `~`, exact — the app + already uses) and move on. +- **Different major → this is a breaking bump, don't take it silently.** Ask the user which + they want: + - **Latest non-breaking** (the safer default if you can't ask — e.g. running + unattended): stay within the app's current major. + ```bash + npm view @dhis2/ui@ version # e.g. npm view @dhis2/ui@9 version + ``` + - **Latest overall**: take the major bump. This needs real verification, not just an + install — expect to fix actual breaking changes, not just lint noise. Check that + library's changelog/migration guide for the specific major(s) being crossed before + starting, and budget real time for it; don't bundle this silently into what's + otherwise a low-risk PR. + If you can't ask (no interactive user available) and choose the non-breaking default, + say so explicitly in the summary — don't just skip the library without mentioning that a + newer major exists. + +Whichever path for whichever library, finish with: + +```bash +pnpm install +pnpm lint # tsc/eslint will flag any prop, export, or i18n API that actually changed +``` + +If a same-major bump breaks anything, that's a signal the release wasn't as non-breaking as +its version number implied — check that library's changelog for the affected +component/export before working around it, don't just silence the error. If a cross-major +bump breaks something, that's expected — fix it for real using the migration guide, don't +revert to avoid the work unless the user explicitly wants to defer it. + +## Step 2: Bump `@dhis2/multi-calendar-dates` (skip if the app doesn't use it) + +Check for `@dhis2/multi-calendar-dates` in `package.json`. Its `3.x` line (`3.0.0`) is now +`latest` — confirm the current version either way: + +```bash +npm view @dhis2/multi-calendar-dates dist-tags +``` + +This is a real breaking change, not a routine bump, regardless of which dist-tag it ships +under — [`dhis2/multi-calendar-dates#102`](https://github.com/dhis2/multi-calendar-dates/pull/102) +removed Temporal types from the public API entirely. Specifically, **`getNowInCalendar` now +returns a plain `{ year, month, day, eraYear? }` object instead of a `Temporal.ZonedDateTime` +— and unlike before, that object carries no time-of-day information at all**, not just a +different date type. Grep for `getNowInCalendar` (the app may wrap it, like +`aggregate-data-entry-app`'s `getNowInCalendarString` does) and check every call site for: + +- **Calling a Temporal method on the result** (`.withCalendar()`, `.startOfDay()`, etc.) — + these don't exist on the plain object anymore. Re-derive what's needed via + `getNowInCalendar`/`convertFromIso8601` again instead. +- **Reading `.hour`/`.minute`/`.second`/timezone-derived fields off the result** — these are + silently `undefined` now (not an error, so this won't fail loudly — check output + correctness, not just that it runs). If time-of-day is genuinely needed alongside the + calendar date, get it separately — but **don't reach for a plain `new Date()`** if the + original code accepted a `timezone` parameter (as `getNowInCalendar` itself still does): + `new Date()` only gives the browser's own local time, silently dropping timezone-correction + behavior the old Temporal-based code had. Use `Intl.DateTimeFormat` with `timeZone` instead, + which needs no extra dependency: + ```javascript + const getTimeOfDayInTimezone = (timezone) => { + const parts = new Intl.DateTimeFormat('en-US', { + timeZone: timezone, + hourCycle: 'h23', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + }).formatToParts(new Date()) + const get = (type) => parts.find((p) => p.type === type)?.value + return { + hour: get('hour'), + minute: get('minute'), + second: get('second'), + } + } + ``` +- Plain reads of `.year`/`.month`/`.day`/`.eraYear` need no changes — that part of the shape + is unchanged. + +### Write a before/after regression test for direct library usage + +Don't just eyeball the call sites above — this is a real behavioral change that can silently +produce wrong output (not a compile error, not a thrown exception), so pin down the exact +current output with a test, confirm it passes against the version installed _right now_ +(before bumping), then bump and re-run the identical test unmodified. If it still passes, +behavior is provably unchanged; if it fails, you've caught a real regression before it ships, +not after. + +**Test every direct import from `@dhis2/multi-calendar-dates` itself, including +`useDatePicker` — the exclusion is `@dhis2/ui`'s own bundled `Calendar`/`DatePicker` +components, not this library's own hook.** It's easy to conflate the two, but they're not +the same thing: + +- **In scope** — anything the app imports directly from `@dhis2/multi-calendar-dates`: + `getNowInCalendar`, `convertFromIso8601`, `convertToIso8601`, the period-calculation + helpers (`getFixedPeriodByDate`, `getAdjacentFixedPeriods`, `generateFixedPeriods`, + `createFixedPeriodFromPeriodId`), `useDatePicker`, or a wrapper the app built on any of + these. **`#102` touched the period-calculation internals extensively** (daily/monthly/ + weekly/yearly period generation, adjacent-period lookup, period-from-id construction) and + rewrote `useDatePicker` to use the new plain-date shape — these are not lower-risk than + `getNowInCalendar` just because they weren't named in the PR's headline breaking-change + note. +- **Out of scope** — `@dhis2/ui`'s `Calendar`/`DatePicker` React components. Those bundle + their _own_ internal `multi-calendar-dates` dependency (a separate resolved version this + app doesn't control — see the Troubleshooting entry on this), have their own test coverage + upstream, and testing them here wouldn't exercise the version this app is actually bumping. + +Cover at least these three calendars for **every** function/hook under test above — they're +the most behaviorally distinct: `gregory` (the default, CLDR-backed), `ethiopian` +(CLDR-backed but uses `eraYear` instead of `year`), and `nepali` (the one calendar this +library implements itself rather than delegating to CLDR — per `#102`'s own description, +this is the part that got rewritten most substantially, so it's the highest-risk case for a +subtle regression, and it's exactly the kind of calendar a period-calculation helper or +`useDatePicker` could be driving just as easily as `getNowInCalendar`). + +**If the app already has a test runner** (jest/vitest — check for an existing +`*.test.ts`/`*.test.js` alongside the file that calls these functions, or a +`jest.config.js`/`vitest.config.ts`), add real, committed tests, following whatever +conventions already exist there (e.g. `jest.useFakeTimers()` + `jest.setSystemTime()` to pin +"now" for date-dependent functions like `getNowInCalendar`, since its output changes daily +otherwise): + +```javascript +// Testing a wrapper around getNowInCalendar directly -- not through a UI component +describe.each(['gregory', 'ethiopian', 'nepali'])('%s calendar', (calendar) => { + it('returns the expected date', () => { + // pin the system time first -- see jest.useFakeTimers() above + expect(getNowInCalendarString({ calendar })).toBe(/* value captured before bumping */) + }) + + it('finds the expected adjacent period', () => { + const period = getFixedPeriodByDate({ periodType: 'Monthly', date: /* fixed date */, calendar }) + const [next] = getAdjacentFixedPeriods({ period, calendar, steps: 1 }) + expect(next).toEqual(/* value captured before bumping */) + }) +}) +``` + +If the app uses `useDatePicker` directly (check for `import { useDatePicker } from +'@dhis2/multi-calendar-dates'`), test it the same way via `@testing-library/react`'s +`renderHook` — it's a headless hook, no `@dhis2/ui` component needed to exercise it: + +```javascript +import { renderHook } from '@testing-library/react' +import { useDatePicker } from '@dhis2/multi-calendar-dates' + +describe.each(['gregory', 'ethiopian', 'nepali'])('%s calendar', (calendar) => { + it('builds the expected calendar grid', () => { + // pin the system time first, same as above -- isToday/isInCurrentMonth depend on it + const { result } = renderHook(() => + useDatePicker({ + date: /* fixed date string */, + options: { calendar }, + onDateSelect: () => {}, + }) + ) + expect(result.current.calendarWeekDays).toEqual(/* value captured before bumping */) + }) +}) +``` + +Run this test _before_ bumping the dependency to confirm it passes against the +currently-installed version — that's what makes it a real baseline, not a guess. Then bump, +reinstall, and run the identical test again, unmodified. Keep it in the codebase +afterward — it's real regression coverage for this library going forward, not a throwaway +migration check. + +**If there's no test runner set up at all**, don't introduce one just for this — that's a +bigger decision than a dependency bump warrants. Instead, do the same before/after +comparison as a temporary standalone script (same idea as this skill's +`pnpm-migration.md` Step 11 smoke-test, but here it's plain Node, no Playwright/browser +needed): + +```javascript +// verify-multi-calendar-dates.mjs -- run once before bumping, once after; diff the two outputs +import { + getNowInCalendar, + convertFromIso8601, + convertToIso8601, + getFixedPeriodByDate, + getAdjacentFixedPeriods, +} from '@dhis2/multi-calendar-dates' + +const CALENDARS = ['gregory', 'ethiopian', 'nepali'] +const FIXED_ISO_DATE = '2024-06-15' // any date currently exercised by the app's real usage + +for (const calendar of CALENDARS) { + const period = getFixedPeriodByDate({ + periodType: 'Monthly', + date: FIXED_ISO_DATE, + calendar, + }) + console.log(calendar, { + now: getNowInCalendar(calendar, 'Etc/UTC'), + fromIso: convertFromIso8601(FIXED_ISO_DATE, calendar), + toIso: convertToIso8601(FIXED_ISO_DATE, calendar), + period, + adjacentPeriods: getAdjacentFixedPeriods({ + period, + calendar, + steps: 1, + }), + // add whichever other functions/period types this app actually calls directly + }) +} +``` + +Redirect the output to a file before bumping (`node verify-multi-calendar-dates.mjs > +/tmp/before.json`), bump the dependency, run it again (`> /tmp/after.json`), and `diff +/tmp/before.json /tmp/after.json`. Anything beyond `getNowInCalendar`'s missing time-of-day +fields (expected — already covered above) is a real behavioral difference to investigate +before shipping. Delete the script afterward — it's a verification aid for this migration, +not something to leave behind in the app's repo. + +**`useDatePicker` doesn't fit this plain-Node script** — it's a React hook, not a pure +function, so exercising it needs at least `@testing-library/react`'s `renderHook`. If the +app uses `useDatePicker` directly but has no test runner at all, that's a real gap worth +raising with the user rather than silently skipping it — a plain-object diff script can't +cover it, only a real (even minimal, temporary) React test can. + +Bump the dependency to whatever version `npm view ... dist-tags` showed above resolves +`latest` to (`3.0.0` as of this writing) and verify: + +```json +"dependencies": { + "@dhis2/multi-calendar-dates": "^3.0.0" +} +``` + +```bash +pnpm install +pnpm lint +pnpm test +``` + +Since this is a real behavioral change (not just a type change), a passing `tsc`/`eslint` +pass doesn't guarantee correctness here on its own — the broken case above (missing +time-of-day fields) compiles and runs fine, it just silently produces wrong output. The +before/after regression test (or standalone script) above is what actually closes that gap; +don't skip it and rely on `tsc`/`eslint`/a generic `pnpm test` pass alone as proof this is +safe. + +## Troubleshooting + +| Symptom | Fix | +| --------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `@dhis2/ui`/`app-runtime`/`d2-i18n` bump surfaces new TypeScript/ESLint errors | If it was a same-major bump, the release wasn't purely non-breaking for a prop/export the app uses — check that library's changelog entry rather than suppressing the error, and consider pinning back a version if it's a real regression. If it was a deliberate cross-major bump, this is expected — fix it using the migration guide. | +| A date/time string looks subtly wrong after bumping `@dhis2/multi-calendar-dates` (e.g. `T undefined:undefined:undefined`) but nothing errors | A `getNowInCalendar` call site is reading `.hour`/`.minute`/`.second` off the new plain object, which no longer has them — see Step 2. This won't throw, so check output values, not just exit codes. | +| A timezone-corrected time string is wrong (e.g. off by the timezone offset) after re-deriving time-of-day | The fix used `new Date()` (browser/system local time) instead of an `Intl.DateTimeFormat`-based lookup for the specific `timezone` the call site was passed — see the code sample in Step 2. | +| The before/after regression test fails on the `nepali` calendar specifically, but `gregory`/`ethiopian` pass | Expected to be the highest-risk case — `#102` rewrote Nepali support from scratch (it's the one calendar this library implements itself rather than delegating to CLDR). Don't assume it's a test bug; diff the actual before/after values and check the library's own changelog/tests for that release before concluding it's safe. | +| No test runner is set up and it's unclear whether to add one just for this | Don't — that's a bigger decision than a dependency bump warrants. Use the standalone verify script instead, and delete it once the bump is confirmed safe. | +| Unsure whether `useDatePicker` is in scope for this app's regression test, since the app also uses `@dhis2/ui`'s `Calendar`/`DatePicker` | Check the import, not the visible UI — `import { useDatePicker } from '@dhis2/multi-calendar-dates'` is in scope (direct usage of the version this app controls); `@dhis2/ui`'s bundled `Calendar`/`DatePicker` components are not (they resolve their own separate internal copy). | diff --git a/src/skills/modernise-apps/references/pnpm-migration.md b/src/skills/modernise-apps/references/pnpm-migration.md new file mode 100644 index 0000000..4fd7d4a --- /dev/null +++ b/src/skills/modernise-apps/references/pnpm-migration.md @@ -0,0 +1,331 @@ +# Migrating a DHIS2 App from Yarn to pnpm + +This is the step-by-step recipe for moving an existing DHIS2 app off Yarn (typically Yarn 1) +and onto pnpm, using the pnpm support added to `@dhis2/cli-app-scripts` in `12.7.0`. Follow +every step in order — later steps assume earlier ones are done. + +Reference migration: [`dhis2/aggregate-data-entry-app#481`](https://github.com/dhis2/aggregate-data-entry-app/pull/481), +enabled by [`dhis2/app-platform#933`](https://github.com/dhis2/app-platform/pull/933). + +--- + +## Step 1: Pre-flight checks + +Confirm before making any changes: + +- `d2.config.js` exists in the project root. +- `yarn.lock` exists, `pnpm-lock.yaml` does not (if it already exists, the app is already + migrated — stop here). +- Read the full `package.json` — note the current `@dhis2/cli-app-scripts` version and the + complete `dependencies`/`devDependencies` lists. You'll need to compare against this after + installing with pnpm to catch anything that was being phantom-hoisted by yarn 1. + +## Step 2: Bump `@dhis2/cli-app-scripts` + +pnpm support requires `@dhis2/cli-app-scripts >= 12.7.0`. Check the current latest version: + +```bash +npm view @dhis2/cli-app-scripts version +``` + +Update `package.json` to that version (or at minimum `^12.7.0`) in `devDependencies`. Don't +run the install yet — do this alongside the other `package.json` changes below so it only +takes one install pass. + +## Step 3: Add `pnpm-workspace.yaml` + +`@dhis2/app-shell` directly imports several dependencies that yarn 1 used to hoist flatly +into `node_modules` without them being declared anywhere. pnpm's stricter resolution needs +these listed explicitly via `publicHoistPattern`. Start with this list (confirmed from both +reference PRs) and extend it if a build/lint/test error points at an unresolved module: + +```yaml +# These hoists are needed for @dhis2/app-shell since it uses directly some libraries that +# were hoisted by yarn 1. This is a better alternative than pnpm's shamefullyHoist +# (https://pnpm.io/settings#shamefullyhoist) until app-shell's dependency handling changes. +publicHoistPattern: + - '@dhis2/*' + - 'typeface-roboto' + - 'prop-types' + - 'post-robot' + - 'styled-jsx' + - '@tanstack/*' + - 'serialize-query-params' +``` + +If `pnpm install` or a subsequent build warns about ignored build scripts, add the relevant +packages to `onlyBuiltDependencies` (allow their build script) or `ignoredBuiltDependencies` +(silence the warning, if the build step genuinely isn't needed): + +```yaml +onlyBuiltDependencies: + - '@dhis2/cli-helpers-engine' + - '@dhis2/cli-style' + - cypress + +ignoredBuiltDependencies: + - core-js-pure + - esbuild +``` + +## Step 4: Convert the lockfile + +Run `pnpm import` first — it reads `yarn.lock` to produce `pnpm-lock.yaml` without doing a +full install, giving pnpm a starting point close to what's currently resolved: + +```bash +pnpm import +``` + +Only after that succeeds, remove the yarn artifacts: + +```bash +rm yarn.lock +rm -rf .yarn .yarnrc .yarnrc.yml # remove whichever of these exist +``` + +## Step 5: Pin the package manager + +Add a `packageManager` field to `package.json` so consumers (and corepack) know which +package manager and version to use. Check Node compatibility _before_ picking a pnpm version, +rather than trying `pnpm@latest` and reacting to a failure — that avoids a confirmed bad +interaction where a failed `corepack use pnpm@latest` still **writes the incompatible version +string into `package.json` before crashing**, leaving something to clean up either way. Don't +ask the user which pnpm version to use: this decision has one objectively correct answer +(whatever satisfies the local Node version) and asking every time would turn a one-line +command into a constant interruption, since most dev environments today are still on Node +<22.13. + +Query what `pnpm@latest` actually requires (don't hardcode a Node version threshold — it's +specific to whichever pnpm major is current and will change as new majors ship), compare +against the local Node version, and pin whichever pnpm line is compatible: + +```bash +REQUIRED_NODE=$(npm view pnpm@latest engines.node) # e.g. ">=22.13" +NODE_OK=$(node -e " + const req = '$REQUIRED_NODE'.replace(/[^0-9.]/g, '').split('.').map(Number) + const cur = process.versions.node.split('.').map(Number) + console.log(cur[0] > req[0] || (cur[0] === req[0] && cur[1] >= (req[1] || 0)) ? 'yes' : 'no') +") + +if [ "$NODE_OK" = "yes" ]; then + corepack use pnpm@latest +else + corepack use pnpm@10 # fully current, actively maintained -- not a legacy fallback +fi +``` + +**Verify** `package.json`'s `packageManager` field actually starts with `pnpm@` afterward — +some environments set `COREPACK_ENABLE_AUTO_PIN=0`, which makes `corepack use` exit `0` +without writing anything at all. If the field is still missing (corepack unavailable, or +silently disabled), set it manually: + +```bash +pnpm --version +# then hand-edit package.json: +# "packageManager": "pnpm@" +``` + +## Step 6: Install and fix breakage + +```bash +pnpm install +pnpm build +pnpm test +``` + +Work through any errors that surface: + +- **Import from a split package fails** (e.g. `@dhis2-ui/checkbox`, `@dhis2/ui-forms`) — + these have been consolidated into `@dhis2/ui`. Update the import to pull from `@dhis2/ui` + instead of trying to make the old package resolve. +- **"Cannot find module" for anything else** — it was being phantom-hoisted by yarn 1. Add + it explicitly to `dependencies` or `devDependencies` in `package.json` (common ones seen + in real migrations: `react`, `react-dom`, `@dhis2/app-adapter`, `@dhis2/pwa`, + `@dhis2/prop-types`). +- **A module `@dhis2/app-shell` needs directly still isn't resolving** — add it to + `publicHoistPattern` in `pnpm-workspace.yaml` (Step 3) rather than as a project dependency. + +Re-run `pnpm install` after each `package.json`/`pnpm-workspace.yaml` change until the build +and tests pass cleanly. + +## Step 7: Update tooling that shells out to yarn + +Search the repo for literal `yarn` invocations outside of `package.json`'s own dependency +list — these won't be caught by the install/build/test cycle above: + +- Husky/git hooks (e.g. `.hooks/pre-commit`, `.hooks/commit-msg`) — replace `yarn ` + with `pnpm `. +- Any `package.json` `scripts` entries that call `yarn` directly. +- CONTRIBUTING docs referencing `yarn install`, `yarn start`, etc. The README itself gets a + fuller pass — see Step 8. + +## Step 8: Update the README + +While the package manager is already being touched, bring the README's own presentation up +to date. Three independent changes — do whichever apply, don't force a section that isn't +relevant: + +**Add a pnpm badge, keep every other badge as-is.** Don't replace or reorder existing badges +(React version, codecov, etc.) — just add pnpm's alongside them, same as any other badge in +that row: + +```markdown +[![pnpm](https://img.shields.io/badge/maintained%20with-pnpm-F69220?logo=pnpm&logoColor=white)](https://pnpm.io/) +``` + +**Refresh the app description from the App Hub, if the app is published there.** Read +`d2.config.js`'s `id` and `title` fields, then check if the app is listed at +`https://apps.dhis2.org/api/v2/apps` (paginated, `?page=&pageSize=25`, no working +server-side search — fetch pages and match client-side). Match on `id` first (most +reliable — `d2.config.js`'s `id` is the same UUID the App Hub uses), falling back to a fuzzy +match on `title` only if `id` isn't set. **The App Hub display name doesn't always match the +repo/title name** — `aggregate-data-entry-app`'s `d2.config.js` has `title: 'Data Entry'`, +and it's genuinely listed under just "Data Entry" on the App Hub, not "Aggregate Data Entry". +If you find a match, replace the paragraph(s) directly under the `#` title with the App Hub +`description` field (it's already written in prose, often markdown-formatted — use it near +verbatim, don't rewrite it). Keep everything else in the README untouched — operational notes +specific to the repo (required authorities, setup caveats, links to diagrams, live-demo URLs) +usually live below that description and aren't part of it. If the app isn't listed there (not +every app is — some are core/bundled, some just aren't published), leave the description as +whatever the README already has and don't fabricate one. + +**Rename "Available Scripts" to "Get Started" and make it concise.** The +`pnpm create @dhis2/app` scaffold's default README has a verbose version of this section — a +`### yarn start`-style subheading plus a full paragraph, repeated separately for each of +`start`/`test`/`build`/`deploy`. Collapse that into one short block, and switch `yarn` to +`pnpm` while doing it: + +````markdown +## Get Started + +```sh +pnpm install # install dependencies +pnpm start # run the app locally +pnpm test # run tests +pnpm build # build for production +pnpm deploy # deploy the built app to a DHIS2 instance +``` +```` + +If the app has no "Available Scripts" section at all (common on older or heavily +customized READMEs — real `aggregate-data-entry-app`'s README has none, just a badge, a +title, a live-demo link, and app-specific docs), add a "Get Started" section rather than +renaming anything; don't invent scripts the app doesn't actually have — check +`package.json`'s `scripts` first. + +## Step 9: Update CI + +In `.github/workflows/*.yml`: + +- Replace direct `yarn install`/`yarn build`/etc. steps with `pnpm install`/`pnpm build`. +- Add `pnpm/action-setup@v4` before `actions/setup-node`, and set `cache: pnpm` on the + `actions/setup-node` step (instead of `cache: yarn`). + +If the app consumes `dhis2/workflows-platform` reusable workflows (`uses: dhis2/workflows-platform/.github/workflows/...@`), +point them at a pnpm-compatible ref instead of the yarn-only `@v1` default — which one +depends on whether this app is also migrating off `@dhis2/cli-style`; see +`references/ci-migration.md` Step 3 for the two cases and which ref fits each. + +## Step 10: Verify + +Do a clean reinstall to catch anything masked by stale `node_modules` state, then re-run the +app's usual checks: + +```bash +rm -rf node_modules +pnpm install +pnpm build +pnpm test +pnpm start +``` + +`pnpm start` here just confirms the dev server boots without crashing — start it, confirm it +compiles and serves on its usual port, then stop it (Ctrl-C). That alone doesn't tell you +whether the app actually works once someone logs in; see Step 11 for that. + +Confirm `pnpm-lock.yaml` is present and tracked in git, and `yarn.lock` is gone. + +## Step 11: Sanity-check against a real server (optional) + +`pnpm build` and `pnpm test` don't start the app or talk to a real DHIS2 server, so they can +miss runtime-only breakage — most commonly a dependency that yarn 1 was phantom-hoisting and +that only gets `import`ed from a code path the build/test suite doesn't exercise (e.g. a +lazy-loaded route, a conditional branch gated on server version). The only way to catch that +class of bug is to actually run the app against a server and see it render. + +This step is **optional** — suggest it when the user wants extra confidence before deploying, +or asks for it directly. Don't run it as a routine part of every migration: it needs outbound +network access to a real DHIS2 instance, downloads a ~100–300MB Chromium binary on first use, +and takes real wall-clock time (dev server boot + login + render, typically 30s–2min). + +A small bundled script (`scripts/smoke-test.mjs`, in this skill's own directory) does this +without adding anything to the migrated app's `package.json` or `pnpm-lock.yaml` — it's a +skill-owned tool with its own isolated dependencies, not a dependency of the app being +migrated. + +### One-time setup + +```bash +cd /scripts +pnpm install +npx playwright install chromium +``` + +(The Chromium download is cached globally under `~/.cache/ms-playwright` and shared across +every project on the machine — this is a one-time cost, not a per-migration one.) + +### Per-run invocation + +```bash +node /scripts/smoke-test.mjs \ + --cwd /path/to/migrated-app \ + --server https://play.im.dhis2.org/stable-2-43-1 \ + --screenshot /tmp/smoke-test-screenshot.png +``` + +`--cwd` must point at the migrated app's directory — that's where the dev server command +actually runs, distinct from wherever the script itself lives. All other flags have sensible +defaults (`admin`/`district` credentials, port 3000/proxy port 8080, a 120s startup timeout) +— read the header comment in `smoke-test.mjs` for the full flag list, or just pass `--cwd` +and accept the rest. + +Point `--screenshot` somewhere outside the app's own working tree (e.g. `/tmp` or this +skill's scratch space) so the check doesn't leave a stray untracked file inside the app's git +repo. + +### Interpreting the result + +The script exits `0` on pass, `1` on any failure, and prints a summary covering: whether +login succeeded (and as which user), whether the expected app-shell element rendered, a count +and preview of browser console errors, any uncaught JS exceptions on the page, and the +screenshot path. Read the screenshot as well as the summary — a blank white page with zero +console errors can still mean something didn't render (e.g. permission-gated content +resolving to nothing). + +If it fails: + +- **Login failed** — check the demo server is actually reachable (`curl -I `) + and that `admin`/`district` (or whatever credentials were passed) are valid on that + instance. +- **Dev server never came up** — read the printed dev-server output; this is almost always a + build error that should have already surfaced in Step 10, not something new. +- **Shell never rendered, but login succeeded** — this is the case this step exists to catch. + Look at the console-error preview in the summary for a "Cannot find module" / import + resolution error — treat it the same as a build-time resolution error: add the missing + dependency to `package.json` or `publicHoistPattern` (see Step 3 and Step 6's + troubleshooting notes), then re-run. +- **App doesn't use the standard `@dhis2/ui` HeaderBar** — re-run with `--selector` pointed + at something else that only appears once logged in (e.g. a known heading or nav element + specific to the app). + +## Troubleshooting + +| Symptom | Fix | +| ------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `pnpm install` succeeds but the app fails at build or runtime | Module resolution errors from missing hoists often don't surface until build/test/start — always run all three, not just install. | +| ESLint or the bundler can't resolve a `@dhis2/*` import that used to work | Check `publicHoistPattern` in `pnpm-workspace.yaml` includes `@dhis2/*`, then reinstall. | +| pnpm prints "Ignored build scripts" warnings | Expected for packages not in `onlyBuiltDependencies`. Only add a package there if the app actually needs its build/postinstall step. | +| The Step 11 smoke-test script reports uncaught page errors or missing-module console errors | This is a runtime-only resolution error the build/test steps didn't catch — treat it like any other missing dependency (Step 3/Step 6): add it to `package.json` or `publicHoistPattern`, reinstall, and re-run the smoke-test. | +| The Step 11 smoke-test script fails to log in | Confirm the demo server URL is reachable and the credentials are valid for that instance before assuming the app is broken — this step depends on network access to a real server. | +| App Hub description not found even though the app is clearly published | The App Hub display name may not match the repo/`d2.config.js` title exactly — match on `d2.config.js`'s `id` (the App Hub UUID) first, and only fall back to fuzzy name matching. See Step 8. | diff --git a/src/skills/modernise-apps/references/style-configs-migration.md b/src/skills/modernise-apps/references/style-configs-migration.md new file mode 100644 index 0000000..77404cb --- /dev/null +++ b/src/skills/modernise-apps/references/style-configs-migration.md @@ -0,0 +1,336 @@ +# Migrating a DHIS2 App from `@dhis2/cli-style` to Shared Configs + +This is the step-by-step recipe for moving an existing DHIS2 app off `@dhis2/cli-style` +(the `d2-style` CLI) and onto the shared `@dhis2/config-*` packages (`config-eslint`, +`config-prettier`, `config-stylelint`, `config-lslint`, `config-commitlint`) with plain +`eslint`/`prettier`/`stylelint`/`ls-lint` and native `husky`/`lint-staged`. This is +independent of the pnpm migration and platform-library bumps (`references/pnpm-migration.md`, +`references/platform-library-bumps.md`) — an app can need any combination, in any order. + +`d2-style` makes every tool opt-in (via a `tools:` block in its config) — most apps only +ever configured eslint + prettier, some also configured stylelint or ls-lint. **Only migrate +the tools the app actually has configured** (Step 1 tells you which). Don't add a stylelint +or ls-lint setup to an app that never had one. + +Reference migrations: [`dhis2/route-manager-app#35`](https://github.com/dhis2/route-manager-app/pull/35) +and [`dhis2/app-runtime#1434`](https://github.com/dhis2/app-runtime/pull/1434) — both cover +only the eslint/prettier/commit-hook piece. `@dhis2/config-stylelint` and +`@dhis2/config-lslint` are newer additions to `style-configs` with no reference PR yet; +follow this doc directly for those. Fresh apps scaffolded via `pnpm create @dhis2/app` +already use the eslint/prettier/commitlint setup by default — this migration brings an +older app in line with what new apps already look like. + +All five `@dhis2/config-*` packages are published to `latest` (`npm view @dhis2/config-eslint +dist-tags` shows the full set; they version in lockstep from the same `style-configs` repo). + +--- + +## Step 1: Pre-flight checks + +Confirm before making any changes — this determines which of Steps 3-6 actually apply: + +- `@dhis2/cli-style` is in `package.json` (`dependencies` or `devDependencies`). +- `.eslintrc.js` and/or `.prettierrc.js` exist and `require('@dhis2/cli-style')` internally + (that's the tell — `d2-style` configs are always proxied through that package, never + self-contained). Almost every app has these. +- `.stylelintrc.js` exists and `require('@dhis2/cli-style')` internally (same proxy + pattern — `module.exports = { extends: [require('@dhis2/cli-style').config.stylelint] }` + or similar). Only some apps configure this. +- `.ls-lint.yml` exists in the project root. Unlike the others, this one is a **plain + copied file with no reference to `cli-style`** — `ls-lint` has no `extends` mechanism, so + its presence alone is the tell, not a `require()`. +- Note whether the app has `.hooks/pre-commit` + `.hooks/commit-msg` (the older + `cli-app-scripts`-generated husky wrapper) or `.husky/pre-commit` already — this + determines whether Steps 7-8 are a hook-manager migration or just a script swap. +- Check what `.hooks/commit-msg` (or `.husky/commit-msg`) actually does — delegates to a CI + job, calls `d2-style check commit` directly, or something else — Step 8 branches on this. +- **`stylelint` and `@ls-lint/ls-lint` are NOT in the app's own `package.json`, even if the + app uses them.** `cli-style` bundles both tools itself and runs its own copies — the app + only ever had the config file, never the binary as a direct dependency. Both need adding + as real devDependencies in Steps 5-6, not just their shared config package. + +## Step 2: Add the shared config packages + +Add the packages for whichever tools Step 1 found configured. **Don't remove +`@dhis2/cli-style` yet** — leave it installed until Step 7, once every config it was +proxying has a working replacement. Removing it up front just means every intermediate step +runs against a broken lint setup for no benefit. + +```bash +npm view @dhis2/config-eslint dist-tags # confirms the current version for all five +``` + +Use whatever version that resolves to — don't pin to the literal `"latest"` tag in +`package.json`, resolve it to a real version number first, same as any other dependency: + +```json +"devDependencies": { + "@dhis2/config-eslint": "^0.4.0", + "@dhis2/config-prettier": "^0.4.0", + "@eslint/compat": "^2.0.0", + "eslint": "^9", + "prettier": "^3", + "husky": "^9.1.7", + "lint-staged": "^16" +} +``` + +(`0.4.0` above is illustrative — use whatever `dist-tags` actually reported.) Add these two +conditionally, only if Step 1 found them configured: + +```json +"devDependencies": { + "@dhis2/config-stylelint": "^0.4.0", + "stylelint": "^16", + "@dhis2/config-lslint": "^0.4.0", + "@ls-lint/ls-lint": "^2" +} +``` + +`@dhis2/config-eslint` requires `eslint >= 9`; `@dhis2/config-prettier` requires +`prettier >= 3.0.0`; `@dhis2/config-stylelint` requires `stylelint >= 11 < 18` (it bundles +its own `postcss-styled-jsx`/`postcss-syntax`/`stylelint-use-logical` transitively — don't +add those to the app directly); `@dhis2/config-lslint` requires `@ls-lint/ls-lint >= 2` +(`cli-style` bundled `1.x` — the YAML rule format is unaffected by this major bump, but +re-run `ls-lint` after Step 6 to confirm). + +## Step 3: Replace the ESLint config + +Delete `.eslintrc.js` (the old config proxies through `@dhis2/cli-style`'s `config.eslintReact` +or similar — it won't work once that package is gone). Add a flat config instead: + +```javascript +// eslint.config.mjs +import config from '@dhis2/config-eslint' +import { defineConfig } from 'eslint/config' +import { includeIgnoreFile } from '@eslint/compat' +import { fileURLToPath } from 'node:url' + +const gitignorePath = fileURLToPath(new URL('.gitignore', import.meta.url)) + +export default defineConfig([ + includeIgnoreFile(gitignorePath, 'Imported .gitignore patterns'), + { + extends: [config], + }, +]) +``` + +`@dhis2/config-eslint` (the base export) already covers standard React apps — that's what a +fresh `pnpm create @dhis2/app` scaffold uses. If the app needs stricter rules or is a +library rather than an app (e.g. exhaustive-deps as an error, tighter `no-unused-vars`), +import `@dhis2/config-eslint/react` instead and layer overrides — see +[`app-runtime#1434`](https://github.com/dhis2/app-runtime/pull/1434)'s `eslint.config.mjs` +for an example, but don't reach for this unless the base config genuinely isn't enough. + +## Step 4: Replace the Prettier config + +Delete `.prettierrc.js`, add: + +```javascript +// .prettierrc.mjs +import prettierConfig from '@dhis2/config-prettier' + +/** + * @type {import("prettier").Config} + */ +const config = { + ...prettierConfig, +} + +export default config +``` + +## Step 5: Replace the Stylelint config (skip if the app doesn't use stylelint) + +Delete `.stylelintrc.js`, add: + +```javascript +// .stylelintrc.mjs +import config from '@dhis2/config-stylelint' + +export default config +``` + +`stylelint` needs `type: "module"` in `package.json` (or a `.cjs`/`.mjs` extension it can +resolve) to load an ESM config — check that's already true from the eslint/prettier flat +configs; if not, add it. `@dhis2/config-stylelint`'s rules match `cli-style`'s bundled +config exactly (the logical-properties warnings, the styled-jsx custom syntax override for +`.jsx`/`.tsx` files) — expect no new violations here, unlike eslint/prettier. + +## Step 6: Replace the ls-lint config (skip if the app doesn't use ls-lint) + +There's no `extends` to wire up — `@dhis2/config-lslint` ships a canonical `.ls-lint.yml` to +copy in, not something you import: + +```bash +cp node_modules/@dhis2/config-lslint/src/ls-lint.yml .ls-lint.yml +``` + +If the app's existing `.ls-lint.yml` has project-specific overrides beyond the base ruleset +(check with `git diff` against a fresh copy before overwriting), keep those — layer them +back in rather than losing them. + +## Step 7: Update `package.json` scripts + +Replace the `d2-style`-based scripts: + +```diff +- "lint": "d2-style check", +- "lint:staged": "d2-style check --staged", +- "format": "d2-style apply", +- "format:staged": "d2-style apply --staged" ++ "lint": "eslint && prettier -c .", ++ "format": "prettier . -w", ++ "prepare": "husky" +``` + +If the app also runs a type-check as part of lint (common — `"lint": "yarn tsc && d2-style check"`), +keep that prefix: `"lint": "tsc --noEmit && eslint && prettier -c ."`. If Step 5/6 applied, +extend the `lint` script rather than leaving stylelint/ls-lint unchecked: +`"lint": "eslint && prettier -c . && stylelint '**/*.{css,js,jsx,ts,tsx}' && ls-lint"`. Drop +the `:staged` variants entirely — `lint-staged` (Step 8) replaces that mechanism. + +## Step 8: Migrate git hooks to native husky + lint-staged + +Add a `lint-staged` block to `package.json`, including stylelint if Step 5 applied +(ls-lint has no per-file mode, so it isn't a `lint-staged` candidate — it stays in the +plain `lint` script from Step 7 and runs against the whole tree): + +```json +"lint-staged": { + "*": ["yarn prettier . --write", "yarn lint"] +} +``` + +If the app has the older `.hooks/pre-commit` + `.hooks/commit-msg` (manually sourcing +`husky.sh`, generated by older `cli-app-scripts` templates), replace them with native husky +v9+ hooks: + +```bash +rm -rf .hooks +mkdir -p .husky +echo 'yarn lint-staged' > .husky/pre-commit +``` + +`.hooks/commit-msg` is handled separately in Step 9 — don't delete it here. + +## Step 9: Replace commit-msg linting with `@dhis2/config-commitlint` + +**If `.hooks/commit-msg` (or `.husky/commit-msg`) just delegates to a CI job** (e.g. the repo +also has `lint-commits: uses: dhis2/workflows-platform/.github/workflows/lint-commits.yml@v1`) +and doesn't run its own linter, it's fine to drop the local hook entirely — CI still catches +bad commit messages, just later. + +**If it calls `d2-style check commit` directly** (`d2-style`'s own commit-message linter, +distinct from any CI job), that check disappears once `cli-style` is gone and needs a real +replacement, not just deletion — otherwise commit messages go unchecked locally even though +the local hook was providing faster, pre-push feedback than CI. Use the shared +`@dhis2/config-commitlint` package instead of hand-rolling a `@commitlint/config-conventional` +setup: + +```bash +npm view @dhis2/config-commitlint version +npm view @commitlint/cli version +``` + +```json +"devDependencies": { + "@commitlint/cli": "^21.2.2", + "@dhis2/config-commitlint": "^0.4.0" +} +``` + +(again, `0.4.0` is illustrative — pin to whatever version the `npm view` command above actually +reported, not the literal `"latest"` tag.) + +```javascript +// commitlint.config.mjs +import config from '@dhis2/config-commitlint' + +export default config +``` + +```bash +echo 'npx --no -- commitlint --edit "$1"' > .husky/commit-msg +``` + +`@dhis2/config-commitlint` already extends `@commitlint/config-conventional` with the same +`header-max-length`/`body-max-line-length`/`[skip release]`/`[skip ci]` rules `d2-style`'s +bundled commitlint config used — don't add `@commitlint/config-conventional` as a direct +dependency or re-declare those rules locally, the shared config covers it. + +## Step 10: Remove `@dhis2/cli-style` + +Now that every config it was proxying (Steps 3-6, whichever applied) has a real +replacement, confirm nothing still references it before removing it: + +```bash +grep -rn "require('@dhis2/cli-style')\|from '@dhis2/cli-style'" --include='*.js' --include='*.mjs' . +grep -n "d2-style" package.json +``` + +Both should come back empty (aside from the `.hooks/` files already deleted in Step 8, if +applicable). Then remove it from `package.json` entirely — don't leave it listed but unused +(the real `route-manager-app#35` migration missed this; it's worth doing properly here +rather than repeating that oversight). + +## Step 11: Update CI + +If `.github/workflows/*.yml` uses a separate reusable `lint` job +(`uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1`) — that reusable workflow +is built around `d2-style` and won't work once it's gone. Fold linting into the existing +test job instead, the same way `route-manager-app#35` did: + +```diff + jobs: + lint-commits: + uses: dhis2/workflows-platform/.github/workflows/lint-commits.yml@v1 +- lint: +- uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1 +- test: ++ lint-and-test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - run: yarn install --frozen-lockfile + - run: yarn build ++ - run: yarn lint + - run: yarn test --coverage +``` + +The inline `yarn lint`/`pnpm lint` step should be whatever Step 7 landed on — if stylelint +or ls-lint got folded into that script, CI picks them up automatically; don't add separate +CI steps for them. + +**`lint-commits` is not unrelated to `cli-style` the way its name suggests.** On `@v1` (and +the `pnpm` branch), `lint-commits.yml` (and `lint-pr-title.yml`, if present) resolves its +commitlint config via `require('@dhis2/cli-style').config.commitlint)` — it breaks with +"Cannot find module '@dhis2/cli-style'" once this migration removes it (Step 10). Point it +at `@v2` instead, which doesn't have this coupling, or replace it with a custom step using +`@dhis2/config-commitlint` (Step 9) — see `references/ci-migration.md` Step 3 for both. + +## Step 12: Verify + +```bash +pnpm install +pnpm lint +pnpm format +``` + +The shared configs aren't a byte-for-byte match for `@dhis2/cli-style`'s rule set — expect +some new violations to fix, mostly formatting (`prettier . -w` handles most of it +automatically) and occasionally a rule that's now an error where it used to be a warning (or +vice versa). Fix real issues; don't disable rules just to make the diff smaller. + +## Troubleshooting + +| Symptom | Fix | +| ------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `eslint` errors "Cannot find config @dhis2/config-eslint" | It wasn't installed, or `.eslintrc.js` (old-style config) still exists alongside the new flat `eslint.config.mjs` and ESLint is confused about which to use — delete the old one. | +| A rule the app relied on is missing after switching | Check if `@dhis2/config-eslint/react` (not the base export) covers it before adding a manual override — see Step 3. | +| `prettier -c .` fails on `pnpm-lock.yaml` or another generated file | Add it to `.prettierignore` — lockfiles and build output shouldn't be formatted. | +| `stylelint` errors "Cannot find module 'stylelint-use-logical'" | A dual-package-hazard/phantom-dependency issue, not a missing install — confirm you're consuming a real published/packed install of `@dhis2/config-stylelint`, not a `pnpm link:`'d local checkout; a real install resolves its bundled plugin correctly. | +| `ls-lint` reports violations that weren't there before | `@dhis2/config-lslint`'s ruleset isn't guaranteed identical to the app's old customized `.ls-lint.yml` — see the note in Step 6 about preserving project-specific overrides. | +| CI still fails after removing the reusable `lint.yml@v1` job | Confirm the new inline `yarn lint`/`pnpm lint` step was actually added to the remaining job — it's easy to drop the job without replacing the step. | +| `npm view @dhis2/config-commitlint` (or any `@dhis2/config-*`) resolves to an unexpectedly old version | Check `npm view @dhis2/config-commitlint dist-tags` and use whatever `latest` currently resolves to. | diff --git a/src/skills/modernise-apps/scripts/README.md b/src/skills/modernise-apps/scripts/README.md new file mode 100644 index 0000000..e56cba3 --- /dev/null +++ b/src/skills/modernise-apps/scripts/README.md @@ -0,0 +1,18 @@ +# modernise-apps smoke-test + +A standalone Playwright script for sanity-checking a migrated DHIS2 app +against a real server. See `../references/pnpm-migration.md` ("Step 11: +Sanity-check against a real server") for full usage docs and when to use it. + +One-time setup: + +```sh +pnpm install +npx playwright install chromium +``` + +Per run (from any directory): + +```sh +node smoke-test.mjs --cwd /path/to/migrated-app +``` diff --git a/src/skills/modernise-apps/scripts/package.json b/src/skills/modernise-apps/scripts/package.json new file mode 100644 index 0000000..37adf98 --- /dev/null +++ b/src/skills/modernise-apps/scripts/package.json @@ -0,0 +1,13 @@ +{ + "name": "modernise-apps-smoke-test", + "private": true, + "version": "0.0.1", + "type": "module", + "description": "Standalone Playwright smoke-test tool bundled with the modernise-apps skill. Not part of any migrated app's dependency tree -- install once with `pnpm install` inside this folder, then run `node smoke-test.mjs`.", + "scripts": { + "smoke-test": "node smoke-test.mjs" + }, + "dependencies": { + "playwright": "^1.62.1" + } +} diff --git a/src/skills/modernise-apps/scripts/pnpm-lock.yaml b/src/skills/modernise-apps/scripts/pnpm-lock.yaml new file mode 100644 index 0000000..454ab6b --- /dev/null +++ b/src/skills/modernise-apps/scripts/pnpm-lock.yaml @@ -0,0 +1,43 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +importers: + + .: + dependencies: + playwright: + specifier: ^1.62.1 + version: 1.62.1 + +packages: + + fsevents@2.3.2: + resolution: {integrity: sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==} + engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} + os: [darwin] + + playwright-core@1.62.1: + resolution: {integrity: sha512-wPYSwEBJY9GHraISXqyqtx0na0LpO3XEX7jNDhntbex7tzUS7kLnZsOlFruFJB4Hi/rhDMjXGqHewDZ68nYZVw==} + engines: {node: '>=20'} + hasBin: true + + playwright@1.62.1: + resolution: {integrity: sha512-0M+L3LAD8/nm554LOla9Ayx0j0tmFZ0FBcoQ7F1VuVHpM/XpiC8RcDzBQB8W5+hA8L22THxELzeF+2WcUzvcLg==} + engines: {node: '>=20'} + hasBin: true + +snapshots: + + fsevents@2.3.2: + optional: true + + playwright-core@1.62.1: {} + + playwright@1.62.1: + dependencies: + playwright-core: 1.62.1 + optionalDependencies: + fsevents: 2.3.2 diff --git a/src/skills/modernise-apps/scripts/smoke-test.mjs b/src/skills/modernise-apps/scripts/smoke-test.mjs new file mode 100644 index 0000000..bea0992 --- /dev/null +++ b/src/skills/modernise-apps/scripts/smoke-test.mjs @@ -0,0 +1,357 @@ +#!/usr/bin/env node +/** + * smoke-test.mjs + * + * Standalone sanity check for a migrated DHIS2 app: starts the app's own dev + * server pointed at a real DHIS2 server via `--proxy`, logs in the same way + * @dhis2/cypress-commands' loginByApi() does (a plain cookie-session login, + * no OAuth), and confirms the authenticated app shell actually renders. + * + * This is a skill-owned tool -- it lives in the skill's own scripts/ folder + * with its own package.json/node_modules, and never touches the target + * app's package.json or lockfile. Point --cwd at the app's directory; this + * script and its dependencies can live anywhere else on disk. + * + * Usage (one-time setup already done, see README.md): + * + * node smoke-test.mjs --cwd /path/to/migrated-app + * + * All flags (all optional): + * --cwd Directory to run the start command in (default: process cwd) + * --start-cmd Full command to start the dev server + * (default: "pnpm start --proxy ", extended with + * --port/--proxyPort only if those were overridden below) + * --server Remote DHIS2 instance to proxy to + * (default: https://play.im.dhis2.org/stable-2-43-1) + * --app-port Port the app's own dev server listens on (default: 3000) + * --proxy-port Port the App Platform's built-in proxy listens on (default: 8080) + * --username Login username (default: admin) + * --password Login password (default: district) + * --timeout Milliseconds to wait for the dev server to come up, and + * for the initial page navigation (default: 120000) + * --screenshot Where to save the post-login screenshot + * (default: ./smoke-test-screenshot.png -- point this + * outside the app's working tree to avoid leaving a stray + * file in `git status`) + * --selector CSS selector that indicates a real logged-in app shell + * has rendered (default: [data-test="headerbar-title"], + * the standard @dhis2/ui HeaderBar's title element). + * Override this if the app doesn't use the standard + * App Platform shell (e.g. a plugin, or a heavily + * customised header). + * + * Exit code: 0 on pass, 1 on any failure (unreachable server, failed login, + * selector never appeared, or an uncaught JS error on the page). + */ + +import { spawn, spawnSync } from 'node:child_process' +import { mkdir } from 'node:fs/promises' +import path from 'node:path' +import { chromium } from 'playwright' + +const DEFAULT_SERVER = 'https://play.im.dhis2.org/stable-2-43-1' +const DEFAULT_APP_PORT = 3000 +const DEFAULT_PROXY_PORT = 8080 +const DEFAULT_TIMEOUT_MS = 120_000 +const DEFAULT_SCREENSHOT = './smoke-test-screenshot.png' +const DEFAULT_SELECTOR = '[data-test="headerbar-title"]' +// How long to wait for the app shell to render *after* the dev server itself +// has already responded and login has already succeeded. Kept separate from +// --timeout (which governs the much slower "dev server boot" wait). +const RENDER_TIMEOUT_MS = 45_000 + +let child = null +let browser = null +let cleanedUp = false + +function parseArgs(argv) { + const out = {} + for (let i = 0; i < argv.length; i++) { + const arg = argv[i] + if (!arg.startsWith('--')) continue + const key = arg.slice(2) + const next = argv[i + 1] + if (next !== undefined && !next.startsWith('--')) { + out[key] = next + i++ + } else { + out[key] = true + } + } + return out +} + +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +function buildDefaultStartCmd({ server, appPort, proxyPort }) { + let cmd = `pnpm start --proxy ${server}` + if (appPort !== DEFAULT_APP_PORT) cmd += ` --port ${appPort}` + if (proxyPort !== DEFAULT_PROXY_PORT) cmd += ` --proxyPort ${proxyPort}` + return cmd +} + +async function waitForPort(url, timeoutMs) { + const start = Date.now() + while (Date.now() - start < timeoutMs) { + try { + // Any response at all (even a Vite error overlay) means + // something is listening -- that's all we need to know here. + await fetch(url, { signal: AbortSignal.timeout(2000) }) + return true + } catch { + // Not up yet -- keep polling. + } + await sleep(1000) + } + return false +} + +async function tryGetMe(request, server) { + try { + const res = await request.get(`${server}/api/me`, { timeout: 15_000 }) + if (res.ok()) { + const body = await res.json() + return { + ok: true, + username: body.username ?? '(unknown)', + } + } + return { ok: false, message: `GET /api/me returned ${res.status()}` } + } catch (e) { + return { ok: false, message: `GET /api/me failed: ${e.message}` } + } +} + +/** + * Mirrors @dhis2/cypress-commands' loginByApi(): a plain cookie-session + * login against the DHIS2 API, no browser form interaction needed. Tries + * the modern JSON endpoint first, falls back to the legacy form-encoded one + * for older core versions, and verifies success via a real authenticated + * GET rather than trusting either POST's status code in isolation. + */ +async function loginByApi({ request, server, username, password }) { + try { + await request.post(`${server}/api/auth/login`, { + data: { username, password }, + timeout: 15_000, + }) + } catch { + // Might be an older core without this endpoint -- checked below. + } + + let me = await tryGetMe(request, server) + if (!me.ok) { + try { + await request.post( + `${server}/dhis-web-commons-security/login.action`, + { + form: { j_username: username, j_password: password }, + timeout: 15_000, + } + ) + } catch { + // Checked below either way. + } + me = await tryGetMe(request, server) + } + + return me.ok + ? { ok: true, username: me.username } + : { ok: false, message: me.message } +} + +async function cleanup() { + if (cleanedUp) return + cleanedUp = true + + if (browser) { + try { + await browser.close() + } catch { + // Best-effort. + } + } + + if (child && child.exitCode === null && !child.killed) { + try { + if (process.platform === 'win32') { + spawnSync('taskkill', ['/pid', String(child.pid), '/t', '/f']) + } else { + // Negative pid targets the whole process group (the dev + // server plus anything it spawned, e.g. esbuild's service + // process) since the child was launched with detached: true. + process.kill(-child.pid, 'SIGTERM') + await sleep(2000) + try { + process.kill(-child.pid, 'SIGKILL') + } catch { + // Already dead -- fine. + } + } + } catch { + // Best-effort. + } + } +} + +async function main() { + const args = parseArgs(process.argv.slice(2)) + + const cwd = args.cwd ?? process.cwd() + const server = args.server ?? DEFAULT_SERVER + const appPort = Number(args['app-port'] ?? DEFAULT_APP_PORT) + const proxyPort = Number(args['proxy-port'] ?? DEFAULT_PROXY_PORT) + const username = args.username ?? 'admin' + const password = args.password ?? 'district' + const timeoutMs = Number(args.timeout ?? DEFAULT_TIMEOUT_MS) + const screenshotPath = args.screenshot ?? DEFAULT_SCREENSHOT + const selector = args.selector ?? DEFAULT_SELECTOR + const startCmd = + args['start-cmd'] ?? + buildDefaultStartCmd({ server, appPort, proxyPort }) + + const appUrl = `http://localhost:${appPort}` + const proxyOrigin = `http://localhost:${proxyPort}` + + console.log(`[smoke-test] cwd: ${cwd}`) + console.log(`[smoke-test] starting dev server: ${startCmd}`) + + let serverOutput = '' + child = spawn(startCmd, { + shell: true, + cwd, + detached: process.platform !== 'win32', + stdio: ['ignore', 'pipe', 'pipe'], + }) + child.stdout.on('data', (d) => (serverOutput += d.toString())) + child.stderr.on('data', (d) => (serverOutput += d.toString())) + + console.log( + `[smoke-test] waiting for ${appUrl} (timeout ${timeoutMs}ms)...` + ) + const up = await waitForPort(appUrl, timeoutMs) + if (!up) { + console.error( + `[smoke-test] FAIL: dev server never responded on ${appUrl} within ${timeoutMs}ms` + ) + console.error('--- dev server output (last 4000 chars) ---') + console.error(serverOutput.slice(-4000)) + return false + } + console.log('[smoke-test] dev server is up') + + browser = await chromium.launch() + // A fresh, non-persisted context matters here: @dhis2/app-adapter checks + // IndexedDB for a previously-saved base URL before falling back to + // localStorage.DHIS2_BASE_URL. A persisted/reused profile could carry a + // stale server URL from a previous run and silently defeat this script. + const context = await browser.newContext() + + console.log(`[smoke-test] logging in to ${proxyOrigin} as ${username}...`) + const loginResult = await loginByApi({ + request: context.request, + server: proxyOrigin, + username, + password, + }) + if (!loginResult.ok) { + console.error( + `[smoke-test] FAIL: login did not succeed: ${loginResult.message}` + ) + console.error( + '[smoke-test] hint: is the proxy actually up on ' + + `${proxyOrigin}? Check --proxy-port matches what the start ` + + 'command uses, and that --server is reachable.' + ) + return false + } + console.log( + `[smoke-test] login OK (logged in as "${loginResult.username}")` + ) + + // Tell the app which server to talk to -- mirrors exactly what the + // login form itself does on submit (`window.localStorage.DHIS2_BASE_URL + // = server`), so the app skips its own login screen entirely and goes + // straight to the authenticated shell using the session cookie we just + // obtained above. + await context.addInitScript((baseUrl) => { + window.localStorage.setItem('DHIS2_BASE_URL', baseUrl) + }, proxyOrigin) + + const consoleErrors = [] + const pageErrors = [] + const page = await context.newPage() + page.on('console', (msg) => { + if (msg.type() === 'error') consoleErrors.push(msg.text()) + }) + page.on('pageerror', (err) => { + pageErrors.push(err.message) + }) + + console.log(`[smoke-test] opening ${appUrl}...`) + await page.goto(appUrl, { + waitUntil: 'domcontentloaded', + timeout: timeoutMs, + }) + + let selectorFound = false + try { + await page.waitForSelector(selector, { timeout: RENDER_TIMEOUT_MS }) + selectorFound = true + } catch { + selectorFound = false + } + + await mkdir(path.dirname(path.resolve(screenshotPath)), { recursive: true }) + await page.screenshot({ path: screenshotPath, fullPage: true }) + + const passed = selectorFound && pageErrors.length === 0 + + console.log('') + console.log('=== smoke-test summary ===') + console.log(`login: OK (as ${loginResult.username})`) + console.log( + `shell rendered: ${selectorFound ? 'YES' : 'NO'} (waited for ${selector})` + ) + console.log(`console errors: ${consoleErrors.length}`) + consoleErrors.slice(0, 10).forEach((m) => console.log(` - ${m}`)) + console.log(`uncaught errors: ${pageErrors.length}`) + pageErrors.forEach((m) => console.log(` - ${m}`)) + console.log(`screenshot: ${path.resolve(screenshotPath)}`) + console.log(`result: ${passed ? 'PASS' : 'FAIL'}`) + + if (!selectorFound) { + console.error( + '[smoke-test] hint: if this app does not use the standard ' + + '@dhis2/ui HeaderBar, re-run with --selector pointed at ' + + 'something else that only appears once logged in.' + ) + } + + return passed +} + +for (const sig of ['SIGINT', 'SIGTERM']) { + process.on(sig, async () => { + await cleanup() + process.exit(1) + }) +} +process.on('uncaughtException', async (err) => { + console.error('[smoke-test] uncaught exception:', err) + await cleanup() + process.exit(1) +}) + +main() + .then(async (passed) => { + await cleanup() + process.exit(passed ? 0 : 1) + }) + .catch(async (err) => { + console.error('[smoke-test] unexpected error:', err) + await cleanup() + process.exit(1) + })