feat: support writing components in TypeScript, with a placeholder OuTree - #1764
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The package-local jest.config.js only fixed `yarn workspace @dhis2-ui/ou-tree test`; the root `yarn test` (used by CI) still failed to resolve `.js` sibling imports from `.ts`/`.tsx` files. Add the moduleNameMapper (re-spreading cli-app-scripts' default mocks) to jest.config.shared.js and jest.config.js instead, and restore ou-tree's conventional test script now that its own jest.config.js is no longer needed.
- enable noUnusedLocals/noUnusedParameters in ou-tree tsconfig and correct the eslintrc comment that falsely claimed TS covers no-unused-vars on its own - fix asymmetric excludedFiles for import/no-extraneous-dependencies - fix extensionAlias ordering in storybook webpack config so it stays inert for node_modules dependencies shipping both .ts and .js - pin typescript to ~5.5.4 to stay within @typescript-eslint/parser's supported range - export OuTreeProps as a type from the ou-tree package entry - correct ou-tree CLAUDE.md's stale reference to a deleted package-local jest.config.js Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The site config set `url` to a sub-path, which Docusaurus 2.4.3 rejects
outright ("The url is not supposed to contain a sub-path"), and sidebars.js
still referenced a recipe deleted in ed10d8a. Both broke `yarn build` on
master, independently of any component work.
`baseUrl` is deliberately left as '/' so Netlify keeps publishing the built
`dist` at a preview root.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TypeScript sources named their compiled output ('./ou-tree.js' for a file on
disk called ou-tree.tsx). TypeScript follows that mapping but three other
resolvers do not, and each needed a workaround: import/no-unresolved disabled,
a .js-stripping moduleNameMapper in BOTH shared jest configs, and
resolve.extensionAlias in the storybook webpack config.
Extensionless imports need none of them — every one of those resolvers already
tries .ts/.tsx. This reverts both jest configs to their master content, so the
branch no longer alters module resolution for the other 125 test suites, and
drops the extensionAlias hunk.
import/extensions is set to 'never' for .ts/.tsx rather than disabled, so the
convention is enforced rather than merely permitted, and an import/resolver
extension list restores import/no-unresolved for TypeScript instead of leaving
it off.
storybook's webpack config gains .ts/.tsx in resolve.extensions: CRA strips them
unless a tsconfig.json exists at the project root, and this repo deliberately
has none.
Trade-off: build/es then carries extensionless specifiers, which Node's native
ESM loader rejects. Inert for DHIS2 apps, which bundle, and exports.require
routes Node to the CJS build. Recorded in the spec and the package CLAUDE.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`yarn build` writes the docusaurus site to dist/ and its cache to docs/.docusaurus/. Neither was in .prettierignore, so running `yarn build` followed by `yarn lint` failed on generated files nobody wrote. Both are already gitignored; prettier's md/json pattern does not consult gitignore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
generate-api-docs.js only globbed .js sources and assumed a src/index.js entry point, so a TypeScript component was skipped entirely and would have vanished from the docs the moment one shipped. react-docgen 5.4.3 already reads TypeScript prop interfaces, so this needs no dependency change: widen the source glob, resolve the entry point across extensions, parse it with the typescript plugin, and fall back to a prop's `tsType` when it has no prop-types `type` (otherwise every TypeScript prop renders as 'undefined'). API.md files are committed alongside the component, as in 9ce851e. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Babel strips types without checking them, so nothing in CI verified them — a type error could reach master with lint, tests and build all green. scripts/typecheck.js discovers packages declaring a `typecheck` script the same way build.js and setup.js discover packages, so a future TypeScript package is covered as soon as it adds one, with nothing to register here. The step runs in the existing lint job rather than a new one: a separate job costs another checkout and install for a check that takes about a second. The job name is unchanged so required-status-check settings keep matching. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other component package has one; ou-tree was the only one without. Notes that the package is an unpublished placeholder, so the usual link to a documentation page would be dead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
API.md was added to .gitignore in ecd2e84 with the intent of generating these on CI instead of tracking them. That intent never took: all 52 API.md files are tracked, and they have been updated in feature commits ever since — 9ce851e added one nine months later. The only effect the rule still had was on NEW packages, whose API.md was silently ignored while every existing component kept theirs, so adding a component needed `git add -f` to stay consistent. Removing the rule makes zero files newly untracked. They stay in .prettierignore: generated output should not be style-gated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build-world.sh had no `set -e` and no `&&`, so its exit status was just the last command's. Every step was non-blocking, including `yarn build:lib` — and the publish job gates on this script, so a broken or missing build/ could reach semantic-release and be published. That also masked two real failures: docusaurus and the storybook build were both broken on master without anyone noticing. Docs and Storybook stay deliberately non-blocking — shipping component fixes matters more than the documentation site building — but they now emit a GitHub Actions warning so the failure is visible in the checks UI instead of buried in a log. The library build and API docs now fail hard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
🚀 Deployed on https://pr-1764--dhis2-ui.netlify.app |
Sonar: node: protocol imports, optional chaining, Boolean as a predicate, and the catch parameter name in scripts/typecheck.js. Comments now describe what the code does rather than how it came to be written that way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o root The styled-jsx and jest-dom ambient declarations apply to any TypeScript package, not just ou-tree, so they move to a root typings/ directory. The compiler options move to tsconfig.base.json for the same reason — a second package extending it cannot silently diverge on settings like isolatedModules, which Babel's file-by-file compilation depends on. include/exclude and the emit paths stay per-package: TypeScript resolves relative paths against the file they are written in, so they cannot be shared. The base config is deliberately not named tsconfig.json — react-scripts treats a root tsconfig.json as 'this is a TypeScript app' and changes the Storybook webpack build accordingly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts the url and sidebars changes. Two reasons: - There is no clearly correct fix. Docusaurus rejects a sub-path in `url`, but the sub-path was what produced correct canonical and sitemap URLs for the developer portal. Moving it to `baseUrl` would prefix every asset path and break the Netlify PR preview, which is served at a domain root and is the only consumer of this build. Picking either side trades one kind of correctness for another, and that is a call for whoever owns the docs. - It was outside the scope of this PR. The docs build failing is now visible rather than silent: build-world.sh reports it as a GitHub Actions warning and the pipeline stays green, which is the intended behaviour for a non-release-blocking step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nsions patch The entry-point glob covered js, ts and tsx but not jsx, which was an oversight rather than a decision — a package entry can be any of the four. The comment on resolve.extensions was wrong. CRA looks for a tsconfig.json in the directory the build runs from, not at the repo root, so the absence of a root tsconfig has nothing to do with it: Storybook builds from storybook/, which is not a TypeScript project. Adding a tsconfig there would also switch on ForkTsCheckerWebpackPlugin and type-check the whole project on every Storybook build, which is why the extensions are added directly instead. The note in tsconfig.base.json repeated the same wrong reasoning for its name; it is named .base.json because it is only ever extended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test files import @testing-library/jest-dom directly, which is the conventional pattern and removes the need for an ambient declaration for it. The remaining comments say what the code does rather than narrating the constraint that led to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
no-undef and no-unused-vars were switched off for .ts/.tsx. Only two
false positives actually justified that, both from no-undef: the ambient
namespaces JSX and NodeJS read as undefined names. Declaring them as globals
fixes those and keeps the rule reporting genuinely undefined identifiers, so
neither rule is disabled any more.
Three other base rules do misfire on TypeScript, but only on overload
signatures and declare blocks, which nothing in the repo uses yet:
no-unused-vars on overload parameters, no-redeclare on function overloads,
and no-dupe-class-members on class method overloads. Left enabled; whoever
writes the first overload can decide.
File globs now use brace expansion consistently rather than mixing separate
entries with partial {ts,tsx} braces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kabaros
approved these changes
Sep 14, 2026
kabaros
left a comment
kabaros
left a comment
Collaborator
There was a problem hiding this comment.
This look great - I have two comments:
- I don't understand why components still need their own tsconfig?
- Instead of d2-style, it'd be great if we move to the shared configs .. that's not necessarily on this PR at all, but I thought it might make some of the boilerplate easier to manage, so maybe worth a quick try.
Type-checking was modelled on the build: a discovery script fanning out to a per-package typecheck script. It is a form of linting, and linting in this repo is global — one command at the root, no per-package script. It now works the same way. tsconfig.base.json becomes the root tsconfig.json and gains the include globs, so `yarn typecheck` is a single `tsc --noEmit` over every package and editors resolve it for any .ts/.tsx file. scripts/typecheck.js and the per-package tsconfig.json and typecheck script are gone. Declaration emit stays per package, because each package publishes its own types and one tsc run writes to one outDir — the same reason d2.config.js is per package. The shared settings move to tsconfig.build.base.json, leaving each package a three-line tsconfig.build.json. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…kage Declaration emit is per package because each package publishes its own types, but expressing that as a tsconfig.build.json in every package restates the compiler options next to the ones in the root tsconfig.json, where the two can drift. scripts/build-types.js reads the options from that config and overrides only rootDir and outDir, so there is one statement of how TypeScript packages build rather than one per package. tsc cannot do this from the command line: include is a config-file field with no CLI equivalent, and --project refuses to be combined with file arguments (TS5042). The compiler API has no such restriction. Output is byte-identical to what tsconfig.build.json produced. Packages now carry no TypeScript config at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removes error handling for malformed-tsconfig cases that fail loudly anyway, and a success log. Behaviour is unchanged and the emitted declarations are byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
3 tasks
kabaros
approved these changes
Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
In the not-too-distant future I will be working on a new organisation unit component. This new component would live alongside the existing
OrganisationUnitTree, because I do not expect it to be a drop-in replacement, but something that apps can adopt/migrate to gradually. I'd like to call the new componentOuTreeand I would like to write it in TypeScript.This current PR is a prelude to the work on the new
OuTreecomponent and makes it possible to write UI components in the repo in TypeScript. You will find a new component in this PR already but it is simply a stub to verify that the TypeScript setup works correctly for tests, stories, etc. This package is currently set to private so nothing will get published.Description
Partial TypeScript support was present already:
cli-app-scriptsalready included@babel/preset-typescript.tsxd2-stylealready globbedts,tsxWhat was missing or providing friction was:
.js./ou-tree), the inverse of the explicit.jsextensions the JavaScript packages require, so the ESLint rule is flipped for.ts/.tsxonlyI tried to make as much as possible global to the repo:
tsconfig.jsonis the TypeScript project for the whole repo, so components need no tsconfig of their own. Editors resolve it for any.ts/.tsxfile toostyled-jsxambient declaration is in the project leveltypings/diryarn typecheckis a singletsc --noEmitover every package. Type-checking is a form of linting, so it works the way linting already does here: one command at the root, no per-package scriptscripts/build-types.jsemits the declarations. Emit has to happen per package, since each publishes its own types and onetscrun writes to oneoutDir— but the script reads the compiler options from the roottsconfig.json, so they can't drift from the ones used to type-checkgenerate-api-docs.jsnow supports packages without asrc/index.jsand documents TypeScript components (this needed no dependency change since react-docgen 5.4.3 already reads TS prop interfaces).Each component will still need a few changes to enable TypeScript, but no new files:
&& node ../../scripts/build-types.jspackage.jsontype fields —typesandexports.typespoint at./build/types/index.d.tsinstead of the hand-writtentypes/dir, andfilesdrops itstypesentryd2.config.jsentry point becomessrc/index.tsInstructions for Claude
I also added some instructions for Claude in
components/ou-tree/CLAUDE.md. I opted to keep this local to the component for 2 reasons:Fixes for indirectly related issues
yarn lintfailed onmaster: generated build output wasn't excluded from Prettier, and a.gitignorerule forAPI.mdcontradicted the fact that all 52 are tracked and updated in feature commits.build-world.shhad noset -e, so its exit status was only the last command's. Every step was non-blocking, includingyarn build:lib, and the publish job gates on it — so a broken or missingbuild/could have reachedsemantic-release. Docs and Storybook stay non-blocking on purpose, since shipping component fixes matters more than the docs site building, but they now emit a warning visible in the checks UI. The library build does fail hard, which makes sense: we don't want to ship a broken or missingbuild/😬 .Not fixed
The docs site is broken on
mastertoo — Docusaurus rejects theurlin its config, and a sidebar entry points at a deleted recipe. I've left these alone, because there is no clearly correct fix: the invalid sub-path inurlis exactly what produced correct canonical URLs for the developer portal, and moving it tobaseUrlwould prefix every asset path and break the Netlify PR preview, which is the only thing that consumes this build. (The new behaviour ofbuild-world.shis visible on this PR — the docs failure is reported as a warning but the pipeline stays green).Checklist