Skip to content

Use React Router framework mode + SPA mode - #3400

Open
david-crespo wants to merge 5 commits into
mainfrom
router-framework
Open

david-crespo wants to merge 5 commits into
mainfrom
router-framework

Conversation

@david-crespo

@david-crespo david-crespo commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

https://reactrouter.com/how-to/spa

The behavior and basic architecture of the console is unchanged: we're still a fully static SPA served by Nexus. This PR just changes how our route files are hooked up to the router. The point of this is to

  1. fix hot reloading, and
  2. get more type safety around stuff like path params through RR framework mode's route typegen.

Hot reloading was broken because our route modules export clientLoader and handle next to the component. React Fast Refresh only accepts modules whose exports are all components, so editing any page triggered a full reload. The framework's Vite plugin knows about route exports and hot-updates them itself (RR docs on supported exports). We could have fixed hot reloading without using framework mode, but it would have be just as long of a PR and we wouldn't get the typegen.

Changes

  • Routes move from app/routes.tsx (JSX <Route> tree with lazy + convert) to app/routes.ts using the framework route helpers. Routes that had an inline handle, an index redirect, or a reused component with a different crumb now point at small modules in app/routes/, since in framework mode a route's handle and loader have to come from the module.
  • Loaders take typed params. Every clientLoader uses the generated Route.ClientLoaderArgs and destructures the keys it needs instead of calling a getXSelector helper. We destructure these params on purpose in order to pass on only what we need: params also holds child-route params, and two query builders (routeList, instanceNetworkInterfaceList) pass their argument through as the API query, so passing params whole would add e.g. route=x to the request and the cache key and break the prefetch. AGENTS.md has a line about this.
  • pb helpers use typed href(), so paths and params are checked against the route config. We could in theory ditch pb altogether and just inline the paths, but I think it's still worth keeping.
  • index.html is gone. root.tsx renders the document, and npm run build prerenders it at build time. This requires binding a localhost port during the build (noted in AGENTS.md for sandboxed agents).
  • CSP: Nexus only allows external scripts, but the framework emits inline bootstrap scripts. A buildEnd step (tools/externalize-spa-scripts.ts) moves each one into a hashed file under assets/, and entry.client.tsx empties those tags back out after they run so hydration sees the DOM shape React Router expects.
  • Build output moves from dist/ to build/client/. React Router always writes the client build to <buildDirectory>/client, so CI, Vercel, npm run preview, and the docs now point there. The tarball is built from inside that directory, so the layout Nexus sees is unchanged. Sourcemaps still ship, same as main.
  • routeDiscovery: 'initial': by default, framework mode uses lazy route discovery. The initial page load gets only the route manifest entries for the current URL, and the client fetches the rest from a /__manifest server endpoint as you navigate. That's an optimization for apps with a lot of routes. We don't have that many, and Nexus doesn't serve that endpoint anyway, so initial mode puts the whole manifest in the initial load (see the bundle comparison below for its size).
  • Tailwind's source scan is limited to app/ (plus the design system), because watched Markdown elsewhere in the repo was triggering full reloads. This drops only .shadow, .!filter, and .[commit:string], which were picked up from words in flake.nix, an e2e test, and a deno tool. This could be done separately from this PR but it was relevant here because it reduces the number events that trigger reloads.
  • Deleted the loader export in OxqlMetric.tsx. It was dead on main too: added in Instance Metrics using OxQL #2654, but nothing ever imported it.
  • Fixed the API_MODE error message, which said the default is msw when it's nexus.

How we checked it

Opus 5.5 did the following during review:

  • Routes: compared app/routes.ts against main's routes.tsx route by route: nesting, index redirects, crumbs, IDs on reused modules, and the old element={null} routes. Everything matches.
  • Pages and forms: compared every page and form against main. No exports were lost, moved validators are byte-identical, and every re-exported loader gets the params it needs where it's mounted.
  • Path builder: checked every URL. They're all identical to main and the snapshot is unchanged. path-builder.spec.ts now builds its routes from the real framework config and module handles instead of the old JSX tree.
  • Prod build under CSP: served the MSW preview build with the same CSP header Nexus sends and loaded deep links in headless Chromium. No CSP violations, hydration errors, or page errors. Index redirects, breadcrumbs, page titles, client-side navigation, and the 404 page all work.
  • HMR: edited ProjectsPage.tsx, which exports clientLoader and handle, with the create form open. It hot-updated without a full reload, and text typed into the form survived.
  • Theme test: the pre-hydration tests in theme.e2e.ts check that theme-init.js sets the theme before React loads. They do this by blocking the app's entry script, which is now entry.client.tsx instead of main.tsx. On main, the tests confirmed the block worked by asserting that #root was empty, but the framework's document has no #root. Now they wait for the blocked request to fail instead. If the entry file is renamed again, the block stops matching and the tests fail, rather than letting React load and passing without testing anything.

