steve/pansy
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

Route-level code splitting for faster mobile first paint (#106) #113

Merged
steve merged 2 commits from feat/code-splitting into main 2026-07-22 06:18:23 +00:00
Conversation 3 Commits 2 Files Changed 3
+39 -7
3 changed files with 39 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 99798db8f6 - Show all commits
web/src/components/layout/AppShell.tsx
+5 -2
View File
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react'
import { Suspense, useEffect, useState } from 'react'
import { Link, Outlet, useMatchRoute, useNavigate, useRouterState } from '@tanstack/react-router'
import { Toaster } from '@/components/ui/toast'
import { useLogout, useMe } from '@/lib/auth'
@@ -93,7 +93,10 @@ export function AppShell() {
showBottomNav && 'pb-[calc(3.5rem+env(safe-area-inset-bottom))] md:pb-6',
)}
>
<Outlet />
{/* Boundary for the lazily-loaded routes (see router.tsx). */}
<Suspense fallback={<p className="p-6 text-sm text-muted">Loading…</p>}>
gitea-actions commented 2026-07-22 06:14:31 +00:00
Review
Copy Link
Copy Source

🟠 Lazy-route chunk load failures are unrecoverable: the error boundary's retry (router.invalidate()) doesn't reload the module, so a stale-deploy client is stuck failing forever

