Skip to content

Remove fetched-data caches from PRReviewStore #351

Description

@jennings

Goal

Make React Query the single cache for all data fetched from the GitHub API. Remove fetched data (and data derived from fetched data) from PRReviewStore. Keep the store for UI state, navigation, drafts, viewed-file tracking, and preferences.

Why

PRReviewStore (src/browser/contexts/pr-review/index.tsx, ~4,355 lines) currently mirrors ~half of what already lives in React Query. This dual-cache setup means:

  • Optimistic updates have to be applied in two places.
  • Fetch orchestration (loadPRData, loadVersionData, etc.) reimplements what React Query already does.
  • Derived indexes recompute imperatively on every setter instead of memoizing over cached data.
  • Tests have to mock GitHubStore and inject fixtures through the store to exercise components.

Consolidating on React Query for fetched data lets us delete ~2,000 lines of store code, drop the store's GitHubStore dependency, and simplify test setup.

What stays in the store

Navigation SHAs (selectedHeadSha, selectedCommitSha, selectedParentSha, compareToSha, compareToCommitSha, interdiffEnabled), selectedFile/selectedFiles, overview/sidebar/scroll-target UI state, diff-viewer state (focused line, selection anchor, goto mode, skip-block expansion), comment focus/editing/replying state, drafts (pendingComments, commentDrafts, reviewBody), preferences (diffViewMode, wordWrap, hideViewed, conversationsFilters), and viewedFiles (the persisted set).

What leaves the store

Direct fetches: pr, files, reviews, reviewThreads, timeline, conversation, comments, commits, pushVersions, checks, checksLastUpdated, workflowRunsAwaitingApproval, branchDeleted, currentUser, viewerPermission, viewerCanMergeAsAdmin, repoAllowMergeCommit/SquashMerge/RebaseMerge, repoHasMergeQueue, prInMergeQueue, loadedDiffs.

Derived from fetched: viewedFilenames, commentRangeLookup, commentAnchorLookup, navigableItems, versionDiffCounts, versionRebaseInfo, commitVersionHistory, commitsByVersion, parentCommitMessages, commitChangeIds, interdiffLoadedDiffs, versionCompareNoChangeFiles.

Orchestration methods deleted: loadPRData, loadVersionData, loadOverviewData, refreshChecks, getChecksStatus, all loading*/versionDataLoaded flags, and every setPr/setFiles/setReviews/etc. setter for fetched fields.

Mutations: mergePR, closePR, reopenPR, dequeuePR, deleteBranch, restoreBranch, convertToDraft, markReadyForReview, updatePR, updateBranch, approveWorkflows and their in-flight/error flags all move to useMutation hooks that own their optimistic setQueryData + invalidateQueries.

Phases

Each phase is a shippable PR with tests written alongside the code.

  • Phase 0 — Fill React Query gaps
  • Phase 1 — Migrate simple mirrored data (in sub-PRs by data type)
  • Phase 2 — Move viewer/repo settings
  • Phase 3 — Move the diff cache
  • Phase 4 — Move version/interdiff derived data
  • Phase 5 — Move derived indexes
  • Phase 6 — Cleanup + test rewrite
  • Phase 7 — Migrate mutations to useMutation

Decisions locked in

  • Mutations are in scope (Phase 7).
  • commitChangeIds and parentCommitMessages get dedicated per-SHA RQ keys, not folded into singleCommit.
  • Tests are written with each phase, not deferred to the end.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Default priority

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions