trails-cool/trails

Watch

Fork 0

feat: migrate to React Router v8 #79

Merged

ullrich merged 1 commit from feat/react-router-v8 into main

2026-08-18 18:22:22 +00:00

ullrich commented

2026-08-18 16:17:15 +00:00

Owner

Replaces #67, #68, #69 and #71. Closes them in favour of one PR, because none of them could ever pass individually.

Why four PRs were all red

react-router ships as one release train — the runtime and the dev/node/serve adapters are published in lockstep and are not compatible across a major. Renovate had them ungrouped, so it opened four v8 PRs, each bumping one catalog entry while the others stayed on ^7.18.2:

PR	bumped	left behind
#71	`react-router ^8.0.0`	dev, node, serve at `^7.18.2`
#67	`@react-router/dev`	the rest at v7
#68	`@react-router/node`	the rest at v7
#69	`@react-router/serve`	the rest at v7

Every one was a v8 runtime against v7 tooling. #71's failure — TS2305: Module 'react-router' has no exported member 'AppLoadContext' — reads like a migration bug but was really a split version set. Same failure shape as the Playwright image in #62: a set that must move together, split across PRs that are each green alone.

All four catalog entries now move to ^8.3.0 in one commit, and renovate.json5 groups them so this cannot recur.

What v8 actually required

Most of v8 does not apply here: no react-router-dom imports anywhere, Vite is already 8.2.1 (v8 needs ≥7), no Cloudflare dev proxy, no architect adapter, no splitRouteModules usage.

AppLoadContext removed. The entry-server hook is now loadContext: RouterContextProvider, verified against react-router@8.3.0's actual HandleDocumentRequestFunction:

(request, responseStatusCode, responseHeaders,
 context: EntryContext, loadContext: RouterContextProvider): Promise<Response> | Response

Both apps' entry.server.tsx are byte-identical and name the parameter _loadContext — deliberately unused — so this is a type annotation swap.

The data arg in meta APIs removed. v8's MetaArgs exposes loaderData, params, location, matches. Seven route modules used it. A grep for meta.*data found only two; the other five destructure and rename ({ data: loaderData }), so pnpm typecheck is what found them — worth noting as the reason typecheck is the real gate on this PR rather than a formality.

future.v8_middleware no longer exists, middleware being the default. The journal had opted in for ActivityPub content negotiation on /users/:username and /activities/:id; under satisfies Config, keeping a removed key is a type error. Both middleware functions destructure only { request }, and nothing in either app reads a loader/action context — so the RouterContextProvider type change is the no-op the original config comment claimed. Typegen confirms it: +future.ts is no longer emitted and MiddlewareFunction is generated unconditionally.

engines.node was ">=20"; v8 requires >=22.22.0. Nothing actually ran on 20 — CI uses node 24, both runtime images are node:26-slim — so this only stops the declared floor understating the requirement.

The react catalog floor also moves ^19.2.5 → ^19.2.7, v8's minimum. It already resolved to 19.2.7, so nothing changes except that the constraint stops lying.

The Renovate rule placement is deliberate

It sits after the majors -> groupName: null rule, unlike testing tooling above it. Later rules win, so a rule placed before that one loses its groupName on a major. That is intentional for test tooling — a major should be reviewed on its own — and exactly wrong here: a react-router major is the case that most needs the four to arrive together. Had this rule gone before it, v8 would have re-split into four PRs and reproduced the bug the rule exists to prevent. addLabels: ["major"] still applies, since rules merge field-wise.

Testing

Verified locally on node 24.16.0 (a stock local asdf may select 22.15.0, which is now below the declared floor):

pnpm typecheck 14/14 — the gate that found five of the seven meta call sites
pnpm lint 14/14
pnpm test 12/12
pnpm build 2/2
Both apps resolve react-router and all three adapters at 8.3.0; the lockfile holds no v7 copy
The ActivityPub middleware is intact on both routes and still typed via generated Route.MiddlewareFunction[]

E2E is left to CI, which boots the app through react-router serve — the path most likely to surface an ESM-only regression, and the one thing I cannot exercise locally without Postgres and BRouter.

Replaces #67, #68, #69 and #71. Closes them in favour of one PR, because none of them could ever pass individually. ## Why four PRs were all red `react-router` ships as one release train — the runtime and the `dev`/`node`/`serve` adapters are published in lockstep and are not compatible across a major. Renovate had them ungrouped, so it opened four v8 PRs, each bumping one catalog entry while the others stayed on `^7.18.2`: | PR | bumped | left behind | |---|---|---| | #71 | `react-router ^8.0.0` | dev, node, serve at `^7.18.2` | | #67 | `@react-router/dev` | the rest at v7 | | #68 | `@react-router/node` | the rest at v7 | | #69 | `@react-router/serve` | the rest at v7 | Every one was a v8 runtime against v7 tooling. #71's failure — `TS2305: Module 'react-router' has no exported member 'AppLoadContext'` — reads like a migration bug but was really a split version set. Same failure shape as the Playwright image in #62: a set that must move together, split across PRs that are each green alone. All four catalog entries now move to `^8.3.0` in one commit, and `renovate.json5` groups them so this cannot recur. ## What v8 actually required Most of v8 does not apply here: no `react-router-dom` imports anywhere, Vite is already 8.2.1 (v8 needs ≥7), no Cloudflare dev proxy, no architect adapter, no `splitRouteModules` usage. **`AppLoadContext` removed.** The entry-server hook is now `loadContext: RouterContextProvider`, verified against `react-router@8.3.0`'s actual `HandleDocumentRequestFunction`: ```ts (request, responseStatusCode, responseHeaders, context: EntryContext, loadContext: RouterContextProvider): Promise<Response> | Response ``` Both apps' `entry.server.tsx` are byte-identical and name the parameter `_loadContext` — deliberately unused — so this is a type annotation swap. **The `data` arg in meta APIs removed.** v8's `MetaArgs` exposes `loaderData`, `params`, `location`, `matches`. **Seven** route modules used it. A grep for `meta.*data` found only two; the other five destructure and rename (`{ data: loaderData }`), so `pnpm typecheck` is what found them — worth noting as the reason typecheck is the real gate on this PR rather than a formality. **`future.v8_middleware` no longer exists**, middleware being the default. The journal had opted in for ActivityPub content negotiation on `/users/:username` and `/activities/:id`; under `satisfies Config`, keeping a removed key is a type error. Both middleware functions destructure only `{ request }`, and nothing in either app reads a loader/action `context` — so the `RouterContextProvider` type change is the no-op the original config comment claimed. Typegen confirms it: `+future.ts` is no longer emitted and `MiddlewareFunction` is generated unconditionally. **`engines.node`** was `">=20"`; v8 requires `>=22.22.0`. Nothing actually ran on 20 — CI uses node 24, both runtime images are `node:26-slim` — so this only stops the declared floor understating the requirement. The `react` catalog floor also moves `^19.2.5` → `^19.2.7`, v8's minimum. It already resolved to 19.2.7, so nothing changes except that the constraint stops lying. ## The Renovate rule placement is deliberate It sits **after** the `majors -> groupName: null` rule, unlike `testing tooling` above it. Later rules win, so a rule placed before that one loses its groupName on a major. That is intentional for test tooling — a major should be reviewed on its own — and exactly wrong here: a react-router major is the case that *most* needs the four to arrive together. Had this rule gone before it, v8 would have re-split into four PRs and reproduced the bug the rule exists to prevent. `addLabels: ["major"]` still applies, since rules merge field-wise. ## Testing Verified locally on node 24.16.0 (a stock local asdf may select 22.15.0, which is now below the declared floor): - `pnpm typecheck` 14/14 — the gate that found five of the seven meta call sites - `pnpm lint` 14/14 - `pnpm test` 12/12 - `pnpm build` 2/2 - Both apps resolve `react-router` and all three adapters at 8.3.0; the lockfile holds no v7 copy - The ActivityPub middleware is intact on both routes and still typed via generated `Route.MiddlewareFunction[]` E2E is left to CI, which boots the app through `react-router serve` — the path most likely to surface an ESM-only regression, and the one thing I cannot exercise locally without Postgres and BRouter.

ullrich added 1 commit

2026-08-18 16:17:16 +00:00

