Skip to content

perf: code-split routes and add per-route titles - #8

Closed
donbr wants to merge 4 commits into
mainfrom
feat/code-split-and-titles
Closed

donbr wants to merge 4 commits into
mainfrom
feat/code-split-and-titles

Conversation

@donbr

@donbr donbr commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Summary

PR A from the next-priorities review: two quick wins with no visual change and no content decisions.

  1. Code-split routes (321ef04): every page shipped one 2.1 MB bundle (678 KB gzipped) containing ECharts, Cytoscape and Leaflet, which only the visualization pages use. HomePage and NotFoundPage stay eager; Projects, Concepts and the six project routes load with React.lazy inside one Suspense boundary. The fallback renders inside Layout, so the nav and footer stay put while a chunk loads.
  2. Per-route titles (6027a0c): all routes shared one tab title. Each page now renders <title>Page Name | Don Branson</title>, which React 19 hoists into <head>. The static title in index.html stays as the no-JS fallback.
  3. Docs (4d570d6): CLAUDE.md said no lazy loading was used; updated, with the title convention noted.

Bundle sizes (vite build)

Before After
Script loaded by the homepage 2,128.6 KB (678.4 KB gz) 275.8 KB (86.1 KB gz)
Largest route chunk (single bundle) SituationalAwareness 1,058.7 KB (351.5 KB gz)

Test plan

Production build served by vite preview, driven with Playwright:

  • npm run build passes; npx tsc -p tsconfig.app.json --noEmit passes
  • npm run lint: 0 errors, same 3 pre-existing warnings
  • Homepage requests only the main script (no route chunks)
  • Direct loads of all 8 non-home routes render their H1 with the correct title and no page errors
  • Cytoscape (3 canvases) and Situational Awareness (ECharts canvas) render; GDELT viewer's Map tab renders Leaflet with 9 markers
  • Client-side navigation Home → Projects → Home updates the tab title each time
  • Trial merge with fix: P0 design-review items — SPA deep links, mobile nav, OG image, card links #7 (feat/redesign) is clean (ProjectsPage.tsx auto-merges)

Notes

Update: Concepts page removal

0a0ad03 deletes src/pages/ConceptsPage.tsx (generated in b03de6c, opening with a fabricated anecdote), its lazy route, and the CLAUDE.md nav mention. /concepts now falls through to the 404 page. The nav link and sitemap entry are removed in #7, which should merge first.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ppa5DyJCZ1CowKtz58QARN

donbr and others added 3 commits October 2, 2026 13:41
All routes shipped in one 2.1 MB bundle (678 KB gzipped), so the homepage
downloaded ECharts, Cytoscape and Leaflet even though only the visualization
pages use them. Projects, Concepts and the six project routes now load with
React.lazy inside one Suspense boundary whose fallback keeps the Layout.
Homepage script: 275.8 KB (86 KB gzipped).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ppa5DyJCZ1CowKtz58QARN
All routes showed the same tab title, so bookmarks and history entries for a
case study were indistinguishable from the homepage. Each page now renders a
<title> ("Page Name | Don Branson") that React 19 hoists into <head>; the
static title in index.html remains the no-JS fallback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ppa5DyJCZ1CowKtz58QARN
@claude

claude Bot commented Oct 2, 2026

Copy link
Copy Markdown

Review: clean, well-scoped change. Bundle savings are real and the test plan is thorough. Nothing blocking; suggestions below.

Suggestions

  1. No error boundary for chunk-load failures (main gap). On GitHub Pages, hashed chunk names change on every deploy. A user with a stale tab who navigates to a lazy route gets a failed dynamic import, and with no error boundary the app unmounts to a blank page. Wrap the Suspense in a small error boundary offering a reload, or retry the import once and then call location.reload().
  2. Layout remounts when the fallback swaps in. RouteFallback renders its own Layout and each page renders another Layout inside Routes. React treats these as different trees, so nav and footer remount rather than staying put as the description says (state and scroll can reset). Hoisting one Layout above Suspense would fix it but touches every page, so it is likely out of scope. On client-side navigation the old page usually stays visible during the transition, so this mostly affects direct loads.
  3. Title duplication. Eleven copies of the Don Branson suffix is fine for now. A tiny PageTitle helper would centralise the format, which helps if fix: P0 design-review items — SPA deep links, mobile nav, OG image, card links #7 touches it.
  4. Follow-ups. Import only the needed ECharts modules (as noted in the PR), and consider prefetching likely routes on idle or hover.