Bundle compared with main (Nexus build)

main this PR
Files 532 832
JS files 258 419
JS raw 2.53 MB 2.83 MB
JS gzipped 844 KB 903 KB
Initial load 15 files, 219 KB gz 41 files, 241 KB gz
  • Most of the extra files come from route-module splitting, which is on by default in RR 8.4: 87 *-client-loader-* chunks and 24 *-main-* chunks, so a route's loader can start fetching before its component code arrives. About 34 more are the tiny route stubs in app/routes/. Most are under 1 KB. splitRouteModules: false turns splitting off if the file count is a problem.
  • No module appears in more than one chunk, and the largest chunks are the same as before (SerialConsolePage 350 KB, silo-create 200 KB, DateTimeRangePicker 181 KB).
  • The one new big file is the route manifest: 260 KB raw, 16.5 KB gzipped, loaded on every page load because of routeDiscovery: 'initial'.

Known dev-only quirk

Index routes that re-export a tab's clientLoader (vpc-index, sled-index, inventory-index, alerting-index, affinity-create) keep the old loader after you edit that tab until a full reload.

Typechecking breadcrumb params against each route's params will come in a followup PR.

@vercel

vercel Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
console Ready Ready Preview Oct 9, 2026 9:19pm UTC

Request Review

@david-crespo david-crespo changed the title Adopt React Router framework mode for the SPA Use React Router framework mode + SPA mode Oct 7, 2026
q(api.instanceView, { path: { instance }, query: { project } })
)
return null
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just unused, apparently — this was not a route file at all.

export const clientLoader = projectLayoutLoader
export async function clientLoader({ params }: Route.ClientLoaderArgs) {
return projectLayoutLoader(params)
}

@david-crespo david-crespo Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so we can check the params against Route.ClientLoaderArgs

export const handle = makeCrumb(
(p) => p.antiAffinityGroup!,
(p) => pb.antiAffinityGroup(getAntiAffinityGroupSelector(p))
)

@david-crespo david-crespo Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a followup PR that typechecks these handles against the route params, avoiding the !, but it's gnarly and hacky TypeScript so I wanted to keep it separate.

@benjaminleonard

benjaminleonard commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Visual regression test against main if not done already would be a good extra validation.

But I am very excited to get hot reload again!

david-crespo added a commit that referenced this pull request Oct 9, 2026
While doing #3400, I realized I wanted a way to test the CSP from Nexus
against our actual production bundle. I decide that because CSP is
enforced in the browser, we don't need Nexus for this at all, we just
need the CSP string. So all we have to do is use regex to extract that
from the pinned Omicron commit, and then we can test the CSP with `vite
preview` and a Playwright test. I was able to confirm the test fails
when it's supposed to: on the #3400 branch, if I comment out the step
that moves React Router's inline bootstrap scripts into separate files,
the app never renders and the test fails with the browser's CSP errors:

```
Error: expect(received).toEqual(expected) // deep equality
- Array []
+ Array [
+   "Executing inline script violates the following Content Security Policy directive 'default-src 'self''. [...] The action has been blocked.",
+   [3 more of those]
+   "CSP violation: script-src-elem inline",
+   "CSP violation: script-src-elem inline",
```

Once I had that in place, I thought about what else we might want to
test about the contract with Nexus, and I remembered that I have
sometimes forgotten to add a new route handler on the Nexus side when we
added a new top-level route prefix in the console. So I added a test
that similarly extracts the console routes from Nexus and makes sure all
the client-side routes are covered by those handlers. That immediately
turned up a bug (go to https://oxide.sys.r3.oxide-preview.com/images,
click an image, and then refresh — 404), which is fixed in
oxidecomputer/omicron#11467. So the test will
fail until that is merged and omicron is bumped.

This is nice and neat because #3402 pulls out the setup.
@david-crespo

Copy link
Copy Markdown
Collaborator Author

Visual regression test is clean, just some weirdness with timing around UUID truncation moving things by a pixel.

This branch was successfully deployed

1 active deployment
Preview — cd5f8ec0 Deployed Oct 9, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants