feature: Performance improvements

[gcgoncalves]

Lazy-load all page components in App.tsx (565KB main chunk -> 200KB,
per-page chunks fetched on nav). Register @fastify/compress globally
on the BFF (gzip/brotli, SSE exempt via reply.hijack()). Cache hashed
/assets/* for 1y immutable; index.html stays no-cache.

Targets FCP regression vs the HTMX app (1.4s vs 0.6s).

Depends on #1

[marekdano]

Frontend: route-level code splitting (src/App.tsx) — looks correct

Backend: compression (server/src/plugins/compress.ts) — correct

Backend: static caching — bug found, confirmed by test

server/src/plugins/static.ts:32-37 sets maxAge: "1y", immutable: true at the root @fastify/static registration (prefix: "/", covering all of server/public/), with the stated intent being to cache only Vite's content-hashed /assets/* files. The SPA-fallback 404 handler correctly overrides this back to maxAge: 0 for index.html — but that override only applies when a request falls through to the 404 handler (i.e. /app/* deep links). It does not apply when a file is served directly by @fastify/static's own route matching.

I verified this empirically with a throwaway Fastify harness against the real plugin:

GET /favicon.ico → cache-control: public, max-age=31536000, immutable
GET /index.html → cache-control: public, max-age=31536000, immutable (same file, direct request)
GET /app/login → cache-control: public, max-age=0 (SPA-fallback path — correct)

favicon.ico is a real, non-hashed file that Vite copies verbatim from public/ (vite.config.ts → outDir: "server/public"), and it's explicitly allowlisted as a known static path in the fallback handler's comment — so it's
a real, reachable route, not a hypothetical. Two concrete problems:

• If the favicon is ever updated, browsers/CDNs that fetched it once will keep serving the stale one for up to a year (immutable also skips conditional revalidation).
• index.html itself is only safe because nothing in this app currently links to /index.html directly (/ redirects server-side to /app/, and all client routes 404-fall-through to the explicit override). But that safety is incidental, not structural — any direct request, bot crawl, or CDN edge caching /index.html verbatim would cache the shell (and its script-tag references to the old hashed bundle) for a year, immutably, defeating the very cache-busting this PR is trying to add.

Fix: don't set maxAge/immutable at the plugin-registration level. Scope the long-cache header to /assets/* only, e.g. via @fastify/static's setHeaders(res, pathName) option:

setHeaders(res, pathName) {
  if (pathName.includes(`${path.sep}assets${path.sep}`)) {
    res.setHeader("Cache-Control", "public, max-age=31536000, immutable");
  }
}

That also lets you drop the now-unnecessary { maxAge: 0, immutable: false } override on the fallback sendFile call, since nothing outside /assets/* would get long-cached in the first place.

[marekdano]

Also, please add tests for changes in App.tsx

[marekdano]

Left some comments

[marekdano]

server/src/plugins/static.ts:38
The 1-year immutable cache header is applied to the whole static root, not just the hashed /assets/* prefix. Direct requests to index.html (or any non-hashed file copied from public/) get cached immutably for a year, so after the next deploy (which wipes and regenerates hashed chunk names) clients can get stuck on a stale shell referencing chunks that no longer exist.

[gcgoncalves]

Thanks for flagging this. I think it's actually already handled: the header is only applied when the resolved file path contains /assets/:

if (pathName.includes(`${path.sep}assets${path.sep}`)) {
  reply.header("Cache-Control", "public, max-age=31536000, immutable");
}

index.html and anything copied verbatim from public/ don't match that condition, so they fall through with no Cache-Control header set (i.e. @fastify/static's default, effectively no long-term caching). It's not being applied to the whole static root.

That said, you were right about the underlying risk: this exact bug (unconditional maxAge: "1y", immutable: true on the whole root) existed one commit back, before Fix caching issues replaced it with this scoped setHeaders check. Might be worth a re-review/resolve on this thread since it looks like the comment landed on the pre-fix version.

[marekdano]

src/App.tsx:13
All ~23 routes were switched to lazy-loaded chunks, but there's no ErrorBoundary anywhere in the app. If a client on a stale build tries to navigate to a route whose chunk was deleted by the last deploy, the dynamic import 404s and React unmounts the tree — blank/crashed page, no retry.

The .then(m => ({ default: m.X })) lazy-load adapter is duplicated 23 times instead of factored into one helper; copy-paste risk plus any future fix (e.g. adding chunk-load retry, which would also help with #2) needs to be repeated 23×.

[marekdano]

server/src/plugins/compress.ts:17
@fastify/compress registered with only { global: true } leaves globalDecompression at its default true, so incoming request bodies with a Content-Encoding header
are now silently auto-decompressed across the whole API surface (including the SSE upstream route) — undocumented and untested side effect beyond the PR's stated intent (response compression).

[gcgoncalves]

Depends on [nope]

EDIT: Commented on wrong PR

[marekdano]

7 findings, ranked most severe first.

Correctness bugs

1. src/App.tsx:115 — Reload guard is cleared before a broken chunk can fail again

The mount-time useEffect that clears the chunk-reload guard fires on App's own successful commit, which happens even while a lazy chunk import is still pending (Suspense fallback showing). So the guard gets cleared before a genuinely broken chunk has even had a chance to fail again.

Impact: For a persistently broken chunk (not just a one-time stale
deploy), this can cause an infinite reload loop instead of the "reload once, then show fallback" behavior the code's own comments promise.

2. src/components/ErrorBoundary.tsx:10 — Crashes on non-Error thrown values

isChunkLoadError() assumes the caught value is a real Error and reads error.name / error.message. If something throws null/undefined (e.g. an empty Promise.reject()), this throws inside componentDidCatch itself.

Impact: Crashes harder than if the boundary didn't exist at all — defeats the exact safety net this PR is adding.

3. server/src/plugins/static.ts:37 — Cache-Control immutable false positive

The new Cache-Control: immutable header is decided by checking whether the absolute filesystem path contains an assets segment, not the URL.

Impact: If the deployed PUBLIC_DIR ever sits under a directory containing "assets" anywhere in its path, non-hashed files — including index.html — get cached immutably for a year, permanently pinning users to a stale app shell after the next deploy.

4. server/src/plugins/static.ts:37 vs :53 — Two divergent asset-path checks

Two different techniques decide "is this an asset path" in the same file (filesystem substring match vs. URL-prefix match), with nothing keeping them in sync if the build layout changes.

Efficiency / cleanup

5. src/App.tsx:25 — Dashboard lazy-load introduces a fetch waterfall

Lazy-loading Dashboard (the default post-login page) turns its data fetch
into a JS-then-data waterfall with no preloading added, working against the
PR's own stated FCP goal.

6. src/App.tsx:22 — Trivial pages unnecessarily code-split

Placeholder pages (ForgotPassword, ResetPassword, ChangePassword) are
code-split into their own chunks even though they're a few lines of static
JSX — likely a net loss vs. just inlining them.

7. server/src/plugins/compress.ts:20 — No test coverage

The new global compression plugin has zero test coverage anywhere in the
repo, including on the SSE/streaming proxy paths it's riskiest for.

[marekdano]

Findings

1. Reload-guard is cleared before the real chunk ever resolves (Critical)

• File: src/App.tsx:60 (ClearReloadGuardOnMount), used at :82 (PublicRoutes) and :100 (PrivateRoutes)
• Bug: ClearReloadGuardOnMount sits inside PublicRoutes' <Suspense> (line 82). On any private route, none of PublicRoutes' three <Route> children match, so they all return null — nothing suspends — so that Suspense boundary commits immediately, firing the effect and deleting sessionStorage['cf:error-boundary-reloaded'] right away, regardless of whether PrivateRoutes' real lazy chunk (line 100) ever loads.
• Failure scenario: A private-route chunk (e.g. /app/gateways) is permanently 404ing post-deploy. ErrorBoundary catches the load failure, checks the guard, finds it already cleared (wiped by the sibling PublicRoutes boundary that never actually suspended), and reloads — then the guard gets wiped again on the next mount before the error is even caught. Result: infinite reload loop, which is the exact failure mode this PR claims to fix.

2. Single top-level ErrorBoundary defeats the route-scoped Suspense design (High)

• File: src/App.tsx:140 (ErrorBoundary wraps <Routes/>, above AppShell at :96)
• Bug: The PR's own comment (line 97) says Suspense was deliberately placed inside AppShell so sidebar/header stay mounted while only the route body shows a fallback. But ErrorBoundary wraps the whole tree above AppShell, so once the one-shot auto-reload fails (or any unrelated page throws), the entire shell — sidebar, header, nav — unmounts too.
• Failure scenario: One broken page (chunk permanently 404ing, or an unrelated render bug) leaves the user on a blank full-page fallback with no navigation, unable to route away to any other (working) page.

Findings 1 and 2 are real functional bugs in the core feature this PR is shipping ("harden route chunk loading"). #1 in particular is self-defeating: it can cause the exact infinite-reload behavior the PR is meant to prevent, and it was independently found and confirmed via two separate code traces (I verified it myself by reading Routes() → PublicRoutes() → Route in src/router/index.tsx, confirming PublicRoutes' Suspense never actually suspends on a private-route path).

Suggested fix for
1: move ClearReloadGuardOnMount so it only lives inside the Suspense boundary that's actually guarding a chunk that can fail on the current path — or simpler, only clear the guard in PrivateRoutes' boundary and drop it from PublicRoutes, since PublicRoutes' three routes are tiny non-suspending stubs anyway per the PR's own comment.

2: is a real design gap but more of a judgment call — worth raising, not necessarily a blocker if the team accepts "full-page fallback for any uncaught error" as intentional scope for now.

[marekdano]

Failing test / red CI — src/App.test.tsx:107
New test asserts the ForgotPassword heading matches /forgot password/i, but the page renders "Reset Password". Confirmed failing in the PR's own CI run ("Lint & Test Client").
ForgotPassword.tsx is unchanged by this PR — the test itself is wrong.