perf: lazy-load term catalogs + vendor code splitting (8.2 MB → ~1.0 MB initial JS)#164
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
caltech-dev | 9e6810e | Commit Preview URL Branch Preview URL |
Jun 11 2026, 04:37 AM |
| export async function loadCourseIndex(term: string): Promise<CourseIndex> { | ||
| const cached = courseIndexCache[term]; | ||
| if (cached) { | ||
| return cached; | ||
| } | ||
| const loader = courseDataLoaders[term]; | ||
| if (!loader) { | ||
| return {}; | ||
| } | ||
| const module = await loader(); | ||
| courseIndexCache[term] = module.default; | ||
| return module.default; | ||
| } |
There was a problem hiding this comment.
📝 Info: Duplicate concurrent loads possible in loadCourseIndex
The loadCourseIndex function at src/courseData.ts:27-39 checks the cache synchronously then awaits the loader. If two callers invoke it for the same term before the first resolves, both will miss the cache and trigger separate dynamic imports. This is harmless (JS runtimes typically deduplicate import() for the same module, and both writes set identical data) but wasteful. Storing the in-flight promise in a secondary map would deduplicate cleanly. In practice this is unlikely to be hit because the useLayoutEffect in src/useAppState.ts:69-89 only fires on realPath changes and the cleanup cancels stale state updates, but React Strict Mode in development will trigger it on every mount.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — concurrent duplicate fetches are harmless (idempotent cache write) and the cancelled flag prevents stale state, so leaving as-is to keep the loader simple.
| chunkSizeWarningLimit: 700, | ||
| rollupOptions: { | ||
| output: { | ||
| manualChunks: { | ||
| react: ["react", "react-dom", "react-dom/client"], | ||
| mui: ["@mui/material", "@mui/icons-material"], | ||
| schedulex: [ | ||
| "@schedule-x/calendar", | ||
| "@schedule-x/react", | ||
| "@schedule-x/theme-default", | ||
| "@schedule-x/events-service", | ||
| ], | ||
| motion: ["motion"], | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
📝 Info: Vite manual chunks configuration looks correct
The manualChunks configuration in vite.config.ts splits React, MUI, schedule-x, and motion into separate vendor chunks. This is a standard code-splitting pattern. The react-dom/client entry in the react chunk is valid—Vite/Rollup resolves it as a subpath of react-dom. The chunkSizeWarningLimit: 700 bump from the default 500kB is reasonable given the large JSON data files that will now be separate chunks due to dynamic imports.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
📝 Info: WorkspaceSearch Fzf instance recreated every render
At src/Workspace.tsx:437, new Fzf(courses, ...) is created on every render of WorkspaceSearch, not memoized. This is a pre-existing issue, not introduced by this PR. With lazy loading, courses is now initially [] and then becomes the full list after loading, so the Fzf instance will be recreated with an empty array first and then with the full array. The firstLoad workaround at line 418 handles the options state, but the Fzf instance itself is always up-to-date since it's recreated from the current courses value.
Was this helpful? React with 👍 or 👎 to provide feedback.
| setWorkspaceIdx(storedIdx ? JSON.parse(storedIdx) : 0); | ||
| }, [realPath, defaultWorkspaces, setWorkspaces, setWorkspaceIdx]); | ||
|
|
||
| const catalogReady = Object.keys(indexedCourses).length > 0; |
There was a problem hiding this comment.
📝 Info: catalogReady blocks all operations during initial async load
The catalogReady guard at line 121 (Object.keys(indexedCourses).length > 0) disables all course operations (add, remove, toggle, lock, navigate arrangements, etc.) while course data is loading. Previously, with static imports, data was immediately available. Now with dynamic imports, there's a loading window where users can see their existing courses (from localStorage) but cannot interact with them. This is actually necessary to prevent crashes — lengthenCourses (src/appContext.ts:80) would produce undefined courseData if indexedCourses is empty. The trade-off is acceptable but represents a UX change: operations silently no-op during loading with no user feedback.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — the guard is intentional to prevent crashes/data loss during the load window. The search Select shows "Loading courses..." as feedback; the load typically completes in well under a second.
| "/fa2026": () => import("./data/IndexedTotalFA2025-26.json"), | ||
| "/wi2026": () => import("./data/IndexedTotalWI2025-26.json"), | ||
| "/sp2026": () => import("./data/IndexedTotalSP2025-26.json"), | ||
| "/fa2027": () => import("./data/IndexedTotalFA2026-27.json"), |
There was a problem hiding this comment.
📝 Info: Invalid terms return {} without caching — acceptable but different from valid empty catalogs
When loadCourseIndex is called with a term that has no entry in courseDataLoaders (e.g., /invalid), it returns {} at src/courseData.ts:17 without caching the result. This means future calls re-check the loaders map. In contrast, a valid term whose JSON happens to be {} gets cached at line 37. Both result in catalogReady = false, blocking operations. The non-caching for invalid terms is fine since the useEffect only calls loadCourseIndex once per realPath change, so there's no repeated lookup concern.
Was this helpful? React with 👍 or 👎 to provide feedback.
| setWorkspaceIdx(storedIdx ? JSON.parse(storedIdx) : 0); | ||
| }, [realPath, defaultWorkspaces, setWorkspaces, setWorkspaceIdx]); | ||
|
|
||
| const catalogReady = Object.keys(indexedCourses).length > 0; |
There was a problem hiding this comment.
📝 Info: catalogReady conflates 'not yet loaded' with 'empty catalog'
At src/useAppState.ts:127, catalogReady is derived as Object.keys(indexedCourses).length > 0. This means an invalid/unknown term (where loadCourseIndex returns {} at src/courseData.ts:17) produces the same catalogReady = false as a term whose data is still loading. All workspace operations are permanently blocked for invalid terms. This is arguably better than the previous behavior (where lengthenCourses would produce undefined courseData for invalid terms), but it means there's no user-facing distinction between 'loading' and 'unsupported term'. A separate isLoading flag or loading an explicit sentinel would make these states distinguishable.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — invalid terms previously also produced a broken state (undefined courseData); now they safely no-op. A distinct "unsupported term" UI would be a nice follow-up but is out of scope for this perf PR.
| setIndexedCourses(cached); | ||
| return; | ||
| } | ||
| setIndexedCourses({}); |
There was a problem hiding this comment.
📝 Info: setIndexedCourses({}) in useLayoutEffect triggers an unnecessary re-render on initial mount
At src/useAppState.ts:76, when the cache is not populated (first visit to a term), setIndexedCourses({}) is called even though the state was just initialized to {} via the lazy initializer at line 65. Since React compares state by reference (Object.is), the new {} is a different object from the initial {}, triggering an extra synchronous re-render before paint. This is harmless but wasteful. Comparing against the current state or using a ref to skip the redundant update would eliminate the extra render cycle.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — the extra pre-paint re-render only happens on the first visit to an uncached term and is not user-visible; keeping the code simple here.
Manual test results — all passedTested in the dev server (recorded): initial load + course search + schedule build on fa2027, term switch to /wi2026 with lazy catalog fetch, and persisted-workspace restore on reload. Verified via Details
Screenshots: Session: https://app.devin.ai/sessions/8b9a591e4ca64813a5ecf3235b854611 |
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
680d747 to
21339b8
Compare
| useLayoutEffect(() => { | ||
| let cancelled = false; | ||
| const cached = getCachedCourseIndex(realPath); | ||
| if (cached) { | ||
| setIndexedCourses(cached); | ||
| return; | ||
| } | ||
| setIndexedCourses({}); | ||
| loadCourseIndex(realPath) | ||
| .then((index) => { | ||
| if (!cancelled) { | ||
| setIndexedCourses(index); | ||
| } | ||
| }) | ||
| .catch((error: unknown) => { | ||
| console.error(`Failed to load course data for ${realPath}:`, error); | ||
| }); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [realPath]); |
There was a problem hiding this comment.
📝 Info: useLayoutEffect with async operation is intentional and correct
Using useLayoutEffect at src/useAppState.ts:69 with an async loadCourseIndex call might look like an anti-pattern, but it serves two purposes: (1) for cache hits, it synchronously sets indexedCourses before paint, preventing a flash of stale data; (2) for cache misses, the synchronous setIndexedCourses({}) at line 76 clears the previous term's data before paint. The async portion (the .then()) naturally completes after paint, which is acceptable since there's no way to avoid a loading state on a cache miss. This is not a bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — yes, the pre-paint sync for cache hits/clears is intentional; the async load completing post-paint is the unavoidable loading state.
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
| useEffect(() => { | ||
| setOptions(Object.values(indexedCourses)); | ||
| }, [indexedCourses]); |
There was a problem hiding this comment.
📝 Info: useEffect resetting options may briefly override active search filter
The new useEffect at src/Workspace.tsx:424-426 resets options to the full course list whenever indexedCourses changes. If a user happened to have an active search query in the dropdown at the exact moment the catalog finishes loading, the filtered results would be replaced by the full list until the user types again. In practice this is nearly impossible since the catalog loads once on page load (before the user can reasonably start searching), so this is not a real concern.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — as noted, the catalog loads once before a user can realistically have an active query, and typing immediately re-filters; leaving as is.
83e84cf
into
devin/1781144816-toolchain-majors
…catalogs, MUI 9, dep-shedding (#166) * deps: bump vite to 7.3.5, wrangler to 4.98.0; npm audit clean Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * chore: upgrade toolchain majors (TypeScript 6, Vite 8, svgr 5, tsconfig-paths 6) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * perf: lazy-load term catalogs and split vendor chunks Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: no-op workspace mutations while term catalog is loading Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: log error when term catalog fails to load Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: sync catalog state before paint when the term changes Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * build: use function-form manualChunks for rolldown compatibility Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * chore: upgrade MUI to v9.1.0 (HelpOutline -> HelpOutlined icon rename) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: use HelpOutlineOutlined to preserve original HelpOutline glyph Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: re-sync search options after async catalog load Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: key search option re-sync on catalog identity Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * chore: upgrade toolchain majors (TypeScript 6, Vite 8, svgr 5, tsconfig-paths 6) (#165) Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * perf: lazy-load term catalogs + vendor code splitting (8.2 MB → ~1.0 MB initial JS) (#164) * perf: lazy-load term catalogs and split vendor chunks Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: no-op workspace mutations while term catalog is loading Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: log error when term catalog fails to load Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: sync catalog state before paint when the term changes Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * build: use function-form manualChunks for rolldown compatibility Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: re-sync search options after async catalog load Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: key search option re-sync on catalog identity Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * chore: upgrade MUI to v9.1.0 (#163) * chore: upgrade MUI to v9.1.0 (HelpOutline -> HelpOutlined icon rename) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: use HelpOutlineOutlined to preserve original HelpOutline glyph Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * Replace flatpickr and react-select with native/MUI inputs, bump auto-animate (#167) Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * build: pin wrangler to exact 4.98.0 (4.99+ wrangler dev assets 404 regression) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * chore: gitignore .wrangler local dev cache Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * review: restore Icon gitignore CRs, drop stale lightningcss errorRecovery (flatpickr removed) Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> * fix: alert instead of silent no-op for Import Workspace / Default Schedule while catalog loads Co-Authored-By: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Rahul Chalamala <22563365+rchalamala@users.noreply.github.com>
Summary
The 14 per-term catalog JSONs (~10 MB) were statically imported in
src/courseData.ts, putting everything in one 8.2 MB chunk. Term catalogs are now loaded on demand via dynamicimport(), and large vendor libs are split into their own chunks.src/courseData.ts: replaces the eagercourseDataSourcesmap withloadCourseIndex(term): Promise<CourseIndex>(per-term() => import(...)loaders + in-memory cache) andgetCachedCourseIndex(term). Term keys (/fa2026,/wi2026, ...) and theCourseIndexshape are unchanged.src/useAppState.ts:indexedCoursesbecomesuseStatepopulated by an effect onrealPath(cached index applied synchronously; otherwise{}while the chunk loads, then the loaded index). Consumers (App.tsxcontext,Workspace.tsx) are unchanged.src/Workspace.tsx: search box shows "Loading courses..." while the catalog is empty.vite.config.ts:manualChunksforreact/react-dom, MUI, Schedule-X, and motion;chunkSizeWarningLimit: 700(the per-term catalog data chunks are intrinsically ~0.5–0.65 MB but are lazy). The >500 kB Rollup warning no longer fires.Persisted workspaces store full
CourseStorage(incl.courseData) in localStorage, so schedules render immediately on reload; the catalog chunk for the active term loads in the background for search/arrangement operations.Bundle sizes (production build)
Before:
index.js: 8,221 kB (1,883 kB gzip) — single chunkAfter (initial load):
index.js421 kB (139 kB gzip)react193 kB (60 kB gzip)schedulex184 kB (51 kB gzip)mui138 kB (46 kB gzip)motion63 kB (22 kB gzip)Lazy (one per visited term):
IndexedTotal*.js514–648 kB (107–136 kB gzip) each.npm run verifypasses. No dependency version changes.Link to Devin session: https://app.devin.ai/sessions/8b9a591e4ca64813a5ecf3235b854611
Requested by: @rchalamala