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; + }) + ); +}