From 057d4ba5038b212c1ec6d5fa02dbe62fb06d15ec Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Fri, 7 Aug 2026 11:51:37 +0200 Subject: [PATCH] perf(client): pick the viewport branch at the route (#1817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every page imported its mobile screen statically and chose while rendering, so each route chunk carried both trees and the viewport only decided which half stayed dark. Worse, because ten route chunks shared the mobile tree, rollup hoisted it into the entry chunk — so every desktop user downloaded the whole mobile shell before seeing a single page. The branch now decides the chunk. A phone never fetches the desktop planner, a desktop never the mobile shell. entry chunk 1,243 -> 581 kB (gzip 349 -> 177) TripPlannerPage 914 -> 340 kB VacayPage 361 -> 57 kB AdminPage 413 -> 225 kB SettingsPage 259 -> 155 kB Splitting inside each page instead was measured and rejected: it leaves the desktop code in the same module as the switch, so a phone would still download it and then wait for a second, serial request on top — 914 kB and a waterfall against 550 kB in parallel. The pass-through wrapper in each page stays deliberately. check-page-pattern scans from the default export to the next top-level function and rejects hooks there; making the desktop component the default export fails it immediately on dashboard, planner and atlas. RouteFallback gets a mobile face. It renders above MobileShell, so the --m-* tokens do not resolve for it and the desktop spinner on bg-surface would be a foreign white sheet in front of the mobile screen. The five scattered viewport spot checks are replaced by one contract covering all eleven routes, which is where the switch now lives. --- client/src/App.tsx | 59 ++++++++-- client/src/App.viewport.test.tsx | 111 ++++++++++++++++++ .../src/mobile/components/MRouteFallback.tsx | 19 +++ client/src/pages/AdminPage.test.tsx | 23 ---- client/src/pages/AdminPage.tsx | 7 +- client/src/pages/AtlasPage.tsx | 7 +- client/src/pages/AtlasPage.wiring.test.tsx | 12 +- client/src/pages/CollectionsPage.tsx | 7 +- .../src/pages/CollectionsPage.wiring.test.tsx | 12 -- .../src/pages/DashboardPage.desktop.test.tsx | 16 --- client/src/pages/DashboardPage.tsx | 9 +- client/src/pages/InAppNotificationsPage.tsx | 7 +- client/src/pages/JourneyDetailPage.tsx | 7 +- client/src/pages/JourneyPage.tsx | 7 +- client/src/pages/SettingsPage.tsx | 7 +- client/src/pages/TripPlannerPage.tsx | 10 +- .../src/pages/TripPlannerPage.wiring.test.tsx | 15 --- client/src/pages/VacayPage.tsx | 7 +- 18 files changed, 209 insertions(+), 133 deletions(-) create mode 100644 client/src/App.viewport.test.tsx create mode 100644 client/src/mobile/components/MRouteFallback.tsx diff --git a/client/src/App.tsx b/client/src/App.tsx index f8ff987c7..ac01a09bb 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -14,6 +14,7 @@ import SaveToCollectionModal from './components/Collections/SaveToCollectionModa import MSaveToCollectionSheet from './components/Collections/MSaveToCollectionSheet' import BackgroundTasksWidget from './components/BackgroundTasks/BackgroundTasksWidget' import MobileShell from './mobile/MobileShell' +import MRouteFallback from './mobile/components/MRouteFallback' import ErrorBoundary from './components/shared/ErrorBoundary' import { lazyWithRetry } from './utils/lazyWithRetry' import { useIsPhone } from './mobile/useIsPhone' @@ -52,6 +53,23 @@ const JoinTripPage = lazyWithRetry(() => import('./pages/JoinTripPage')) const InAppNotificationsPage = lazyWithRetry(() => import('./pages/InAppNotificationsPage.tsx')) const OAuthAuthorizePage = lazyWithRetry(() => import('./pages/OAuthAuthorizePage')) +// The ten phone screens are chunks of their own, alongside the desktop pages +// rather than inside them. Each page used to import its M screen statically and +// decide while rendering, so the route chunk always carried both trees and the +// viewport only picked which half stayed dark. Here the branch decides the +// chunk: a phone never loads the desktop planner, a desktop never the mobile +// shell. +const MDashboardScreen = lazyWithRetry(() => import('./mobile/screens/dashboard/MDashboard')) +const MTripScreen = lazyWithRetry(() => import('./mobile/screens/trip/MTripShell')) +const MAdminScreen = lazyWithRetry(() => import('./mobile/screens/admin/MAdmin')) +const MSettingsScreen = lazyWithRetry(() => import('./mobile/screens/settings/MSettings')) +const MVacayScreen = lazyWithRetry(() => import('./mobile/screens/vacay/MVacay')) +const MAtlasScreen = lazyWithRetry(() => import('./mobile/screens/atlas/MAtlas')) +const MJourneyScreen = lazyWithRetry(() => import('./mobile/screens/journey/MJourney')) +const MJourneyDetailScreen = lazyWithRetry(() => import('./mobile/screens/journey/MJourneyDetail')) +const MCollectionsScreen = lazyWithRetry(() => import('./mobile/screens/collections/MCollections')) +const MNotificationsScreen = lazyWithRetry(() => import('./mobile/screens/notifications/MNotifications')) + interface ProtectedRouteProps { children: ReactNode adminRequired?: boolean @@ -140,6 +158,20 @@ function PublicRoute({ children }: { children: React.ReactNode }) { ) } +/** + * Picks the chunk, not just the branch. Both sides come through lazyWithRetry, + * so exactly one of them is fetched for a given viewport — which is the whole + * point: the ternary that used to sit in each page only ran once the browser + * had already paid for both trees. + */ +function ViewportRoute({ phone: Phone, desktop: Desktop }: { + phone: React.ComponentType + desktop: React.ComponentType +}): React.ReactElement { + const isPhone = useIsPhone() + return isPhone ? : +} + function RootRedirect() { const { isAuthenticated, isLoading } = useAuthStore() @@ -160,6 +192,11 @@ function RootRedirect() { * get to inherit its raw slate. */ function RouteFallback() { + // The fallback renders above MobileShell, so it has to pick its own palette — + // on a phone the desktop spinner on bg-surface would be a foreign white sheet + // in front of the mobile screen. + const isPhone = useIsPhone() + if (isPhone) return return (
@@ -290,7 +327,7 @@ export default function App() { path="/dashboard" element={ - + } /> @@ -324,7 +361,7 @@ export default function App() { path="/trips/:id" element={ - + } /> @@ -340,7 +377,7 @@ export default function App() { path="/admin" element={ - + } /> @@ -348,7 +385,7 @@ export default function App() { path="/settings" element={ - + } /> @@ -364,7 +401,7 @@ export default function App() { path="/vacay" element={ - + } /> @@ -372,7 +409,7 @@ export default function App() { path="/atlas" element={ - + } /> @@ -380,7 +417,7 @@ export default function App() { path="/journey" element={ - + } /> @@ -388,7 +425,7 @@ export default function App() { path="/journey/:id" element={ - + } /> @@ -396,7 +433,7 @@ export default function App() { path="/collections" element={ - + } /> @@ -404,7 +441,7 @@ export default function App() { path="/collections/:id" element={ - + } /> @@ -412,7 +449,7 @@ export default function App() { path="/notifications" element={ - + } /> diff --git a/client/src/App.viewport.test.tsx b/client/src/App.viewport.test.tsx new file mode 100644 index 000000000..13c37d5e5 --- /dev/null +++ b/client/src/App.viewport.test.tsx @@ -0,0 +1,111 @@ +import React from 'react' +import { render, screen } from '@testing-library/react' +import { MemoryRouter } from 'react-router' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { useAuthStore } from './store/authStore' +import { useAddonStore } from './store/addonStore' +import { resetAllStores } from '../tests/helpers/store' +import { buildUser } from '../tests/helpers/factories' +import App from './App' + +/** + * The viewport decides which chunk gets fetched, so the switch lives in App.tsx + * and no longer in the pages. This is that contract, for all ten routes in one + * place — it used to be five scattered spot checks, one per page test. + * + * A stub per screen keeps the test about one thing: below the breakpoint the M + * root hangs off the route, and the desktop page is never rendered at all. + */ + +const isPhone = vi.hoisted(() => ({ value: false })) +vi.mock('./mobile/useIsPhone', () => ({ useIsPhone: () => isPhone.value })) + +vi.mock('./mobile/screens/dashboard/MDashboard', () => ({ default: () =>
m-dashboard
})) +vi.mock('./mobile/screens/trip/MTripShell', () => ({ default: () =>
m-trip
})) +vi.mock('./mobile/screens/admin/MAdmin', () => ({ default: () =>
m-admin
})) +vi.mock('./mobile/screens/settings/MSettings', () => ({ default: () =>
m-settings
})) +vi.mock('./mobile/screens/vacay/MVacay', () => ({ default: () =>
m-vacay
})) +vi.mock('./mobile/screens/atlas/MAtlas', () => ({ default: () =>
m-atlas
})) +vi.mock('./mobile/screens/journey/MJourney', () => ({ default: () =>
m-journey
})) +vi.mock('./mobile/screens/journey/MJourneyDetail', () => ({ default: () =>
m-journey-detail
})) +vi.mock('./mobile/screens/collections/MCollections', () => ({ default: () =>
m-collections
})) +vi.mock('./mobile/screens/notifications/MNotifications', () => ({ default: () =>
m-notifications
})) + +vi.mock('./pages/DashboardPage', () => ({ default: () =>
d-dashboard
})) +vi.mock('./pages/TripPlannerPage', () => ({ default: () =>
d-trip
})) +vi.mock('./pages/AdminPage', () => ({ default: () =>
d-admin
})) +vi.mock('./pages/SettingsPage', () => ({ default: () =>
d-settings
})) +vi.mock('./pages/VacayPage', () => ({ default: () =>
d-vacay
})) +vi.mock('./pages/AtlasPage', () => ({ default: () =>
d-atlas
})) +vi.mock('./pages/JourneyPage', () => ({ default: () =>
d-journey
})) +vi.mock('./pages/JourneyDetailPage', () => ({ default: () =>
d-journey-detail
})) +vi.mock('./pages/CollectionsPage', () => ({ default: () =>
d-collections
})) +vi.mock('./pages/InAppNotificationsPage.tsx', () => ({ default: () =>
d-notifications
})) + +// The notification listener opens a WebSocket on mount. +vi.mock('./hooks/useInAppNotificationListener.ts', () => ({ + useInAppNotificationListener: vi.fn(), +})) + +/** path, mobile marker, desktop marker */ +const ROUTES: [string, string, string][] = [ + ['/dashboard', 'm-dashboard', 'd-dashboard'], + ['/trips/1', 'm-trip', 'd-trip'], + ['/admin', 'm-admin', 'd-admin'], + ['/settings', 'm-settings', 'd-settings'], + ['/vacay', 'm-vacay', 'd-vacay'], + ['/atlas', 'm-atlas', 'd-atlas'], + ['/journey', 'm-journey', 'd-journey'], + ['/journey/1', 'm-journey-detail', 'd-journey-detail'], + ['/collections', 'm-collections', 'd-collections'], + ['/collections/1', 'm-collections', 'd-collections'], + ['/notifications', 'm-notifications', 'd-notifications'], +] + +function renderAt(path: string) { + return render( + + + + ) +} + +beforeEach(() => { + resetAllStores() + vi.clearAllMocks() + isPhone.value = false + // An admin passes the role check on /admin; every other route ignores it. + useAuthStore.setState({ + isLoading: false, + isAuthenticated: true, + user: buildUser({ role: 'admin', mfa_enabled: true }), + appRequireMfa: false, + loadUser: vi.fn().mockResolvedValue(undefined), + }) + // /journey and /collections sit behind addons; without this they redirect to + // the dashboard and the route under test never renders. + useAddonStore.setState({ loaded: true, isEnabled: () => true }) +}) + +describe('App — viewport routing', () => { + it.each(ROUTES)( + 'FE-COMP-APPVP-001: %s renders the mobile screen below the breakpoint', + async (path, mobile, desktop) => { + isPhone.value = true + renderAt(path) + + expect(await screen.findByText(mobile)).toBeInTheDocument() + expect(screen.queryByText(desktop)).not.toBeInTheDocument() + } + ) + + it.each(ROUTES)( + 'FE-COMP-APPVP-002: %s renders the desktop page above the breakpoint', + async (path, mobile, desktop) => { + renderAt(path) + + expect(await screen.findByText(desktop)).toBeInTheDocument() + expect(screen.queryByText(mobile)).not.toBeInTheDocument() + } + ) +}) diff --git a/client/src/mobile/components/MRouteFallback.tsx b/client/src/mobile/components/MRouteFallback.tsx new file mode 100644 index 000000000..ba802c0e3 --- /dev/null +++ b/client/src/mobile/components/MRouteFallback.tsx @@ -0,0 +1,19 @@ +/** + * Placeholder while a route chunk is in flight — the phone counterpart to + * RouteFallback in App.tsx. It carries m-root itself, because it renders outside + * MobileShell and the --m-* tokens would not resolve otherwise. + * + * An outline rather than a spinner: on a phone an empty screen is the loudest + * state the app has, and a pulsing silhouette of what is about to arrive reads + * as "loading" instead of "broken". + */ +export default function MRouteFallback() { + return ( +
+
+
+
+
+
+ ) +} diff --git a/client/src/pages/AdminPage.test.tsx b/client/src/pages/AdminPage.test.tsx index 463383a74..1cd09f0f4 100644 --- a/client/src/pages/AdminPage.test.tsx +++ b/client/src/pages/AdminPage.test.tsx @@ -69,19 +69,8 @@ vi.mock('../components/Admin/DefaultUserSettingsTab', () => ({ default: () =>
, })); -vi.mock('../mobile/screens/admin/MAdmin', () => ({ - default: () =>
, -})); - -// Viewport switch — flipped per test instead of at module scope. -let phoneViewport = false; -vi.mock('../mobile/useIsPhone', () => ({ - useIsPhone: () => phoneViewport, -})); - beforeEach(() => { resetAllStores(); - phoneViewport = false; }); describe('AdminPage', () => { @@ -1617,18 +1606,6 @@ describe('AdminPage', () => { }); }); - describe('FE-PAGE-ADMIN-058: Phone viewport', () => { - it('renders the mobile admin screen instead of the desktop layout', async () => { - phoneViewport = true; - seedStore(useAuthStore, { isAuthenticated: true, user: buildAdmin() }); - - render(); - - expect(screen.getByTestId('mobile-admin')).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /^users$/i })).not.toBeInTheDocument(); - }); - }); - describe('FE-PAGE-ADMIN-059: Stats fallbacks', () => { it('shows a zero file count when the API omits totalFiles', async () => { server.use( diff --git a/client/src/pages/AdminPage.tsx b/client/src/pages/AdminPage.tsx index ba5233b09..8b3a73f23 100644 --- a/client/src/pages/AdminPage.tsx +++ b/client/src/pages/AdminPage.tsx @@ -14,8 +14,6 @@ import AdminMcpTokensPanel from '../components/Admin/AdminMcpTokensPanel' import AdminPluginsPanel from '../components/Admin/AdminPluginsPanel' import { Users, Map, Briefcase, Shield, FileText, SlidersHorizontal, UserCog, Puzzle, Blocks, Settings as SettingsIcon, Bell, Database, ScrollText, KeyRound, GitBranch, Bug } from 'lucide-react' import PageSidebar, { type PageSidebarTab } from '../components/Layout/PageSidebar' -import { useIsPhone } from '../mobile/useIsPhone' -import MAdmin from '../mobile/screens/admin/MAdmin' import { useAdmin } from './admin/useAdmin' import AdminUpdateBanner from './admin/AdminUpdateBanner' import AdminStatCard from './admin/AdminStatCard' @@ -25,8 +23,9 @@ import AdminNotificationsTab from './admin/AdminNotificationsTab' import AdminUserModals from './admin/AdminUserModals' export default function AdminPage(): React.ReactElement { - const isPhone = useIsPhone() - return isPhone ? : + // ViewportRoute in App.tsx picks the branch now, so the phone screen is a + // chunk of its own instead of a dead limb in this one. + return } function AdminPageDesktop(): React.ReactElement { diff --git a/client/src/pages/AtlasPage.tsx b/client/src/pages/AtlasPage.tsx index f259c9e27..716707354 100644 --- a/client/src/pages/AtlasPage.tsx +++ b/client/src/pages/AtlasPage.tsx @@ -13,12 +13,11 @@ import AtlasCountrySearch from './atlas/AtlasCountrySearch' import AtlasLayerToggle from './atlas/AtlasLayerToggle' import { useToast } from '../components/shared/Toast' import { getApiErrorMessage } from '../types' -import { useIsPhone } from '../mobile/useIsPhone' -import MAtlas from '../mobile/screens/atlas/MAtlas' export default function AtlasPage(): React.ReactElement { - const isPhone = useIsPhone() - return isPhone ? : + // ViewportRoute in App.tsx picks the branch now, so the phone screen is a + // chunk of its own instead of a dead limb in this one. + return } function AtlasPageDesktop(): React.ReactElement { diff --git a/client/src/pages/AtlasPage.wiring.test.tsx b/client/src/pages/AtlasPage.wiring.test.tsx index 32e2883d6..40c1a098e 100644 --- a/client/src/pages/AtlasPage.wiring.test.tsx +++ b/client/src/pages/AtlasPage.wiring.test.tsx @@ -14,10 +14,9 @@ import AtlasPage from './AtlasPage'; // confirm popup and the sidebar can be driven into every state directly, which the // map-click driven suite in AtlasPage.test.tsx cannot reach. -const mocks = vi.hoisted(() => ({ atlas: {} as AtlasController, isPhone: false })); +const mocks = vi.hoisted(() => ({ atlas: {} as AtlasController })); vi.mock('./atlas/useAtlas', () => ({ useAtlas: () => mocks.atlas })); -vi.mock('../mobile/useIsPhone', () => ({ useIsPhone: () => mocks.isPhone })); vi.mock('../components/Layout/Navbar', () => ({ default: () => React.createElement('nav', { 'data-testid': 'navbar' }), })); @@ -35,7 +34,6 @@ function selectTriggers(): HTMLElement[] { } beforeEach(() => { - mocks.isPhone = false; toast.mockClear(); window.__addToast = toast; setAtlas(); @@ -54,14 +52,6 @@ afterEach(() => { }); describe('AtlasPage wiring', () => { - it('FE-PAGE-ATLASW-001: hands the whole screen to the mobile atlas on a phone', () => { - mocks.isPhone = true; - render(); - - expect(screen.getByRole('button', { name: 'atlas.bucketTab' })).toBeInTheDocument(); - expect(screen.queryByTestId('navbar')).not.toBeInTheDocument(); - }); - it('FE-PAGE-ATLASW-002: shows navbar and spinner while the atlas is loading', () => { setAtlas({ loading: true }); render(); diff --git a/client/src/pages/CollectionsPage.tsx b/client/src/pages/CollectionsPage.tsx index 4ad98fbc1..ccbe9c8a9 100644 --- a/client/src/pages/CollectionsPage.tsx +++ b/client/src/pages/CollectionsPage.tsx @@ -16,15 +16,14 @@ import CollectionPlaceDetail from '../components/Collections/CollectionPlaceDeta import LabelManager from '../components/Collections/LabelManager' import BulkAssignLabelModal from '../components/Collections/BulkAssignLabelModal' import { useCollections } from './collections/useCollections' -import { useIsPhone } from '../mobile/useIsPhone' -import MCollections from '../mobile/screens/collections/MCollections' import EmptyState from '../components/shared/EmptyState' import '../styles/dashboard.css' import '../styles/collections.css' export default function CollectionsPage(): React.ReactElement { - const isPhone = useIsPhone() - return isPhone ? : + // ViewportRoute in App.tsx picks the branch now, so the phone screen is a + // chunk of its own instead of a dead limb in this one. + return } function CollectionsPageDesktop(): React.ReactElement { diff --git a/client/src/pages/CollectionsPage.wiring.test.tsx b/client/src/pages/CollectionsPage.wiring.test.tsx index a13f395e0..59e1d4a6c 100644 --- a/client/src/pages/CollectionsPage.wiring.test.tsx +++ b/client/src/pages/CollectionsPage.wiring.test.tsx @@ -8,10 +8,7 @@ import { render, screen, fireEvent } from '../../tests/helpers/render' import CollectionsPage from './CollectionsPage' vi.mock('../components/Layout/Navbar', () => ({ default: () =>