From 3b9e5606cea7dd875dcce3e03254b771812b8a34 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 01:01:41 +0000 Subject: [PATCH] refactor(client): the planner's lazy panels and their boundary get modules of their own The lazyWithRetry declarations of the planner page move to pages/tripPlanner/plannerLazy.ts, a plain module so Fast Refresh keeps treating the page as a component boundary, and LazyPanel moves to pages/tripPlanner/LazyPanel.tsx. The page imports both. Comments travel with their code; two of them lose a dash. Nothing renders or loads differently. --- client/src/pages/TripPlannerPage.tsx | 71 ++----------------- .../src/pages/tripPlanner/LazyPanel.test.tsx | 53 ++++++++++++++ client/src/pages/tripPlanner/LazyPanel.tsx | 29 ++++++++ .../src/pages/tripPlanner/plannerLazy.test.ts | 65 +++++++++++++++++ client/src/pages/tripPlanner/plannerLazy.ts | 38 ++++++++++ 5 files changed, 191 insertions(+), 65 deletions(-) create mode 100644 client/src/pages/tripPlanner/LazyPanel.test.tsx create mode 100644 client/src/pages/tripPlanner/LazyPanel.tsx create mode 100644 client/src/pages/tripPlanner/plannerLazy.test.ts create mode 100644 client/src/pages/tripPlanner/plannerLazy.ts diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index d91991588..c185b45aa 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -36,7 +36,6 @@ import type { BookingExpenseRequest } from '../components/Planner/BookingCostsSe import type { BudgetItem } from '../types' import PluginFrame from '../components/Plugins/PluginFrame' import ErrorBoundary from '../components/shared/ErrorBoundary' -import { lazyWithRetry } from '../utils/lazyWithRetry' import { getDayBookendHotels } from '../utils/dayOrder' import TripWarningsBanner from '../components/Planner/TripWarningsBanner' import Navbar from '../components/Layout/Navbar' @@ -57,75 +56,17 @@ import { usePlannerHistory } from '../hooks/usePlannerHistory' import type { Accommodation, TripMember, Day, Place, Reservation, PackingItem, TodoItem } from '../types' import { ListTodo, ListPlus, Download, Plus, FolderPlus } from 'lucide-react' import { useTripPlannerPage } from './tripPlanner/useTripPlannerPage' +import { + ReservationsPanel, PackingListPanel, TodoListPanel, FileManager, CostsPanel, ExpenseModal, CollabPanel, + RoadtripSidebar, RoadtripCorridorPanel, RoadtripLimitsCard, RoadtripStopPopup, RoadtripStayModal, RoadtripTrackModal, RoadtripAlternativesBar, + TourPlannerRail, TourPlannerToursRail, TransportModal, +} from './tripPlanner/plannerLazy' +import { LazyPanel } from './tripPlanner/LazyPanel' import { useMergedMapPois } from '../components/Map/useMergedMapPois' import PoiCategoryPill from '../components/Map/PoiCategoryPill' import { useTouchDragBridge } from '../hooks/useTouchDragBridge' import PanelResizeHandle from '../components/Planner/PanelResizeHandle' -// The tab panels are the planner's dead weight: each one mounts only while its -// own tab is active, so the page chunk carried code most sessions never run. They -// load on demand now, through the same lazyWithRetry the route chunks use. -// -// PluginFrame stays static on purpose: DayDetailPanel and PlaceInspector import it -// too and both belong to the plan tab, so splitting it here would move nothing. -const ReservationsPanel = lazyWithRetry(() => import('../components/Planner/ReservationsPanel')) -const PackingListPanel = lazyWithRetry(() => import('../components/Packing/PackingListPanel')) -const TodoListPanel = lazyWithRetry(() => import('../components/Todo/TodoListPanel')) -const FileManager = lazyWithRetry(() => import('../components/Files/FileManager')) -const CostsPanel = lazyWithRetry(() => import('../components/Budget/CostsPanel')) -// Named export, so it needs the extra hop. Importing it statically would keep the -// whole CostsPanel module in the page chunk and undo the split above. -const ExpenseModal = lazyWithRetry(() => - import('../components/Budget/CostsPanel').then(m => ({ default: m.ExpenseModal })) -) -const CollabPanel = lazyWithRetry(() => import('../components/Collab/CollabPanel')) -const RoadtripSidebar = lazyWithRetry(() => import('../components/Roadtrip/RoadtripSidebar')) -const RoadtripCorridorPanel = lazyWithRetry(() => import('../components/Roadtrip/RoadtripCorridorPanel')) -const RoadtripLimitsCard = lazyWithRetry(() => import('../components/Roadtrip/RoadtripLimitsCard')) -const RoadtripStopPopup = lazyWithRetry(() => import('../components/Roadtrip/RoadtripStopPopup')) -const RoadtripStayModal = lazyWithRetry(() => import('../components/Roadtrip/RoadtripStayModal')) -const RoadtripTrackModal = lazyWithRetry(() => import('../components/Roadtrip/RoadtripTrackModal')) -const RoadtripAlternativesBar = lazyWithRetry(() => import('../components/Roadtrip/RoadtripAlternativesBar')) -const TourPlannerRail = lazyWithRetry(() => - import('../components/Tours/planner/TourPlannerPanels').then(module => ({ default: module.TourPlannerRail })) -) -const TourPlannerToursRail = lazyWithRetry(() => - import('../components/Tours/planner/TourPlannerPanels').then(module => ({ default: module.TourPlannerToursRail })) -) -// Already rendered conditionally, so lazy bites immediately. Worth it beyond its -// own 63 kB: it is the only path to TransitSearchPanel, which drags in tz-lookup -// — about 200 kB of packed zone geometry that every trip used to load. -const TransportModal = lazyWithRetry(() => - import('../components/Planner/TransportModal').then(m => ({ default: m.TransportModal })) -) - -/** - * One tab panel, with its own net. - * - * The boundary sits outside the Suspense, not inside: Suspense owns the pending - * promise, a rejected one throws straight past it. And it has to be per panel — - * a single boundary around the whole content area would already be mounted with - * the visible tab, so switching tabs would swap the entire planner for the - * placeholder instead of just the part that is still loading. - * - * No label: ErrorBoundary lets label win over the panel level and would title a - * broken packing list "This plugin could not be shown". - */ -function LazyPanel({ id, children, overlay }: { id: string; children: React.ReactNode; overlay?: boolean }): React.ReactElement { - return ( - - {/* A panel holds its place with a skeleton while its chunk arrives; a dialog has no - place to hold. Drawn in the page flow, that skeleton was a pale block flashing - under the planner the first time each dialog was ever opened, and never again - once the chunk was cached. Nothing is the right placeholder for something that - is about to cover the screen anyway. */} - }> - {children} - - - ) -} - function ListsContainer({ tripId, packingItems, todoItems }: { tripId: number; packingItems: PackingItem[]; todoItems: TodoItem[] }) { const [subTab, setSubTab] = useState<'packing' | 'todo'>(() => { return (sessionStorage.getItem(`trip-lists-subtab-${tripId}`) as 'packing' | 'todo') || 'packing' diff --git a/client/src/pages/tripPlanner/LazyPanel.test.tsx b/client/src/pages/tripPlanner/LazyPanel.test.tsx new file mode 100644 index 000000000..56ad3bc10 --- /dev/null +++ b/client/src/pages/tripPlanner/LazyPanel.test.tsx @@ -0,0 +1,53 @@ +// FE-PAGE-PLANNER-LAZYPANEL-001 to FE-PAGE-PLANNER-LAZYPANEL-004 +import React, { lazy } from 'react' +import { render, screen } from '../../../tests/helpers/render' +import { LazyPanel } from './LazyPanel' + +// A chunk that never arrives, so the fallback stays on screen. +const Pending = lazy(() => new Promise<{ default: React.ComponentType }>(() => {})) + +let consoleError: ReturnType +beforeEach(() => { + // React and the boundary both log a caught error; keep the output readable. + consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) +}) +afterEach(() => { + consoleError.mockRestore() +}) + +describe('LazyPanel', () => { + it('FE-PAGE-PLANNER-LAZYPANEL-001: renders the panel once its chunk is there', async () => { + const Loaded = lazy(async () => ({ default: () =>