error-handling · flagged by 1 model

  • web/src/components/layout/AppShell.tsx:97 (Suspense boundary for the new lazy routes) + web/src/router.tsx:14-33 (the lazy() definitions) + web/src/components/RouteError.tsx:14: no handling for a chunk-load failure on a stale client. React.lazy(load) memoizes the import() promise once per component definition — if it rejects (network blip, or the classic case here: the SPA stays open in a tab across a deploy, and the hashed chunk filename referenced by the already-loaded `index.htm…

🪰 Gadfly · advisory

🟠 **Lazy-route chunk load failures are unrecoverable: the error boundary's retry (router.invalidate()) doesn't reload the module, so a stale-deploy client is stuck failing forever** _error-handling · flagged by 1 model_ - `web/src/components/layout/AppShell.tsx:97` (Suspense boundary for the new lazy routes) + `web/src/router.tsx:14-33` (the `lazy()` definitions) + `web/src/components/RouteError.tsx:14`: no handling for a chunk-load failure on a stale client. `React.lazy(load)` memoizes the `import()` promise once per component definition — if it rejects (network blip, or the classic case here: the SPA stays open in a tab across a deploy, and the hashed chunk filename referenced by the already-loaded `index.htm… <sub>🪰 Gadfly · advisory</sub>
<Outlet />
</Suspense>
</main>
{showBottomNav && <BottomNav sections={visibleSections} />}
web/src/router.tsx
+20 -5
View File
@@ -1,3 +1,4 @@
import { lazy } from 'react'
import {
createRootRouteWithContext,
createRoute,
@@ -9,13 +10,27 @@ import { AppShell } from '@/components/layout/AppShell'
import { NotFound } from '@/components/NotFound'
import { RouteError } from '@/components/RouteError'
import { LoginPage } from '@/pages/LoginPage'
import { RegisterPage } from '@/pages/RegisterPage'
import { GardensPage } from '@/pages/GardensPage'
import { GardenEditorPage } from '@/pages/GardenEditorPage'
import { PublicGardenPage } from '@/pages/PublicGardenPage'
import { PlantsPage } from '@/pages/PlantsPage'
import { SettingsPage } from '@/pages/SettingsPage'
import { meQueryOptions } from '@/lib/auth'
// Code-split the heavier / deeper routes so a phone on cell data doesn't download
// the whole app (notably the canvas editor with its gesture + geometry deps)
// before the first screen paints. Login and the gardens list — the entry points —
// stay eager to avoid a fallback flash on landing; AppShell wraps the Outlet in a
// Suspense boundary for the rest.
const GardenEditorPage = lazy(() =>
gitea-actions commented 2026-07-22 06:14:31 +00:00
Outdated
Review
Copy Link
Copy Source

🟠 No recovery path for a failed lazy-route chunk fetch (stale deploy hash or dropped mobile request) — RouteError's retry doesn't refetch the script and React.lazy never retries a rejected import

correctness, error-handling, maintainability, performance · flagged by 3 models

  • web/src/router.tsx:21-36 — the five lazy(...) declarations sit between the static imports at lines 1-14 and further static imports at lines 34-36 (queryClient, safeRedirectPath, getLastGardenId), splitting the file's import block in two. Confirmed against the actual file: lines 34-36 are genuine import statements that land after code (the lazy consts), not before it. Other files in the codebase (e.g. web/src/pages/GardensPage.tsx:1-12) keep all imports contiguous at the top. This…

🪰 Gadfly · advisory

🟠 **No recovery path for a failed lazy-route chunk fetch (stale deploy hash or dropped mobile request) — RouteError's retry doesn't refetch the script and React.lazy never retries a rejected import** _correctness, error-handling, maintainability, performance · flagged by 3 models_ - `web/src/router.tsx:21-36` — the five `lazy(...)` declarations sit between the static imports at lines 1-14 and further static imports at lines 34-36 (`queryClient`, `safeRedirectPath`, `getLastGardenId`), splitting the file's import block in two. Confirmed against the actual file: lines 34-36 are genuine `import` statements that land after code (the lazy consts), not before it. Other files in the codebase (e.g. `web/src/pages/GardensPage.tsx:1-12`) keep all imports contiguous at the top. This… <sub>🪰 Gadfly · advisory</sub>
import('@/pages/GardenEditorPage').then((m) => ({ default: m.GardenEditorPage })),
)
const PublicGardenPage = lazy(() =>
import('@/pages/PublicGardenPage').then((m) => ({ default: m.PublicGardenPage })),
)
const PlantsPage = lazy(() => import('@/pages/PlantsPage').then((m) => ({ default: m.PlantsPage })))
gitea-actions commented 2026-07-22 06:14:31 +00:00
Outdated
Review
Copy Link
Copy Source

⚪ Inconsistent formatting of lazy() declarations in same block

maintainability · flagged by 2 models

  • web/src/router.tsx:34-36 — Import ordering violation: import { queryClient }, import { safeRedirectPath }, and import { getLastGardenId } are placed after the const lazy declarations. Standard convention (and the pattern used everywhere else in web/src/) keeps all imports at the top of the file. This hurts readability and will likely trigger lint rules. - web/src/router.tsx:27 — Formatting inconsistency: PlantsPage is flattened to a single line while every other `lazy()…

🪰 Gadfly · advisory

⚪ **Inconsistent formatting of lazy() declarations in same block** _maintainability · flagged by 2 models_ - `web/src/router.tsx:34-36` — **Import ordering violation**: `import { queryClient }`, `import { safeRedirectPath }`, and `import { getLastGardenId }` are placed after the `const` lazy declarations. Standard convention (and the pattern used everywhere else in `web/src/`) keeps all imports at the top of the file. This hurts readability and will likely trigger lint rules. - `web/src/router.tsx:27` — **Formatting inconsistency**: `PlantsPage` is flattened to a single line while every other `lazy()… <sub>🪰 Gadfly · advisory</sub>
const SettingsPage = lazy(() =>
import('@/pages/SettingsPage').then((m) => ({ default: m.SettingsPage })),
)
const RegisterPage = lazy(() =>
import('@/pages/RegisterPage').then((m) => ({ default: m.RegisterPage })),
)
import { queryClient } from '@/lib/queryClient'
import { safeRedirectPath } from '@/lib/redirect'
import { getLastGardenId } from '@/lib/lastGarden'
web/vite.config.ts
+14
View File
@@ -29,6 +29,20 @@ export default defineConfig(({ mode }) => {
build: {
outDir: 'dist',
sourcemap: true,
rollupOptions: {
output: {
// One vendor chunk for ALL node_modules: it's rarely-changing, so it
// caches across app deploys while the tiny app chunk churns. Kept as a
// SINGLE chunk on purpose — splitting react-dom/scheduler into their own
// chunk reorders their module init across chunk boundaries and breaks
// React 19 at load ("Cannot set 'Activity' of undefined"). The heavy
// routes are code-split separately via React.lazy (router.tsx), which is
// where the real first-paint win is.
manualChunks(id) {
gitea-actions commented 2026-07-22 06:14:31 +00:00
Review
Copy Link
Copy Source

🟠 manualChunks forces editor-only deps (@use-gesture, geometry) into the eager vendor chunk, defeating the first-paint split for those libraries

performance · flagged by 1 model

  • web/vite.config.ts:41-43 — the gesture/geometry deps the PR wants off the first-paint path are actually pulled into it. The manualChunks(id) function unconditionally returns 'vendor' for every node_modules module, and that vendor chunk is on the critical path for every route (including the eager /login and /gardens). I verified that @use-gesture/react (the dep the PR description names as the biggest surface to keep lazy) is only referenced from `web/src/editor/useView…

🪰 Gadfly · advisory

🟠 **manualChunks forces editor-only deps (@use-gesture, geometry) into the eager vendor chunk, defeating the first-paint split for those libraries** _performance · flagged by 1 model_ - **`web/vite.config.ts:41-43` — the gesture/geometry deps the PR wants off the first-paint path are actually pulled *into* it.** The `manualChunks(id)` function unconditionally returns `'vendor'` for **every** `node_modules` module, and that vendor chunk is on the critical path for *every* route (including the eager `/login` and `/gardens`). I verified that `@use-gesture/react` (the dep the PR description names as the biggest surface to keep lazy) is only referenced from `web/src/editor/useView… <sub>🪰 Gadfly · advisory</sub>
return id.includes('node_modules') ? 'vendor' : undefined
},
},
},
},
}
})