Repository navigation
chore(GAT-9528): React Router build scaffolding, ESLint 9 and CI wiring - #137
Merged
Merged
Conversation
Lands the React Router v7 build toolchain on the GAT-9016 integration branch
so that everything above it can have green CI: package.json + lockfile,
tsconfig, vite and react-router config, Dockerfiles, public assets, a minimal
app/root.tsx shell, an empty app/routes.ts, an ESLint 9 flat config, and CI
wiring.
React Router cannot build without a root module, and routes.ts hard-fails on
any entry whose file is missing, so the toolchain has to land as one floor
before any application code. The root.tsx here is a deliberate stub — Layout,
Outlet and ErrorBoundary only, no loader, no middleware, no ~/lib import. The
real one arrives with the UI shell: it dynamically imports auth.server,
retention.server and schema.server, which Vite resolves statically at build
time, so it cannot land before the background-jobs PR.
Three fixes beyond the toolchain itself:
- npm start bound port 3000, not 3001. react-router-serve defaults to 3000 and
does not read .env at runtime (dotenv is loaded by vite.config.ts, which only
runs at build/dev time). The chart routes targetPort 3001 and the old Express
service defaulted to 3001, so shipping this as-is would have deployed a
container listening on the wrong port. The start script now defaults PORT to
3001, still overridable from the environment.
- env.example on the POC branch points SCHEMA_LOCATION at HDRUK/schemata; dev
correctly uses HDRUK/schemata-2. CI copies this file, so the POC value would
have pointed schema loading at the wrong repo. Kept dev's value and carried
over the POC additions.
- The Express service called require('dotenv').config() at startup
(src/app.js:15), so .env was read by the running server. In the rewrite
dotenv is only invoked from vite.config.ts, which does not run under
react-router-serve, so a built server saw none of its configuration: with
SCHEMA_LOCATION unset the schema loader falls back to a relative path, tries
to open /available.json, and every schema-backed endpoint 500s. npm start now
uses node --env-file-if-exists, so a deployment with no .env file
(Kubernetes, where config arrives as real environment variables) is
unaffected. This does not remove the need to populate deployment.yaml, which
currently sets no environment variables at all.
CI gains Typecheck, Lint and Build steps, and runs the suite against a started
server. Its branch filters match feat/GAT-* so that every PR in the migration
stack is covered regardless of which ticket it lands under.
The harvested fixture corpus, its replay harness and the old-vs-new differences writeup live in an untracked regression-suite/ directory rather than in the repo. The corpus is roughly a megabyte of captured production responses and nothing in the build or the test suite needs it, so committing it taxed every clone, every diff and every review for an artefact used by one workflow.
|
🎉 Great job! Your PR title follows the correct format. 🚀 |
calmacx
added this pull request to stack #139
September 22, 2026 09:17
Three additions were made to give the stacked migration PRs any CI at all. Two of them should never have been committed, and they appeared in two places each: the push/pull_request trigger lists and the expression that picks the GitHub environment. feature/GAT-9016-migration is gone entirely. That integration branch was retired and deleted on 2026-09-17; `git ls-remote --heads origin feature/GAT-9016-migration` returns nothing. Left in, dev and main would carry config naming a branch that does not exist, in perpetuity. feat/GAT-* is gone from the push list. Push and pull_request both fire for a branch with an open PR, so every commit ran the suite twice against the same SHA — sha e0f2c1d has one push run and two pull_request runs on record. The pull_request trigger already covers every branch that has a PR open. feat/GAT-* stays in pull_request, and in the environment expression, because both are load-bearing while this stack exists. A stacked PR is matched on its *base* branch, so without it every PR based on another feat/GAT-* branch gets zero CI and only the title check — the exact bug these entries were added to fix. It is still temporary. Once the stack has merged to dev there are no feat/GAT-* bases left and it should go, returning the lists to dev and main. That removal belongs in a PR targeting dev after the stack lands, not in the stack itself, where it would pull the trigger config out from under the PRs still depending on it.
tsconfig.json sets include "**/*" with no exclude, so tsc walks the whole working directory rather than the sources. Anything a developer has on disk becomes part of the typecheck program even when git ignores it. This is not hypothetical. A local checkout of the production regression suite in regression-suite/ breaks `npm run typecheck` outright: regression-suite/overlay/tests/regression-replay.test.ts(9,26): error TS2307: Cannot find module './helpers' That file is written to sit at tests/ inside this repo and resolve ./helpers there; sitting in an untracked overlay it cannot. The same applies to the ~1300-file data/ runtime cache, which is JSON that resolveJsonModule will happily pull in. CI never sees any of this — it starts from a clean checkout — so the failure only ever appears on a developer machine, which is the worst place for it. eslint.config.js already ignores the same directories.
equinoxmatt
approved these changes
Sep 22, 2026
This branch was successfully deployed
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
2 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.
What
Lands the React Router v7 build toolchain on the GAT-9016 integration branch:
package.json+ lockfile, tsconfig, Vite and React Router config, Dockerfiles, public assets, a minimalapp/root.tsxshell, an emptyapp/routes.ts, an ESLint 9 flat config, and CI wiring.Why
React Router cannot build without a root module, and
routes.tshard-fails on any entry whose file is missing, so the toolchain has to land as one floor before any application code in the migration stack.Testing
npm run typecheck,npm run lintandnpm run buildall exit 0, and CI now runs all three plus the suite against a started server. ESLint was checked to be non-vacuous with a probe file (const x: any→ correctly flagged).Notes
app/root.tsxis a deliberate stub —Layout,OutletandErrorBoundaryonly, no loader, no middleware, no~/libimport. The real one dynamically importsauth.server,retention.serverandschema.server, which Vite resolves statically at build time, so it cannot land until those exist.npm startbound port 3000 while the chart routestargetPort: 3001;env.exampleon the POC branch pointedSCHEMA_LOCATIONatHDRUK/schematarather thanschemata-2; and.envwas not read at runtime at all, because dotenv only ran fromvite.config.ts, whichreact-router-servenever invokes. The last one would have 500'd every schema-backed endpoint.deployment.yamlstill sets no environment variables. That is a separate deploy-config task and it blocks promotion, not this PR.Recreated 2026-09-21 from #132. Base moved from the regression-fixtures branch to
dev— that PR is closed and its corpus now lives outside the repo. Also adds/regression-suite/to.gitignore.