packing list

})) + render() + + expect(await screen.findByText('packing list')).toBeInTheDocument() + }) + + it('FE-PAGE-PLANNER-LAZYPANEL-002: a panel holds its place with a skeleton while loading', () => { + const { container } = render() + + const skeleton = container.querySelector('.animate-pulse') + expect(skeleton).not.toBeNull() + expect(skeleton).toHaveClass('min-h-[180px]', 'bg-surface-secondary') + }) + + it('FE-PAGE-PLANNER-LAZYPANEL-003: an overlay shows nothing while its chunk loads', () => { + const { container } = render() + + expect(container).toBeEmptyDOMElement() + }) + + it('FE-PAGE-PLANNER-LAZYPANEL-004: a chunk that fails stays inside its own panel', async () => { + const Broken = lazy(() => Promise.reject(new Error('panel exploded'))) + render( +
+

planner frame

+ +
, + ) + + expect(await screen.findByRole('alert')).toBeInTheDocument() + expect(screen.getByText('planner frame')).toBeInTheDocument() + expect(consoleError).toHaveBeenCalledWith('[ErrorBoundary:planner-panel:collab]', expect.any(Error), expect.anything()) + }) +}) diff --git a/client/src/pages/tripPlanner/LazyPanel.tsx b/client/src/pages/tripPlanner/LazyPanel.tsx new file mode 100644 index 000000000..3318faf8f --- /dev/null +++ b/client/src/pages/tripPlanner/LazyPanel.tsx @@ -0,0 +1,29 @@ +import React, { Suspense } from 'react' +import ErrorBoundary from '../../components/shared/ErrorBoundary' + +/** + * One tab panel, with its own net. + * + * The boundary sits outside the Suspense, not inside: Suspense owns the pending + * promise, a rejected one throws straight past it. And it has to be per panel: + * a single boundary around the whole content area would already be mounted with + * the visible tab, so switching tabs would swap the entire planner for the + * placeholder instead of just the part that is still loading. + * + * No label: ErrorBoundary lets label win over the panel level and would title a + * broken packing list "This plugin could not be shown". + */ +export function LazyPanel({ id, children, overlay }: Readonly<{ id: string; children: React.ReactNode; overlay?: boolean }>): React.ReactElement { + return ( + + {/* A panel holds its place with a skeleton while its chunk arrives; a dialog has no + place to hold. Drawn in the page flow, that skeleton was a pale block flashing + under the planner the first time each dialog was ever opened, and never again + once the chunk was cached. Nothing is the right placeholder for something that + is about to cover the screen anyway. */} + }> + {children} + + + ) +} diff --git a/client/src/pages/tripPlanner/plannerLazy.test.ts b/client/src/pages/tripPlanner/plannerLazy.test.ts new file mode 100644 index 000000000..dcf60c73d --- /dev/null +++ b/client/src/pages/tripPlanner/plannerLazy.test.ts @@ -0,0 +1,65 @@ +// FE-PAGE-PLANNER-LAZY-001 to FE-PAGE-PLANNER-LAZY-002 +// +// Every planner panel loads as a chunk of its own. The targets are stubbed so +// each loader, and each hop to a named export, is checked by what it renders. +import { createElement, Suspense, type ComponentType } from 'react' +import { render, screen } from '@testing-library/react' +import * as lazy from './plannerLazy' + +// Hoisted with the mocks: each stub renders a line of text naming its module. +const label = vi.hoisted(() => (text: string) => () => text) + +vi.mock('../../components/Planner/ReservationsPanel', () => ({ default: label('reservations panel') })) +vi.mock('../../components/Packing/PackingListPanel', () => ({ default: label('packing list panel') })) +vi.mock('../../components/Todo/TodoListPanel', () => ({ default: label('todo list panel') })) +vi.mock('../../components/Files/FileManager', () => ({ default: label('file manager') })) +vi.mock('../../components/Budget/CostsPanel', () => ({ default: label('costs panel'), ExpenseModal: label('expense modal') })) +vi.mock('../../components/Collab/CollabPanel', () => ({ default: label('collab panel') })) +vi.mock('../../components/Roadtrip/RoadtripSidebar', () => ({ default: label('roadtrip sidebar') })) +vi.mock('../../components/Roadtrip/RoadtripCorridorPanel', () => ({ default: label('roadtrip corridor panel') })) +vi.mock('../../components/Roadtrip/RoadtripLimitsCard', () => ({ default: label('roadtrip limits card') })) +vi.mock('../../components/Roadtrip/RoadtripStopPopup', () => ({ default: label('roadtrip stop popup') })) +vi.mock('../../components/Roadtrip/RoadtripStayModal', () => ({ default: label('roadtrip stay modal') })) +vi.mock('../../components/Roadtrip/RoadtripTrackModal', () => ({ default: label('roadtrip track modal') })) +vi.mock('../../components/Roadtrip/RoadtripAlternativesBar', () => ({ default: label('roadtrip alternatives bar') })) +vi.mock('../../components/Tours/planner/TourPlannerPanels', () => ({ + TourPlannerRail: label('tour planner rail'), + TourPlannerToursRail: label('tour planner tours rail'), +})) +vi.mock('../../components/Planner/TransportModal', () => ({ TransportModal: label('transport modal') })) + +const cases: Array<[keyof typeof lazy, string]> = [ + ['ReservationsPanel', 'reservations panel'], + ['PackingListPanel', 'packing list panel'], + ['TodoListPanel', 'todo list panel'], + ['FileManager', 'file manager'], + ['CostsPanel', 'costs panel'], + ['ExpenseModal', 'expense modal'], + ['CollabPanel', 'collab panel'], + ['RoadtripSidebar', 'roadtrip sidebar'], + ['RoadtripCorridorPanel', 'roadtrip corridor panel'], + ['RoadtripLimitsCard', 'roadtrip limits card'], + ['RoadtripStopPopup', 'roadtrip stop popup'], + ['RoadtripStayModal', 'roadtrip stay modal'], + ['RoadtripTrackModal', 'roadtrip track modal'], + ['RoadtripAlternativesBar', 'roadtrip alternatives bar'], + ['TourPlannerRail', 'tour planner rail'], + ['TourPlannerToursRail', 'tour planner tours rail'], + ['TransportModal', 'transport modal'], +] + +function renderLazy(name: keyof typeof lazy) { + const Panel = lazy[name] as ComponentType + return render(createElement(Suspense, { fallback: 'loading' }, createElement(Panel))) +} + +describe('plannerLazy', () => { + it('FE-PAGE-PLANNER-LAZY-001: every panel the planner loads on demand is listed here', () => { + expect(Object.keys(lazy).sort()).toEqual(cases.map(([name]) => name).sort()) + }) + + it.each(cases)('FE-PAGE-PLANNER-LAZY-002: %s loads its own component', async (name, text) => { + renderLazy(name) + expect(await screen.findByText(text)).toBeInTheDocument() + }) +}) diff --git a/client/src/pages/tripPlanner/plannerLazy.ts b/client/src/pages/tripPlanner/plannerLazy.ts new file mode 100644 index 000000000..e94c9efac --- /dev/null +++ b/client/src/pages/tripPlanner/plannerLazy.ts @@ -0,0 +1,38 @@ +import { lazyWithRetry } from '../../utils/lazyWithRetry' + +// The tab panels are the planner's dead weight: each one mounts only while its +// own tab is active, so the page chunk carried code most sessions never run. They +// load on demand now, through the same lazyWithRetry the route chunks use. +// +// PluginFrame stays static on purpose: DayDetailPanel and PlaceInspector import it +// too and both belong to the plan tab, so splitting it here would move nothing. +export const ReservationsPanel = lazyWithRetry(() => import('../../components/Planner/ReservationsPanel')) +export const PackingListPanel = lazyWithRetry(() => import('../../components/Packing/PackingListPanel')) +export const TodoListPanel = lazyWithRetry(() => import('../../components/Todo/TodoListPanel')) +export const FileManager = lazyWithRetry(() => import('../../components/Files/FileManager')) +export const CostsPanel = lazyWithRetry(() => import('../../components/Budget/CostsPanel')) +// Named export, so it needs the extra hop. Importing it statically would keep the +// whole CostsPanel module in the page chunk and undo the split above. +export const ExpenseModal = lazyWithRetry(() => + import('../../components/Budget/CostsPanel').then(m => ({ default: m.ExpenseModal })) +) +export const CollabPanel = lazyWithRetry(() => import('../../components/Collab/CollabPanel')) +export const RoadtripSidebar = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripSidebar')) +export const RoadtripCorridorPanel = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripCorridorPanel')) +export const RoadtripLimitsCard = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripLimitsCard')) +export const RoadtripStopPopup = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripStopPopup')) +export const RoadtripStayModal = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripStayModal')) +export const RoadtripTrackModal = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripTrackModal')) +export const RoadtripAlternativesBar = lazyWithRetry(() => import('../../components/Roadtrip/RoadtripAlternativesBar')) +export const TourPlannerRail = lazyWithRetry(() => + import('../../components/Tours/planner/TourPlannerPanels').then(module => ({ default: module.TourPlannerRail })) +) +export const TourPlannerToursRail = lazyWithRetry(() => + import('../../components/Tours/planner/TourPlannerPanels').then(module => ({ default: module.TourPlannerToursRail })) +) +// Already rendered conditionally, so lazy bites immediately. Worth it beyond its +// own 63 kB: it is the only path to TransitSearchPanel, which drags in tz-lookup, +// about 200 kB of packed zone geometry that every trip used to load. +export const TransportModal = lazyWithRetry(() => + import('../../components/Planner/TransportModal').then(m => ({ default: m.TransportModal })) +)