feat: add mock dataset API layer#22
Conversation
martian56
left a comment
There was a problem hiding this comment.
I pulled this branch and ran it. With a connected account in the store, every /app page renders a permanently blank screen. Not a spinner, not an error message, an empty body, and the console is silent. It behaves identically with VITE_USE_MOCK_API=true and with it unset, I tested both. Typecheck and build pass, which is exactly why the verification bar for frontend changes here is opening the browser, not the compiler.
What is happening, because the mechanism is worth understanding:
- Throwing a promise from render is the Suspense protocol: React catches it, abandons the render, and retries when that promise resolves. Yours is
new Promise(() => {}), which by construction never resolves. - A component that suspends never commits, and effects only run after commit. So the useEffect above never runs, fetchDataset is never called, setData never fires, and there is no second render. The hang is total and mode-independent.
- Sidebar also calls useDataset, and it sits outside the page-level Suspense boundary in App.tsx. When it suspends there is no boundary to catch it, so React renders nothing at all. That is why there is not even a loading spinner.
On scope: #15 asks for a mock API server, something actually serving the dataset over the agreed routes, with the frontend able to target it via an env flag. This PR has the flag but no server. The non-mock path fetches /api/dataset, which nothing anywhere serves. So against the acceptance list: routes not served, flag exists but undocumented, shapes fine.
How I would shape round two:
- The default path keeps the app working with zero config: useDataset stays synchronous and in-process, exactly as on main. The env flag opts in to the HTTP path, not out of it.
- The server piece is the heart of the issue: a small Vite dev middleware (configureServer in vite.config.ts) or a standalone Bun script that serves GET /api/dataset from getDataset. Add a .env.example documenting the flag.
- For the HTTP path, no fake Suspense. Fetch at the shell level with a visible loading state, or cache the in-flight promise per account and throw that, which is how Suspense data fetching actually works. This part is worth a design chat before you write it, come grab me.
One process note: the checklist says docs were updated and the version bumped, but neither is in the diff. The checklist is how a reviewer calibrates trust. An unchecked box is fine, a wrongly checked one is not.
The instinct to put an API seam between components and the data layer is right, and lib/api is the right home for it. The execution needs another pass.
| }, [accountId]); | ||
|
|
||
| if (!data) { | ||
| throw new Promise(() => {}); |
There was a problem hiding this comment.
This promise never resolves, and the component suspends on first render, so the effect above never gets a chance to run. Effects run after commit; a suspended render never commits. Nothing can ever un-suspend this. Every consumer of the hook hangs forever, and since Sidebar consumes it outside any Suspense boundary, the entire app shell renders blank. Verified in the browser on this branch: empty body after 10 seconds, with the mock flag on and off.
| useEffect(() => { | ||
| if (!accountId) return; | ||
|
|
||
| fetchDataset(accountId).then(setData); |
There was a problem hiding this comment.
Two more things to handle in the rework: there is no error path (a rejected fetch leaves the page blank forever with an unhandled rejection), and no stale-response guard (switch accounts quickly and the slow response for the old account can land after, and overwrite, the fast one for the new). The usual shape is an effect-scoped cancelled flag or an AbortController.
| */ | ||
| export async function fetchDataset(accountId: string): Promise<Dataset> { | ||
| // MOCK MODE | ||
| if (import.meta.env.VITE_USE_MOCK_API === "true") { |
There was a problem hiding this comment.
Flag direction: unset should mean the app works, which today means the in-process dataset. Make the HTTP path the opt-in, not the default. And document the flag in a .env.example, otherwise nobody can discover it.
| } | ||
|
|
||
| // REAL BACKEND | ||
| const res = await fetch(`/api/dataset?accountId=${accountId}`); |
There was a problem hiding this comment.
Nothing serves this route, in dev or anywhere else, so this fetch can only fail today. The server side of this endpoint is the actual deliverable of #15: a Vite configureServer middleware or a small Bun script can serve it from getDataset in a few dozen lines. Also encodeURIComponent(accountId), and include res.status in the error message so failures are diagnosable.
What does this change?
Introduces a mock API layer for dataset to support frontend development without a backend service.
Adds a Vite development middleware that serves
/api/datasetduring local development.Implements an API abstraction (
fetchDataset) that supports two modes:getDataset)VITE_USE_MOCK_API=true): fetches from/api/datasetEnsures dataset shape remains fully compatible with
@cleat/contracts.Keeps existing dataset generation logic intact and reusable for both local and mock server modes.
Related issue
Closes #15
Area
Screenshots
Not applicable (no UI changes)
Checklist
bun run typecheck && bun run buildinapps/web(for frontend changes)