From 942771dc81df587a02881f66b89db7c1210c74b8 Mon Sep 17 00:00:00 2001 From: Mozafar Haider Date: Mon, 14 Sep 2026 09:27:19 +0100 Subject: [PATCH 1/5] feat: add modernise repo skill Covers two independent modernisation tasks for existing DHIS2 apps: migrating the package manager from Yarn to pnpm, and moving off @dhis2/cli-style onto the shared @dhis2/config-* packages (eslint, prettier, stylelint, ls-lint, commitlint -- whichever the app already had configured), plus a non-breaking @dhis2/ui version bump. Removes @dhis2/cli-style only once every config it was proxying has a real replacement. Co-Authored-By: Claude Sonnet 5 --- .gitignore | 4 + README.md | 99 +++++ pnpm-lock.yaml | 2 + src/skills/dhis2-apps/dhis2-apps | 1 + src/skills/modernise-apps/SKILL.md | 100 +++++ src/skills/modernise-apps/evals/evals.json | 168 +++++++++ src/skills/modernise-apps/package.json | 12 + .../references/pnpm-migration.md | 287 ++++++++++++++ .../references/style-configs-migration.md | 352 +++++++++++++++++ src/skills/modernise-apps/scripts/README.md | 18 + .../modernise-apps/scripts/package.json | 13 + .../modernise-apps/scripts/pnpm-lock.yaml | 43 +++ .../modernise-apps/scripts/smoke-test.mjs | 357 ++++++++++++++++++ 13 files changed, 1456 insertions(+) create mode 120000 src/skills/dhis2-apps/dhis2-apps create mode 100644 src/skills/modernise-apps/SKILL.md create mode 100644 src/skills/modernise-apps/evals/evals.json create mode 100644 src/skills/modernise-apps/package.json create mode 100644 src/skills/modernise-apps/references/pnpm-migration.md create mode 100644 src/skills/modernise-apps/references/style-configs-migration.md create mode 100644 src/skills/modernise-apps/scripts/README.md create mode 100644 src/skills/modernise-apps/scripts/package.json create mode 100644 src/skills/modernise-apps/scripts/pnpm-lock.yaml create mode 100644 src/skills/modernise-apps/scripts/smoke-test.mjs 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/README.md b/README.md index 43bc722..0135b3a 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,80 @@ 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 two +independent migrations — an app can need either, both, or neither: + +- **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 + `@dhis2/config-eslint`/`@dhis2/config-prettier`, a flat `eslint.config.mjs` and + `.prettierrc.mjs`, and migrates git hooks from the old `.hooks/` + `d2-style` setup to + native `husky`/`lint-staged` (with `commitlint` standing in for any commit-message check + that used to go through `d2-style`) — the same setup a freshly scaffolded app already uses + by default. + +Either migration 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 +102,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/dhis2-apps/dhis2-apps b/src/skills/dhis2-apps/dhis2-apps new file mode 120000 index 0000000..a780761 --- /dev/null +++ b/src/skills/dhis2-apps/dhis2-apps @@ -0,0 +1 @@ +/home/mozafar/code/dhis/others/testing-skill/.agents/skills/dhis2-apps \ No newline at end of file diff --git a/src/skills/modernise-apps/SKILL.md b/src/skills/modernise-apps/SKILL.md new file mode 100644 index 0000000..5353d24 --- /dev/null +++ b/src/skills/modernise-apps/SKILL.md @@ -0,0 +1,100 @@ +--- +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 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 to a newer version, in the + context of an existing DHIS2 app. 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 two independent tasks — an app can need either, both, or neither: + +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. +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. This + task also includes bumping `@dhis2/ui` to the latest non-breaking version while the + app's style tooling is already being touched. + +## 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 | + +## What does the user need? + +| Scenario | References (read in order) | +| ---------------------------------------------------------------------------- | ------------------------------------------------- | +| Migrate an app from yarn to pnpm | `references/pnpm-migration.md` | +| Sanity-check a migrated app renders correctly against a real server (opt-in) | `references/pnpm-migration.md` (Step 10) | +| Move off `@dhis2/cli-style`/`d2-style` onto shared lint/format configs | `references/style-configs-migration.md` | +| Bump `@dhis2/ui` to the latest non-breaking version | `references/style-configs-migration.md` (Step 12) | + +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. +- **If the app's CI calls `dhis2/workflows-platform` reusable workflows, point them at the + `pnpm` branch** (`uses: dhis2/workflows-platform/.github/workflows/.yml@pnpm`), + the same way `aggregate-data-entry-app#481` did — pnpm support hasn't been merged into + the default `@v1` tag yet, but the `pnpm` branch has it. Confirm the branch still exists + first (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) rather than + assuming it forever, since this is expected to eventually merge into `@v1`. +- **The Step 10 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` bump must stay non-breaking** — update within the app's current major + version only. If it surfaces new type or lint errors, that's a real regression to + investigate via the changelog, not something to silence. + +## 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. | diff --git a/src/skills/modernise-apps/evals/evals.json b/src/skills/modernise-apps/evals/evals.json new file mode 100644 index 0000000..40daf1d --- /dev/null +++ b/src/skills/modernise-apps/evals/evals.json @@ -0,0 +1,168 @@ +{ + "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 separate lint-commits CI job (unrelated, should stay untouched), a lint: uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1 job (should be folded into an inline step), 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 CI job is left untouched (unrelated to this migration)", + "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 -- 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 CI job is left untouched", + "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)" + ] + } + ] +} 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/pnpm-migration.md b/src/skills/modernise-apps/references/pnpm-migration.md new file mode 100644 index 0000000..4885c17 --- /dev/null +++ b/src/skills/modernise-apps/references/pnpm-migration.md @@ -0,0 +1,287 @@ +# 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. +- README/CONTRIBUTING docs referencing `yarn install`, `yarn start`, etc. + +## Step 8: 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 the `pnpm` branch instead of the default `@v1` tag — the same approach +`aggregate-data-entry-app#481` used: + +```yaml +uses: dhis2/workflows-platform/.github/workflows/test.yml@pnpm +``` + +pnpm support hasn't been merged into `@v1` yet as of this writing, so `@v1` workflows will +still run `yarn install` internally and fail once `yarn.lock` is gone. Confirm the `pnpm` +branch still exists before using it — it's expected to eventually merge into `@v1`, at which +point this step becomes unnecessary: + +```bash +gh api repos/dhis2/workflows-platform/branches --jq '.[].name' +``` + +## Step 9: 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 10 for that. + +Confirm `pnpm-lock.yaml` is present and tracked in git, and `yarn.lock` is gone. + +## Step 10: 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 9, 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 10 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 10 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. | 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..04ebd59 --- /dev/null +++ b/src/skills/modernise-apps/references/style-configs-migration.md @@ -0,0 +1,352 @@ +# 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 (`references/pnpm-migration.md`) — an app can do either, +both, or neither. If doing both, order doesn't matter; do whichever the user asked for. + +`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 still in prerelease, published under the `alpha` +dist-tag (`npm view @dhis2/config-eslint dist-tags` shows the full set — they version in +lockstep from the same `style-configs` repo). Use the `alpha` tag for all of them until +they're promoted to `latest`; check dist-tags again if any install unexpectedly 404s. + +--- + +## 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 alpha tag + current version for all five +``` + +```json +"devDependencies": { + "@dhis2/config-eslint": "alpha", + "@dhis2/config-prettier": "alpha", + "@eslint/compat": "^2.0.0", + "eslint": "^9", + "prettier": "^3", + "husky": "^9.1.7", + "lint-staged": "^16" +} +``` + +Add these two conditionally, only if Step 1 found them configured: + +```json +"devDependencies": { + "@dhis2/config-stylelint": "alpha", + "stylelint": "^16", + "@dhis2/config-lslint": "alpha", + "@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@alpha version # check current alpha version +npm view @commitlint/cli version +``` + +```json +"devDependencies": { + "@commitlint/cli": "^21.2.2", + "@dhis2/config-commitlint": "alpha" +} +``` + +```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` (commit message format) is a separate reusable workflow unrelated to +`d2-style` — leave it as-is. + +## Step 12: Bump `@dhis2/ui` to the latest non-breaking version + +While touching the app's style/UI tooling, also bring `@dhis2/ui` up to date within its +current major — a low-risk win that's easy to bundle into the same PR, but keep it a +**non-breaking** bump: don't cross a major version here, that's a separate, riskier task the +user didn't ask for. + +```bash +node -e "console.log(require('./package.json').dependencies['@dhis2/ui'])" # current range +npm view @dhis2/ui@ version # e.g. npm view @dhis2/ui@9 version +``` + +Update `package.json` to that version (keep the same range style the app already uses — +`^`, `~`, or exact), then: + +```bash +pnpm install +pnpm lint # tsc/eslint will flag any prop or export that actually did change +``` + +If anything breaks, that's a signal the release wasn't as non-breaking as its version number +implied — check the `@dhis2/ui` changelog for the affected component before working around +it, don't just silence the error. + +## Step 13: 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@alpha` (or any `@dhis2/config-*@alpha`) 404s | The `style-configs` prerelease hasn't published yet, or `alpha` has since been promoted to `latest` — check `npm view @dhis2/config-commitlint dist-tags` and use whichever tag/version actually resolves. | +| `@dhis2/ui` bump surfaces new TypeScript/ESLint errors | The release wasn't purely non-breaking for a prop/export the app uses — check that component's changelog entry rather than suppressing the error; consider pinning back a version if it's a real regression. | diff --git a/src/skills/modernise-apps/scripts/README.md b/src/skills/modernise-apps/scripts/README.md new file mode 100644 index 0000000..230e802 --- /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 10: +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) + }) From 90c0154de85c114e5b13f8e58482026f87e98f5f Mon Sep 17 00:00:00 2001 From: Mozafar Haider Date: Wed, 16 Sep 2026 19:20:16 +0300 Subject: [PATCH 2/5] feat(skill-modernise-apps): add CI modernisation task and README refresh step Adds a third independent modernisation task -- moving bespoke GitHub Actions workflows onto dhis2/workflows-platform reusable workflows and bumping outdated actions/Node versions -- documented in the new references/ci-migration.md, including the known gap where lint.yml/lint-commits.yml/lint-pr-title.yml internally require @dhis2/cli-style and the pnpm-no-cli-style branch that fixes it. Also adds a README-refresh step to the pnpm migration (pnpm badge, an App Hub-sourced app description matched by d2.config.js's id, and a concise "Get Started" section replacing the scaffold's verbose per-script "Available Scripts" layout), extends the platform-library bump step to cover @dhis2/app-runtime and @dhis2/d2-i18n alongside @dhis2/ui (non-breaking by default, with an explicit choice offered if latest would cross a major), and extends the eval suite with dedicated CI-migration fixtures (a real app-management-app snapshot and a synthetic one to avoid the confound of a baseline agent finding an already-fixed real repo to copy). Co-Authored-By: Claude Sonnet 5 --- README.md | 32 ++-- src/skills/dhis2-apps/dhis2-apps | 1 - src/skills/modernise-apps/SKILL.md | 82 ++++++--- src/skills/modernise-apps/evals/evals.json | 34 +++- .../modernise-apps/references/ci-migration.md | 174 ++++++++++++++++++ .../references/pnpm-migration.md | 72 +++++++- .../references/style-configs-migration.md | 89 ++++++--- src/skills/modernise-apps/scripts/README.md | 2 +- 8 files changed, 408 insertions(+), 78 deletions(-) delete mode 120000 src/skills/dhis2-apps/dhis2-apps create mode 100644 src/skills/modernise-apps/references/ci-migration.md diff --git a/README.md b/README.md index 0135b3a..9352a47 100644 --- a/README.md +++ b/README.md @@ -67,23 +67,31 @@ 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 two -independent migrations — an app can need either, both, or neither: +An AI skill for bringing an existing DHIS2 app's tooling up to date. It covers five 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 - `@dhis2/config-eslint`/`@dhis2/config-prettier`, a flat `eslint.config.mjs` and - `.prettierrc.mjs`, and migrates git hooks from the old `.hooks/` + `d2-style` setup to - native `husky`/`lint-staged` (with `commitlint` standing in for any commit-message check - that used to go through `d2-style`) — the same setup a freshly scaffolded app already uses - by default. - -Either migration 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. +- **`@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. +- **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. + +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 diff --git a/src/skills/dhis2-apps/dhis2-apps b/src/skills/dhis2-apps/dhis2-apps deleted file mode 120000 index a780761..0000000 --- a/src/skills/dhis2-apps/dhis2-apps +++ /dev/null @@ -1 +0,0 @@ -/home/mozafar/code/dhis/others/testing-skill/.agents/skills/dhis2-apps \ No newline at end of file diff --git a/src/skills/modernise-apps/SKILL.md b/src/skills/modernise-apps/SKILL.md index 5353d24..cbe1f31 100644 --- a/src/skills/modernise-apps/SKILL.md +++ b/src/skills/modernise-apps/SKILL.md @@ -4,51 +4,66 @@ 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 in the context of an - existing DHIS2 app. Also use it when the user wants to move an app off @dhis2/cli-style + 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 to a newer version, in the - context of an existing DHIS2 app. This skill is for modernising an already-existing app - — for scaffolding a brand new app, use the dhis2-apps skill instead. + commitlint, migrating husky/git hooks, or bumping @dhis2/ui, @dhis2/app-runtime, or + @dhis2/d2-i18n 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. + 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 two independent tasks — an app can need either, both, or neither: +covers three 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. This - task also includes bumping `@dhis2/ui` to the latest non-breaking version while the - app's style tooling is already being touched. + task also includes bumping `@dhis2/ui`, `@dhis2/app-runtime`, and `@dhis2/d2-i18n` while + the app's style tooling is already being touched — non-breaking by default, with an + explicit choice offered if `latest` would cross a major version for any of them. +3. **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. ## 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 | +| 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 | +| `.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 | ## What does the user need? -| Scenario | References (read in order) | -| ---------------------------------------------------------------------------- | ------------------------------------------------- | -| Migrate an app from yarn to pnpm | `references/pnpm-migration.md` | -| Sanity-check a migrated app renders correctly against a real server (opt-in) | `references/pnpm-migration.md` (Step 10) | -| Move off `@dhis2/cli-style`/`d2-style` onto shared lint/format configs | `references/style-configs-migration.md` | -| Bump `@dhis2/ui` to the latest non-breaking version | `references/style-configs-migration.md` (Step 12) | +| 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/style-configs-migration.md` (Step 12) | +| Modernise CI: adopt `dhis2/workflows-platform` reusable workflows, bump outdated actions/Node version | `references/ci-migration.md` | More modernisation tasks (dependency upgrades, router migrations, etc.) may be added here in the future — this table is deliberately structured to grow. @@ -69,7 +84,13 @@ in the future — this table is deliberately structured to grow. the default `@v1` tag yet, but the `pnpm` branch has it. Confirm the branch still exists first (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) rather than assuming it forever, since this is expected to eventually merge into `@v1`. -- **The Step 10 sanity e2e check (`references/pnpm-migration.md`) is optional and opt-in.** +- **`dhis2/workflows-platform`'s `lint.yml`, `lint-commits.yml`, and `lint-pr-title.yml` + reusable workflows all internally require `@dhis2/cli-style` to be installed in the app** + (on both `@v1` and `@pnpm`) — don't adopt them as-is for an app that has migrated off + `cli-style`, they'll fail with "Cannot find module '@dhis2/cli-style'". Check whether the + `pnpm-no-cli-style` branch exists yet (it fixes all three) before falling back to a custom + step. 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 @@ -85,9 +106,18 @@ in the future — this table is deliberately structured to grow. `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` bump must stay non-breaking** — update within the app's current major - version only. If it surfaces new type or lint errors, that's a real regression to - investigate via the changelog, not something to silence. +- **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. +- **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. ## Troubleshooting @@ -98,3 +128,5 @@ in the future — this table is deliberately structured to grow. | 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`. | diff --git a/src/skills/modernise-apps/evals/evals.json b/src/skills/modernise-apps/evals/evals.json index 40daf1d..39cf52b 100644 --- a/src/skills/modernise-apps/evals/evals.json +++ b/src/skills/modernise-apps/evals/evals.json @@ -94,7 +94,7 @@ "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 separate lint-commits CI job (unrelated, should stay untouched), a lint: uses: dhis2/workflows-platform/.github/workflows/lint.yml@v1 job (should be folded into an inline step), and `@dhis2/ui` at `^10.16.1` that should get a non-breaking bump.", + "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", @@ -104,7 +104,7 @@ "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 CI job is left untouched (unrelated to this migration)", + "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", @@ -140,7 +140,7 @@ "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 -- 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.", + "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", @@ -153,7 +153,7 @@ "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 CI job is left untouched", + "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", @@ -163,6 +163,32 @@ "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" + ] } ] } 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..cb5db14 --- /dev/null +++ b/src/skills/modernise-apps/references/ci-migration.md @@ -0,0 +1,174 @@ +# 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: Check `@v1` vs `@pnpm` and known `cli-style` coupling before adopting a job + +Not every reusable workflow is a drop-in replacement for every app — check both of these +before wiring one in: + +- **If this app is also migrating to pnpm** (`references/pnpm-migration.md`), every reusable + workflow's `@v1` tag runs `yarn install --frozen-lockfile` internally — point the `uses:` + ref at the `@pnpm` branch instead (`uses: dhis2/workflows-platform/.github/workflows/.yml@pnpm`), + the same way `aggregate-data-entry-app#481` did. Confirm the branch still exists first + (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) — this is expected to + eventually merge into `@v1`. +- **`lint.yml`, `lint-commits.yml`, and `lint-pr-title.yml` all internally require + `@dhis2/cli-style` to be installed in the app**, on both `@v1` and `@pnpm` — `lint.yml` runs + `d2-style check` directly, and the other two resolve their commitlint config via + `require('@dhis2/cli-style').config.commitlint`. If this app has migrated off `cli-style` + (`references/style-configs-migration.md`), all three break with "Cannot find module + '@dhis2/cli-style'" once adopted as-is. Don't adopt them for such an app as-is — check for a + fixed branch first, and fall back to a custom step if none exists yet: + + ```bash + gh api repos/dhis2/workflows-platform/branches --jq '.[].name' + ``` + + A `pnpm-no-cli-style` branch fixes all three (built on top of `@pnpm`: `lint.yml` runs the + app's own `pnpm lint` instead of `pnpm d2-style check`, and `lint-commits.yml`/ + `lint-pr-title.yml` point commitlint straight at the app's own `commitlint.config.mjs` + instead of resolving through `cli-style`). If it exists, use it — + `uses: dhis2/workflows-platform/.github/workflows/.yml@pnpm-no-cli-style` — the same + way the `@pnpm` branch itself gets used once confirmed to exist (this is expected to + eventually merge into `@pnpm`/`@v1`, at which point drop the special-cased ref). If it + doesn't show up in that branch list yet (not pushed, or a yarn-based app with no equivalent + branch), fall back to a small custom step: + + ```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 the fixed branch 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'" | That job (`lint.yml`, `lint-commits.yml`, or `lint-pr-title.yml`) depends on `cli-style` internally — see Step 3. Check for the `pnpm-no-cli-style` branch first; fall back to a custom step if it isn't available. | +| `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`. | +| `pnpm install --frozen-lockfile` fails inside a reusable workflow job | The job is still pinned at `@v1` (yarn-based) rather than `@pnpm` — see Step 3. | diff --git a/src/skills/modernise-apps/references/pnpm-migration.md b/src/skills/modernise-apps/references/pnpm-migration.md index 4885c17..4e80168 100644 --- a/src/skills/modernise-apps/references/pnpm-migration.md +++ b/src/skills/modernise-apps/references/pnpm-migration.md @@ -157,9 +157,64 @@ 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. -- README/CONTRIBUTING docs referencing `yarn install`, `yarn start`, etc. +- CONTRIBUTING docs referencing `yarn install`, `yarn start`, etc. The README itself gets a + fuller pass — see Step 8. -## Step 8: Update CI +## 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`: @@ -184,7 +239,7 @@ point this step becomes unnecessary: gh api repos/dhis2/workflows-platform/branches --jq '.[].name' ``` -## Step 9: Verify +## Step 10: Verify Do a clean reinstall to catch anything masked by stale `node_modules` state, then re-run the app's usual checks: @@ -199,11 +254,11 @@ 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 10 for that. +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 10: Sanity-check against a real server (optional) +## 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 @@ -266,7 +321,7 @@ If it fails: 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 9, not something new. + 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 @@ -283,5 +338,6 @@ If it fails: | `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 10 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 10 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. | +| 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 index 04ebd59..5b3c544 100644 --- a/src/skills/modernise-apps/references/style-configs-migration.md +++ b/src/skills/modernise-apps/references/style-configs-migration.md @@ -298,32 +298,67 @@ The inline `yarn lint`/`pnpm lint` step should be whatever Step 7 landed on — or ls-lint got folded into that script, CI picks them up automatically; don't add separate CI steps for them. -`lint-commits` (commit message format) is a separate reusable workflow unrelated to -`d2-style` — leave it as-is. - -## Step 12: Bump `@dhis2/ui` to the latest non-breaking version - -While touching the app's style/UI tooling, also bring `@dhis2/ui` up to date within its -current major — a low-risk win that's easy to bundle into the same PR, but keep it a -**non-breaking** bump: don't cross a major version here, that's a separate, riskier task the -user didn't ask for. +**`lint-commits` is not unrelated to `cli-style` the way it looks.** Despite the name +suggesting it only checks commit message format, `dhis2/workflows-platform`'s +`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). Check whether the +`pnpm-no-cli-style` branch exists yet (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) +and point at that instead of `@v1`/`@pnpm` if so; otherwise replace it with a custom step +using `@dhis2/config-commitlint` (Step 9) instead of leaving it pointed at the reusable +workflow. See `references/ci-migration.md` Step 3 for the full writeup and the exact +replacement YAML — this doc's job is just to flag it here since Step 10 is what triggers it. + +## Step 12: Bump `@dhis2/ui`, `@dhis2/app-runtime`, and `@dhis2/d2-i18n` + +While touching the app's style/UI tooling, also bring these three platform libraries up to +date — a low-risk win that's easy to bundle into the same PR. Check each one independently; +they don't necessarily move majors at the same time: ```bash -node -e "console.log(require('./package.json').dependencies['@dhis2/ui'])" # current range -npm view @dhis2/ui@ version # e.g. npm view @dhis2/ui@9 version +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 ``` -Update `package.json` to that version (keep the same range style the app already uses — -`^`, `~`, or exact), then: +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 or export that actually did change +pnpm lint # tsc/eslint will flag any prop, export, or i18n API that actually changed ``` -If anything breaks, that's a signal the release wasn't as non-breaking as its version number -implied — check the `@dhis2/ui` changelog for the affected component before working around -it, don't just silence the error. +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 13: Verify @@ -340,13 +375,13 @@ 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@alpha` (or any `@dhis2/config-*@alpha`) 404s | The `style-configs` prerelease hasn't published yet, or `alpha` has since been promoted to `latest` — check `npm view @dhis2/config-commitlint dist-tags` and use whichever tag/version actually resolves. | -| `@dhis2/ui` bump surfaces new TypeScript/ESLint errors | The release wasn't purely non-breaking for a prop/export the app uses — check that component's changelog entry rather than suppressing the error; consider pinning back a version if it's a real regression. | +| 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@alpha` (or any `@dhis2/config-*@alpha`) 404s | The `style-configs` prerelease hasn't published yet, or `alpha` has since been promoted to `latest` — check `npm view @dhis2/config-commitlint dist-tags` and use whichever tag/version actually resolves. | +| `@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. | diff --git a/src/skills/modernise-apps/scripts/README.md b/src/skills/modernise-apps/scripts/README.md index 230e802..e56cba3 100644 --- a/src/skills/modernise-apps/scripts/README.md +++ b/src/skills/modernise-apps/scripts/README.md @@ -1,7 +1,7 @@ # 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 10: +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: From 83367e820a610f70ebc5eac597cc5f542d900a43 Mon Sep 17 00:00:00 2001 From: Mozafar Haider Date: Tue, 22 Sep 2026 10:03:08 +0100 Subject: [PATCH 3/5] feat(skill-modernise-apps): add platform-library-bumps step and fix CI ref guidance Extracts platform-library version bumps (@dhis2/ui, @dhis2/app-runtime, @dhis2/d2-i18n, @dhis2/multi-calendar-dates) out of style-configs-migration.md into its own references/platform-library-bumps.md -- it's an orthogonal concern, not something that belongs in the cli-style migration doc. The @dhis2/multi-calendar-dates step now covers useDatePicker and the period-calculation helpers (getFixedPeriodByDate, getAdjacentFixedPeriods, etc.), not just getNowInCalendar -- dhis2/multi-calendar-dates#102 touched all of them. Adds guidance to write a before/after regression test across gregory/ethiopic/nepali before bumping, as a real committed test if the app has a test runner or a temporary standalone script otherwise. Also reflects that 3.x is now published as latest, not alpha-only. Corrects the dhis2/workflows-platform CI guidance: @v1 is yarn-only and its lint/lint-commits/lint-pr-title jobs require @dhis2/cli-style internally; the pnpm branch is pnpm-native but keeps that cli-style coupling; @v2 is pnpm-native without it. Which ref to use depends on the app's pnpm and cli-style status independently, not just "point at @pnpm". Co-Authored-By: Claude Sonnet 5 --- README.md | 6 +- src/skills/modernise-apps/SKILL.md | 107 ++++--- src/skills/modernise-apps/evals/evals.json | 11 + .../modernise-apps/references/ci-migration.md | 101 +++---- .../references/platform-library-bumps.md | 279 ++++++++++++++++++ .../references/pnpm-migration.md | 18 +- .../references/style-configs-migration.md | 92 ++---- 7 files changed, 419 insertions(+), 195 deletions(-) create mode 100644 src/skills/modernise-apps/references/platform-library-bumps.md diff --git a/README.md b/README.md index 9352a47..bd4a337 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,11 @@ an app can need any combination of, or none: 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. + 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". diff --git a/src/skills/modernise-apps/SKILL.md b/src/skills/modernise-apps/SKILL.md index cbe1f31..fcc2130 100644 --- a/src/skills/modernise-apps/SKILL.md +++ b/src/skills/modernise-apps/SKILL.md @@ -10,8 +10,8 @@ description: > 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, or - @dhis2/d2-i18n to a newer version, in the + 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. @@ -22,7 +22,7 @@ description: > # Modernising DHIS2 Apps You are helping a developer bring an existing DHIS2 app up to date. "Modernising" currently -covers three independent tasks — an app can need any combination of them, or none: +covers four 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 @@ -35,35 +35,39 @@ covers three independent tasks — an app can need any combination of them, or n `@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. This - task also includes bumping `@dhis2/ui`, `@dhis2/app-runtime`, and `@dhis2/d2-i18n` while - the app's style tooling is already being touched — non-breaking by default, with an - explicit choice offered if `latest` would cross a major version for any of them. -3. **Modernising the CI pipeline** — replacing bespoke GitHub Actions workflows with the + 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. ## 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 | -| `.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 | +| 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 | ## 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/style-configs-migration.md` (Step 12) | -| Modernise CI: adopt `dhis2/workflows-platform` reusable workflows, bump outdated actions/Node version | `references/ci-migration.md` | +| 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` | More modernisation tasks (dependency upgrades, router migrations, etc.) may be added here in the future — this table is deliberately structured to grow. @@ -78,18 +82,11 @@ in the future — this table is deliberately structured to grow. 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. -- **If the app's CI calls `dhis2/workflows-platform` reusable workflows, point them at the - `pnpm` branch** (`uses: dhis2/workflows-platform/.github/workflows/.yml@pnpm`), - the same way `aggregate-data-entry-app#481` did — pnpm support hasn't been merged into - the default `@v1` tag yet, but the `pnpm` branch has it. Confirm the branch still exists - first (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) rather than - assuming it forever, since this is expected to eventually merge into `@v1`. -- **`dhis2/workflows-platform`'s `lint.yml`, `lint-commits.yml`, and `lint-pr-title.yml` - reusable workflows all internally require `@dhis2/cli-style` to be installed in the app** - (on both `@v1` and `@pnpm`) — don't adopt them as-is for an app that has migrated off - `cli-style`, they'll fail with "Cannot find module '@dhis2/cli-style'". Check whether the - `pnpm-no-cli-style` branch exists yet (it fixes all three) before falling back to a custom - step. See `references/ci-migration.md` Step 3. +- **`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 @@ -113,6 +110,25 @@ in the future — this table is deliberately structured to grow. 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 @@ -121,12 +137,13 @@ in the future — this table is deliberately structured to grow. ## 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`. | +| 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. | diff --git a/src/skills/modernise-apps/evals/evals.json b/src/skills/modernise-apps/evals/evals.json index 39cf52b..ce924c9 100644 --- a/src/skills/modernise-apps/evals/evals.json +++ b/src/skills/modernise-apps/evals/evals.json @@ -189,6 +189,17 @@ "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/references/ci-migration.md b/src/skills/modernise-apps/references/ci-migration.md index cb5db14..e0cb8bd 100644 --- a/src/skills/modernise-apps/references/ci-migration.md +++ b/src/skills/modernise-apps/references/ci-migration.md @@ -84,60 +84,41 @@ Delete the bespoke workflow file(s) each wrapper replaces once the wrapper is in 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: Check `@v1` vs `@pnpm` and known `cli-style` coupling before adopting a job - -Not every reusable workflow is a drop-in replacement for every app — check both of these -before wiring one in: - -- **If this app is also migrating to pnpm** (`references/pnpm-migration.md`), every reusable - workflow's `@v1` tag runs `yarn install --frozen-lockfile` internally — point the `uses:` - ref at the `@pnpm` branch instead (`uses: dhis2/workflows-platform/.github/workflows/.yml@pnpm`), - the same way `aggregate-data-entry-app#481` did. Confirm the branch still exists first - (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) — this is expected to - eventually merge into `@v1`. -- **`lint.yml`, `lint-commits.yml`, and `lint-pr-title.yml` all internally require - `@dhis2/cli-style` to be installed in the app**, on both `@v1` and `@pnpm` — `lint.yml` runs - `d2-style check` directly, and the other two resolve their commitlint config via - `require('@dhis2/cli-style').config.commitlint`. If this app has migrated off `cli-style` - (`references/style-configs-migration.md`), all three break with "Cannot find module - '@dhis2/cli-style'" once adopted as-is. Don't adopt them for such an app as-is — check for a - fixed branch first, and fall back to a custom step if none exists yet: - - ```bash - gh api repos/dhis2/workflows-platform/branches --jq '.[].name' - ``` - - A `pnpm-no-cli-style` branch fixes all three (built on top of `@pnpm`: `lint.yml` runs the - app's own `pnpm lint` instead of `pnpm d2-style check`, and `lint-commits.yml`/ - `lint-pr-title.yml` point commitlint straight at the app's own `commitlint.config.mjs` - instead of resolving through `cli-style`). If it exists, use it — - `uses: dhis2/workflows-platform/.github/workflows/.yml@pnpm-no-cli-style` — the same - way the `@pnpm` branch itself gets used once confirmed to exist (this is expected to - eventually merge into `@pnpm`/`@v1`, at which point drop the special-cased ref). If it - doesn't show up in that branch list yet (not pushed, or a yarn-based app with no equivalent - branch), fall back to a small custom step: - - ```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 the fixed branch 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 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 @@ -166,9 +147,9 @@ 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'" | That job (`lint.yml`, `lint-commits.yml`, or `lint-pr-title.yml`) depends on `cli-style` internally — see Step 3. Check for the `pnpm-no-cli-style` branch first; fall back to a custom step if it isn't available. | -| `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`. | -| `pnpm install --frozen-lockfile` fails inside a reusable workflow job | The job is still pinned at `@v1` (yarn-based) rather than `@pnpm` — see Step 3. | +| 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 index 4e80168..4fd7d4a 100644 --- a/src/skills/modernise-apps/references/pnpm-migration.md +++ b/src/skills/modernise-apps/references/pnpm-migration.md @@ -223,21 +223,9 @@ In `.github/workflows/*.yml`: `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 the `pnpm` branch instead of the default `@v1` tag — the same approach -`aggregate-data-entry-app#481` used: - -```yaml -uses: dhis2/workflows-platform/.github/workflows/test.yml@pnpm -``` - -pnpm support hasn't been merged into `@v1` yet as of this writing, so `@v1` workflows will -still run `yarn install` internally and fail once `yarn.lock` is gone. Confirm the `pnpm` -branch still exists before using it — it's expected to eventually merge into `@v1`, at which -point this step becomes unnecessary: - -```bash -gh api repos/dhis2/workflows-platform/branches --jq '.[].name' -``` +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 diff --git a/src/skills/modernise-apps/references/style-configs-migration.md b/src/skills/modernise-apps/references/style-configs-migration.md index 5b3c544..0640972 100644 --- a/src/skills/modernise-apps/references/style-configs-migration.md +++ b/src/skills/modernise-apps/references/style-configs-migration.md @@ -4,8 +4,8 @@ This is the step-by-step recipe for moving an existing DHIS2 app off `@dhis2/cli (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 (`references/pnpm-migration.md`) — an app can do either, -both, or neither. If doing both, order doesn't matter; do whichever the user asked for. +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 @@ -298,69 +298,14 @@ The inline `yarn lint`/`pnpm lint` step should be whatever Step 7 landed on — 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 it looks.** Despite the name -suggesting it only checks commit message format, `dhis2/workflows-platform`'s -`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). Check whether the -`pnpm-no-cli-style` branch exists yet (`gh api repos/dhis2/workflows-platform/branches --jq '.[].name'`) -and point at that instead of `@v1`/`@pnpm` if so; otherwise replace it with a custom step -using `@dhis2/config-commitlint` (Step 9) instead of leaving it pointed at the reusable -workflow. See `references/ci-migration.md` Step 3 for the full writeup and the exact -replacement YAML — this doc's job is just to flag it here since Step 10 is what triggers it. +**`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: Bump `@dhis2/ui`, `@dhis2/app-runtime`, and `@dhis2/d2-i18n` - -While touching the app's style/UI tooling, also bring these three platform libraries up to -date — a low-risk win that's easy to bundle into the same PR. 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 13: Verify +## Step 12: Verify ```bash pnpm install @@ -375,13 +320,12 @@ 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@alpha` (or any `@dhis2/config-*@alpha`) 404s | The `style-configs` prerelease hasn't published yet, or `alpha` has since been promoted to `latest` — check `npm view @dhis2/config-commitlint dist-tags` and use whichever tag/version actually resolves. | -| `@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. | +| 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@alpha` (or any `@dhis2/config-*@alpha`) 404s | The `style-configs` prerelease hasn't published yet, or `alpha` has since been promoted to `latest` — check `npm view @dhis2/config-commitlint dist-tags` and use whichever tag/version actually resolves. | From a9ca2d4a1c838f533c30f98763d46b35bb85e201 Mon Sep 17 00:00:00 2001 From: Mozafar Haider Date: Wed, 23 Sep 2026 14:18:16 +0100 Subject: [PATCH 4/5] fix(skill-modernise-apps): update style-configs guidance to published @dhis2/config-* packages @dhis2/config-eslint/prettier/stylelint/lslint/commitlint are now on the latest dist-tag, not alpha-only -- drop the alpha-tag guidance. Package.json examples resolve the actual current version via npm view/dist-tags rather than pinning the literal "latest" tag or a hardcoded version number. Co-Authored-By: Claude Sonnet 5 --- .../references/style-configs-migration.md | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/skills/modernise-apps/references/style-configs-migration.md b/src/skills/modernise-apps/references/style-configs-migration.md index 0640972..77404cb 100644 --- a/src/skills/modernise-apps/references/style-configs-migration.md +++ b/src/skills/modernise-apps/references/style-configs-migration.md @@ -20,10 +20,8 @@ follow this doc directly for those. Fresh apps scaffolded via `pnpm create @dhis 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 still in prerelease, published under the `alpha` -dist-tag (`npm view @dhis2/config-eslint dist-tags` shows the full set — they version in -lockstep from the same `style-configs` repo). Use the `alpha` tag for all of them until -they're promoted to `latest`; check dist-tags again if any install unexpectedly 404s. +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). --- @@ -59,13 +57,16 @@ proxying has a working replacement. Removing it up front just means every interm runs against a broken lint setup for no benefit. ```bash -npm view @dhis2/config-eslint dist-tags # confirms the alpha tag + current version for all five +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": "alpha", - "@dhis2/config-prettier": "alpha", + "@dhis2/config-eslint": "^0.4.0", + "@dhis2/config-prettier": "^0.4.0", "@eslint/compat": "^2.0.0", "eslint": "^9", "prettier": "^3", @@ -74,13 +75,14 @@ npm view @dhis2/config-eslint dist-tags # confirms the alpha tag + current ver } ``` -Add these two conditionally, only if Step 1 found them configured: +(`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": "alpha", + "@dhis2/config-stylelint": "^0.4.0", "stylelint": "^16", - "@dhis2/config-lslint": "alpha", + "@dhis2/config-lslint": "^0.4.0", "@ls-lint/ls-lint": "^2" } ``` @@ -228,17 +230,20 @@ the local hook was providing faster, pre-push feedback than CI. Use the shared setup: ```bash -npm view @dhis2/config-commitlint@alpha version # check current alpha version +npm view @dhis2/config-commitlint version npm view @commitlint/cli version ``` ```json "devDependencies": { "@commitlint/cli": "^21.2.2", - "@dhis2/config-commitlint": "alpha" + "@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' @@ -320,12 +325,12 @@ 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@alpha` (or any `@dhis2/config-*@alpha`) 404s | The `style-configs` prerelease hasn't published yet, or `alpha` has since been promoted to `latest` — check `npm view @dhis2/config-commitlint dist-tags` and use whichever tag/version actually resolves. | +| 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. | From 80f054720ce84170be3e75b60322267245a176db Mon Sep 17 00:00:00 2001 From: Mozafar Haider Date: Fri, 9 Oct 2026 12:49:49 +0100 Subject: [PATCH 5/5] feat(skill-modernise-apps): add App Hub continuous delivery task Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- README.md | 9 +- src/skills/modernise-apps/SKILL.md | 44 +++-- .../references/apphub-release.md | 167 ++++++++++++++++++ 4 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 src/skills/modernise-apps/references/apphub-release.md 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 bd4a337..1401ea3 100644 --- a/README.md +++ b/README.md @@ -67,8 +67,8 @@ 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 five things -an app can need any combination of, or none: +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 @@ -92,6 +92,11 @@ an app can need any combination of, or none: - **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 diff --git a/src/skills/modernise-apps/SKILL.md b/src/skills/modernise-apps/SKILL.md index fcc2130..c19836b 100644 --- a/src/skills/modernise-apps/SKILL.md +++ b/src/skills/modernise-apps/SKILL.md @@ -15,6 +15,9 @@ description: > 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. --- @@ -22,7 +25,7 @@ description: > # Modernising DHIS2 Apps You are helping a developer bring an existing DHIS2 app up to date. "Modernising" currently -covers four independent tasks — an app can need any combination of them, or none: +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 @@ -44,18 +47,25 @@ covers four independent tasks — an app can need any combination of them, or no 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 | +| 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? @@ -68,6 +78,7 @@ covers four independent tasks — an app can need any combination of them, or no | 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. @@ -134,6 +145,18 @@ in the future — this table is deliberately structured to grow. `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 @@ -147,3 +170,4 @@ in the future — this table is deliberately structured to grow. | 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/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. |