Other areas

  • Security: no concerns.
  • Tests: no test framework exists, so the Playwright verification is appreciated. A stale-chunk test would be worth adding if tests are introduced.
  • Docs: the CLAUDE.md updates are accurate.

Good to merge once the error-boundary point is considered.

🤖 Generated with Claude Code

The page was generated in b03de6c and opened with an anecdote that never
happened. Removes ConceptsPage.tsx, its lazy route and the CLAUDE.md nav
mention. /concepts now falls through to the 404 page. The nav link and sitemap
entry are removed in #7, which should merge first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ppa5DyJCZ1CowKtz58QARN
donbr added a commit that referenced this pull request Oct 2, 2026
The Concepts page is being removed (its route and file go in #8). Removes the
nav item and the /concepts sitemap URL so nothing on the site links to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ppa5DyJCZ1CowKtz58QARN
@claude

claude Bot commented Oct 2, 2026

Copy link
Copy Markdown

Review

Clean, well-scoped PR. Lazy routes with one Suspense boundary, the Layout-wrapped fallback, and per-page <title> (React 19 hoisting) are all correct, and the bundle numbers are a real win. CLAUDE.md is updated to match.

Things to consider

  1. Double Layout mount on route transitions. RouteFallback renders its own <Layout>, and each page renders <Layout> again. When the chunk resolves, React swaps one Layout tree for another, so Layout state (e.g. a mobile menu open state) resets and the nav may flash. A parent layout route (<Route element={<Layout><Outlet/></Layout>}>) would avoid this, but it's a bigger refactor. Fine to leave for now.
  2. No error boundary for chunk load failures. After a GitHub Pages redeploy, hashed chunk filenames change. A user with a stale tab who navigates to a lazy route gets a failed dynamic import, which means a blank page, since there's no boundary. A small ErrorBoundary offering a reload would cover this.
  3. Default-export assumption. lazy(() => import(...)) needs a default export in each module. They all have one today and tsc would catch a miss. This is only a note for new routes.
  4. Deep-link 404 on GitHub Pages. Not new in this PR, but BrowserRouter on Pages needs a 404.html redirect for direct loads of /assets/projects/.... vite preview has SPA fallback, so your testing doesn't exercise this. Worth confirming that production behaves the same.
  5. Concepts removal is mixed into a "no content decisions" PR. 0a0ad03 deletes a page and makes /concepts a 404. It depends on fix: P0 design-review items — SPA deep links, mobile nav, OG image, card links #7 merging first (nav link and sitemap entry). If this merges first, the nav links to a 404. Consider stacking on fix: P0 design-review items — SPA deep links, mobile nav, OG image, card links #7 or keeping the ordering note prominent.
  6. Titles. The <title> strings are duplicated as literals across 10 files. Fine at this size. A tiny <PageTitle> helper would centralize the | Don Branson suffix if more pages are added.
  7. Tests. No test framework is configured (per CLAUDE.md), so the Playwright checks in the description are the only coverage. Nothing to add for this PR.

Security / performance: No concerns. SituationalAwareness is still a ~1 MB chunk, so a manualChunks split for echarts, or a prefetch on link hover/focus, could be a follow-up.

Overall: LGTM. #2 (error boundary) and #5 (merge ordering) are the most worthwhile to address.

@donbr

donbr commented Oct 2, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #9, which combines #7 and #8 into one PR (same changes, plus the Concepts page removal as a single commit).

@donbr donbr closed this Oct 2, 2026
@donbr
donbr deleted the feat/code-split-and-titles branch October 3, 2026 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant