From 4ab8c0ce28ee2eb2b9299354131d8b846f835d07 Mon Sep 17 00:00:00 2001 From: bhavik-mangla Date: Sun, 30 Aug 2026 10:53:25 +0530 Subject: [PATCH] fix: restore analysis on reload instead of rendering a blank page The analytical model lived only in React state, so any fresh page load discarded it. Five pages then returned null and rendered nothing between the navbar and the footer, and /network threw "Cannot read properties of null (reading 'allRepos')". Cache the analysis in IndexedDB with a one-hour TTL, matching the cache the landing page already advertises, and restore it on startup so a reload, bookmark or shared link costs no API calls. IndexedDB rather than localStorage because a single model can run to several megabytes. Add a RequireAnalysis route guard for the case where there is genuinely nothing to restore: send people to the organization picker, which is the only place an analysis can be started. It waits for the cache read so a reload is not redirected away a moment before its own data arrives, and keeps pages mounted while explore() refetches so their skeletons show. Also add the missing null guard to NetworkPage so it is safe on its own, matching the other data pages. --- src/App.jsx | 15 ++-- src/components/RequireAnalysis.jsx | 27 +++++++ src/components/RequireAnalysis.test.jsx | 67 ++++++++++++++++++ src/context/AppContext.jsx | 60 +++++++++++++++- src/pages/NetworkPage.jsx | 3 + src/services/cache.js | 94 +++++++++++++++++++++++++ 6 files changed, 257 insertions(+), 9 deletions(-) create mode 100644 src/components/RequireAnalysis.jsx create mode 100644 src/components/RequireAnalysis.test.jsx create mode 100644 src/services/cache.js diff --git a/src/App.jsx b/src/App.jsx index 94f8d27..56d854d 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -15,6 +15,7 @@ import GovernancePage from './pages/GovernancePage' import SettingsPage from './pages/SettingsPage' import Footer from './components/layout/Footer' import Support from './pages/Support' +import RequireAnalysis from './components/RequireAnalysis' function Layout({ children }) { return ( @@ -32,13 +33,13 @@ function AppContent() { } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> } /> } /> } /> diff --git a/src/components/RequireAnalysis.jsx b/src/components/RequireAnalysis.jsx new file mode 100644 index 0000000..36643f8 --- /dev/null +++ b/src/components/RequireAnalysis.jsx @@ -0,0 +1,27 @@ +import { Navigate } from 'react-router-dom' +import { useApp } from '../context/AppContext' + +/** + * Guards the routes that can only render once an organization has been analyzed. + * + * The cached analysis is restored asynchronously on startup, so this waits for + * `hydrating` to settle before deciding anything — otherwise a reload would + * redirect away a fraction of a second before its own data arrived. + * + * When there is genuinely nothing to show (no cache, or an expired one), send + * people to the organization picker, which is the only place an analysis can be + * started. Previously those routes rendered an empty page between the navbar and + * the footer, and /network threw "Cannot read properties of null". + * + * `loading` keeps the children mounted while explore() refetches — it clears the + * model first, and each page shows its own skeleton during that window. + */ +export default function RequireAnalysis({ children }) { + const { model, loading, hydrating } = useApp() + + if (hydrating) return null + + if (!model && !loading) return + + return children +} diff --git a/src/components/RequireAnalysis.test.jsx b/src/components/RequireAnalysis.test.jsx new file mode 100644 index 0000000..404341c --- /dev/null +++ b/src/components/RequireAnalysis.test.jsx @@ -0,0 +1,67 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import { MemoryRouter, Routes, Route } from 'react-router-dom' +import RequireAnalysis from './RequireAnalysis' + +const app = vi.hoisted(() => ({ + state: { model: null, loading: false, hydrating: false }, +})) + +vi.mock('../context/AppContext', () => ({ useApp: () => app.state })) + +function renderGuarded() { + return render( + + + org picker} /> +
analysis dashboard
} + /> + + + ) +} + +describe('RequireAnalysis', () => { + beforeEach(() => { + app.state = { model: null, loading: false, hydrating: false } + }) + + it('redirects to the org picker when there is no analysis to show', () => { + renderGuarded() + + expect(screen.getByText('org picker')).toBeInTheDocument() + expect(screen.queryByText('analysis dashboard')).not.toBeInTheDocument() + }) + + it('renders the page once an analysis is loaded', () => { + app.state = { model: { totalRepos: [] }, loading: false, hydrating: false } + + renderGuarded() + + expect(screen.getByText('analysis dashboard')).toBeInTheDocument() + }) + + it('waits while the cached analysis is being restored', () => { + // A reload restores from IndexedDB asynchronously — redirecting here would + // bounce the user away a moment before their own data arrived. + app.state = { model: null, loading: false, hydrating: true } + + renderGuarded() + + expect(screen.queryByText('org picker')).not.toBeInTheDocument() + expect(screen.queryByText('analysis dashboard')).not.toBeInTheDocument() + }) + + it('keeps the page mounted while explore() refetches', () => { + // explore() clears the model before refetching; the page shows its own + // skeleton during that window rather than being redirected away. + app.state = { model: null, loading: true, hydrating: false } + + renderGuarded() + + expect(screen.getByText('analysis dashboard')).toBeInTheDocument() + expect(screen.queryByText('org picker')).not.toBeInTheDocument() + }) +}) diff --git a/src/context/AppContext.jsx b/src/context/AppContext.jsx index a013159..66f7de0 100644 --- a/src/context/AppContext.jsx +++ b/src/context/AppContext.jsx @@ -1,6 +1,7 @@ -import { createContext, useContext, useState, useCallback, useEffect, useMemo } from 'react' +import { createContext, useContext, useState, useCallback, useEffect, useMemo, useRef } from 'react' import { fetchOrg, fetchRepos, fetchContributors, fetchIssues, fetchRateLimit, fetchPulls } from '../services/github' import { buildAnalyticalModel, getTopRepositories } from '../services/analytics' +import { saveAnalysis, loadAnalysis } from '../services/cache' const Ctx = createContext(null) @@ -41,6 +42,61 @@ export function AppProvider({ children }) { const [isComplete, setIsComplete] = useState(false) const [auditComplete, setAuditComplete] = useState(false) const [lastOrgNames, setLastOrgNames] = useState([]) + // True until the cached analysis has been read, so routes that need a model + // wait for the restore instead of bouncing to the picker on first paint. + const [hydrating, setHydrating] = useState(true) + // Set when state came straight from the cache, so the write-back effect can + // skip it. Re-saving an untouched restore would stamp a fresh savedAt on + // every page load and the entry would never reach its TTL. + const restoredFromCache = useRef(false) + + // Restore the last analysis on startup. The model is held in memory, so + // without this a reload, bookmark or shared link loses it entirely. + useEffect(() => { + let cancelled = false + + loadAnalysis() + .then(cached => { + if (cancelled || !cached) return + + restoredFromCache.current = true + + setOrgs(cached.orgs || []) + setModel(cached.model) + setTotalRepo(cached.totalRepo || 0) + setIsComplete(!!cached.isComplete) + setLastOrgNames(cached.lastOrgNames || []) + setIssuesData(cached.issuesData || {}) + setPullsData(cached.pullsData || {}) + setAuditComplete(!!cached.auditComplete) + setAdvanceAnalyticsComplete(!!cached.advanceAnalyticsComplete) + }) + .finally(() => { + if (!cancelled) setHydrating(false) + }) + + return () => { cancelled = true } + }, []) + + // Persist the analysis whenever it changes, including audit and analytics + // results — those are the most expensive data to refetch. + useEffect(() => { + if (hydrating || !model) return + + // Skip the write that would immediately follow a restore. + if (restoredFromCache.current) { + restoredFromCache.current = false + return + } + + saveAnalysis({ + orgs, model, totalRepo, isComplete, lastOrgNames, + issuesData, pullsData, auditComplete, advanceAnalyticsComplete + }) + }, [ + hydrating, orgs, model, totalRepo, isComplete, lastOrgNames, + issuesData, pullsData, auditComplete, advanceAnalyticsComplete + ]) useEffect(() => { const handler = e => { @@ -327,7 +383,7 @@ export function AppProvider({ children }) { rateLimit, loading, loadMsg, govLoading, error, totalRepo, runAdvanceAnalytics, refreshRateLimit, advanceAnalyticsLoading, advanceAnalyticsComplete, runFullAnalytics, - isComplete, auditComplete, lastOrgNames, + isComplete, auditComplete, lastOrgNames, hydrating, explore, runFullExplore, runAudit, runGovernanceAnalysis, setError, staleRepoStats }}> {children} diff --git a/src/pages/NetworkPage.jsx b/src/pages/NetworkPage.jsx index 105e30f..eb26e57 100644 --- a/src/pages/NetworkPage.jsx +++ b/src/pages/NetworkPage.jsx @@ -170,6 +170,9 @@ export default function NetworkPage() { const navigate = useNavigate() if(loading) return + // Matches the guard the other data pages already have: this page reads + // model.allRepos directly and threw a TypeError without it. + if (!model) return null return (
diff --git a/src/services/cache.js b/src/services/cache.js new file mode 100644 index 0000000..22c818a --- /dev/null +++ b/src/services/cache.js @@ -0,0 +1,94 @@ +/** + * Persistent cache for the last analysis. + * + * The analytical model is expensive to rebuild — it costs dozens of GitHub API + * calls against a 60 req/hr unauthenticated budget — but it lived only in React + * state, so every reload discarded it. Storing it in IndexedDB lets a refresh, + * a bookmark or a shared link restore the analysis with no API calls at all. + * + * IndexedDB is used rather than localStorage because a single model can run to + * several megabytes (239 repositories for an org the size of Vercel), well past + * the ~5MB localStorage quota. + * + * Every operation is best-effort: the cache is an optimisation, so a browser + * with IndexedDB unavailable or blocked (private windows, storage disabled, + * jsdom) degrades to the previous behaviour instead of breaking the app. + */ + +const DB_NAME = 'orgexplorer' +const DB_VERSION = 1 +const STORE = 'analysis' +const KEY = 'latest' + +/** Matches the "1HR intelligent cache" the landing page advertises. */ +export const CACHE_TTL_MS = 60 * 60 * 1000 + +function openDb() { + return new Promise((resolve, reject) => { + if (typeof indexedDB === 'undefined') { + reject(new Error('IndexedDB unavailable')) + return + } + + const req = indexedDB.open(DB_NAME, DB_VERSION) + + req.onupgradeneeded = () => { + const db = req.result + if (!db.objectStoreNames.contains(STORE)) db.createObjectStore(STORE) + } + req.onsuccess = () => resolve(req.result) + req.onerror = () => reject(req.error) + req.onblocked = () => reject(new Error('IndexedDB blocked')) + }) +} + +function withStore(mode, run) { + return openDb().then(db => + new Promise((resolve, reject) => { + let result + const tx = db.transaction(STORE, mode) + const req = run(tx.objectStore(STORE)) + + if (req) req.onsuccess = () => { result = req.result } + tx.oncomplete = () => { db.close(); resolve(result) } + tx.onerror = () => { db.close(); reject(tx.error) } + tx.onabort = () => { db.close(); reject(tx.error) } + }) + ) +} + +/** Persist the current analysis. Never throws. */ +export async function saveAnalysis(payload) { + try { + await withStore('readwrite', store => store.put({ ...payload, savedAt: Date.now() }, KEY)) + } catch { + // Best effort — a full or unavailable store must not break analysis. + } +} + +/** Return the cached analysis, or null when absent, stale or unreadable. */ +export async function loadAnalysis() { + try { + const record = await withStore('readonly', store => store.get(KEY)) + + if (!record?.model) return null + + if (Date.now() - record.savedAt > CACHE_TTL_MS) { + await clearAnalysis() + return null + } + + return record + } catch { + return null + } +} + +/** Drop the cached analysis. Never throws. */ +export async function clearAnalysis() { + try { + await withStore('readwrite', store => store.delete(KEY)) + } catch { + // Best effort. + } +}