feat: migrate to React Router v8

CD Staging / Build & Push Docker Images (pull_request) Has been skipped

Details

CD Staging / Build & Push Docker Images-1 (pull_request) Has been skipped

Details

CD Staging / Deploy Staging (pull_request) Has been skipped

Details

CD Staging / Deploy PR Preview (pull_request) Has been skipped

Details

CI / Security Scan (pull_request) Successful in 2m24s

Details

CI / Dockerfile Package Check (pull_request) Successful in 26s

Details

CI / Checks (pull_request) Successful in 10m17s

Details

CI / Visual Tests (pull_request) Successful in 4m37s

Details

CI / Journal Image Smoke Test (pull_request) Successful in 27m38s

Details

CI / E2E Tests (pull_request) Successful in 9m21s

Details

Cancel superseded CI / Cancel in-flight CI (pull_request) Successful in 20s

Details

CD Staging / Tear Down PR Preview (pull_request) Successful in 43s

Details

223cbe5ce4

Replaces #67, #68, #69 and #71, which could never pass individually.

react-router ships as one release train: the runtime and the dev/node/serve
adapters are published in lockstep and are not compatible across a major.
Renovate had them ungrouped, so it opened four separate v8 PRs — each bumping
one catalog entry while the others stayed on ^7.18.2. Every one was therefore a
v8 runtime against v7 tooling. #71 failed with "Module 'react-router' has no
exported member 'AppLoadContext'", which reads like a migration bug but was
really a split version set. renovate.json5 now groups the four.

All four catalog entries go to ^8.3.0 together. The migration itself is small,
because most of v8's breaking changes do not apply here: no `react-router-dom`
imports anywhere, Vite is already 8.2.1 (v8 needs >=7), no Cloudflare dev
proxy, no architect adapter, no splitRouteModules usage.

What v8 actually required:

  AppLoadContext is gone. The entry-server hook is now
  `loadContext: RouterContextProvider` (verified against react-router@8.3.0's
  HandleDocumentRequestFunction). Both apps' entry.server.tsx are byte-identical
  and name the parameter `_loadContext` — deliberately unused — so this is a
  type annotation swap and nothing more.

  The deprecated `data` arg in meta APIs is gone; v8's MetaArgs exposes
  loaderData, params, location and matches. Seven route modules used it. A grep
  for `meta.*data` found only two of them — the other five destructure and
  rename (`{ data: loaderData }`), so `pnpm typecheck` is what actually found
  them. Five became `{ loaderData }` (the local name was already right) and two
  became `{ loaderData: d }`.

  future.v8_middleware no longer exists as a flag, middleware being the default.
  The journal had opted in for ActivityPub content negotiation on
  /users/:username and /activities/:id; under `satisfies Config` keeping a
  removed key is a type error. The two middleware functions destructure only
  `{ request }`, and nothing in either app reads a loader/action `context`, so
  the RouterContextProvider type change is the no-op the original comment
  claimed. Typegen confirms it: `+future.ts` is no longer emitted and
  MiddlewareFunction is now generated unconditionally.

  engines.node was ">=20"; v8 requires >=22.22.0. Nothing actually ran on 20 —
  CI uses node 24 and both runtime images are node:26-slim — so this only stops
  the declared floor understating the requirement. Note it is now above the
  22.15.0 that a stock local asdf may select; this was verified on 24.16.0.

The react catalog floor also moves ^19.2.5 -> ^19.2.7, v8's minimum. It already
resolved to 19.2.7, so nothing changes but the constraint stops lying.

The renovate rule is placed AFTER the majors -> groupName: null rule, unlike
"testing tooling" above it. Later rules win, so a rule placed before that one
loses its groupName on a major — deliberate for test tooling, and exactly wrong
here: a major is the case that most needs the four to arrive together. Had it
gone before, v8 would have re-split into four PRs and reproduced the bug the
rule exists to prevent.

Verified locally on node 24.16.0: typecheck 14/14, lint 14/14, test 12/12,
build 2/2. Both apps resolve react-router and all three adapters at 8.3.0, and
the lockfile holds no v7 copy. E2E is left to CI, which boots the app through
`react-router serve` — the path most likely to surface an ESM-only regression.