refactor(frontend): centralize fetch wrapper, stale-response guard and LRU cache#795
Open
tdjager wants to merge 2 commits into
Open
refactor(frontend): centralize fetch wrapper, stale-response guard and LRU cache#795tdjager wants to merge 2 commits into
tdjager wants to merge 2 commits into
Conversation
…d LRU cache Consolidates three patterns that were copy-pasted across the editor SPA and deletes the dead X-Editor-Session machinery. Pure refactor: all user-visible behavior and (Dutch) error texts are preserved exactly. - lib/apiFetch.js: thin fetch wrapper (ok-check, ApiError with status/body/contentType, optional JSON/text parsing). Returns the raw Response so the ETag/If-Match concurrency plumbing keeps reading headers unchanged. 46 of 52 raw fetch sites migrated; the 6 that remain are deliberate multi-status branches (documented inline). - lib/useLatest.js: the "generation counter" guard against stale async writes, previously re-implemented 11 times (useLaw, useNotes x2, useTrajectDocuments, useLawGraph, LibraryView x2, EditorView x2, ScenarioBuilder x2, SearchPopover). - lib/lruMap.js: Map-based LRU with onEvict, replacing the three hand-rolled copies in useLaw, useCorpusLaws and useEngine. - Remove dead X-Editor-Session plumbing: no backend code reads the header; writes are scoped by the traject in the URL path. The still-used lastSavedPr/sanitizeSavedPr move from useEditorSession.js (deleted) to useSavedPr.js with corrected docs. - Bare catch blocks keep their swallow decision but now say why. All three helpers are unit-tested; the full vitest suite (302 tests) and the production build pass.
Preview Deployment — editor — editorYour changes have been deployed to a preview environment: URL: https://editor.pr795.rig.prd1.gn2.quattro.rijksapps.nl This deployment will be automatically cleaned up when the PR is closed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements audit advice #8: consolidates three patterns that were copy-pasted across the editor SPA and deletes the dead
X-Editor-Sessionmachinery. This is a pure refactor — all user-visible behavior and Dutch error texts are preserved byte-for-byte.1.
lib/apiFetch.js— thin fetch wrapperApiErrorcarryingstatus, best-effortbodytext andcontentType; callers branch on.status === 404or surface the backend's own message.apiFetchJson/apiFetchTextfor the common parse cases; plainapiFetchreturns the rawResponseso the recently-added ETag/If-Match optimistic-concurrency plumbing (useLaw, useScenarios, useTrajectDocuments) keeps reading headers unchanged, including the 412 conflict texts.allowStatuseslets 404/412/503/401-branching call sites keep their normal-branch handling;errorMessage(status, body, contentType)keeps every existing user-facing text identical (incl. the text/plain proxy-HTML guard inuseLaw.saveLaw/useDraftNotes.saveToRepo).apiAuthGuard.js, which wrapswindow.fetchand therefore still sits underapiFetch.useScenarios.fetchScenarios,useTrajects.loadTrajects,useTrajectDocumentsopen/save/HEAD/delete) — each now carries a comment saying so.2.
lib/useLatest.js— the stale-response "generation counter"claim()starts a generation and returns anisCurrent()predicate. Replaces 11 hand-rolled counters:useLaw(load/switch),useNotes(×2),useTrajectDocuments.openDocument,useLawGraph.rebuild,LibraryView(×2),EditorView(×2),ScenarioBuilder(×2),SearchPopover.3.
lib/lruMap.js— Map-based LRU withonEvictReplaces the three copy-pasted LRUs in
useLaw(cap 50),useCorpusLaws(cap 5, companion promise map synced viaonEvict) anduseEngine(cap 50, WASM unload viaonEvict).peek()preserves the one non-touching read inuseCorpusLaws.4. Dead
X-Editor-SessionremovalVerified by grepping
packages/: no backend code reads the header (editor-api never extracts it; theeditor/session-*branch code inpackages/corpusis server-side and header-independent). The header is no longer sent on law/scenario saves; writes are scoped by the traject in the URL path.useEditorSession.jsis deleted — its still-used exportslastSavedPr/sanitizeSavedPrmove tocomposables/useSavedPr.jswith corrected docs.Bare
catch {}blocksKept their swallow decisions (no new error UI in this PR) but each now states why the failure is ignored.
Testing
apiFetch.test.js,useLatest.test.js,lruMap.test.js, 23 tests).toHaveBeenCalledWith(url)assertions updated to account for the wrapper's explicit init argument.npm run buildpasses (no new warnings).Review notes (round 1)
useDependencies) — it stays retryable like other failures.useEngine's LRU now touches on cache hits too, so actively-used laws are less likely to be evicted (and WASM-unloaded) under cap pressure. The old code only bumped recency on insert.