From 96e4ff5932411457b05ee11c0872c15c838d1e3c Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Fri, 7 Aug 2026 10:22:51 +0200 Subject: [PATCH] perf(client): load the page chunks on demand (#1812) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(client): retry a failed page chunk once before giving up A dynamic import fails for two different reasons, and only one of them is worth a second attempt. If the file is gone because a deploy happened while the tab was open, nothing but a reload helps and reloadOnceForChunk already owns that path. If the fetch itself went wrong — hotel wifi, an aborted request, a service worker handing back a truncated entry — one retry fixes it. The retry cannot reuse the original specifier, since Vite rewrites it into a hashed asset URL at build time; the only usable URL is the one the browser names in the failure message, and a fresh query on it misses both the HTTP cache and the Workbox precache. This is also where a healthy load is acknowledged. Clearing the reload marker used to sit in main.tsx right after render(), which runs before React commits and long before any chunk is fetched, so the marker was gone before it was ever needed. * perf(client): load the page chunks on demand Every one of the twenty pages was a static import, so the entry chunk carried the planner, the journal, the atlas and the vacation planner for anyone who only opened the dashboard. They now load through lazyWithRetry; the entry chunk drops from 5,183 kB to 1,243 kB (gzip 1,300 -> 349). LoginPage stays static on purpose: anyone logged out lands there, and every other route redirects there first. The Suspense boundary sits above rather than inside ProtectedRoute so it stays mounted across navigations. react-router runs location updates inside a transition, which keeps the current page on screen while the next chunk arrives instead of flashing a spinner on every jump. The precache is unchanged by design: workbox picks up the new chunks, so the install size stays where it was (107 -> 178 entries, +31 KiB) and every route remains available offline. This buys first paint, not install size. --- client/src/App.test.tsx | 4 +- client/src/App.tsx | 353 +++++++++++++----------- client/src/main.tsx | 4 - client/src/utils/lazyWithRetry.test.tsx | 89 ++++++ client/src/utils/lazyWithRetry.ts | 69 +++++ 5 files changed, 352 insertions(+), 167 deletions(-) create mode 100644 client/src/utils/lazyWithRetry.test.tsx create mode 100644 client/src/utils/lazyWithRetry.ts diff --git a/client/src/App.test.tsx b/client/src/App.test.tsx index 610bd05dd..5eae3cab1 100644 --- a/client/src/App.test.tsx +++ b/client/src/App.test.tsx @@ -169,6 +169,8 @@ describe('ProtectedRoute — admin role check', () => { // ── Public routes ────────────────────────────────────────────────────────────── describe('Public routes', () => { + // Synchronous on purpose: LoginPage is the one page still statically imported, + // so this also holds the line against someone making it lazy later. it('FE-COMP-APP-012: /login is accessible without authentication', async () => { seedAuth({ isAuthenticated: false }) renderApp('/login') @@ -178,7 +180,7 @@ describe('Public routes', () => { it('FE-COMP-APP-013: /shared/:token is accessible without authentication', async () => { seedAuth({ isAuthenticated: false }) renderApp('/shared/sometoken') - expect(screen.getByText('SharedTrip')).toBeInTheDocument() + expect(await screen.findByText('SharedTrip')).toBeInTheDocument() }) it('FE-COMP-APP-014: unknown routes redirect to / which then redirects to /login', async () => { diff --git a/client/src/App.tsx b/client/src/App.tsx index e415eef28..f8ff987c7 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -1,36 +1,21 @@ -import React, { useEffect, ReactNode } from 'react' +import React, { useEffect, ReactNode, Suspense } from 'react' import { Routes, Route, Navigate, useLocation } from 'react-router' import { useAuthStore } from './store/authStore' import { useSettingsStore } from './store/settingsStore' import { applyAppearance } from './theme/applyAppearance' import { useAddonStore } from './store/addonStore' import { usePluginStore } from './store/pluginStore' -import PluginPage from './pages/PluginPage' +// The one page that stays in the entry chunk. Anyone logged out lands here, and +// every other route redirects here first — a chunk round trip in front of the login +// form would slow down the single screen that has to be there immediately. import LoginPage from './pages/LoginPage' -import ForgotPasswordPage from './pages/ForgotPasswordPage' -import ResetPasswordPage from './pages/ResetPasswordPage' -import DashboardPage from './pages/DashboardPage' -import TripPlannerPage from './pages/TripPlannerPage' -import FilesPage from './pages/FilesPage' -import AdminPage from './pages/AdminPage' -import SettingsPage from './pages/SettingsPage' -import VacayPage from './pages/VacayPage' -import HelpPage from './pages/HelpPage' -import AtlasPage from './pages/AtlasPage' -import JourneyPage from './pages/JourneyPage' -import JourneyDetailPage from './pages/JourneyDetailPage' -import CollectionsPage from './pages/CollectionsPage' -import JourneyPublicPage from './pages/JourneyPublicPage' -import SharedTripPage from './pages/SharedTripPage' -import JoinTripPage from './pages/JoinTripPage' -import InAppNotificationsPage from './pages/InAppNotificationsPage.tsx' -import OAuthAuthorizePage from './pages/OAuthAuthorizePage' import { ToastContainer } from './components/shared/Toast' import SaveToCollectionModal from './components/Collections/SaveToCollectionModal' import MSaveToCollectionSheet from './components/Collections/MSaveToCollectionSheet' import BackgroundTasksWidget from './components/BackgroundTasks/BackgroundTasksWidget' import MobileShell from './mobile/MobileShell' import ErrorBoundary from './components/shared/ErrorBoundary' +import { lazyWithRetry } from './utils/lazyWithRetry' import { useIsPhone } from './mobile/useIsPhone' import { TranslationProvider, useTranslation } from './i18n' import { authApi } from './api/client' @@ -42,6 +27,31 @@ import { SystemNoticeHost } from './components/SystemNotices/SystemNoticeHost.js // Notice action registrations (side-effect imports): import './pages/Trips/noticeActions.js' +// Every page below loads on demand. The entry chunk used to carry all twenty of +// them eagerly, so opening /dashboard also paid for the planner, the journal, the +// atlas and the vacation planner. lazyWithRetry rather than lazy: a chunk that +// fails once gets a second, cache-busted attempt before the route boundary reaches +// for a reload. +const PluginPage = lazyWithRetry(() => import('./pages/PluginPage')) +const ForgotPasswordPage = lazyWithRetry(() => import('./pages/ForgotPasswordPage')) +const ResetPasswordPage = lazyWithRetry(() => import('./pages/ResetPasswordPage')) +const DashboardPage = lazyWithRetry(() => import('./pages/DashboardPage')) +const TripPlannerPage = lazyWithRetry(() => import('./pages/TripPlannerPage')) +const FilesPage = lazyWithRetry(() => import('./pages/FilesPage')) +const AdminPage = lazyWithRetry(() => import('./pages/AdminPage')) +const SettingsPage = lazyWithRetry(() => import('./pages/SettingsPage')) +const VacayPage = lazyWithRetry(() => import('./pages/VacayPage')) +const HelpPage = lazyWithRetry(() => import('./pages/HelpPage')) +const AtlasPage = lazyWithRetry(() => import('./pages/AtlasPage')) +const JourneyPage = lazyWithRetry(() => import('./pages/JourneyPage')) +const JourneyDetailPage = lazyWithRetry(() => import('./pages/JourneyDetailPage')) +const CollectionsPage = lazyWithRetry(() => import('./pages/CollectionsPage')) +const JourneyPublicPage = lazyWithRetry(() => import('./pages/JourneyPublicPage')) +const SharedTripPage = lazyWithRetry(() => import('./pages/SharedTripPage')) +const JoinTripPage = lazyWithRetry(() => import('./pages/JoinTripPage')) +const InAppNotificationsPage = lazyWithRetry(() => import('./pages/InAppNotificationsPage.tsx')) +const OAuthAuthorizePage = lazyWithRetry(() => import('./pages/OAuthAuthorizePage')) + interface ProtectedRouteProps { children: ReactNode adminRequired?: boolean @@ -144,6 +154,19 @@ function RootRedirect() { return } +/** + * Shown while a route chunk is in flight. Same geometry as the auth spinner above, + * but on theme tokens — that one predates the styleguide and a new surface does not + * get to inherit its raw slate. + */ +function RouteFallback() { + return ( +
+
+
+ ) +} + export default function App() { const { loadUser, isAuthenticated, demoMode, setDemoMode, setDevMode, setIsPrerelease, setAppVersion, setHasMapsKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled, setPlacesPhotosEnabled, setPlacesAutocompleteEnabled, setPlacesDetailsEnabled } = useAuthStore() const { loadSettings } = useSettingsStore() @@ -248,148 +271,154 @@ export default function App() { {!isAuthPage && } {!isAuthPage && (isPhone ? : )} - - } /> - } /> - } /> - } /> - } /> - } /> - } /> - {/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */} - } /> - - - - } - /> - {/* Trip invite link (#1143) — behind ProtectedRoute so an anonymous - visitor is redirected to /login (never registration) and returns here. */} - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - - - - } - /> - } /> - + {/* One boundary for all route chunks, above so it stays mounted + across navigations. react-router runs location updates inside a transition, + so a mounted boundary keeps the current page on screen instead of flashing + a spinner on every jump — the spinner is for the first paint of a deep link. */} + }> + + } /> + } /> + } /> + } /> + } /> + } /> + } /> + {/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */} + } /> + + + + } + /> + {/* Trip invite link (#1143) — behind ProtectedRoute so an anonymous + visitor is redirected to /login (never registration) and returns here. */} + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + + + + } + /> + } /> + + ) } diff --git a/client/src/main.tsx b/client/src/main.tsx index 9a2c326c6..7d06410e9 100644 --- a/client/src/main.tsx +++ b/client/src/main.tsx @@ -36,7 +36,6 @@ import { startConnectivityProbe } from './sync/connectivity' import { requestPersistentStorage } from './sync/persistentStorage' import ErrorBoundary, { RootErrorFallback } from './components/shared/ErrorBoundary' import { installGlobalErrorHandlers } from './utils/globalErrorHandlers' -import { clearChunkReloadMarker } from './utils/chunkReload' maybeInstallTouchDragPolyfill() startConnectivityProbe() @@ -57,6 +56,3 @@ ReactDOM.createRoot(document.getElementById('root')!).render( , ) - -// The app rendered, so a later deploy is allowed to trigger its own reload. -clearChunkReloadMarker() diff --git a/client/src/utils/lazyWithRetry.test.tsx b/client/src/utils/lazyWithRetry.test.tsx new file mode 100644 index 000000000..27059049b --- /dev/null +++ b/client/src/utils/lazyWithRetry.test.tsx @@ -0,0 +1,89 @@ +import React, { Suspense } from 'react' +import { render, screen } from '@testing-library/react' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import ErrorBoundary from '../components/shared/ErrorBoundary' +import { lazyWithRetry, retryUrlFor } from './lazyWithRetry' + +// Every case needs its own URL: the one-shot guard is a module-level Set that +// lives for the whole file. +const viteMessage = (url: string) => `Failed to fetch dynamically imported module: ${url}` + +beforeEach(() => { + sessionStorage.clear() +}) + +describe('retryUrlFor', () => { + it('FE-UTIL-LAZYRETRY-001: busts the cache on the URL the browser named', () => { + const url = retryUrlFor(new Error(viteMessage('https://trek.example/assets/AtlasPage-a1b2.js'))) + expect(url).toMatch(/^https:\/\/trek\.example\/assets\/AtlasPage-a1b2\.js\?t=\d+$/) + }) + + it('FE-UTIL-LAZYRETRY-002: retries a given chunk only once', () => { + const message = viteMessage('https://trek.example/assets/VacayPage-c3d4.js') + expect(retryUrlFor(new Error(message))).not.toBeNull() + // A second attempt would be the loop the route boundary is there to break. + expect(retryUrlFor(new Error(message))).toBeNull() + }) + + it('FE-UTIL-LAZYRETRY-003: leaves ordinary errors to the boundary', () => { + expect(retryUrlFor(new Error('Cannot read properties of undefined'))).toBeNull() + }) + + it('FE-UTIL-LAZYRETRY-004: gives up when the message names no URL', () => { + // Safari's wording, which carries no URL at all. + expect(retryUrlFor(new Error('Importing a module script failed.'))).toBeNull() + // And a relative path leaves nothing to bust. + expect(retryUrlFor(new Error(viteMessage('/assets/JourneyPage-e5f6.js')))).toBeNull() + }) + + it('FE-UTIL-LAZYRETRY-005: does not try to import a stylesheet', () => { + const css = new Error('Unable to preload CSS for https://trek.example/assets/PhotoLightbox-g7h8.css') + expect(retryUrlFor(css)).toBeNull() + }) +}) + +describe('lazyWithRetry', () => { + it('FE-UTIL-LAZYRETRY-006: renders the default export once the chunk arrives', async () => { + const Page = lazyWithRetry(async () => ({ default: () =>
Atlas
})) + render( + loading}> + + + ) + expect(await screen.findByText('Atlas')).toBeInTheDocument() + }) + + it('FE-UTIL-LAZYRETRY-007: clears the reload marker once a chunk has loaded', async () => { + // Set by reloadOnceForChunk() before the reload that got us here. Clearing it + // any earlier — main.tsx used to do it right after render() — would let a + // genuinely missing chunk reload the tab over and over. + sessionStorage.setItem('trek:chunk-reload', '1750000000000') + + const Page = lazyWithRetry(async () => ({ default: () =>
Dashboard
})) + render( + loading}> + + + ) + + expect(await screen.findByText('Dashboard')).toBeInTheDocument() + expect(sessionStorage.getItem('trek:chunk-reload')).toBeNull() + }) + + it('FE-UTIL-LAZYRETRY-008: hands a non-chunk failure to the boundary', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + const Page = lazyWithRetry(() => Promise.reject(new Error('boom'))) + + render( + caught}> + loading}> + + + + ) + + expect(await screen.findByText('caught')).toBeInTheDocument() + // The marker belongs to reloadOnceForChunk; a plain throw must not touch it. + expect(sessionStorage.getItem('trek:chunk-reload')).toBeNull() + }) +}) diff --git a/client/src/utils/lazyWithRetry.ts b/client/src/utils/lazyWithRetry.ts new file mode 100644 index 000000000..22a4b502e --- /dev/null +++ b/client/src/utils/lazyWithRetry.ts @@ -0,0 +1,69 @@ +import { lazy } from 'react'; +import type { ComponentType, LazyExoticComponent } from 'react'; +import { clearChunkReloadMarker, isChunkLoadError } from './chunkReload'; + +/** + * React.lazy with a single second attempt — and the place where a healthy load is + * acknowledged. + * + * A route chunk fails for two very different reasons. Either the file is gone + * because a deploy happened while the tab was open; nothing but a reload helps, and + * reloadOnceForChunk() already owns that path through the route boundary. Or the + * fetch itself went wrong: a blip on hotel wifi, an aborted request, a service + * worker handing back a truncated entry. That one heals on a second try, and it is + * the only case this wrapper exists for. + * + * The retry cannot reuse the original specifier. Vite rewrites + * import('./pages/AtlasPage') into a hashed asset URL at build time, so a query + * appended in source stops it being statically analysable: the build leaves a + * runtime path that resolves against the wrong directory, while dev — which really + * does serve the source file — looks perfectly fine. The only usable URL is the one + * the browser names in the failure message. A fresh query on it misses the HTTP + * cache and the Workbox precache, both of which key on the full URL. + */ + +const retried = new Set(); + +/** The cache-busted URL for one failed chunk, or null when a retry is pointless. */ +export function retryUrlFor(error: unknown): string | null { + if (!isChunkLoadError(error)) return null; + + const message = error instanceof Error ? error.message : String(error); + const match = /https?:\/\/[^\s'")]+/.exec(message); + // Safari says "Importing a module script failed." and names no URL at all, and a + // relative path gives us nothing to bust. Both belong to the boundary, not here. + if (!match) return null; + + const url = match[0]; + // The CSS preload wording carries a stylesheet, which is not ours to import. + if (url.endsWith('.css') || retried.has(url)) return null; + retried.add(url); + + const busted = new URL(url); + busted.searchParams.set('t', String(Date.now())); + return busted.href; +} + +export function lazyWithRetry>( + load: () => Promise<{ default: T }> +): LazyExoticComponent { + return lazy(() => + load() + .catch((error: unknown) => { + const url = retryUrlFor(error); + // Rethrown on purpose: React.lazy hands the rejection to the nearest + // boundary, and that one knows how to reload once per session. + if (!url) throw error; + return import(/* @vite-ignore */ url) as Promise<{ default: T }>; + }) + .then(module => { + // A chunk arrived, so the asset pipeline is intact and a later deploy may + // spend its own reload. This used to sit in main.tsx right after render(), + // which runs before React commits and long before any chunk is fetched — the + // marker was cleared before it was ever needed, and a chunk that is genuinely + // missing could put the tab in the reload loop the marker exists to prevent. + clearChunkReloadMarker(); + return module; + }) + ); +}