From 40a0d19d9f3cd58a3d8efc6f104870fb01a07771 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Mon, 10 Aug 2026 17:03:23 +0100 Subject: [PATCH 1/2] Declare cache effects on the API and dispatch them in lite lite decided which caches to refresh in two hand-written places: a table of which watcher events make each query stale, and per-mutation invalidation lists in every mutation hook. Both restate facts the backend owns, in another language, where a forgotten entry silently never refreshes. Both are now derived from one vocabulary, `CacheTag` in but-api: a tag names one kind of cached state, and three declarations say everything that happens to it. * A read declares what its result is made of: `#[but_api(provides = [Reviews])]` * A mutation declares what it makes stale: `#[but_api(invalidates = [Reviews])]` * An event declares what it makes stale: `WatcherEventKind::invalidates` A mutation that only writes to the repository declares nothing -- the watcher observes the repository and the event carries the invalidation. Forge, app, and config writes declare, because nothing watches those. An endpoint either provides or invalidates, never both; naming a tag that does not exist is a compile error; and `None` stays distinguishable from `[]`, since "unclassified" is not the same answer as "no tag". The SDK carries all three tables as `cache-tags` (`apiProvides`, `apiInvalidates`, `watcherInvalidates`, and the `CacheTag` union), replacing `apiStaleAfter`. In lite, `api/tags.ts` connects them to the query cache: events meet queries where their tags intersect, and a mutation carrying its endpoint as `mutationKey` -- arranged by `apiMutation` -- has its declared tags applied by one mutation-cache hook. The hand lists in the mutation hooks are deleted; what remains locally is remedy machinery: optimistic patches, their rollbacks, and the two event exceptions (worktreeChanges pushes the changes it carries, target commits re-read only after the review refresh). 15 cache keys are renamed to their endpoint names so tag-to-query derivation is a lookup, and `projectQueryKeys` stays hand-written on purpose: it is what lite caches, which the backend does not know. Every tag has at least one provider and every project query one declaration. Dry runs are outside the system, as a ruling: a dry run is an imperative measurement, memoized under a key carrying the operation and changes it was measured against, and nothing refreshes it in place -- users do not expect a hover preview to update itself, and mutations serialize behind the project lock anyway. `dryRun` is a `LocalQueryKey` beside the drafts, and events no longer touch it. Parity, checked by replaying both dispatch paths against the previous behaviour, leaves two deliberate deltas: publishReview also refreshes the single-review cache, since both listings provide `Reviews`; and dry-run previews are no longer refreshed by repository events. `watcher.ts` becomes `project-events.ts`, since from lite's side the file watching is a backend detail and what the renderer has is a per-project event subscription. --- apps/lite/ui/src/api/mutations.ts | 301 +++++------------- apps/lite/ui/src/api/queries.ts | 163 +++++----- apps/lite/ui/src/api/tags.test.ts | 75 +++++ apps/lite/ui/src/api/tags.ts | 102 ++++++ apps/lite/ui/src/main.tsx | 13 +- apps/lite/ui/src/operations/operation.ts | 3 +- apps/lite/ui/src/project-events.test.ts | 75 +++++ apps/lite/ui/src/project-events.ts | 105 ++++++ apps/lite/ui/src/routes/project/$id/route.tsx | 4 +- .../$id/workspace/Settings/github-oauth.ts | 3 +- apps/lite/ui/src/watcher.ts | 93 ------ crates/but-api-macros/src/lib.rs | 107 ++++++- crates/but-api-macros/tests/src/lib.rs | 9 + .../ui/fail/base_invalid_attr_key.stderr | 2 +- .../ui/fail/base_provides_requires_napi.rs | 17 + .../fail/base_provides_requires_napi.stderr | 5 + .../ui/fail/napi_list_attr_unsupported.stderr | 2 +- .../ui/fail/napi_provides_and_invalidates.rs | 16 + .../fail/napi_provides_and_invalidates.stderr | 5 + .../tests/ui/fail/napi_provides_duplicated.rs | 15 + .../ui/fail/napi_provides_duplicated.stderr | 5 + crates/but-api/src/bitbucket.rs | 4 +- crates/but-api/src/branch.rs | 6 +- crates/but-api/src/comments.rs | 2 +- crates/but-api/src/diff.rs | 6 +- crates/but-api/src/github.rs | 4 +- crates/but-api/src/gitlab.rs | 4 +- crates/but-api/src/legacy/absorb.rs | 2 +- crates/but-api/src/legacy/config.rs | 4 +- crates/but-api/src/legacy/forge.rs | 58 ++-- crates/but-api/src/legacy/git.rs | 2 +- crates/but-api/src/legacy/projects.rs | 4 +- crates/but-api/src/legacy/repo.rs | 2 +- crates/but-api/src/legacy/workspace.rs | 6 +- crates/but-api/src/lib.rs | 3 + crates/but-api/src/resolve/mod.rs | 2 +- crates/but-api/src/tags.rs | 88 +++++ crates/but-api/src/target_commits.rs | 2 +- crates/but-api/src/watcher.rs | 70 ++++ crates/but-api/src/workspace.rs | 4 +- crates/but-schemars/src/lib.rs | 11 + crates/but-ts/src/main.rs | 81 ++++- packages/but-sdk/package.json | 34 +- .../src/generated/graph/cacheTags.d.ts | 73 +++++ .../but-sdk/src/generated/graph/cacheTags.js | 71 +++++ .../src/generated/linear/cacheTags.d.ts | 73 +++++ .../but-sdk/src/generated/linear/cacheTags.js | 71 +++++ 47 files changed, 1346 insertions(+), 461 deletions(-) create mode 100644 apps/lite/ui/src/api/tags.test.ts create mode 100644 apps/lite/ui/src/api/tags.ts create mode 100644 apps/lite/ui/src/project-events.test.ts create mode 100644 apps/lite/ui/src/project-events.ts delete mode 100644 apps/lite/ui/src/watcher.ts create mode 100644 crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.rs create mode 100644 crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.stderr create mode 100644 crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.rs create mode 100644 crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.stderr create mode 100644 crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.rs create mode 100644 crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.stderr create mode 100644 crates/but-api/src/tags.rs create mode 100644 packages/but-sdk/src/generated/graph/cacheTags.d.ts create mode 100644 packages/but-sdk/src/generated/graph/cacheTags.js create mode 100644 packages/but-sdk/src/generated/linear/cacheTags.d.ts create mode 100644 packages/but-sdk/src/generated/linear/cacheTags.js diff --git a/apps/lite/ui/src/api/mutations.ts b/apps/lite/ui/src/api/mutations.ts index 32e2098742d..bb426da23be 100644 --- a/apps/lite/ui/src/api/mutations.ts +++ b/apps/lite/ui/src/api/mutations.ts @@ -3,18 +3,15 @@ import { decodeBytes, encodeBytes } from "#ui/api/bytes.ts"; import { getHeadInfoIndex } from "#ui/api/ref-info.ts"; import { currentForgeLoginQueryOptions, - gbConfigQueryOptions, - getReviewMergeStatusQueryOptions, getReviewQueryOptions, headInfoQueryOptions, guiSettingsQueryOptions, - signingSettingsQueryOptions, listCommentReactionsQueryOptions, listReviewCommentsQueryOptions, listReviewReactionsQueryOptions, workspaceFetchQueryOptions, - type QueryKey, } from "#ui/api/queries.ts"; +import { apiMutation, type DeclaredMutation } from "#ui/api/tags.ts"; import { shortCommitId } from "#ui/commit.ts"; import { errorMessageForToast } from "#ui/errors.ts"; import { createDiffSpec, resolveDiffSpecs } from "#ui/operations/diff-specs.ts"; @@ -177,12 +174,7 @@ export const usePublishReview = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.publishReview, - onSuccess: async (_response, input, _context, mutation) => { - await mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }); - }, + ...apiMutation("publishReview"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -201,18 +193,7 @@ export const useUpdateReview = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.updateReview, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: getReviewQueryOptions({ projectId: input.projectId, reviewId: input.reviewId }) - .queryKey, - }), - ]); - }, + ...apiMutation("updateReview"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -231,17 +212,7 @@ export const useAddReviewLabels = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.addReviewLabels, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - ]); - }, + ...apiMutation("addReviewLabels"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -260,17 +231,7 @@ export const useRemoveReviewLabel = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.removeReviewLabel, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - ]); - }, + ...apiMutation("removeReviewLabel"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -337,7 +298,7 @@ export const useAddReviewReaction = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.addReviewReaction, + ...apiMutation("addReviewReaction"), onMutate: async (input, ctx) => { const key = listReviewReactionsQueryOptions(input).queryKey; await ctx.client.cancelQueries({ queryKey: key }); @@ -354,10 +315,13 @@ export const useAddReviewReaction = () => { return prev; }, - onSettled: (_response, _err, input, _prev, ctx) => - ctx.client.invalidateQueries({ queryKey: listReviewReactionsQueryOptions(input).queryKey }), onError: (error, input, prev, ctx) => { + // Roll the optimistic write back, then refetch: the rollback snapshot + // may itself be stale by now. if (prev) ctx.client.setQueryData(listReviewReactionsQueryOptions(input).queryKey, prev); + void ctx.client.invalidateQueries({ + queryKey: listReviewReactionsQueryOptions(input).queryKey, + }); // oxlint-disable-next-line no-console console.error(error); @@ -376,7 +340,7 @@ export const useRemoveReviewReaction = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.removeReviewReaction, + ...apiMutation("removeReviewReaction"), onMutate: async (input, ctx) => { const key = listReviewReactionsQueryOptions(input).queryKey; await ctx.client.cancelQueries({ queryKey: key }); @@ -388,10 +352,13 @@ export const useRemoveReviewReaction = () => { return prev; }, - onSettled: (_response, _err, input, _prev, ctx) => - ctx.client.invalidateQueries({ queryKey: listReviewReactionsQueryOptions(input).queryKey }), onError: (error, input, prev, ctx) => { + // Roll the optimistic write back, then refetch: the rollback snapshot + // may itself be stale by now. if (prev) ctx.client.setQueryData(listReviewReactionsQueryOptions(input).queryKey, prev); + void ctx.client.invalidateQueries({ + queryKey: listReviewReactionsQueryOptions(input).queryKey, + }); // oxlint-disable-next-line no-console console.error(error); @@ -415,6 +382,7 @@ export const useAddCommentReaction = () => { const toastManager = Toast.useToastManager(); return useMutation({ + ...apiMutation("addCommentReaction"), // `reviewId` keys the cache below; the forge addresses comments by id, // so it is not part of what the endpoint takes. mutationFn: ({ @@ -446,14 +414,15 @@ export const useAddCommentReaction = () => { return { prevReactions, prevComments }; }, - onSettled: (_response, _err, input, _prev, ctx) => - Promise.all([ - ctx.client.invalidateQueries({ - queryKey: listCommentReactionsQueryOptions(input).queryKey, - }), - ctx.client.invalidateQueries({ queryKey: listReviewCommentsQueryOptions(input).queryKey }), - ]), onError: (error, input, prev, ctx) => { + // Roll the optimistic writes back, then refetch: the rollback + // snapshots may themselves be stale by now. + void ctx.client.invalidateQueries({ + queryKey: listCommentReactionsQueryOptions(input).queryKey, + }); + void ctx.client.invalidateQueries({ + queryKey: listReviewCommentsQueryOptions(input).queryKey, + }); if (prev?.prevReactions) { ctx.client.setQueryData( listCommentReactionsQueryOptions(input).queryKey, @@ -511,14 +480,15 @@ export const useRemoveCommentReaction = () => { return { prevReactions, prevComments }; }, - onSettled: (_response, _err, input, _prev, ctx) => - Promise.all([ - ctx.client.invalidateQueries({ - queryKey: listCommentReactionsQueryOptions(input).queryKey, - }), - ctx.client.invalidateQueries({ queryKey: listReviewCommentsQueryOptions(input).queryKey }), - ]), onError: (error, input, prev, ctx) => { + // Roll the optimistic writes back, then refetch: the rollback + // snapshots may themselves be stale by now. + void ctx.client.invalidateQueries({ + queryKey: listCommentReactionsQueryOptions(input).queryKey, + }); + void ctx.client.invalidateQueries({ + queryKey: listReviewCommentsQueryOptions(input).queryKey, + }); if (prev?.prevReactions) { ctx.client.setQueryData( listCommentReactionsQueryOptions(input).queryKey, @@ -545,20 +515,7 @@ export const useRequestReview = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.requestReview, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["reviewTimelineEvents" satisfies QueryKey, input.projectId], - }), - ]); - }, + ...apiMutation("requestReview"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -577,17 +534,7 @@ export const useWithdrawReviewRequest = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.withdrawReviewRequest, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - ]); - }, + ...apiMutation("withdrawReviewRequest"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -606,7 +553,7 @@ export const useCreateReviewComment = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.createReviewComment, + ...apiMutation("createReviewComment"), onMutate: async (input, ctx) => { const key = listReviewCommentsQueryOptions(input).queryKey; await ctx.client.cancelQueries({ queryKey: key }); @@ -628,10 +575,13 @@ export const useCreateReviewComment = () => { return prev; }, - onSettled: (_response, _err, input, _prev, ctx) => - ctx.client.invalidateQueries({ queryKey: listReviewCommentsQueryOptions(input).queryKey }), onError: (error, input, prev, ctx) => { + // Roll the optimistic write back, then refetch: the rollback snapshot + // may itself be stale by now. if (prev) ctx.client.setQueryData(listReviewCommentsQueryOptions(input).queryKey, prev); + void ctx.client.invalidateQueries({ + queryKey: listReviewCommentsQueryOptions(input).queryKey, + }); // oxlint-disable-next-line no-console console.error(error); @@ -652,16 +602,12 @@ export const useUpdateReviewComment = () => { return useMutation({ // `reviewId` keys the cache below; the forge addresses comments by id, // so it is not part of what the endpoint takes. + ...apiMutation("updateReviewComment"), mutationFn: ({ reviewId: _reviewId, ...params }: PayloadFor<"updateReviewComment"> & { reviewId: number }) => window.lite.updateReviewComment(params), - onSuccess: async (_response, input, _context, mutation) => { - await mutation.client.invalidateQueries({ - queryKey: ["reviewComments" satisfies QueryKey, input.projectId, input.reviewId], - }); - }, onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -682,16 +628,12 @@ export const useDeleteReviewComment = () => { return useMutation({ // `reviewId` keys the cache below; the forge addresses comments by id, // so it is not part of what the endpoint takes. + ...apiMutation("deleteReviewComment"), mutationFn: ({ reviewId: _reviewId, ...params }: PayloadFor<"deleteReviewComment"> & { reviewId: number }) => window.lite.deleteReviewComment(params), - onSuccess: async (_response, input, _context, mutation) => { - await mutation.client.invalidateQueries({ - queryKey: ["reviewComments" satisfies QueryKey, input.projectId, input.reviewId], - }); - }, onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -710,9 +652,9 @@ export const useSetReviewAutoMerge = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.setReviewAutoMerge, + ...apiMutation("setReviewAutoMerge"), onMutate: async (input, ctx) => { - const reviewsPrefix = ["reviews" satisfies QueryKey, input.projectId]; + const reviewsPrefix = ["listReviews", input.projectId] as const; await ctx.client.cancelQueries({ queryKey: reviewsPrefix }); // The flag lives on every reviews listing (the key varies by cache @@ -735,17 +677,12 @@ export const useSetReviewAutoMerge = () => { return { prev, prevSingle }; }, - onSettled: (_response, _err, input, _prev, ctx) => - Promise.all([ - ctx.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - ctx.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - ]), onError: (error, input, prev, ctx) => { + // Roll the optimistic writes back, then refetch: the rollback + // snapshots may themselves be stale by now. for (const [key, data] of prev?.prev ?? []) ctx.client.setQueryData(key, data); + void ctx.client.invalidateQueries({ queryKey: ["listReviews", input.projectId] }); + void ctx.client.invalidateQueries({ queryKey: ["getReview", input.projectId] }); if (prev?.prevSingle) { ctx.client.setQueryData( getReviewQueryOptions({ projectId: input.projectId, reviewId: input.reviewId }).queryKey, @@ -770,24 +707,13 @@ export const useMergeReview = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.mergeReview, + ...apiMutation("mergeReview"), onSuccess: async (_response, input, _context, mutation) => { - // Checks 422 once the branch is merged; refetch so the badge clears. - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["reviewMergeStatus" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["ciChecks" satisfies QueryKey, input.projectId], - }), - ]); - + // The merge moved the target branch on the remote, but nothing local, so + // the branch keeps looking un-integrated until remote-tracking refs catch + // up. Fetch through the shared query (dedupes with auto-fetch), then + // re-read head info rather than waiting on watcher delivery. A failed + // fetch is not a failed merge, so neither reaches onError. await mutation.client .fetchQuery({ ...workspaceFetchQueryOptions(input.projectId), staleTime: 0 }) .then(() => @@ -816,24 +742,7 @@ export const useSetReviewDraftiness = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.setReviewDraftiness, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: getReviewQueryOptions({ projectId: input.projectId, reviewId: input.reviewId }) - .queryKey, - }), - mutation.client.invalidateQueries({ - queryKey: getReviewMergeStatusQueryOptions({ - projectId: input.projectId, - reviewId: input.reviewId, - }).queryKey, - }), - ]); - }, + ...apiMutation("setReviewDraftiness"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -852,19 +761,7 @@ export const useSetGbConfig = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.setGbConfig, - onSuccess: async (_response, input, _context, mutation) => { - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: gbConfigQueryOptions(input.projectId).queryKey, - }), - // The stored settings are what signing was checked against, so a change - // retires the previous verdict. - mutation.client.invalidateQueries({ - queryKey: signingSettingsQueryOptions(input.projectId).queryKey, - }), - ]); - }, + ...apiMutation("setGbConfig"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -883,12 +780,7 @@ export const useDeleteAllData = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.deleteAllData, - onSuccess: async (_response, _input, _context, mutation) => { - await mutation.client.invalidateQueries({ - queryKey: ["projects" satisfies QueryKey], - }); - }, + ...apiMutation("deleteAllData"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -903,28 +795,17 @@ export const useDeleteAllData = () => { }); }; -/** - * Every forge's accounts live under one key root, so any change to any of them - * refreshes the lot — and the per-project login they resolve to. - */ -const invalidateForgeAccounts = async (client: QueryClient): Promise => { - await Promise.all([ - client.invalidateQueries({ queryKey: ["forgeAccounts" satisfies QueryKey] }), - client.invalidateQueries({ queryKey: ["currentForgeLogin" satisfies QueryKey] }), - ]); -}; - -const useForgeAccountMutation = ( - mutationFn: (input: TInput) => Promise, +const useForgeAccountMutation = ( + mutation: { + mutationKey: readonly [DeclaredMutation]; + mutationFn: (input: Input) => Promise; + }, failureTitle: string, ) => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn, - onSuccess: async (_response, _input, _context, mutation) => { - await invalidateForgeAccounts(mutation.client); - }, + ...mutation, onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -940,33 +821,28 @@ const useForgeAccountMutation = ( }; export const useForgetGithubAccount = () => - useForgeAccountMutation(window.lite.forgetGithubAccount, "Failed to forget account"); + useForgeAccountMutation(apiMutation("forgetGithubAccount"), "Failed to forget account"); export const useForgetGitlabAccount = () => - useForgeAccountMutation(window.lite.forgetGitlabAccount, "Failed to forget account"); + useForgeAccountMutation(apiMutation("forgetGitlabAccount"), "Failed to forget account"); export const useForgetBitbucketAccount = () => - useForgeAccountMutation(window.lite.forgetBitbucketAccount, "Failed to forget account"); + useForgeAccountMutation(apiMutation("forgetBitbucketAccount"), "Failed to forget account"); export const useStoreGithubPat = () => - useForgeAccountMutation(window.lite.storeGithubPat, "Failed to add GitHub account"); + useForgeAccountMutation(apiMutation("storeGithubPat"), "Failed to add GitHub account"); export const useStoreGitlabPat = () => - useForgeAccountMutation(window.lite.storeGitlabPat, "Failed to add GitLab account"); + useForgeAccountMutation(apiMutation("storeGitlabPat"), "Failed to add GitLab account"); export const useStoreBitbucketApiToken = () => - useForgeAccountMutation(window.lite.storeBitbucketApiToken, "Failed to add Bitbucket account"); + useForgeAccountMutation(apiMutation("storeBitbucketApiToken"), "Failed to add Bitbucket account"); export const useDeleteProject = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.deleteProject, - onSuccess: async (_response, _input, _context, mutation) => { - await mutation.client.invalidateQueries({ - queryKey: ["projects" satisfies QueryKey], - }); - }, + ...apiMutation("deleteProject"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -985,12 +861,7 @@ export const useUpdateProjectSettings = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.updateProjectSettings, - onSuccess: async (_response, _input, _context, mutation) => { - await mutation.client.invalidateQueries({ - queryKey: ["projects" satisfies QueryKey], - }); - }, + ...apiMutation("updateProjectSettings"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); @@ -1451,31 +1322,7 @@ export const useWorkspaceBranchAndAncestorsPush = () => { const toastManager = Toast.useToastManager(); return useMutation({ - mutationFn: window.lite.workspaceBranchAndAncestorsPush, - onSuccess: async (_response, input, _context, mutation) => { - // A push moves the review's head, so the cached reviews, their mergeability, - // and the checks for the new sha are all stale. - await Promise.all([ - mutation.client.invalidateQueries({ - queryKey: ["headInfo" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["reviews" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["review" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["reviewMergeStatus" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["ciChecks" satisfies QueryKey, input.projectId], - }), - mutation.client.invalidateQueries({ - queryKey: ["reviewTimelineEvents" satisfies QueryKey, input.projectId], - }), - ]); - }, + ...apiMutation("workspaceBranchAndAncestorsPush"), onError: (error) => { // oxlint-disable-next-line no-console console.error(error); diff --git a/apps/lite/ui/src/api/queries.ts b/apps/lite/ui/src/api/queries.ts index 4a7dbe1d7c1..840337a1ada 100644 --- a/apps/lite/ui/src/api/queries.ts +++ b/apps/lite/ui/src/api/queries.ts @@ -9,37 +9,39 @@ import * as ms from "ms"; * Keyed `[key, projectId, ...]`. The fixed position is what lets `handleWatcher` * invalidate a whole query root holding nothing but a project id. */ -export type ProjectQueryKey = - | "branchCannedName" - | "branchDetails" - | "branchDiff" - | "branchList" - | "changesInWorktree" - | "ciChecks" - | "comments" - | "commitConflicts" - | "commitDetailsWithLineStats" - | "forgeInfo" - | "headInfo" - | "currentForgeLogin" - | "repoLabels" - | "review" - | "reviewComments" - | "reviewSubmissions" - | "reviewTimelineEvents" - | "reviewReactions" - | "commentReactions" - | "reviewMergeStatus" - | "reviewerCandidates" - | "reviews" - | "gbConfig" - | "signingSettings" - | "treeChangeDiffs" - | "absorptionPlan" - | "dryRun" - | "workspaceFetch" - | "workspaceFetchStatus" - | "workspaceTargetCommits"; +export const projectQueryKeys = [ + "branchCannedName", + "branchDetails", + "branchDiff", + "branchList", + "changesInWorktree", + "commitConflicts", + "listCiChecks", + "commentsList", + "commitDetailsWithLineStats", + "forgeInfo", + "headInfo", + "currentForgeLogin", + "listRepoLabels", + "getReview", + "listReviewComments", + "listReviewSubmissions", + "listReviewTimelineEvents", + "listReviewReactions", + "listCommentReactions", + "getReviewMergeStatus", + "listReviewerCandidates", + "listReviews", + "getGbConfig", + "checkSigningSettings", + "treeChangeDiffs", + "absorptionPlan", + "workspaceFetchFromRemotes", + "workspaceFetchStatus", + "workspaceTargetCommits", +] as const; + +export type ProjectQueryKey = (typeof projectQueryKeys)[number]; /** Keyed without a project id, so no project event can invalidate them. */ type GlobalQueryKey = @@ -50,40 +52,59 @@ type GlobalQueryKey = | "projects" | "guiSettings"; -export type QueryKey = ProjectQueryKey | GlobalQueryKey; +/** + * Client state kept in the query cache, so nothing declares for them. `dryRun` + * memoizes an imperative preview: its key carries the operation and changes it + * was measured against, and nothing refreshes it in place. + */ +type LocalQueryKey = "commitMessageDraft" | "dryRun" | "prMergeMethod" | "prDraft"; + +export type QueryKey = ProjectQueryKey | GlobalQueryKey | LocalQueryKey; + +declare module "@tanstack/react-query" { + interface Register { + /** + * Every query key in the app starts with one of ours, so a typo is a type + * error wherever a key is written — building one, invalidating it, or + * reading it back — without each site having to say so. + */ + queryKey: readonly [QueryKey, ...ReadonlyArray]; + } +} /** * The name the backend would generate for a branch created right now. Used to - * name the branch a commit is about to create before it exists, so it goes - * stale as soon as any branch is created — see `refreshedBy` in `watcher.ts`. + * name the branch a commit is about to create before it exists. Derived from + * the branch namespace, so the endpoint provides `Branches` and any ref + * movement refreshes it. */ export const branchCannedNameQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["branchCannedName" satisfies QueryKey, projectId], + queryKey: ["branchCannedName", projectId], queryFn: () => window.lite.branchCannedName(projectId), }); export const branchDetailsQueryOptions = ({ projectId, ...params }: PayloadFor<"branchDetails">) => queryOptions({ - queryKey: ["branchDetails" satisfies QueryKey, projectId, params], + queryKey: ["branchDetails", projectId, params], queryFn: () => window.lite.branchDetails({ projectId, ...params }), }); export const branchDiffQueryOptions = ({ projectId, ...params }: PayloadFor<"branchDiff">) => queryOptions({ - queryKey: ["branchDiff" satisfies QueryKey, projectId, params], + queryKey: ["branchDiff", projectId, params], queryFn: () => window.lite.branchDiff({ projectId, ...params }), }); export const branchListQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["branchList" satisfies QueryKey, projectId], + queryKey: ["branchList", projectId], queryFn: () => window.lite.branchList(projectId), }); export const changesInWorktreeQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["changesInWorktree" satisfies QueryKey, projectId], + queryKey: ["changesInWorktree", projectId], queryFn: () => window.lite.changesInWorktree({ projectId, @@ -94,7 +115,7 @@ export const changesInWorktreeQueryOptions = (projectId: string) => export const commentsQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["comments" satisfies QueryKey, projectId], + queryKey: ["commentsList", projectId], queryFn: () => window.lite.commentsList(projectId), }); @@ -103,7 +124,7 @@ export const commitDetailsWithLineStatsQueryOptions = ({ ...params }: PayloadFor<"commitDetailsWithLineStats">) => queryOptions({ - queryKey: ["commitDetailsWithLineStats" satisfies QueryKey, projectId, params], + queryKey: ["commitDetailsWithLineStats", projectId, params], queryFn: () => window.lite.commitDetailsWithLineStats({ projectId, ...params }), }); @@ -120,7 +141,7 @@ export const commitConflictsQueryOptions = ({ ...params }: PayloadFor<"commitConflicts"> & { enabled: boolean }) => queryOptions({ - queryKey: ["commitConflicts" satisfies QueryKey, projectId, params], + queryKey: ["commitConflicts", projectId, params], queryFn: () => window.lite.commitConflicts({ projectId, ...params }), enabled, staleTime: Infinity, @@ -132,25 +153,25 @@ export const commitConflictsQueryOptions = ({ export const forgeInfoOptions = (projectId: string) => queryOptions({ - queryKey: ["forgeInfo" satisfies QueryKey, projectId], + queryKey: ["forgeInfo", projectId], queryFn: () => window.lite.forgeInfo(projectId), }); export const headInfoQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["headInfo" satisfies QueryKey, projectId], + queryKey: ["headInfo", projectId], queryFn: () => window.lite.headInfo(projectId), }); export const getReviewQueryOptions = ({ projectId, reviewId }: PayloadFor<"getReview">) => queryOptions({ - queryKey: ["review" satisfies QueryKey, projectId, reviewId], + queryKey: ["getReview", projectId, reviewId], queryFn: () => window.lite.getReview({ projectId, reviewId }), }); export const workspaceTargetCommitsQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["workspaceTargetCommits" satisfies QueryKey, projectId], + queryKey: ["workspaceTargetCommits", projectId], queryFn: () => window.lite.workspaceTargetCommits({ projectId, from: null, limit: null }), }); @@ -188,7 +209,7 @@ export const refreshIntegratedReviews = async ( export const workspaceFetchStatusQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["workspaceFetchStatus" satisfies QueryKey, projectId], + queryKey: ["workspaceFetchStatus", projectId], queryFn: () => window.lite.workspaceFetchStatus(projectId), }); @@ -205,7 +226,7 @@ export const workspaceFetchQueryOptions = ( } return queryOptions({ - queryKey: ["workspaceFetch" satisfies QueryKey, projectId], + queryKey: ["workspaceFetchFromRemotes", projectId], queryFn: () => window.lite.workspaceFetchFromRemotes({ projectId, action: null }).then( // RQ treats undefined results in queries as errors. @@ -227,7 +248,7 @@ export const listReviewCommentsQueryOptions = ({ reviewId, }: PayloadFor<"listReviewComments">) => queryOptions({ - queryKey: ["reviewComments" satisfies QueryKey, projectId, reviewId], + queryKey: ["listReviewComments", projectId, reviewId], queryFn: () => window.lite.listReviewComments({ projectId, reviewId }), // Fresh forge fetch each time; keep a gentle poll while the tab is open // so replies from others appear without a manual refresh. @@ -237,7 +258,7 @@ export const listReviewCommentsQueryOptions = ({ export const gbConfigQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["gbConfig" satisfies QueryKey, projectId], + queryKey: ["getGbConfig", projectId], queryFn: () => window.lite.getGbConfig(projectId), }); @@ -247,7 +268,7 @@ export const gbConfigQueryOptions = (projectId: string) => */ export const signingSettingsQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["signingSettings" satisfies QueryKey, projectId], + queryKey: ["checkSigningSettings", projectId], queryFn: () => window.lite.checkSigningSettings(projectId), enabled: false, retry: false, @@ -256,7 +277,7 @@ export const signingSettingsQueryOptions = (projectId: string) => export const currentForgeLoginQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["currentForgeLogin" satisfies QueryKey, projectId], + queryKey: ["currentForgeLogin", projectId], queryFn: () => window.lite.currentForgeLogin(projectId), // Resolved from local account storage; changes only on re-auth. staleTime: Number.POSITIVE_INFINITY, @@ -265,7 +286,7 @@ export const currentForgeLoginQueryOptions = (projectId: string) => /** Gate on the forge being GitHub; other forges reject this call. */ export const repoLabelsQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["repoLabels" satisfies QueryKey, projectId], + queryKey: ["listRepoLabels", projectId], queryFn: () => window.lite.listRepoLabels(projectId), // Label definitions rarely change. staleTime: 5 * 60_000, @@ -274,7 +295,7 @@ export const repoLabelsQueryOptions = (projectId: string) => /** Gate on the forge being GitHub; other forges reject this call. */ export const reviewerCandidatesQueryOptions = (projectId: string) => queryOptions({ - queryKey: ["reviewerCandidates" satisfies QueryKey, projectId], + queryKey: ["listReviewerCandidates", projectId], queryFn: () => window.lite.listReviewerCandidates(projectId), // Collaborator lists rarely change. staleTime: 5 * 60_000, @@ -286,7 +307,7 @@ export const listReviewSubmissionsQueryOptions = ({ reviewId, }: PayloadFor<"getReview">) => queryOptions({ - queryKey: ["reviewSubmissions" satisfies QueryKey, projectId, reviewId], + queryKey: ["listReviewSubmissions", projectId, reviewId], queryFn: () => window.lite.listReviewSubmissions({ projectId, reviewId }), // Same freshness posture as the comments: fresh fetch, gentle poll. staleTime: 60_000, @@ -299,7 +320,7 @@ export const listReviewTimelineEventsQueryOptions = ({ reviewId, }: PayloadFor<"getReview">) => queryOptions({ - queryKey: ["reviewTimelineEvents" satisfies QueryKey, projectId, reviewId], + queryKey: ["listReviewTimelineEvents", projectId, reviewId], queryFn: () => window.lite.listReviewTimelineEvents({ projectId, reviewId }), // Same freshness posture as the comments: fresh fetch, gentle poll. staleTime: 60_000, @@ -309,7 +330,7 @@ export const listReviewTimelineEventsQueryOptions = ({ /** This query should be gated by PR capability lest it fail. */ export const listReviewReactionsQueryOptions = ({ projectId, reviewId }: PayloadFor<"getReview">) => queryOptions({ - queryKey: ["reviewReactions" satisfies QueryKey, projectId, reviewId], + queryKey: ["listReviewReactions", projectId, reviewId], queryFn: () => window.lite.listReviewReactions({ projectId, reviewId }), // Same freshness posture as the comments: fresh fetch, gentle poll. staleTime: 60_000, @@ -329,7 +350,7 @@ export const listCommentReactionsQueryOptions = ({ commentId: number; }) => queryOptions({ - queryKey: ["commentReactions" satisfies QueryKey, projectId, commentId], + queryKey: ["listCommentReactions", projectId, commentId], queryFn: () => window.lite.listCommentReactions({ projectId, commentId }), staleTime: 60_000, }); @@ -339,7 +360,7 @@ export const getReviewMergeStatusQueryOptions = ({ reviewId, }: PayloadFor<"getReview">) => queryOptions({ - queryKey: ["reviewMergeStatus" satisfies QueryKey, projectId, reviewId], + queryKey: ["getReviewMergeStatus", projectId, reviewId], queryFn: () => window.lite.getReviewMergeStatus({ projectId, reviewId }), staleTime: ({ state: { data } }) => (data?.isMergeable ? 30_000 : 10_000), // Mergeability flips from the forge side (checks finish, approvals @@ -352,7 +373,7 @@ export const getReviewMergeStatusQueryOptions = ({ /** This query should be gated by PR capability lest it fail. */ export const listReviewsQueryOptions = ({ projectId, ...params }: PayloadFor<"listReviews">) => queryOptions({ - queryKey: ["reviews" satisfies QueryKey, projectId, params], + queryKey: ["listReviews", projectId, params], queryFn: () => window.lite.listReviews({ projectId, ...params }), select: (reviews) => { const reviewsBySourceBranch = new Map(); @@ -380,38 +401,38 @@ const backendPlatform = (platform: string): string => /** Terminals are per-platform, and the platform cannot change while running. */ export const terminalsQueryOptions = queryOptions({ - queryKey: ["terminals" satisfies QueryKey], + queryKey: ["terminals"], queryFn: () => window.lite.getTerminalOptionsForPlatform(backendPlatform(window.lite.platform)), staleTime: Number.POSITIVE_INFINITY, }); export const userProfileQueryOptions = queryOptions({ - queryKey: ["userProfile" satisfies QueryKey], + queryKey: ["userProfile"], queryFn: () => window.lite.getUserProfileLocal(), }); export const githubAccountsQueryOptions = queryOptions({ - queryKey: ["forgeAccounts" satisfies QueryKey, "github"], + queryKey: ["forgeAccounts", "github"], queryFn: () => window.lite.listKnownGithubAccounts(), }); export const gitlabAccountsQueryOptions = queryOptions({ - queryKey: ["forgeAccounts" satisfies QueryKey, "gitlab"], + queryKey: ["forgeAccounts", "gitlab"], queryFn: () => window.lite.listKnownGitlabAccounts(), }); export const bitbucketAccountsQueryOptions = queryOptions({ - queryKey: ["forgeAccounts" satisfies QueryKey, "bitbucket"], + queryKey: ["forgeAccounts", "bitbucket"], queryFn: () => window.lite.listKnownBitbucketAccounts(), }); export const listProjectsQueryOptions = queryOptions({ - queryKey: ["projects" satisfies QueryKey], + queryKey: ["projects"], queryFn: () => window.lite.listProjectsStateless(), }); export const listEditorsQueryOptions = queryOptions({ - queryKey: ["editors" satisfies QueryKey], + queryKey: ["editors"], queryFn: () => window.lite.listEditors(), }); @@ -425,7 +446,7 @@ export const listCIChecksQueryOptions = ({ polling: "passive" | "priority"; }) => queryOptions({ - queryKey: ["ciChecks" satisfies QueryKey, projectId, reference], + queryKey: ["listCiChecks", projectId, reference], queryFn: async () => { // Aggregated data is needed in queryFn to adjust refetching behaviour. Aggregating here, for // use as mentioned and also at call sites, is more efficient. @@ -482,17 +503,17 @@ export const listCIChecksQueryOptions = ({ export const treeChangeDiffsQueryOptions = ({ projectId, change }: PayloadFor<"treeChangeDiffs">) => queryOptions({ - queryKey: ["treeChangeDiffs" satisfies QueryKey, projectId, change], + queryKey: ["treeChangeDiffs", projectId, change], queryFn: () => window.lite.treeChangeDiffs({ projectId, change }), }); export const absorptionPlanQueryOptions = ({ projectId, target }: PayloadFor<"absorptionPlan">) => queryOptions({ - queryKey: ["absorptionPlan" satisfies QueryKey, projectId, target], + queryKey: ["absorptionPlan", projectId, target], queryFn: () => window.lite.absorptionPlan({ projectId, target }), }); export const guiSettingsQueryOptions = queryOptions({ - queryKey: ["guiSettings" satisfies QueryKey], + queryKey: ["guiSettings"], queryFn: () => window.lite.readGUISettings(), }); diff --git a/apps/lite/ui/src/api/tags.test.ts b/apps/lite/ui/src/api/tags.test.ts new file mode 100644 index 00000000000..eabac01577d --- /dev/null +++ b/apps/lite/ui/src/api/tags.test.ts @@ -0,0 +1,75 @@ +import { invalidateDeclared, invalidateTags } from "#ui/api/tags.ts"; +import { apiInvalidates, type CacheTag } from "@gitbutler/but-sdk/cache-tags"; +import type { QueryClient } from "@tanstack/react-query"; +import { describe, expect, it } from "vitest"; + +const declared = apiInvalidates as Record>; + +const recording = () => { + const invalidated: Array> = []; + const client = { + invalidateQueries: ({ queryKey }: { queryKey: ReadonlyArray }) => { + invalidated.push(queryKey); + return Promise.resolve(); + }, + } as unknown as QueryClient; + return { client, invalidated }; +}; + +describe("invalidateTags", () => { + it("scopes project queries to the project", async () => { + const { client, invalidated } = recording(); + await invalidateTags(client, ["Reviews"], "p1"); + expect(invalidated).toEqual( + expect.arrayContaining([ + ["getReview", "p1"], + ["listReviews", "p1"], + ]), + ); + }); + + it("falls back to a key prefix without a project id", async () => { + const { client, invalidated } = recording(); + await invalidateTags(client, ["ForgeLogin"]); + expect(invalidated).toEqual([["currentForgeLogin"]]); + }); + + it("reaches global queries", async () => { + const { client, invalidated } = recording(); + await invalidateTags(client, ["Projects", "ForgeAccounts"], "p1"); + expect(invalidated).toEqual(expect.arrayContaining([["projects"], ["forgeAccounts"]])); + }); +}); + +describe("declared mutations", () => { + // A tag no query provides invalidates nothing: the declaration is dead + // and the mutation author believes otherwise. + it.each(Object.entries(declared))( + "%s only names tags some query provides", + async (_endpoint, tags) => { + const { client, invalidated } = recording(); + await invalidateTags(client, tags, "p1"); + expect(invalidated.length).toBeGreaterThan(0); + }, + ); + + it("applies a mutation's declaration from its key", async () => { + const { client, invalidated } = recording(); + await invalidateDeclared(client, ["mergeReview"], { projectId: "p1" }); + expect(invalidated).toEqual( + expect.arrayContaining([ + ["getReview", "p1"], + ["listReviews", "p1"], + ["getReviewMergeStatus", "p1"], + ["listCiChecks", "p1"], + ]), + ); + }); + + it("ignores mutations that declared nothing", async () => { + const { client, invalidated } = recording(); + await invalidateDeclared(client, ["commitCreate"], { projectId: "p1" }); + await invalidateDeclared(client, undefined, { projectId: "p1" }); + expect(invalidated).toEqual([]); + }); +}); diff --git a/apps/lite/ui/src/api/tags.ts b/apps/lite/ui/src/api/tags.ts new file mode 100644 index 00000000000..f37d0c2529a --- /dev/null +++ b/apps/lite/ui/src/api/tags.ts @@ -0,0 +1,102 @@ +/** + * @file The cache-tag declarations, turned into invalidation. + * + * The backend describes its cache effects in three declarations: each read + * endpoint provides tags, each mutation invalidates tags, and each watcher + * event invalidates tags. This file connects them to the queries lite + * actually caches, so "which caches to drop" is derived, never guessed. + */ + +import { projectQueryKeys, type QueryKey } from "#ui/api/queries.ts"; +import { apiInvalidates, apiProvides, type CacheTag } from "@gitbutler/but-sdk/cache-tags"; +import type { QueryClient } from "@tanstack/react-query"; + +/** + * Global queries by the tag they provide. The backend cannot know these: + * they are what lite caches without a project scope, under keys of its own. + */ +const globalProviders: Partial>> = { + Projects: ["projects"], + ForgeAccounts: ["forgeAccounts"], +}; + +/** + * Every query providing each tag, with the scope its key carries. + * + * The `apiProvides` index is the gate: a project query naming no declared + * endpoint does not compile, so it has to be declared in Rust or become a + * `LocalQueryKey`. + */ +const providers = new Map>(); +const provide = (tag: CacheTag, query: QueryKey, projectScoped: boolean) => { + const queries = providers.get(tag); + if (queries) queries.push({ query, projectScoped }); + else providers.set(tag, [{ query, projectScoped }]); +}; +for (const query of projectQueryKeys) + for (const tag of apiProvides[query]) provide(tag, query, true); +for (const [tag, queries] of Object.entries(globalProviders) as Array< + [CacheTag, ReadonlyArray] +>) + for (const query of queries) provide(tag, query, false); + +/** + * Drop every cache providing the given tags. Without a project id, + * project-scoped queries are invalidated across all projects by key prefix. + */ +export const invalidateTags = ( + client: QueryClient, + tags: ReadonlyArray, + projectId?: string, +): Promise => + Promise.all( + tags.flatMap((tag) => + (providers.get(tag) ?? []).map(({ query, projectScoped }) => + client.invalidateQueries({ + queryKey: projectScoped && projectId !== undefined ? [query, projectId] : [query], + }), + ), + ), + ); + +/** A mutation endpoint that declared what it invalidates. */ +export type DeclaredMutation = keyof typeof apiInvalidates & keyof typeof window.lite; + +/** + * The mutation options binding an endpoint to its declaration: the key names + * the endpoint, so on success the endpoint's `invalidates` tags are applied + * by the mutation cache. Spread it, overriding `mutationFn` when the call + * needs wrapping. + */ +export const apiMutation = (endpoint: Endpoint) => ({ + mutationKey: [endpoint] as const, + mutationFn: window.lite[endpoint], +}); + +/** The declarations by endpoint name, since a mutation key arrives as `unknown`. */ +const declaredInvalidates = new Map>( + Object.entries(apiInvalidates), +); + +/** + * Apply a finished mutation's declared invalidations. Wired once into the + * query client's mutation cache; mutations opt in by carrying their endpoint + * as `mutationKey`, which `apiMutation` arranges. + */ +export const invalidateDeclared = ( + client: QueryClient, + mutationKey: ReadonlyArray | undefined, + variables: unknown, +): Promise => { + const endpoint = mutationKey?.[0]; + const tags = typeof endpoint === "string" ? declaredInvalidates.get(endpoint) : undefined; + if (!tags) return Promise.resolve(); + const projectId = + typeof variables === "object" && + variables !== null && + "projectId" in variables && + typeof variables.projectId === "string" + ? variables.projectId + : undefined; + return invalidateTags(client, tags, projectId); +}; diff --git a/apps/lite/ui/src/main.tsx b/apps/lite/ui/src/main.tsx index 87aa2a2206d..2e32e392c20 100644 --- a/apps/lite/ui/src/main.tsx +++ b/apps/lite/ui/src/main.tsx @@ -1,6 +1,7 @@ -import { QueryClient, focusManager } from "@tanstack/react-query"; +import { MutationCache, QueryClient, focusManager } from "@tanstack/react-query"; import { createRouter } from "@tanstack/react-router"; import { App } from "#ui/App.tsx"; +import { invalidateDeclared } from "#ui/api/tags.ts"; import { routeTree } from "#ui/routeTree.ts"; import { createRoot } from "react-dom/client"; import "./global.css"; @@ -9,7 +10,9 @@ import { errorMessageForToast } from "#ui/errors.ts"; const toastManager = Toast.createToastManager(); -const queryClient = new QueryClient({ +// Annotated because the mutation-cache callback below refers back to the +// client: tsc cannot infer a type that appears in its own initializer. +const queryClient: QueryClient = new QueryClient({ defaultOptions: { queries: { // We don't expect network errors over the Node API. @@ -17,6 +20,12 @@ const queryClient = new QueryClient({ staleTime: Number.POSITIVE_INFINITY, }, }, + // A mutation's cache effects come from its endpoint's `invalidates` + // declaration; per-mutation handlers keep only toasts and pushes. + mutationCache: new MutationCache({ + onSuccess: (_data, variables, _context, mutation) => + invalidateDeclared(queryClient, mutation.options.mutationKey, variables), + }), }); // By default React Query uses `visibilitychange`, but this doesn't seem to work diff --git a/apps/lite/ui/src/operations/operation.ts b/apps/lite/ui/src/operations/operation.ts index c1e75a5cb9c..c49f106f856 100644 --- a/apps/lite/ui/src/operations/operation.ts +++ b/apps/lite/ui/src/operations/operation.ts @@ -10,7 +10,6 @@ import { Toast } from "@base-ui/react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { Match } from "effect"; -import type { QueryKey } from "#ui/api/queries.ts"; import { rejectedChangesToastOptions } from "#ui/operations/toastOptions.tsx"; import type { DiffSpec, InsertSide, RelativeTo } from "@gitbutler/but-sdk"; import { type Operand, operandEquals, operandFileParent } from "#ui/operands.ts"; @@ -201,7 +200,7 @@ export const useDryRunOperation = ({ return useQuery({ enabled: !!operation, - queryKey: ["dryRun" satisfies QueryKey, projectId, operation, changes], + queryKey: ["dryRun", projectId, operation, changes], queryFn: () => { if (!operation) return null; return executeOperation({ diff --git a/apps/lite/ui/src/project-events.test.ts b/apps/lite/ui/src/project-events.test.ts new file mode 100644 index 00000000000..f3f3c872bb4 --- /dev/null +++ b/apps/lite/ui/src/project-events.test.ts @@ -0,0 +1,75 @@ +import { projectQueryKeys } from "#ui/api/queries.ts"; +import { handleProjectEvent } from "#ui/project-events.ts"; +import type { WatcherEvent } from "@gitbutler/but-sdk"; +import { apiProvides, watcherInvalidates } from "@gitbutler/but-sdk/cache-tags"; +import type { QueryClient } from "@tanstack/react-query"; +import { describe, expect, it } from "vitest"; + +const provides = apiProvides as Record | undefined>; +const eventTags = watcherInvalidates as Record>; + +/** The queries `handleProjectEvent` invalidates, and the ones it pushes. */ +const react = (event: string) => { + const invalidated: Array = []; + const pushed: Array = []; + const client = { + invalidateQueries: ({ queryKey }: { queryKey: ReadonlyArray }) => { + invalidated.push(queryKey[0]); + return Promise.resolve(); + }, + setQueryData: (queryKey: ReadonlyArray) => pushed.push(queryKey[0]), + fetchQuery: () => Promise.reject(new Error("offline")), + } as unknown as QueryClient; + + const subject = event === "worktreeChanges" ? { changes: {} } : null; + handleProjectEvent( + { name: event, payload: { type: event, subject } } as WatcherEvent, + "p1", + client, + ); + return { invalidated, pushed }; +}; + +describe("tags declared in Rust", () => { + // Guards the generated map: if it ever arrives empty, every query silently + // stops refreshing and nothing else here would notice. + it("answers for most of the queries", () => { + expect(projectQueryKeys.filter((query) => query in provides).length).toBeGreaterThan(20); + }); + + it("has a declaration for every project query", () => { + expect(projectQueryKeys.filter((query) => !(query in provides))).toEqual([]); + }); + + it.each( + projectQueryKeys.filter((query) => + (provides[query] ?? []).some((tag) => + Object.values(eventTags).some((tags) => tags.includes(tag)), + ), + ), + )("refreshes %s after each event invalidating its tags", async (query) => { + const tags = provides[query] ?? []; + for (const [event, invalidatedTags] of Object.entries(eventTags)) { + if (!tags.some((tag) => invalidatedTags.includes(tag))) continue; + const { invalidated, pushed } = react(event); + // Some are refreshed after an await rather than straight away. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect([...invalidated, ...pushed], `after ${event}`).toContain(query); + } + }); +}); + +describe("handled separately", () => { + it("pushes worktree changes rather than invalidating them", () => { + const { invalidated, pushed } = react("worktreeChanges"); + expect(pushed).toEqual(["changesInWorktree"]); + expect(invalidated).not.toContain("changesInWorktree"); + }); + + it("still re-reads target commits after a fetch, once reviews have landed", async () => { + const { invalidated } = react("gitFetch"); + expect(invalidated).not.toContain("workspaceTargetCommits"); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(invalidated).toContain("workspaceTargetCommits"); + }); +}); diff --git a/apps/lite/ui/src/project-events.ts b/apps/lite/ui/src/project-events.ts new file mode 100644 index 00000000000..f2894f9936f --- /dev/null +++ b/apps/lite/ui/src/project-events.ts @@ -0,0 +1,105 @@ +/** + * @file React to events the backend sends about a subscribed project. + * + * Each event says what happened in the repository, never what the UI should do + * about it. What it should do is derived: the event declares the tags it makes + * stale, each query declares the tags it provides, and where they meet, the + * query is refreshed. The two cases where invalidating is not the best move + * are handled separately below. + */ + +import { + projectQueryKeys, + refreshIntegratedReviews, + type ProjectQueryKey, +} from "#ui/api/queries.ts"; +import type { WatcherEvent } from "@gitbutler/but-sdk"; +import { apiProvides, watcherInvalidates, type CacheTag } from "@gitbutler/but-sdk/cache-tags"; +import type { QueryClient } from "@tanstack/react-query"; + +/** What happened in the project, without the detail the event carried. */ +type ProjectEvent = WatcherEvent["payload"]["type"]; + +// The event table must answer for every event the backend can send. +const eventTags: Record> = watcherInvalidates; + +/** The events invalidating each tag, inverted once from the event table. */ +const eventsInvalidating = new Map>(); +for (const [event, tags] of Object.entries(eventTags) as Array< + [ProjectEvent, ReadonlyArray] +>) { + for (const tag of tags) { + const events = eventsInvalidating.get(tag); + if (events) events.push(event); + else eventsInvalidating.set(tag, [event]); + } +} + +/** The queries providing a tag the event invalidates. */ +type QueriesStaleAfter = { + [Q in keyof typeof apiProvides]: (typeof apiProvides)[Q][number] & + (typeof watcherInvalidates)[Event][number] extends never + ? never + : Q; +}[keyof typeof apiProvides] & + ProjectQueryKey; + +/** + * Queries left out of `invalidateOn`, because for these `handleProjectEvent` + * has a better answer than invalidating. Each entry says what it does instead. + * + * They are stale — the exception is about the remedy, not the diagnosis — so + * the `satisfies` holds them to that: naming a query the event does not make + * stale is a type error rather than a disagreement. + */ +const handledSeparately: Partial>> = { + // Pushes instead: the event carries the new changes, so invalidating would + // spend a round trip fetching what is already in hand. + worktreeChanges: new Set(["changesInWorktree"]), + // Invalidates later instead: re-reading before the review refresh lands + // returns commits without their annotations. + gitFetch: new Set(["workspaceTargetCommits"]), +} satisfies { [Event in ProjectEvent]?: ReadonlySet> }; + +/** + * The queries to invalidate when an event arrives, inverted once so an event + * costs a lookup rather than a walk of every query. + */ +const invalidateOn = new Map>(); +for (const query of projectQueryKeys) { + for (const tag of apiProvides[query]) { + for (const event of eventsInvalidating.get(tag) ?? []) { + if (handledSeparately[event]?.has(query)) continue; + const queries = invalidateOn.get(event); + if (!queries) invalidateOn.set(event, [query]); + else if (!queries.includes(query)) queries.push(query); + } + } +} + +export const handleProjectEvent = ( + event: WatcherEvent, + projectId: string, + client: QueryClient, +): void => { + const { payload } = event; + + if (payload.type === "worktreeChanges") + client.setQueryData(["changesInWorktree", projectId], () => payload.subject.changes); + + for (const query of invalidateOn.get(payload.type) ?? []) + void client.invalidateQueries({ queryKey: [query, projectId] }); + + // The annotations read the backend's review cache, so integrated reviews have + // to land before the listing is re-read. A failed refresh degrades to + // unannotated commits until the next fetch. + if (payload.type === "gitFetch") { + void refreshIntegratedReviews(client, projectId) + .catch(() => undefined) + .finally(() => { + void client.invalidateQueries({ + queryKey: ["workspaceTargetCommits", projectId], + }); + }); + } +}; diff --git a/apps/lite/ui/src/routes/project/$id/route.tsx b/apps/lite/ui/src/routes/project/$id/route.tsx index bc8395ea5c5..a3a363ccd8d 100644 --- a/apps/lite/ui/src/routes/project/$id/route.tsx +++ b/apps/lite/ui/src/routes/project/$id/route.tsx @@ -1,6 +1,6 @@ import { createRoute, notFound, Outlet } from "@tanstack/react-router"; import { Route as rootRoute } from "#ui/routes/__root.tsx"; -import { handleWatcher } from "#ui/watcher.ts"; +import { handleProjectEvent } from "#ui/project-events.ts"; export const Route = createRoute({ getParentRoute: () => rootRoute, @@ -16,7 +16,7 @@ export const Route = createRoute({ // Allow the route to render and handle failure via its queries. try { const subscriptionId = await window.lite.watcherSubscribe(params.id, (event) => - handleWatcher(event, params.id, context.queryClient), + handleProjectEvent(event, params.id, context.queryClient), ); return { subscriptionId }; } catch { diff --git a/apps/lite/ui/src/routes/project/$id/workspace/Settings/github-oauth.ts b/apps/lite/ui/src/routes/project/$id/workspace/Settings/github-oauth.ts index 066996dc6c9..ae7f79e27f8 100644 --- a/apps/lite/ui/src/routes/project/$id/workspace/Settings/github-oauth.ts +++ b/apps/lite/ui/src/routes/project/$id/workspace/Settings/github-oauth.ts @@ -1,3 +1,4 @@ +import { invalidateTags } from "#ui/api/tags.ts"; import type { QueryClient } from "@tanstack/react-query"; import { pollUntilSuccess } from "./poll.ts"; @@ -51,5 +52,5 @@ export const signInWithGithub = async ({ isRetryable: worthRetrying, }); - await client.invalidateQueries({ queryKey: ["forgeAccounts"] }); + await invalidateTags(client, ["ForgeAccounts"]); }; diff --git a/apps/lite/ui/src/watcher.ts b/apps/lite/ui/src/watcher.ts deleted file mode 100644 index bf90165b89e..00000000000 --- a/apps/lite/ui/src/watcher.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { refreshIntegratedReviews, type ProjectQueryKey } from "#ui/api/queries.ts"; -import type { WatcherEvent } from "@gitbutler/but-sdk"; -import type { QueryClient } from "@tanstack/react-query"; - -type WatcherEventType = WatcherEvent["payload"]["type"]; - -/** - * A `Record`, not a `Partial`: a new project query has to declare what refreshes - * it, or this stops compiling. - */ -const refreshedBy: Record> = { - absorptionPlan: ["gitActivity", "workspaceActivity", "worktreeChanges"], - // The generated name is deduped against local branches and the short names of - // remote-tracking branches, so anything that moves a ref can invalidate it. - branchCannedName: ["gitFetch", "gitActivity", "workspaceActivity"], - // A fetch changes no local commit, but it moves remote-tracking refs. - branchDetails: ["gitFetch", "gitActivity", "workspaceActivity"], - branchDiff: ["gitFetch", "gitActivity", "workspaceActivity"], - branchList: ["gitFetch", "gitActivity", "workspaceActivity"], - // Pushed below rather than invalidated: `worktreeChanges` carries the data. - changesInWorktree: ["gitActivity", "workspaceActivity"], - ciChecks: [], - commentReactions: [], - comments: ["gitActivity", "workspaceActivity", "worktreeChanges"], - // Derived from the trees the commit itself carries, so nothing outside it - // can change the answer. Resolving rewrites the commit, which moves the - // query to a new key rather than staling this one. - commitConflicts: [], - commitDetailsWithLineStats: ["gitActivity", "workspaceActivity"], - currentForgeLogin: [], - dryRun: ["gitActivity", "workspaceActivity", "worktreeChanges"], - forgeInfo: [], - gbConfig: [], - headInfo: ["gitActivity", "workspaceActivity"], - repoLabels: [], - review: ["gitFetch"], - reviewComments: [], - reviewMergeStatus: [], - reviewReactions: [], - reviewSubmissions: [], - reviewTimelineEvents: [], - reviewerCandidates: [], - reviews: ["gitFetch"], - signingSettings: [], - treeChangeDiffs: ["gitActivity", "workspaceActivity", "worktreeChanges"], - workspaceFetch: [], - workspaceFetchStatus: ["gitFetch"], - // `gitFetch` refreshes this too, but only once reviews land — see below. - workspaceTargetCommits: ["gitActivity", "workspaceActivity"], -}; - -/** Inverted once, so an event costs a lookup rather than a walk of every query. */ -const staleAfter = new Map>(); -for (const [key, events] of Object.entries(refreshedBy) as Array< - [ProjectQueryKey, ReadonlyArray] ->) { - for (const event of events) { - const keys = staleAfter.get(event); - if (keys) keys.push(key); - else staleAfter.set(event, [key]); - } -} - -export const handleWatcher = ( - event: WatcherEvent, - projectId: string, - client: QueryClient, -): void => { - const { payload } = event; - - if (payload.type === "worktreeChanges") { - client.setQueryData( - ["changesInWorktree" satisfies ProjectQueryKey, projectId], - () => payload.subject.changes, - ); - } - - for (const key of staleAfter.get(payload.type) ?? []) - void client.invalidateQueries({ queryKey: [key, projectId] }); - - // The annotations read the backend's review cache, so integrated reviews have - // to land before the listing is re-read. A failed refresh degrades to - // unannotated commits until the next fetch. - if (payload.type === "gitFetch") { - void refreshIntegratedReviews(client, projectId) - .catch(() => undefined) - .finally(() => { - void client.invalidateQueries({ - queryKey: ["workspaceTargetCommits" satisfies ProjectQueryKey, projectId], - }); - }); - } -}; diff --git a/crates/but-api-macros/src/lib.rs b/crates/but-api-macros/src/lib.rs index 15e13fdae94..7adade97c13 100644 --- a/crates/but-api-macros/src/lib.rs +++ b/crates/but-api-macros/src/lib.rs @@ -21,6 +21,23 @@ use syn::{FnArg, ItemFn, Pat, parse_macro_input}; /// - `Result>` converts each `T` into `JSONReturnType`. /// - Controls how the actual return value is fallibly converted for JSON serialization in `func_json` and `func_cmd`. /// +/// * `provides = [CacheTag, ..]` +/// - Use it on a read like `but_api(napi, provides = [Reviews])` to declare which tags its +/// result is made of. Clients refresh their cache of it whenever a mutation or a watcher +/// event invalidates one of those tags. +/// - Omitting it means "not classified yet", which stays distinguishable from `[]`, meaning +/// "no tag — nothing refreshes this". +/// * `invalidates = [CacheTag, ..]` +/// - Use it on a mutation like `but_api(napi, invalidates = [Reviews])` to declare which tags +/// it makes stale, so clients drop those caches when the call succeeds. +/// - Only for state the repository watcher cannot observe: forge, app, and config writes. A +/// mutation that writes to the repository declares nothing — the watcher reports the change +/// and the event carries the invalidation. +/// +/// An endpoint either provides or invalidates, never both. Naming a tag that does not exist in +/// `crate::tags::CacheTag` is a compile error. The SDK exports both maps (`apiProvides`, +/// `apiInvalidates`) together with the event table as `cache-tags`. +/// /// # Parameter Attributes /// /// Function parameters may use `#[but_api(TransportType)]` to keep the implementation @@ -319,15 +336,44 @@ pub fn but_api(attr: TokenStream, item: TokenStream) -> TokenStream { .collect(); let js_name_str = js_name.clone().unwrap_or_else(|| fn_name.to_string()); + // `None` stays distinguishable from an empty list: unclassified is not the + // same answer as "no tag". + let tag_list_value = |tags: &Option>| match tags { + None => quote! { None }, + Some(tags) => { + let names: Vec = tags.iter().map(|tag| tag.to_string()).collect(); + quote! { Some(&[#(#names),*]) } + } + }; + let provides_value = tag_list_value(&opts.provides); + let invalidates_value = tag_list_value(&opts.invalidates); + + // Naming a tag that does not exist fails here rather than shipping a name + // nothing ever matches. + let tag_checks = opts + .provides + .iter() + .chain(opts.invalidates.iter()) + .flatten() + .map(|tag| { + quote! { + const _: crate::tags::CacheTag = crate::tags::CacheTag::#tag; + } + }); + // Registered whenever the attribute opts into napi, deliberately not // behind `cfg(feature = "napi")`: but-ts reads this registry and builds // but-api without that feature. The entry is inert metadata either way. let napi_registry_entry = if opts.napi { quote! { + #(#tag_checks)* + ::but_schemars::internal_submit! { ::but_schemars::ApiFnEntry { js_name: #js_name_str, params: &[#(#napi_param_js_names),*], + provides: #provides_value, + invalidates: #invalidates_value, } } } @@ -1105,6 +1151,16 @@ struct Options { /// If `true`, generate a `_napi` function for Node.js bindings. /// Enabled by writing `#[but_api(napi)]` or `#[but_api(napi, try_from = Foo)]`. napi: bool, + /// `CacheTag` variants this read's result is made of, written + /// `#[but_api(napi, provides = [Reviews])]`. + /// + /// `None` means unclassified; `Some([])` means classified as "no tag". + /// Consumers need to tell those apart. + provides: Option>, + /// `CacheTag` variants this mutation makes stale, written + /// `#[but_api(napi, invalidates = [Reviews])]`. Mutually exclusive with + /// `provides`. + invalidates: Option>, } struct ResultConversion { @@ -1124,15 +1180,50 @@ fn parse_options( ) -> syn::Result { let mut napi = false; let mut conversion_path: Option<(FromMode, syn::Path)> = None; + let mut provides: Option> = None; + let mut invalidates: Option> = None; + let mut tag_list_ident: Option = None; while !input.is_empty() { if input.peek(syn::Ident) && input.peek2(syn::Token![=]) { - // try_from = Path + // try_from = Path, or provides/invalidates = [Tag, ..] let ident: syn::Ident = input.parse()?; + if ident == "provides" || ident == "invalidates" { + input.parse::()?; + let content; + syn::bracketed!(content in input); + let tags = + syn::punctuated::Punctuated::::parse_terminated( + &content, + )?; + let list = if ident == "provides" { + &mut provides + } else { + &mut invalidates + }; + if list.is_some() { + return Err(syn::Error::new_spanned( + &ident, + format!("Only one `{ident}` list may be specified"), + )); + } + *list = Some(tags.into_iter().collect()); + if provides.is_some() && invalidates.is_some() { + return Err(syn::Error::new_spanned( + &ident, + "An endpoint either provides tags or invalidates them, not both", + )); + } + tag_list_ident = Some(ident); + if !input.is_empty() { + input.parse::()?; + } + continue; + } if ident != "try_from" { return Err(syn::Error::new_spanned( ident, - "Expected `try_from = Type`; only `try_from` is supported as a key", + "Expected `try_from = Type`, `provides = [Tag, ..]`, or `invalidates = [Tag, ..]`", )); } input.parse::()?; @@ -1164,6 +1255,16 @@ fn parse_options( } } + // `napi` may follow the list in the attribute, so this can only be judged here. + if let Some(ident) = &tag_list_ident + && !napi + { + return Err(syn::Error::new_spanned( + ident, + format!("`{ident}` requires `napi`: tag declarations are read from the napi registry"), + )); + } + let result_conversion = conversion_path.map(|(mode, p)| { let base_ty = syn::Type::Path(syn::TypePath { qself: None, @@ -1185,6 +1286,8 @@ fn parse_options( Ok(Options { result_conversion, napi, + provides, + invalidates, }) } diff --git a/crates/but-api-macros/tests/src/lib.rs b/crates/but-api-macros/tests/src/lib.rs index b1089344e4d..f2dd9e6310a 100644 --- a/crates/but-api-macros/tests/src/lib.rs +++ b/crates/but-api-macros/tests/src/lib.rs @@ -13,6 +13,15 @@ use std::str::FromStr; +pub mod tags { + /// Stand-in for `but_api::tags::CacheTag`, holding just enough variants + /// for the `provides`/`invalidates` expansion checks. + pub enum CacheTag { + Reviews, + Checks, + } +} + pub mod panic_capture { pub fn panic_payload_to_anyhow( function_name: &str, diff --git a/crates/but-api-macros/tests/tests/ui/fail/base_invalid_attr_key.stderr b/crates/but-api-macros/tests/tests/ui/fail/base_invalid_attr_key.stderr index addb08ad812..28c15736f8b 100644 --- a/crates/but-api-macros/tests/tests/ui/fail/base_invalid_attr_key.stderr +++ b/crates/but-api-macros/tests/tests/ui/fail/base_invalid_attr_key.stderr @@ -1,4 +1,4 @@ -error: Expected `try_from = Type`; only `try_from` is supported as a key +error: Expected `try_from = Type`, `provides = [Tag, ..]`, or `invalidates = [Tag, ..]` --> tests/ui/fail/base_invalid_attr_key.rs:8:11 | 8 | #[but_api(from = json::HexHash)] diff --git a/crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.rs b/crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.rs new file mode 100644 index 00000000000..92916435692 --- /dev/null +++ b/crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.rs @@ -0,0 +1,17 @@ +// Case: an endpoint declaring `provides` without `napi` should fail — the +// declaration is only read from the napi registry, so it would be silently +// dropped. +// Extend when: tag declarations gain a non-napi consumer. + +use but_api_macros::but_api; + +pub use but_api_macros_tests::{json, panic_capture, tags}; + +#[but_api(provides = [Reviews])] +pub fn provides_without_napi() -> anyhow::Result { + Ok(json::HexHash( + "0123456789abcdef0123456789abcdef01234567".into(), + )) +} + +fn main() {} diff --git a/crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.stderr b/crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.stderr new file mode 100644 index 00000000000..224f97c56f9 --- /dev/null +++ b/crates/but-api-macros/tests/tests/ui/fail/base_provides_requires_napi.stderr @@ -0,0 +1,5 @@ +error: `provides` requires `napi`: tag declarations are read from the napi registry + --> tests/ui/fail/base_provides_requires_napi.rs:10:11 + | +10 | #[but_api(provides = [Reviews])] + | ^^^^^^^^ diff --git a/crates/but-api-macros/tests/tests/ui/fail/napi_list_attr_unsupported.stderr b/crates/but-api-macros/tests/tests/ui/fail/napi_list_attr_unsupported.stderr index c1ac307fcc6..506994e4853 100644 --- a/crates/but-api-macros/tests/tests/ui/fail/napi_list_attr_unsupported.stderr +++ b/crates/but-api-macros/tests/tests/ui/fail/napi_list_attr_unsupported.stderr @@ -1,4 +1,4 @@ -error: Expected `try_from = Type`; only `try_from` is supported as a key +error: Expected `try_from = Type`, `provides = [Tag, ..]`, or `invalidates = [Tag, ..]` --> tests/ui/fail/napi_list_attr_unsupported.rs:8:17 | 8 | #[but_api(napi, from = json::HexHash)] diff --git a/crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.rs b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.rs new file mode 100644 index 00000000000..9c2b199ddf6 --- /dev/null +++ b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.rs @@ -0,0 +1,16 @@ +// Case: an endpoint declaring both `provides` and `invalidates` should fail — +// it is either a read or a mutation, never both. +// Extend when: that exclusivity rule changes. + +use but_api_macros::but_api; + +pub use but_api_macros_tests::{json, panic_capture, tags}; + +#[but_api(napi, provides = [Reviews], invalidates = [Checks])] +pub fn provides_and_invalidates() -> anyhow::Result { + Ok(json::HexHash( + "0123456789abcdef0123456789abcdef01234567".into(), + )) +} + +fn main() {} diff --git a/crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.stderr b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.stderr new file mode 100644 index 00000000000..ca9d1f34ee3 --- /dev/null +++ b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_and_invalidates.stderr @@ -0,0 +1,5 @@ +error: An endpoint either provides tags or invalidates them, not both + --> tests/ui/fail/napi_provides_and_invalidates.rs:9:39 + | +9 | #[but_api(napi, provides = [Reviews], invalidates = [Checks])] + | ^^^^^^^^^^^ diff --git a/crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.rs b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.rs new file mode 100644 index 00000000000..960e606ba90 --- /dev/null +++ b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.rs @@ -0,0 +1,15 @@ +// Case: `provides` given twice should fail macro option parsing. +// Extend when: the accepted shape of the tag lists changes. + +use but_api_macros::but_api; + +pub use but_api_macros_tests::{json, panic_capture, tags}; + +#[but_api(napi, provides = [Reviews], provides = [Checks])] +pub fn duplicated_provides() -> anyhow::Result { + Ok(json::HexHash( + "0123456789abcdef0123456789abcdef01234567".into(), + )) +} + +fn main() {} diff --git a/crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.stderr b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.stderr new file mode 100644 index 00000000000..f2dd52a7d7a --- /dev/null +++ b/crates/but-api-macros/tests/tests/ui/fail/napi_provides_duplicated.stderr @@ -0,0 +1,5 @@ +error: Only one `provides` list may be specified + --> tests/ui/fail/napi_provides_duplicated.rs:8:39 + | +8 | #[but_api(napi, provides = [Reviews], provides = [Checks])] + | ^^^^^^^^ diff --git a/crates/but-api/src/bitbucket.rs b/crates/but-api/src/bitbucket.rs index f36edc08265..5c32fc82e2d 100644 --- a/crates/but-api/src/bitbucket.rs +++ b/crates/but-api/src/bitbucket.rs @@ -19,7 +19,7 @@ use tracing::instrument; /// /// * `Ok(_)` - The token is valid and stored /// * `Err(_)` - If the token is invalid or storage fails -#[but_api(napi, json::BitbucketAuthStatusResponse)] +#[but_api(napi, json::BitbucketAuthStatusResponse, invalidates = [ForgeAccounts, ForgeLogin])] #[instrument(err(Debug))] pub async fn store_bitbucket_api_token( email: String, @@ -38,7 +38,7 @@ pub async fn store_bitbucket_api_token( /// # Returns /// /// * `Ok(())` - Always succeeds, even if no token was found -#[but_api(napi)] +#[but_api(napi, invalidates = [ForgeAccounts, ForgeLogin])] #[instrument(err(Debug))] pub fn forget_bitbucket_account(account: but_bitbucket::BitbucketAccountIdentifier) -> Result<()> { let storage = but_forge_storage::Controller::from_path(but_path::app_data_dir()?); diff --git a/crates/but-api/src/branch.rs b/crates/but-api/src/branch.rs index 1b8e62df509..9cdb5a92380 100644 --- a/crates/but-api/src/branch.rs +++ b/crates/but-api/src/branch.rs @@ -907,7 +907,7 @@ pub fn apply_with_perm( /// exists. Uniqueness only holds at the time of the call: the name is /// deduplicated against local branches and the short names of remote-tracking /// branches, both of which can change afterwards. -#[but_api(napi)] +#[but_api(napi, provides = [Branches])] #[instrument(err(Debug))] pub fn branch_canned_name(ctx: &Context) -> anyhow::Result { let _guard = ctx.shared_worktree_access(); @@ -1665,7 +1665,7 @@ fn checkout_ref_with_perm( /// `branch` is resolved by name in the repository referenced by `ctx`, and the /// diff is computed against the current workspace state. For lower-level /// implementation details, see [`but_workspace::ui::diff::changes_in_branch()`]. -#[but_api(napi)] +#[but_api(napi, provides = [Branches])] #[instrument(err(Debug))] pub fn branch_diff(ctx: &Context, branch: String) -> anyhow::Result { let (_guard, repo, ws, _) = ctx.workspace_and_db()?; @@ -1682,7 +1682,7 @@ pub fn branch_diff(ctx: &Context, branch: String) -> anyhow::Result /// ordered most recently updated first; group by `status` to lead with the /// workspace-related ones. Ahead-counts are relative to the /// project's configured target branch, which clients know from the project APIs. -#[but_api(napi, json::ListedStack)] +#[but_api(napi, json::ListedStack, provides = [Branches])] #[instrument(err(Debug))] pub fn branch_list(ctx: &Context) -> anyhow::Result> { let meta = ctx.meta()?; diff --git a/crates/but-api/src/comments.rs b/crates/but-api/src/comments.rs index 2dfa8070ed2..d0474ead338 100644 --- a/crates/but-api/src/comments.rs +++ b/crates/but-api/src/comments.rs @@ -58,7 +58,7 @@ pub fn comment_create_with_perm( /// List all unarchived comments, re-anchored against the current diffs. /// /// See [`but_comments::list_comments`] for the re-anchoring and auto-archiving semantics. -#[but_api(napi)] +#[but_api(napi, provides = [Comments])] #[instrument(skip(ctx), err(Debug))] pub fn comments_list(ctx: &Context) -> anyhow::Result> { let guard = ctx.shared_worktree_access(); diff --git a/crates/but-api/src/diff.rs b/crates/but-api/src/diff.rs index 352345f579e..559afa0501b 100644 --- a/crates/but-api/src/diff.rs +++ b/crates/but-api/src/diff.rs @@ -81,7 +81,7 @@ pub fn commit_details( /// /// This exists for callers that always want line statistics without passing /// `line_stats` explicitly. -#[but_api(napi, json::CommitDetails)] +#[but_api(napi, json::CommitDetails, provides = [Commits])] #[instrument(err(Debug))] pub fn commit_details_with_line_stats( ctx: &Context, @@ -94,7 +94,7 @@ pub fn commit_details_with_line_stats( /// /// `change` must not be a type change or a submodule change. For lower-level /// implementation details, see [`but_core::TreeChange::unified_patch()`]. -#[but_api(napi)] +#[but_api(napi, provides = [Diffs])] #[instrument(err(Debug))] pub fn tree_change_diffs( ctx: &Context, @@ -106,7 +106,7 @@ pub fn tree_change_diffs( } /// See [`changes_in_worktree_with_perm()`]. -#[but_api(napi)] +#[but_api(napi, provides = [WorktreeChanges])] #[instrument(err(Debug))] pub fn changes_in_worktree( ctx: &Context, diff --git a/crates/but-api/src/github.rs b/crates/but-api/src/github.rs index 7e0b5613517..3af634c398a 100644 --- a/crates/but-api/src/github.rs +++ b/crates/but-api/src/github.rs @@ -58,7 +58,7 @@ pub async fn check_github_auth_status(device_code: String) -> Result) -> Result { let storage = but_forge_storage::Controller::from_path(but_path::app_data_dir()?); @@ -101,7 +101,7 @@ pub async fn store_github_enterprise_pat( /// # Returns /// /// * `Ok(())` - Always succeeds, even if no token was found -#[but_api(napi)] +#[but_api(napi, invalidates = [ForgeAccounts, ForgeLogin])] #[instrument(err(Debug))] pub fn forget_github_account(account: but_github::GithubAccountIdentifier) -> Result<()> { let storage = but_forge_storage::Controller::from_path(but_path::app_data_dir()?); diff --git a/crates/but-api/src/gitlab.rs b/crates/but-api/src/gitlab.rs index f0433819e1a..798f3ccf738 100644 --- a/crates/but-api/src/gitlab.rs +++ b/crates/but-api/src/gitlab.rs @@ -17,7 +17,7 @@ use tracing::instrument; /// /// * `Ok(_)` - The token is valid and stored /// * `Err(_)` - If the token is invalid or storage fails -#[but_api(napi, json::GitlabAuthStatusResponse)] +#[but_api(napi, json::GitlabAuthStatusResponse, invalidates = [ForgeAccounts, ForgeLogin])] #[instrument(err(Debug))] pub async fn store_gitlab_pat(access_token: Sensitive) -> Result { let storage = but_forge_storage::Controller::from_path(but_path::app_data_dir()?); @@ -60,7 +60,7 @@ pub async fn store_gitlab_selfhosted_pat( /// # Returns /// /// * `Ok(())` - Always succeeds, even if no token was found -#[but_api(napi)] +#[but_api(napi, invalidates = [ForgeAccounts, ForgeLogin])] #[instrument(err(Debug))] pub fn forget_gitlab_account(account: but_gitlab::GitlabAccountIdentifier) -> Result<()> { let storage = but_forge_storage::Controller::from_path(but_path::app_data_dir()?); diff --git a/crates/but-api/src/legacy/absorb.rs b/crates/but-api/src/legacy/absorb.rs index 0e26993bb5d..904dd333550 100644 --- a/crates/but-api/src/legacy/absorb.rs +++ b/crates/but-api/src/legacy/absorb.rs @@ -97,7 +97,7 @@ pub fn absorb_with_perm( /// Build an absorption plan for `target` using the behavior documented by /// [`absorption_plan_with_perm()`]. -#[but_api(napi)] +#[but_api(napi, provides = [AbsorptionPlan])] #[instrument(err(Debug))] pub fn absorption_plan( ctx: &mut Context, diff --git a/crates/but-api/src/legacy/config.rs b/crates/but-api/src/legacy/config.rs index ff9ace1257a..f628ea18916 100644 --- a/crates/but-api/src/legacy/config.rs +++ b/crates/but-api/src/legacy/config.rs @@ -6,14 +6,14 @@ use gix::bstr::BString; use serde::Serialize; use tracing::instrument; -#[but_api(napi)] +#[but_api(napi, provides = [GbConfig])] #[instrument(err(Debug))] pub fn get_gb_config(ctx: &but_ctx::Context) -> Result { let repo = ctx.repo.get()?; repo.git_settings().map(Into::into) } -#[but_api(napi)] +#[but_api(napi, invalidates = [GbConfig, SigningSettings])] #[instrument(err(Debug))] pub fn set_gb_config(ctx: &but_ctx::Context, config: GitConfigSettings) -> Result<()> { ctx.repo.get()?.set_git_settings(&config.into()) diff --git a/crates/but-api/src/legacy/forge.rs b/crates/but-api/src/legacy/forge.rs index 97788009ef3..0a9466fc70b 100644 --- a/crates/but-api/src/legacy/forge.rs +++ b/crates/but-api/src/legacy/forge.rs @@ -104,7 +104,7 @@ pub fn forge_provider(ctx: &Context) -> Result> { /// Per-project forge display + URL config. Lets the renderer build /// commit/PR URLs and pick labels without branching on forge name. /// Returns no value when the project has no target yet or its target forge is unknown. -#[but_api(napi)] +#[but_api(napi, provides = [ForgeInfo])] #[instrument(err(Debug))] pub fn forge_info(ctx: &Context) -> Result> { let project_meta = ctx.project_meta()?; @@ -265,7 +265,7 @@ pub fn set_review_template(ctx: &but_ctx::Context, template_path: Option repo.set_git_settings(&git_config) } -#[but_api(napi)] +#[but_api(napi, provides = [Reviews])] #[instrument(err(Debug))] pub fn list_reviews( ctx: &Context, @@ -692,7 +692,7 @@ pub async fn get_review_base_repo_url( } /// List the top-level conversation comments on a review, oldest first. -#[but_api(napi)] +#[but_api(napi, provides = [ReviewComments])] #[instrument(err(Debug))] pub async fn list_review_comments( ctx: ThreadSafeContext, @@ -704,7 +704,7 @@ pub async fn list_review_comments( } /// List the individual reactions (with who reacted) on a review itself. -#[but_api(napi)] +#[but_api(napi, provides = [ReviewReactions])] #[instrument(err(Debug))] pub async fn list_review_reactions( ctx: ThreadSafeContext, @@ -716,7 +716,7 @@ pub async fn list_review_reactions( } /// List the individual reactions (with who reacted) on one comment. -#[but_api(napi)] +#[but_api(napi, provides = [CommentReactions])] #[instrument(err(Debug))] pub async fn list_comment_reactions( ctx: ThreadSafeContext, @@ -733,7 +733,7 @@ pub async fn list_comment_reactions( } /// Add the caller's reaction to a review itself. -#[but_api(napi)] +#[but_api(napi, invalidates = [ReviewReactions])] #[instrument(err(Debug))] pub async fn add_review_reaction( ctx: ThreadSafeContext, @@ -752,7 +752,7 @@ pub async fn add_review_reaction( } /// Remove one of the caller's reactions from a review itself. -#[but_api(napi)] +#[but_api(napi, invalidates = [ReviewReactions])] #[instrument(err(Debug))] pub async fn remove_review_reaction( ctx: ThreadSafeContext, @@ -771,7 +771,7 @@ pub async fn remove_review_reaction( } /// Add the caller's reaction to one comment. -#[but_api(napi)] +#[but_api(napi, invalidates = [CommentReactions, ReviewComments])] #[instrument(err(Debug))] pub async fn add_comment_reaction( ctx: ThreadSafeContext, @@ -790,7 +790,7 @@ pub async fn add_comment_reaction( } /// Remove one of the caller's reactions from one comment. -#[but_api(napi)] +#[but_api(napi, invalidates = [CommentReactions, ReviewComments])] #[instrument(err(Debug))] pub async fn remove_comment_reaction( ctx: ThreadSafeContext, @@ -809,7 +809,7 @@ pub async fn remove_comment_reaction( } /// List the pushed commits and review requests on a review's timeline. -#[but_api(napi)] +#[but_api(napi, provides = [ReviewTimeline])] #[instrument(err(Debug))] pub async fn list_review_timeline_events( ctx: ThreadSafeContext, @@ -826,7 +826,7 @@ pub async fn list_review_timeline_events( } /// List the submitted reviews (approvals, change requests) on a review. -#[but_api(napi)] +#[but_api(napi, provides = [ReviewSubmissions])] #[instrument(err(Debug))] pub async fn list_review_submissions( ctx: ThreadSafeContext, @@ -838,7 +838,7 @@ pub async fn list_review_submissions( } /// Edit a top-level conversation comment on a review. -#[but_api(napi)] +#[but_api(napi, invalidates = [ReviewComments])] #[instrument(err(Debug))] pub async fn update_review_comment( ctx: ThreadSafeContext, @@ -857,7 +857,7 @@ pub async fn update_review_comment( } /// Delete a top-level conversation comment on a review. -#[but_api(napi)] +#[but_api(napi, invalidates = [ReviewComments])] #[instrument(err(Debug))] pub async fn delete_review_comment(ctx: ThreadSafeContext, comment_id: i64) -> Result<()> { let (storage, forge_repo_info, preferred_forge_user) = forge_endpoint_context(ctx)?; @@ -872,7 +872,7 @@ pub async fn delete_review_comment(ctx: ThreadSafeContext, comment_id: i64) -> R /// The login this project's forge calls authenticate as, if any account is /// configured. Resolved from stored accounts; no network. -#[but_api(napi)] +#[but_api(napi, provides = [ForgeLogin])] #[instrument(err(Debug))] pub fn current_forge_login(ctx: &Context) -> Result> { let project_meta = ctx.project_meta()?; @@ -891,7 +891,7 @@ pub fn current_forge_login(ctx: &Context) -> Result> { } /// List the labels defined on the repository backing this project's reviews. -#[but_api(napi)] +#[but_api(napi, provides = [RepoLabels])] #[instrument(err(Debug))] pub async fn list_repo_labels(ctx: ThreadSafeContext) -> Result> { let (storage, forge_repo_info, preferred_forge_user) = forge_endpoint_context(ctx)?; @@ -899,7 +899,7 @@ pub async fn list_repo_labels(ctx: ThreadSafeContext) -> Result Result { let (storage, forge_repo_info, preferred_forge_user) = { @@ -1063,7 +1063,7 @@ pub async fn get_repo_info(ctx: ThreadSafeContext) -> Result Result { Ok(size) } -#[but_api(napi)] +#[but_api(napi, invalidates = [Projects])] #[instrument(err(Debug))] pub fn delete_all_data() -> Result<()> { for project in gitbutler_project::dangerously_list_projects_without_migration() diff --git a/crates/but-api/src/legacy/projects.rs b/crates/but-api/src/legacy/projects.rs index 2e26cdde075..366c84b9213 100644 --- a/crates/but-api/src/legacy/projects.rs +++ b/crates/but-api/src/legacy/projects.rs @@ -30,7 +30,7 @@ pub struct ProjectSettingsUpdate { but_schemars::register_sdk_type!(ProjectSettingsUpdate); /// Change the stored settings of a project, leaving absent fields as they were. -#[but_api(napi)] +#[but_api(napi, invalidates = [Projects])] #[instrument(err(Debug))] pub fn update_project_settings( project_id: ProjectHandleOrLegacyProjectId, @@ -161,7 +161,7 @@ pub fn list_projects( }) } -#[but_api(napi)] +#[but_api(napi, invalidates = [Projects])] #[instrument(err(Debug))] pub fn delete_project(project_id: ProjectHandleOrLegacyProjectId) -> Result<()> { delete_project_at_app_data_dir(but_path::app_data_dir()?, project_id) diff --git a/crates/but-api/src/legacy/repo.rs b/crates/but-api/src/legacy/repo.rs index a29d4a6735a..e1a68be887a 100644 --- a/crates/but-api/src/legacy/repo.rs +++ b/crates/but-api/src/legacy/repo.rs @@ -11,7 +11,7 @@ use gitbutler_repo::{ }; use tracing::instrument; -#[but_api(napi)] +#[but_api(napi, provides = [SigningSettings])] #[instrument(err(Debug))] pub fn check_signing_settings(ctx: &Context) -> Result { ctx.check_signing_settings() diff --git a/crates/but-api/src/legacy/workspace.rs b/crates/but-api/src/legacy/workspace.rs index 76505d9b005..9214629da30 100644 --- a/crates/but-api/src/legacy/workspace.rs +++ b/crates/but-api/src/legacy/workspace.rs @@ -25,7 +25,7 @@ use tracing::instrument; use crate::json::HexHash; -#[but_api(napi, try_from = but_workspace::ui::RefInfo)] +#[but_api(napi, try_from = but_workspace::ui::RefInfo, provides = [Workspace])] #[instrument(err(Debug))] pub fn head_info(ctx: &but_ctx::Context) -> Result { let traversal = ctx.graph_options(but_graph::init::Options::limited())?; @@ -222,7 +222,7 @@ fn handle_gerrit( Ok(()) } -#[but_api(napi)] +#[but_api(napi, provides = [Branches])] #[instrument(err(Debug))] pub fn branch_details( ctx: &but_ctx::Context, @@ -427,7 +427,7 @@ pub fn target_commits( } /// Push a branch and any parent references that lie within the current workspace projection. -#[but_api(napi, json::PushResult)] +#[but_api(napi, json::PushResult, invalidates = [Workspace, Reviews, MergeStatus, Checks, ReviewTimeline])] #[instrument(err(Debug))] pub async fn workspace_branch_and_ancestors_push( ctx: ThreadSafeContext, diff --git a/crates/but-api/src/lib.rs b/crates/but-api/src/lib.rs index ab4ee788e86..d2d7f3bce3c 100644 --- a/crates/but-api/src/lib.rs +++ b/crates/but-api/src/lib.rs @@ -72,6 +72,9 @@ pub mod panic_capture; #[cfg(feature = "export-schema")] pub mod watcher; +/// The tag vocabulary clients cache API results under. +pub mod tags; + /// Functions for workspace state. pub mod workspace_state; diff --git a/crates/but-api/src/resolve/mod.rs b/crates/but-api/src/resolve/mod.rs index 595e131a057..6ce2e5e94ae 100644 --- a/crates/but-api/src/resolve/mod.rs +++ b/crates/but-api/src/resolve/mod.rs @@ -80,7 +80,7 @@ but_schemars::register_sdk_type!(ConflictedFile); /// result. Fails for commits whose conflicts have no hunk representation /// (deletions/renames, binaries, oversized files, marker-like content) — those /// need manual resolution in edit mode. -#[but_api(napi, try_from = crate::resolve::json::CommitConflicts)] +#[but_api(napi, try_from = crate::resolve::json::CommitConflicts, provides = [])] #[instrument(err(Debug))] pub fn commit_conflicts( ctx: &but_ctx::Context, diff --git a/crates/but-api/src/tags.rs b/crates/but-api/src/tags.rs new file mode 100644 index 00000000000..976e1837863 --- /dev/null +++ b/crates/but-api/src/tags.rs @@ -0,0 +1,88 @@ +//! The tag vocabulary clients cache API results under. +//! +//! A tag names one kind of cached state. Three declarations, all in Rust, +//! describe everything that happens to it: +//! +//! * a read endpoint says what its result is made of: `#[but_api(provides = [Reviews])]` +//! * a mutation says what it makes stale: `#[but_api(invalidates = [Reviews])]` +//! * a watcher event says what it makes stale: [`crate::watcher::WatcherEventKind::invalidates`] +//! +//! Clients derive every cache refresh from those three, so which caches to +//! drop after a mutation or an event is never guessed on the frontend. +//! Mutations that only write to the repository declare nothing: the watcher +//! observes the repository, and the event carries the invalidation. + +macro_rules! cache_tags { + ($($(#[$doc:meta])+ $name:ident,)+) => { + /// One kind of cached state a client may hold. See the module docs. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub enum CacheTag { + $($(#[$doc])+ $name,)+ + } + + impl CacheTag { + /// Every tag, for the SDK generator to enumerate. + pub const ALL: &'static [CacheTag] = &[$(CacheTag::$name,)+]; + + /// The tag's name as clients see it. + pub fn name(self) -> &'static str { + match self { + $(CacheTag::$name => stringify!($name),)+ + } + } + } + }; +} + +cache_tags! { + /// The branch listing and per-branch details and diffs. + Branches, + /// Commits on the workspace's target branch. + TargetCommits, + /// The workspace head: applied stacks and their segments. + Workspace, + /// A single commit's details. + Commits, + /// Diffs of individual changes, committed or not. + Diffs, + /// Uncommitted file changes with their assignments. + WorktreeChanges, + /// Where uncommitted changes would absorb into existing commits. + AbsorptionPlan, + /// GitButler's own diff comments. + Comments, + /// When the workspace last fetched. + FetchStatus, + /// Forge reviews, listed or single. + Reviews, + /// Comments on a forge review. + ReviewComments, + /// A forge review's timeline. + ReviewTimeline, + /// A forge review's submissions. + ReviewSubmissions, + /// Whether a forge review can merge. + MergeStatus, + /// CI check runs. + Checks, + /// Reactions on a forge review. + ReviewReactions, + /// Reactions on a forge review comment. + CommentReactions, + /// The labels a repository offers. + RepoLabels, + /// Who could review. + ReviewerCandidates, + /// Which forge the repository talks to. + ForgeInfo, + /// Who the current project is logged in as on its forge. + ForgeLogin, + /// The forge accounts known to the app. + ForgeAccounts, + /// The project's GitButler configuration. + GbConfig, + /// Whether the repository's signing configuration produces a signature. + SigningSettings, + /// The projects known to the app. + Projects, +} diff --git a/crates/but-api/src/target_commits.rs b/crates/but-api/src/target_commits.rs index 335303cb78c..ab719584cf0 100644 --- a/crates/but-api/src/target_commits.rs +++ b/crates/but-api/src/target_commits.rs @@ -38,7 +38,7 @@ const TARGET_COMMITS_PAGE_SIZE: usize = 50; /// the forge reports them; it reads only the local cache and performs no /// network requests or diffs, and enrichment failures degrade to unannotated /// commits. -#[but_api(napi, json::TargetCommitPage)] +#[but_api(napi, json::TargetCommitPage, provides = [TargetCommits])] #[instrument(err(Debug))] pub fn workspace_target_commits( ctx: &but_ctx::Context, diff --git a/crates/but-api/src/watcher.rs b/crates/but-api/src/watcher.rs index 9d97129be73..4563e39b09e 100644 --- a/crates/but-api/src/watcher.rs +++ b/crates/but-api/src/watcher.rs @@ -2,6 +2,7 @@ //! //! These are intended for export into type bindings for e.g. the but-sdk. +use crate::tags::CacheTag; use but_hunk_assignment::WorktreeChanges; use gitbutler_operating_modes::OperatingMode; use schemars::JsonSchema; @@ -27,6 +28,75 @@ pub enum WatcherPayload { #[cfg(feature = "export-schema")] but_schemars::register_sdk_type!(WatcherPayload); +/// Which watcher event happened, without the payload it carried. +/// +/// Each kind declares the tags it makes stale in [`WatcherEventKind::invalidates`]; +/// the SDK exports that table so clients can derive invalidation from it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WatcherEventKind { + /// See [`WatcherPayload::GitFetch`]. + GitFetch, + /// See [`WatcherPayload::GitHead`]. + GitHead, + /// See [`WatcherPayload::GitActivity`]. + GitActivity, + /// See [`WatcherPayload::WorktreeChanges`]. + WorktreeChanges, + /// See [`WatcherPayload::WorkspaceActivity`]. + WorkspaceActivity, +} + +impl WatcherEventKind { + /// Every kind, for the SDK generator to enumerate. + pub const ALL: &'static [WatcherEventKind] = &[ + WatcherEventKind::GitFetch, + WatcherEventKind::GitHead, + WatcherEventKind::GitActivity, + WatcherEventKind::WorktreeChanges, + WatcherEventKind::WorkspaceActivity, + ]; + + /// The event's name as clients see it, matching the payload's serde tag. + pub fn name(self) -> &'static str { + match self { + WatcherEventKind::GitFetch => "gitFetch", + WatcherEventKind::GitHead => "gitHead", + WatcherEventKind::GitActivity => "gitActivity", + WatcherEventKind::WorktreeChanges => "worktreeChanges", + WatcherEventKind::WorkspaceActivity => "workspaceActivity", + } + } + + /// The tags this event makes stale. + /// + /// This is the event-side third of the tag declarations — see + /// [`crate::tags`]. A fetch moves remote-tracking refs and refreshes the + /// forge cache; activity means the repository changed; worktree changes + /// mean the files did. + pub fn invalidates(self) -> &'static [CacheTag] { + use CacheTag as T; + match self { + WatcherEventKind::GitFetch => { + &[T::Branches, T::TargetCommits, T::FetchStatus, T::Reviews] + } + WatcherEventKind::GitHead => &[], + WatcherEventKind::GitActivity | WatcherEventKind::WorkspaceActivity => &[ + T::Branches, + T::TargetCommits, + T::Workspace, + T::Commits, + T::Diffs, + T::WorktreeChanges, + T::AbsorptionPlan, + T::Comments, + ], + WatcherEventKind::WorktreeChanges => { + &[T::Diffs, T::WorktreeChanges, T::AbsorptionPlan, T::Comments] + } + } + } +} + /// Git fetch event #[derive(Debug, Clone, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] diff --git a/crates/but-api/src/workspace.rs b/crates/but-api/src/workspace.rs index 9d5b72c1bce..c29df97fd2b 100644 --- a/crates/but-api/src/workspace.rs +++ b/crates/but-api/src/workspace.rs @@ -64,7 +64,7 @@ impl TryFrom for WorkspaceFetchStatus { /// reacts to updated remote refs. Within this process, overlapping fetch calls for the same /// repository serialize among themselves so concurrent `git fetch` runs cannot trip over Git's /// per-ref locks; fetches from other processes are not affected. -#[but_api(napi)] +#[but_api(napi, provides = [])] #[instrument(skip_all, err(Debug))] pub fn workspace_fetch_from_remotes( ctx: &mut but_ctx::Context, @@ -180,7 +180,7 @@ pub(crate) fn prune_missing_branch_stack_order(ctx: &but_ctx::Context) -> anyhow /// /// A project that hasn't used the workspace fetch API returns an empty status. Legacy fetch state /// is intentionally not imported. -#[but_api(napi)] +#[but_api(napi, provides = [FetchStatus])] #[instrument(skip_all, err(Debug))] pub fn workspace_fetch_status(ctx: &but_ctx::Context) -> anyhow::Result { ctx.db diff --git a/crates/but-schemars/src/lib.rs b/crates/but-schemars/src/lib.rs index b2b8caf40c4..f9a78316d8e 100644 --- a/crates/but-schemars/src/lib.rs +++ b/crates/but-schemars/src/lib.rs @@ -393,6 +393,17 @@ pub struct ApiFnEntry { pub js_name: &'static str, /// Parameter names in call order, camelCased to match the declaration. pub params: &'static [&'static str], + /// The cache tags this read's result is made of, declared with + /// `#[but_api(provides = [Reviews, ..])]`. + /// + /// `None` is unclassified and `Some(&[])` is "no tag — nothing refreshes + /// this" — a consumer driving cache invalidation from this has to tell + /// them apart, since only the second is an answer. + pub provides: Option<&'static [&'static str]>, + /// The cache tags this mutation makes stale, declared with + /// `#[but_api(invalidates = [Reviews, ..])]`. Mutually exclusive with + /// `provides`. + pub invalidates: Option<&'static [&'static str]>, } inventory::collect!(ApiFnEntry); diff --git a/crates/but-ts/src/main.rs b/crates/but-ts/src/main.rs index 14bf15328e7..e09ac88338e 100644 --- a/crates/but-ts/src/main.rs +++ b/crates/but-ts/src/main.rs @@ -17,8 +17,6 @@ //! use anyhow::bail; -// Link but-api to ensure types are correctly collected. -use but_api as _; use serde_json::Value; const MARKER: &str = "// Auto-generated by but-ts. Do not edit manually."; @@ -161,6 +159,85 @@ fn write_api_param_names(output_path: &Path, declarations: &str) -> anyhow::Resu std::fs::write(dir.join("apiParamNames.js"), js)?; std::fs::write(dir.join("apiParamNames.d.ts"), dts)?; eprintln!("Wrote parameter names for {} endpoints", entries.len()); + + let in_this_variant: Vec<&str> = entries.iter().map(|(name, _)| *name).collect(); + write_cache_tags(dir, &in_this_variant)?; + Ok(()) +} + +/// The cache-tag declarations: what each read provides, what each mutation +/// invalidates, and what each watcher event invalidates, plus the tag +/// vocabulary itself as a literal union. +/// +/// Only endpoints that declared are written, so absence means unclassified and +/// an empty list means "no tag". A consumer driving invalidation from these +/// needs that distinction: only the second is an answer. +fn write_cache_tags(dir: &Path, in_this_variant: &[&str]) -> anyhow::Result<()> { + let mut js = String::from("// Auto-generated by but-ts. Do not edit manually.\n\n"); + let mut dts = js.clone(); + + let tag_union: Vec = but_api::tags::CacheTag::ALL + .iter() + .map(|tag| format!("\"{}\"", tag.name())) + .collect(); + writeln!(dts, "export type CacheTag = {};\n", tag_union.join(" | "))?; + + let write_map = |name: &str, + entries: &[(&str, Vec<&str>)], + js: &mut String, + dts: &mut String| + -> anyhow::Result<()> { + writeln!(js, "export const {name} = {{")?; + writeln!(dts, "export declare const {name}: {{")?; + for (key, tags) in entries { + let quoted: Vec = tags.iter().map(|tag| format!("\"{tag}\"")).collect(); + writeln!(js, "\t{key}: [{}],", quoted.join(", "))?; + writeln!(dts, "\treadonly {key}: readonly [{}];", quoted.join(", "))?; + } + writeln!(js, "}};")?; + writeln!(dts, "}};")?; + Ok(()) + }; + + let declared = |pick: fn(&but_schemars::ApiFnEntry) -> Option<&'static [&'static str]>| { + let mut entries: Vec<(&str, Vec<&str>)> = inventory::iter:: + .into_iter() + // Only what survived this SDK variant's feature gates. + .filter(|entry| in_this_variant.contains(&entry.js_name)) + .filter_map(|entry| Some((entry.js_name, pick(entry)?.to_vec()))) + .collect(); + entries.sort_by_key(|(name, _)| *name); + entries + }; + + let provides = declared(|entry| entry.provides); + write_map("apiProvides", &provides, &mut js, &mut dts)?; + writeln!(js)?; + writeln!(dts)?; + + let invalidates = declared(|entry| entry.invalidates); + write_map("apiInvalidates", &invalidates, &mut js, &mut dts)?; + writeln!(js)?; + writeln!(dts)?; + + let mut events: Vec<(&str, Vec<&str>)> = but_api::watcher::WatcherEventKind::ALL + .iter() + .map(|kind| { + let tags: Vec<&str> = kind.invalidates().iter().map(|tag| tag.name()).collect(); + (kind.name(), tags) + }) + .collect(); + events.sort_by_key(|(name, _)| *name); + write_map("watcherInvalidates", &events, &mut js, &mut dts)?; + + std::fs::write(dir.join("cacheTags.js"), js)?; + std::fs::write(dir.join("cacheTags.d.ts"), dts)?; + eprintln!( + "Wrote cache tags: {} providers, {} invalidators, {} events", + provides.len(), + invalidates.len(), + events.len() + ); Ok(()) } diff --git a/packages/but-sdk/package.json b/packages/but-sdk/package.json index b6580b8f840..09825c04de5 100644 --- a/packages/but-sdk/package.json +++ b/packages/but-sdk/package.json @@ -63,7 +63,11 @@ "./src/generated/linear/apiParamNames.js", "./src/generated/linear/apiParamNames.d.ts", "./src/generated/graph/apiParamNames.js", - "./src/generated/graph/apiParamNames.d.ts" + "./src/generated/graph/apiParamNames.d.ts", + "./src/generated/linear/cacheTags.js", + "./src/generated/linear/cacheTags.d.ts", + "./src/generated/graph/cacheTags.js", + "./src/generated/graph/cacheTags.d.ts" ], "exports": { ".": { @@ -135,6 +139,34 @@ "types": "./src/generated/graph/apiParamNames.d.ts", "default": "./src/generated/graph/apiParamNames.js" } + }, + "./cache-tags": { + "browser": { + "types": "./src/generated/linear/cacheTags.d.ts", + "default": "./src/generated/linear/cacheTags.js" + }, + "import": { + "types": "./src/generated/linear/cacheTags.d.ts", + "default": "./src/generated/linear/cacheTags.js" + }, + "require": { + "types": "./src/generated/linear/cacheTags.d.ts", + "default": "./src/generated/linear/cacheTags.js" + } + }, + "./graph/cache-tags": { + "browser": { + "types": "./src/generated/graph/cacheTags.d.ts", + "default": "./src/generated/graph/cacheTags.js" + }, + "import": { + "types": "./src/generated/graph/cacheTags.d.ts", + "default": "./src/generated/graph/cacheTags.js" + }, + "require": { + "types": "./src/generated/graph/cacheTags.d.ts", + "default": "./src/generated/graph/cacheTags.js" + } } } } diff --git a/packages/but-sdk/src/generated/graph/cacheTags.d.ts b/packages/but-sdk/src/generated/graph/cacheTags.d.ts new file mode 100644 index 00000000000..7230313a088 --- /dev/null +++ b/packages/but-sdk/src/generated/graph/cacheTags.d.ts @@ -0,0 +1,73 @@ +// Auto-generated by but-ts. Do not edit manually. + +export type CacheTag = "Branches" | "TargetCommits" | "Workspace" | "Commits" | "Diffs" | "WorktreeChanges" | "AbsorptionPlan" | "Comments" | "FetchStatus" | "Reviews" | "ReviewComments" | "ReviewTimeline" | "ReviewSubmissions" | "MergeStatus" | "Checks" | "ReviewReactions" | "CommentReactions" | "RepoLabels" | "ReviewerCandidates" | "ForgeInfo" | "ForgeLogin" | "ForgeAccounts" | "GbConfig" | "SigningSettings" | "Projects"; + +export declare const apiProvides: { + readonly absorptionPlan: readonly ["AbsorptionPlan"]; + readonly branchCannedName: readonly ["Branches"]; + readonly branchDetails: readonly ["Branches"]; + readonly branchDiff: readonly ["Branches"]; + readonly branchList: readonly ["Branches"]; + readonly changesInWorktree: readonly ["WorktreeChanges"]; + readonly checkSigningSettings: readonly ["SigningSettings"]; + readonly commentsList: readonly ["Comments"]; + readonly commitConflicts: readonly []; + readonly commitDetailsWithLineStats: readonly ["Commits"]; + readonly currentForgeLogin: readonly ["ForgeLogin"]; + readonly forgeInfo: readonly ["ForgeInfo"]; + readonly getGbConfig: readonly ["GbConfig"]; + readonly getReview: readonly ["Reviews"]; + readonly getReviewMergeStatus: readonly ["MergeStatus"]; + readonly headInfo: readonly ["Workspace"]; + readonly listCiChecks: readonly ["Checks"]; + readonly listCommentReactions: readonly ["CommentReactions"]; + readonly listRepoLabels: readonly ["RepoLabels"]; + readonly listReviewComments: readonly ["ReviewComments"]; + readonly listReviewReactions: readonly ["ReviewReactions"]; + readonly listReviewSubmissions: readonly ["ReviewSubmissions"]; + readonly listReviewTimelineEvents: readonly ["ReviewTimeline"]; + readonly listReviewerCandidates: readonly ["ReviewerCandidates"]; + readonly listReviews: readonly ["Reviews"]; + readonly treeChangeDiffs: readonly ["Diffs"]; + readonly workspaceFetchFromRemotes: readonly []; + readonly workspaceFetchStatus: readonly ["FetchStatus"]; + readonly workspaceTargetCommits: readonly ["TargetCommits"]; +}; + +export declare const apiInvalidates: { + readonly addCommentReaction: readonly ["CommentReactions", "ReviewComments"]; + readonly addReviewLabels: readonly ["Reviews"]; + readonly addReviewReaction: readonly ["ReviewReactions"]; + readonly createReviewComment: readonly ["ReviewComments"]; + readonly deleteAllData: readonly ["Projects"]; + readonly deleteProject: readonly ["Projects"]; + readonly deleteReviewComment: readonly ["ReviewComments"]; + readonly forgetBitbucketAccount: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly forgetGithubAccount: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly forgetGitlabAccount: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly mergeReview: readonly ["Reviews", "MergeStatus", "Checks"]; + readonly publishReview: readonly ["Reviews"]; + readonly removeCommentReaction: readonly ["CommentReactions", "ReviewComments"]; + readonly removeReviewLabel: readonly ["Reviews"]; + readonly removeReviewReaction: readonly ["ReviewReactions"]; + readonly requestReview: readonly ["Reviews", "ReviewTimeline"]; + readonly setGbConfig: readonly ["GbConfig", "SigningSettings"]; + readonly setReviewAutoMerge: readonly ["Reviews"]; + readonly setReviewDraftiness: readonly ["Reviews", "MergeStatus"]; + readonly storeBitbucketApiToken: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly storeGithubPat: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly storeGitlabPat: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly updateProjectSettings: readonly ["Projects"]; + readonly updateReview: readonly ["Reviews"]; + readonly updateReviewComment: readonly ["ReviewComments"]; + readonly withdrawReviewRequest: readonly ["Reviews"]; + readonly workspaceBranchAndAncestorsPush: readonly ["Workspace", "Reviews", "MergeStatus", "Checks", "ReviewTimeline"]; +}; + +export declare const watcherInvalidates: { + readonly gitActivity: readonly ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"]; + readonly gitFetch: readonly ["Branches", "TargetCommits", "FetchStatus", "Reviews"]; + readonly gitHead: readonly []; + readonly workspaceActivity: readonly ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"]; + readonly worktreeChanges: readonly ["Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"]; +}; diff --git a/packages/but-sdk/src/generated/graph/cacheTags.js b/packages/but-sdk/src/generated/graph/cacheTags.js new file mode 100644 index 00000000000..c29811b08e3 --- /dev/null +++ b/packages/but-sdk/src/generated/graph/cacheTags.js @@ -0,0 +1,71 @@ +// Auto-generated by but-ts. Do not edit manually. + +export const apiProvides = { + absorptionPlan: ["AbsorptionPlan"], + branchCannedName: ["Branches"], + branchDetails: ["Branches"], + branchDiff: ["Branches"], + branchList: ["Branches"], + changesInWorktree: ["WorktreeChanges"], + checkSigningSettings: ["SigningSettings"], + commentsList: ["Comments"], + commitConflicts: [], + commitDetailsWithLineStats: ["Commits"], + currentForgeLogin: ["ForgeLogin"], + forgeInfo: ["ForgeInfo"], + getGbConfig: ["GbConfig"], + getReview: ["Reviews"], + getReviewMergeStatus: ["MergeStatus"], + headInfo: ["Workspace"], + listCiChecks: ["Checks"], + listCommentReactions: ["CommentReactions"], + listRepoLabels: ["RepoLabels"], + listReviewComments: ["ReviewComments"], + listReviewReactions: ["ReviewReactions"], + listReviewSubmissions: ["ReviewSubmissions"], + listReviewTimelineEvents: ["ReviewTimeline"], + listReviewerCandidates: ["ReviewerCandidates"], + listReviews: ["Reviews"], + treeChangeDiffs: ["Diffs"], + workspaceFetchFromRemotes: [], + workspaceFetchStatus: ["FetchStatus"], + workspaceTargetCommits: ["TargetCommits"], +}; + +export const apiInvalidates = { + addCommentReaction: ["CommentReactions", "ReviewComments"], + addReviewLabels: ["Reviews"], + addReviewReaction: ["ReviewReactions"], + createReviewComment: ["ReviewComments"], + deleteAllData: ["Projects"], + deleteProject: ["Projects"], + deleteReviewComment: ["ReviewComments"], + forgetBitbucketAccount: ["ForgeAccounts", "ForgeLogin"], + forgetGithubAccount: ["ForgeAccounts", "ForgeLogin"], + forgetGitlabAccount: ["ForgeAccounts", "ForgeLogin"], + mergeReview: ["Reviews", "MergeStatus", "Checks"], + publishReview: ["Reviews"], + removeCommentReaction: ["CommentReactions", "ReviewComments"], + removeReviewLabel: ["Reviews"], + removeReviewReaction: ["ReviewReactions"], + requestReview: ["Reviews", "ReviewTimeline"], + setGbConfig: ["GbConfig", "SigningSettings"], + setReviewAutoMerge: ["Reviews"], + setReviewDraftiness: ["Reviews", "MergeStatus"], + storeBitbucketApiToken: ["ForgeAccounts", "ForgeLogin"], + storeGithubPat: ["ForgeAccounts", "ForgeLogin"], + storeGitlabPat: ["ForgeAccounts", "ForgeLogin"], + updateProjectSettings: ["Projects"], + updateReview: ["Reviews"], + updateReviewComment: ["ReviewComments"], + withdrawReviewRequest: ["Reviews"], + workspaceBranchAndAncestorsPush: ["Workspace", "Reviews", "MergeStatus", "Checks", "ReviewTimeline"], +}; + +export const watcherInvalidates = { + gitActivity: ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"], + gitFetch: ["Branches", "TargetCommits", "FetchStatus", "Reviews"], + gitHead: [], + workspaceActivity: ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"], + worktreeChanges: ["Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"], +}; diff --git a/packages/but-sdk/src/generated/linear/cacheTags.d.ts b/packages/but-sdk/src/generated/linear/cacheTags.d.ts new file mode 100644 index 00000000000..7230313a088 --- /dev/null +++ b/packages/but-sdk/src/generated/linear/cacheTags.d.ts @@ -0,0 +1,73 @@ +// Auto-generated by but-ts. Do not edit manually. + +export type CacheTag = "Branches" | "TargetCommits" | "Workspace" | "Commits" | "Diffs" | "WorktreeChanges" | "AbsorptionPlan" | "Comments" | "FetchStatus" | "Reviews" | "ReviewComments" | "ReviewTimeline" | "ReviewSubmissions" | "MergeStatus" | "Checks" | "ReviewReactions" | "CommentReactions" | "RepoLabels" | "ReviewerCandidates" | "ForgeInfo" | "ForgeLogin" | "ForgeAccounts" | "GbConfig" | "SigningSettings" | "Projects"; + +export declare const apiProvides: { + readonly absorptionPlan: readonly ["AbsorptionPlan"]; + readonly branchCannedName: readonly ["Branches"]; + readonly branchDetails: readonly ["Branches"]; + readonly branchDiff: readonly ["Branches"]; + readonly branchList: readonly ["Branches"]; + readonly changesInWorktree: readonly ["WorktreeChanges"]; + readonly checkSigningSettings: readonly ["SigningSettings"]; + readonly commentsList: readonly ["Comments"]; + readonly commitConflicts: readonly []; + readonly commitDetailsWithLineStats: readonly ["Commits"]; + readonly currentForgeLogin: readonly ["ForgeLogin"]; + readonly forgeInfo: readonly ["ForgeInfo"]; + readonly getGbConfig: readonly ["GbConfig"]; + readonly getReview: readonly ["Reviews"]; + readonly getReviewMergeStatus: readonly ["MergeStatus"]; + readonly headInfo: readonly ["Workspace"]; + readonly listCiChecks: readonly ["Checks"]; + readonly listCommentReactions: readonly ["CommentReactions"]; + readonly listRepoLabels: readonly ["RepoLabels"]; + readonly listReviewComments: readonly ["ReviewComments"]; + readonly listReviewReactions: readonly ["ReviewReactions"]; + readonly listReviewSubmissions: readonly ["ReviewSubmissions"]; + readonly listReviewTimelineEvents: readonly ["ReviewTimeline"]; + readonly listReviewerCandidates: readonly ["ReviewerCandidates"]; + readonly listReviews: readonly ["Reviews"]; + readonly treeChangeDiffs: readonly ["Diffs"]; + readonly workspaceFetchFromRemotes: readonly []; + readonly workspaceFetchStatus: readonly ["FetchStatus"]; + readonly workspaceTargetCommits: readonly ["TargetCommits"]; +}; + +export declare const apiInvalidates: { + readonly addCommentReaction: readonly ["CommentReactions", "ReviewComments"]; + readonly addReviewLabels: readonly ["Reviews"]; + readonly addReviewReaction: readonly ["ReviewReactions"]; + readonly createReviewComment: readonly ["ReviewComments"]; + readonly deleteAllData: readonly ["Projects"]; + readonly deleteProject: readonly ["Projects"]; + readonly deleteReviewComment: readonly ["ReviewComments"]; + readonly forgetBitbucketAccount: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly forgetGithubAccount: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly forgetGitlabAccount: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly mergeReview: readonly ["Reviews", "MergeStatus", "Checks"]; + readonly publishReview: readonly ["Reviews"]; + readonly removeCommentReaction: readonly ["CommentReactions", "ReviewComments"]; + readonly removeReviewLabel: readonly ["Reviews"]; + readonly removeReviewReaction: readonly ["ReviewReactions"]; + readonly requestReview: readonly ["Reviews", "ReviewTimeline"]; + readonly setGbConfig: readonly ["GbConfig", "SigningSettings"]; + readonly setReviewAutoMerge: readonly ["Reviews"]; + readonly setReviewDraftiness: readonly ["Reviews", "MergeStatus"]; + readonly storeBitbucketApiToken: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly storeGithubPat: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly storeGitlabPat: readonly ["ForgeAccounts", "ForgeLogin"]; + readonly updateProjectSettings: readonly ["Projects"]; + readonly updateReview: readonly ["Reviews"]; + readonly updateReviewComment: readonly ["ReviewComments"]; + readonly withdrawReviewRequest: readonly ["Reviews"]; + readonly workspaceBranchAndAncestorsPush: readonly ["Workspace", "Reviews", "MergeStatus", "Checks", "ReviewTimeline"]; +}; + +export declare const watcherInvalidates: { + readonly gitActivity: readonly ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"]; + readonly gitFetch: readonly ["Branches", "TargetCommits", "FetchStatus", "Reviews"]; + readonly gitHead: readonly []; + readonly workspaceActivity: readonly ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"]; + readonly worktreeChanges: readonly ["Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"]; +}; diff --git a/packages/but-sdk/src/generated/linear/cacheTags.js b/packages/but-sdk/src/generated/linear/cacheTags.js new file mode 100644 index 00000000000..c29811b08e3 --- /dev/null +++ b/packages/but-sdk/src/generated/linear/cacheTags.js @@ -0,0 +1,71 @@ +// Auto-generated by but-ts. Do not edit manually. + +export const apiProvides = { + absorptionPlan: ["AbsorptionPlan"], + branchCannedName: ["Branches"], + branchDetails: ["Branches"], + branchDiff: ["Branches"], + branchList: ["Branches"], + changesInWorktree: ["WorktreeChanges"], + checkSigningSettings: ["SigningSettings"], + commentsList: ["Comments"], + commitConflicts: [], + commitDetailsWithLineStats: ["Commits"], + currentForgeLogin: ["ForgeLogin"], + forgeInfo: ["ForgeInfo"], + getGbConfig: ["GbConfig"], + getReview: ["Reviews"], + getReviewMergeStatus: ["MergeStatus"], + headInfo: ["Workspace"], + listCiChecks: ["Checks"], + listCommentReactions: ["CommentReactions"], + listRepoLabels: ["RepoLabels"], + listReviewComments: ["ReviewComments"], + listReviewReactions: ["ReviewReactions"], + listReviewSubmissions: ["ReviewSubmissions"], + listReviewTimelineEvents: ["ReviewTimeline"], + listReviewerCandidates: ["ReviewerCandidates"], + listReviews: ["Reviews"], + treeChangeDiffs: ["Diffs"], + workspaceFetchFromRemotes: [], + workspaceFetchStatus: ["FetchStatus"], + workspaceTargetCommits: ["TargetCommits"], +}; + +export const apiInvalidates = { + addCommentReaction: ["CommentReactions", "ReviewComments"], + addReviewLabels: ["Reviews"], + addReviewReaction: ["ReviewReactions"], + createReviewComment: ["ReviewComments"], + deleteAllData: ["Projects"], + deleteProject: ["Projects"], + deleteReviewComment: ["ReviewComments"], + forgetBitbucketAccount: ["ForgeAccounts", "ForgeLogin"], + forgetGithubAccount: ["ForgeAccounts", "ForgeLogin"], + forgetGitlabAccount: ["ForgeAccounts", "ForgeLogin"], + mergeReview: ["Reviews", "MergeStatus", "Checks"], + publishReview: ["Reviews"], + removeCommentReaction: ["CommentReactions", "ReviewComments"], + removeReviewLabel: ["Reviews"], + removeReviewReaction: ["ReviewReactions"], + requestReview: ["Reviews", "ReviewTimeline"], + setGbConfig: ["GbConfig", "SigningSettings"], + setReviewAutoMerge: ["Reviews"], + setReviewDraftiness: ["Reviews", "MergeStatus"], + storeBitbucketApiToken: ["ForgeAccounts", "ForgeLogin"], + storeGithubPat: ["ForgeAccounts", "ForgeLogin"], + storeGitlabPat: ["ForgeAccounts", "ForgeLogin"], + updateProjectSettings: ["Projects"], + updateReview: ["Reviews"], + updateReviewComment: ["ReviewComments"], + withdrawReviewRequest: ["Reviews"], + workspaceBranchAndAncestorsPush: ["Workspace", "Reviews", "MergeStatus", "Checks", "ReviewTimeline"], +}; + +export const watcherInvalidates = { + gitActivity: ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"], + gitFetch: ["Branches", "TargetCommits", "FetchStatus", "Reviews"], + gitHead: [], + workspaceActivity: ["Branches", "TargetCommits", "Workspace", "Commits", "Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"], + worktreeChanges: ["Diffs", "WorktreeChanges", "AbsorptionPlan", "Comments"], +}; From b9a6fe71024497d989d4a3065109e937ee851d00 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Tue, 11 Aug 2026 22:52:37 +0100 Subject: [PATCH 2/2] Recognize a mutation's endpoint by its function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sam's review argued the apiMutation wrapper was needless indirection. Following that through: the manual mutation key it replaced is also a hand-written copy of a fact every mutation already carries — its mutationFn. The mutation cache now resolves the endpoint by function identity (a lazy map over window.lite) and applies the endpoint's declared invalidations. There is no second declaration to forget, so the forget hole is closed by construction rather than by a test. This fixes removeCommentReaction, whose declared invalidations never ran: it was written keyless. The four comment hooks that wrapped their endpoint to strip a cache-keying reviewId take it as a hook argument instead, so every mutationFn is the endpoint function itself. The last pending-state lookup filters by the same function identity, so no mutation keys remain at all. The same move, three more times: - A mutation's failure toast is declared as meta.failureTitle and shown by the mutation cache, which also logs every error once. Hooks write onError only for their own work — rollbacks and dynamic wording — which dissolves two dozen identical toast handlers. - projectQueryKeys was character-for-character the keys of the generated apiProvides; derive the type and list from it, so a query name the backend doesn't declare is a type error and there is no list to keep in step. - exposedEndpoints looked like an allowlist but wasn't one: the 15 endpoints it left out are merely unused, not more sensitive than what it let through. Derive the exposed set from apiParamNames; the sender-frame validation in main stays the security boundary. --- apps/lite/electron/src/ipc.ts | 139 +-- apps/lite/electron/src/main.ts | 4 +- apps/lite/ui/src/api/mutations.ts | 890 ++++-------------- apps/lite/ui/src/api/queries.ts | 67 +- apps/lite/ui/src/api/tags.test.ts | 16 +- apps/lite/ui/src/api/tags.ts | 48 +- apps/lite/ui/src/main.tsx | 23 +- apps/lite/ui/src/project-events.test.ts | 10 +- .../project/$id/workspace/CommitForm.tsx | 5 +- .../$id/workspace/PullRequestComments.tsx | 11 +- 10 files changed, 270 insertions(+), 943 deletions(-) diff --git a/apps/lite/electron/src/ipc.ts b/apps/lite/electron/src/ipc.ts index 6e9aa940175..e2d86904d66 100644 --- a/apps/lite/electron/src/ipc.ts +++ b/apps/lite/electron/src/ipc.ts @@ -1,14 +1,14 @@ import type { WatcherEvent, AskpassPromptEvent } from "@gitbutler/but-sdk"; import type * as sdk from "@gitbutler/but-sdk"; -import type { apiParamNames } from "@gitbutler/but-sdk/api-param-names"; +import { apiParamNames } from "@gitbutler/but-sdk/api-param-names"; import type { GUISettings } from "./settings.js"; /** - * What the renderer can call: the exposed endpoints, whose signatures are the + * What the renderer can call: every SDK endpoint, whose signatures are the * SDK's, plus the members electron implements itself. */ export type LiteElectronApi = { - [K in ExposedKey]: EndpointFn; + [K in Endpoint]: EndpointFn; } & { onAskpassPrompt: (callback: (event: AskpassPromptEvent) => void) => () => void; askpassSubmitPromptResponse: (params: AskpassSubmitPromptResponseParams) => Promise; @@ -28,131 +28,12 @@ export type LiteElectronApi = { }; /** - * The SDK endpoints the renderer may call. This list is the decision — the - * signatures, payloads and handlers all follow from it — and an endpoint's - * name is its IPC channel, so there is nothing else to keep in step. + * The SDK endpoints the renderer can call: all of them, each under its own + * name as the IPC channel, so a new declaration in Rust reaches `window.lite` + * with nothing to keep in step. */ -export const exposedEndpoints = [ - "absorb", - "absorptionPlan", - "addCommentReaction", - "addReviewLabels", - "addReviewReaction", - "apply", - "applyBranchIntegration", - "assignHunk", - "branchCannedName", - "branchCheckout", - "branchCheckoutNew", - "branchCreate", - "branchDetails", - "branchDiff", - "branchList", - "branchRemove", - "branchRename", - "changesInWorktree", - "checkGithubAuthStatus", - "checkSigningSettings", - "commentArchive", - "commentCreate", - "commentUpdate", - "commentsList", - "commitAmend", - "commitConflicts", - "commitCreate", - "commitDetailsWithLineStats", - "commitDiscard", - "commitDiscardChanges", - "commitInsertBlank", - "commitMove", - "commitMoveChangesBetween", - "commitReword", - "commitSquash", - "commitUncommit", - "commitUncommitChanges", - "createReviewComment", - "currentForgeLogin", - "deleteAllData", - "deleteProject", - "deleteUser", - "deleteReviewComment", - "discardWorktreeChanges", - "forgeCompareBranchUrl", - "forgeInfo", - "forgeProvider", - "forgetBitbucketAccount", - "forgetGithubAccount", - "forgetGitlabAccount", - "getGbConfig", - "getLoginToken", - "getInitialBranchIntegration", - "getRedoTargetSnapshot", - "getRepoInfo", - "gitTestFetch", - "gitTestPush", - "getReview", - "getReviewBaseRepoUrl", - "getReviewMergeStatus", - "getTerminalOptionsForPlatform", - "getUndoTargetSnapshot", - "getUserProfileLocal", - "initGithubDeviceOauth", - "headInfo", - "listAvailableReviewTemplates", - "loginAndPersist", - "listCiChecks", - "listCommentReactions", - "listEditors", - "listKnownBitbucketAccounts", - "listKnownGithubAccounts", - "listKnownGitlabAccounts", - "listPrograms", - "listProjectsStateless", - "listRepoLabels", - "listReviewComments", - "listReviewReactions", - "listReviewSubmissions", - "listReviewTimelineEvents", - "listReviewerCandidates", - "listReviews", - "listReviewsForBranch", - "mergeReview", - "moveBranch", - "openInProgram", - "openInTerminal", - "peelRestoreSnapshot", - "publishReview", - "removeCommentReaction", - "removeReviewLabel", - "removeReviewReaction", - "requestReview", - "resolveCommitConflictHunks", - "restoreSnapshotWithKind", - "reviewTemplate", - "setGbConfig", - "storeBitbucketApiToken", - "storeGithubPat", - "storeGitlabPat", - "setReviewAutoMerge", - "setReviewDraftiness", - "setReviewTemplate", - "setTargetRefAndInitProject", - "tearOffBranch", - "treeChangeDiffs", - "unapplyStack", - "updateProfileAndPersist", - "updateProjectSettings", - "updateReview", - "updateReviewComment", - "updateReviewFooters", - "warmCiChecksCache", - "withdrawReviewRequest", - "workspaceBranchAndAncestorsPush", - "workspaceFetchFromRemotes", - "workspaceFetchStatus", - "workspaceIntegrateUpstream", - "workspaceTargetCommits", -] satisfies Array; +// `Object.keys` erases key types; the record's keys are exactly these. +export const exposedEndpoints = Object.keys(apiParamNames) as ReadonlyArray; /** Members the main process answers itself rather than forwarding to the SDK. */ export const localEndpoints = [ @@ -172,8 +53,6 @@ export const localEndpoints = [ "writeGUISettings", ] as const; -// Everything below derives the surface above from the list above. - /** An endpoint the SDK exposes to JavaScript. */ export type Endpoint = keyof typeof apiParamNames & keyof typeof sdk; @@ -205,8 +84,6 @@ type EndpointFn = (typeof apiParamNames)[K]["length"] extend ? (arg: Parameters<(typeof sdk)[K]>[0]) => Result : (params: PayloadFor) => Result; -export type ExposedKey = (typeof exposedEndpoints)[number]; - // Shapes electron owns: no SDK declaration behind them, so they are written // out by hand — the only types in this file that are. diff --git a/apps/lite/electron/src/main.ts b/apps/lite/electron/src/main.ts index 04925979319..7854cc0b3c1 100644 --- a/apps/lite/electron/src/main.ts +++ b/apps/lite/electron/src/main.ts @@ -5,7 +5,7 @@ import { apiParamNames } from "@gitbutler/but-sdk/api-param-names"; import { exposedEndpoints, type PayloadFor, - type ExposedKey, + type Endpoint, type LiteElectronApi, type ShowNativeMenuParams, type WatcherSubscribeParams, @@ -308,7 +308,7 @@ type OverrideKey = keyof typeof ipcHandlerOverrides; type DerivedKey = Exclude; /** Narrowing rather than asserting: an exposed endpoint may be either. */ -const isOverride = (key: ExposedKey): key is ExposedKey & OverrideKey => key in ipcHandlerOverrides; +const isOverride = (key: Endpoint): key is Endpoint & OverrideKey => key in ipcHandlerOverrides; /** * Every other endpoint reads its arguments out of the payload by name, so diff --git a/apps/lite/ui/src/api/mutations.ts b/apps/lite/ui/src/api/mutations.ts index bb426da23be..de23b52afee 100644 --- a/apps/lite/ui/src/api/mutations.ts +++ b/apps/lite/ui/src/api/mutations.ts @@ -1,4 +1,3 @@ -import type { PayloadFor } from "#electron/ipc.ts"; import { decodeBytes, encodeBytes } from "#ui/api/bytes.ts"; import { getHeadInfoIndex } from "#ui/api/ref-info.ts"; import { @@ -11,7 +10,6 @@ import { listReviewReactionsQueryOptions, workspaceFetchQueryOptions, } from "#ui/api/queries.ts"; -import { apiMutation, type DeclaredMutation } from "#ui/api/tags.ts"; import { shortCommitId } from "#ui/commit.ts"; import { errorMessageForToast } from "#ui/errors.ts"; import { createDiffSpec, resolveDiffSpecs } from "#ui/operations/diff-specs.ts"; @@ -39,6 +37,18 @@ import { type QueryClient, useMutation, useQueryClient } from "@tanstack/react-q import type { GUISettings } from "#electron/settings.ts"; import { moveDraftPR } from "#ui/pr.ts"; +declare module "@tanstack/react-query" { + interface Register { + /** + * A mutation's failure toast is declared, not coded: the mutation cache + * logs every error and shows `failureTitle` when one is given. Hooks + * write `onError` only for work of their own, like rolling back an + * optimistic write or wording a title dynamically. + */ + mutationMeta: { failureTitle?: string }; + } +} + const pluralRules = new Intl.PluralRules("en"); // oxlint-disable-next-line typescript/no-explicit-any @@ -70,27 +80,14 @@ export const syncCoreCaches = ( ); }; -export const useAbsorb = ({ projectId }: { projectId: string }) => { - const toastManager = Toast.useToastManager(); - - return useMutation({ +export const useAbsorb = ({ projectId }: { projectId: string }) => + useMutation({ mutationFn: (absorptionPlan: Array | undefined) => { if (!absorptionPlan) return Promise.resolve(null); return window.lite.absorb({ projectId, absorptionPlan }); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to absorb", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to absorb" }, }); -}; export const useApply = () => { const dispatch = useAppDispatch(); @@ -133,118 +130,44 @@ export const useApply = () => { }); } }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to apply branch", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to apply branch" }, }); }; export const useBranchCreate = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.branchCreate, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to create branch", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to create branch" }, }); }; -export const usePublishReview = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("publishReview"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to create pull request", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const usePublishReview = () => + useMutation({ + mutationFn: window.lite.publishReview, + meta: { failureTitle: "Failed to create pull request" }, }); -}; -export const useUpdateReview = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("updateReview"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to update pull request", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useUpdateReview = () => + useMutation({ + mutationFn: window.lite.updateReview, + meta: { failureTitle: "Failed to update pull request" }, }); -}; - -export const useAddReviewLabels = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("addReviewLabels"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - toastManager.add({ - type: "error", - title: "Failed to add label", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useAddReviewLabels = () => + useMutation({ + mutationFn: window.lite.addReviewLabels, + meta: { failureTitle: "Failed to add label" }, }); -}; - -export const useRemoveReviewLabel = () => { - const toastManager = Toast.useToastManager(); - return useMutation({ - ...apiMutation("removeReviewLabel"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to remove label", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useRemoveReviewLabel = () => + useMutation({ + mutationFn: window.lite.removeReviewLabel, + meta: { failureTitle: "Failed to remove label" }, }); -}; /** * Optimistic entries carry negative forge ids until the settle refetch @@ -294,11 +217,10 @@ const withCommentReactionCount = ( return { ...comment, reactions }; }); -export const useAddReviewReaction = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("addReviewReaction"), +export const useAddReviewReaction = () => + useMutation({ + mutationFn: window.lite.addReviewReaction, + meta: { failureTitle: "Failed to add reaction" }, onMutate: async (input, ctx) => { const key = listReviewReactionsQueryOptions(input).queryKey; await ctx.client.cancelQueries({ queryKey: key }); @@ -322,25 +244,13 @@ export const useAddReviewReaction = () => { void ctx.client.invalidateQueries({ queryKey: listReviewReactionsQueryOptions(input).queryKey, }); - - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to add reaction", - description: errorMessageForToast(error), - priority: "high", - }); }, }); -}; -export const useRemoveReviewReaction = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("removeReviewReaction"), +export const useRemoveReviewReaction = () => + useMutation({ + mutationFn: window.lite.removeReviewReaction, + meta: { failureTitle: "Failed to remove reaction" }, onMutate: async (input, ctx) => { const key = listReviewReactionsQueryOptions(input).queryKey; await ctx.client.cancelQueries({ queryKey: key }); @@ -359,40 +269,24 @@ export const useRemoveReviewReaction = () => { void ctx.client.invalidateQueries({ queryKey: listReviewReactionsQueryOptions(input).queryKey, }); - - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to remove reaction", - description: errorMessageForToast(error), - priority: "high", - }); }, }); -}; /** * A comment reaction spans two caches — the count summary on the comments * listing and the names on the per-comment reactions listing — so the * optimistic write and its rollback patch both. */ -export const useAddCommentReaction = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("addCommentReaction"), - // `reviewId` keys the cache below; the forge addresses comments by id, - // so it is not part of what the endpoint takes. - mutationFn: ({ - reviewId: _reviewId, - ...params - }: PayloadFor<"addCommentReaction"> & { reviewId: number }) => - window.lite.addCommentReaction(params), +export const useAddCommentReaction = ({ reviewId }: { reviewId: number }) => + useMutation({ + mutationFn: window.lite.addCommentReaction, + meta: { failureTitle: "Failed to add reaction" }, onMutate: async (input, ctx) => { const reactionsKey = listCommentReactionsQueryOptions(input).queryKey; - const commentsKey = listReviewCommentsQueryOptions(input).queryKey; + const commentsKey = listReviewCommentsQueryOptions({ + projectId: input.projectId, + reviewId, + }).queryKey; await Promise.all([ ctx.client.cancelQueries({ queryKey: reactionsKey }), ctx.client.cancelQueries({ queryKey: commentsKey }), @@ -415,50 +309,30 @@ export const useAddCommentReaction = () => { return { prevReactions, prevComments }; }, onError: (error, input, prev, ctx) => { + const reactionsKey = listCommentReactionsQueryOptions(input).queryKey; + const commentsKey = listReviewCommentsQueryOptions({ + projectId: input.projectId, + reviewId, + }).queryKey; // Roll the optimistic writes back, then refetch: the rollback // snapshots may themselves be stale by now. - void ctx.client.invalidateQueries({ - queryKey: listCommentReactionsQueryOptions(input).queryKey, - }); - void ctx.client.invalidateQueries({ - queryKey: listReviewCommentsQueryOptions(input).queryKey, - }); - if (prev?.prevReactions) { - ctx.client.setQueryData( - listCommentReactionsQueryOptions(input).queryKey, - prev.prevReactions, - ); - } - if (prev?.prevComments) - ctx.client.setQueryData(listReviewCommentsQueryOptions(input).queryKey, prev.prevComments); - - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to add reaction", - description: errorMessageForToast(error), - priority: "high", - }); + if (prev?.prevReactions) ctx.client.setQueryData(reactionsKey, prev.prevReactions); + if (prev?.prevComments) ctx.client.setQueryData(commentsKey, prev.prevComments); + void ctx.client.invalidateQueries({ queryKey: reactionsKey }); + void ctx.client.invalidateQueries({ queryKey: commentsKey }); }, }); -}; - -export const useRemoveCommentReaction = () => { - const toastManager = Toast.useToastManager(); - return useMutation({ - // `reviewId` keys the cache below; the forge addresses comments by id, - // so it is not part of what the endpoint takes. - mutationFn: ({ - reviewId: _reviewId, - ...params - }: PayloadFor<"removeCommentReaction"> & { reviewId: number }) => - window.lite.removeCommentReaction(params), +export const useRemoveCommentReaction = ({ reviewId }: { reviewId: number }) => + useMutation({ + mutationFn: window.lite.removeCommentReaction, + meta: { failureTitle: "Failed to remove reaction" }, onMutate: async (input, ctx) => { const reactionsKey = listCommentReactionsQueryOptions(input).queryKey; - const commentsKey = listReviewCommentsQueryOptions(input).queryKey; + const commentsKey = listReviewCommentsQueryOptions({ + projectId: input.projectId, + reviewId, + }).queryKey; await Promise.all([ ctx.client.cancelQueries({ queryKey: reactionsKey }), ctx.client.cancelQueries({ queryKey: commentsKey }), @@ -481,79 +355,36 @@ export const useRemoveCommentReaction = () => { return { prevReactions, prevComments }; }, onError: (error, input, prev, ctx) => { + const reactionsKey = listCommentReactionsQueryOptions(input).queryKey; + const commentsKey = listReviewCommentsQueryOptions({ + projectId: input.projectId, + reviewId, + }).queryKey; // Roll the optimistic writes back, then refetch: the rollback // snapshots may themselves be stale by now. - void ctx.client.invalidateQueries({ - queryKey: listCommentReactionsQueryOptions(input).queryKey, - }); - void ctx.client.invalidateQueries({ - queryKey: listReviewCommentsQueryOptions(input).queryKey, - }); - if (prev?.prevReactions) { - ctx.client.setQueryData( - listCommentReactionsQueryOptions(input).queryKey, - prev.prevReactions, - ); - } - if (prev?.prevComments) - ctx.client.setQueryData(listReviewCommentsQueryOptions(input).queryKey, prev.prevComments); - - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to remove reaction", - description: errorMessageForToast(error), - priority: "high", - }); + if (prev?.prevReactions) ctx.client.setQueryData(reactionsKey, prev.prevReactions); + if (prev?.prevComments) ctx.client.setQueryData(commentsKey, prev.prevComments); + void ctx.client.invalidateQueries({ queryKey: reactionsKey }); + void ctx.client.invalidateQueries({ queryKey: commentsKey }); }, }); -}; - -export const useRequestReview = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("requestReview"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - toastManager.add({ - type: "error", - title: "Failed to request review", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useRequestReview = () => + useMutation({ + mutationFn: window.lite.requestReview, + meta: { failureTitle: "Failed to request review" }, }); -}; -export const useWithdrawReviewRequest = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("withdrawReviewRequest"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to withdraw review request", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useWithdrawReviewRequest = () => + useMutation({ + mutationFn: window.lite.withdrawReviewRequest, + meta: { failureTitle: "Failed to withdraw review request" }, }); -}; - -export const useCreateReviewComment = () => { - const toastManager = Toast.useToastManager(); - return useMutation({ - ...apiMutation("createReviewComment"), +export const useCreateReviewComment = () => + useMutation({ + mutationFn: window.lite.createReviewComment, + meta: { failureTitle: "Failed to post comment" }, onMutate: async (input, ctx) => { const key = listReviewCommentsQueryOptions(input).queryKey; await ctx.client.cancelQueries({ queryKey: key }); @@ -582,77 +413,26 @@ export const useCreateReviewComment = () => { void ctx.client.invalidateQueries({ queryKey: listReviewCommentsQueryOptions(input).queryKey, }); - - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to post comment", - description: errorMessageForToast(error), - priority: "high", - }); }, }); -}; - -export const useUpdateReviewComment = () => { - const toastManager = Toast.useToastManager(); - return useMutation({ - // `reviewId` keys the cache below; the forge addresses comments by id, - // so it is not part of what the endpoint takes. - ...apiMutation("updateReviewComment"), - mutationFn: ({ - reviewId: _reviewId, - ...params - }: PayloadFor<"updateReviewComment"> & { reviewId: number }) => - window.lite.updateReviewComment(params), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to update comment", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useUpdateReviewComment = () => + useMutation({ + mutationFn: window.lite.updateReviewComment, + meta: { failureTitle: "Failed to update comment" }, }); -}; - -export const useDeleteReviewComment = () => { - const toastManager = Toast.useToastManager(); - return useMutation({ - // `reviewId` keys the cache below; the forge addresses comments by id, - // so it is not part of what the endpoint takes. - ...apiMutation("deleteReviewComment"), - mutationFn: ({ - reviewId: _reviewId, - ...params - }: PayloadFor<"deleteReviewComment"> & { reviewId: number }) => - window.lite.deleteReviewComment(params), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to delete comment", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useDeleteReviewComment = () => + useMutation({ + mutationFn: window.lite.deleteReviewComment, + meta: { failureTitle: "Failed to delete comment" }, }); -}; export const useSetReviewAutoMerge = () => { const toastManager = Toast.useToastManager(); return useMutation({ - ...apiMutation("setReviewAutoMerge"), + mutationFn: window.lite.setReviewAutoMerge, onMutate: async (input, ctx) => { const reviewsPrefix = ["listReviews", input.projectId] as const; await ctx.client.cancelQueries({ queryKey: reviewsPrefix }); @@ -681,17 +461,14 @@ export const useSetReviewAutoMerge = () => { // Roll the optimistic writes back, then refetch: the rollback // snapshots may themselves be stale by now. for (const [key, data] of prev?.prev ?? []) ctx.client.setQueryData(key, data); - void ctx.client.invalidateQueries({ queryKey: ["listReviews", input.projectId] }); - void ctx.client.invalidateQueries({ queryKey: ["getReview", input.projectId] }); if (prev?.prevSingle) { ctx.client.setQueryData( getReviewQueryOptions({ projectId: input.projectId, reviewId: input.reviewId }).queryKey, prev.prevSingle, ); } - - // oxlint-disable-next-line no-console - console.error(error); + void ctx.client.invalidateQueries({ queryKey: ["listReviews", input.projectId] }); + void ctx.client.invalidateQueries({ queryKey: ["getReview", input.projectId] }); toastManager.add({ type: "error", @@ -703,11 +480,9 @@ export const useSetReviewAutoMerge = () => { }); }; -export const useMergeReview = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("mergeReview"), +export const useMergeReview = () => + useMutation({ + mutationFn: window.lite.mergeReview, onSuccess: async (_response, input, _context, mutation) => { // The merge moved the target branch on the remote, but nothing local, so // the branch keeps looking un-integrated until remote-tracking refs catch @@ -718,190 +493,92 @@ export const useMergeReview = () => { .fetchQuery({ ...workspaceFetchQueryOptions(input.projectId), staleTime: 0 }) .then(() => mutation.client.fetchQuery({ - ...headInfoQueryOptions(input.projectId), - staleTime: 0, - }), - ) - .catch(() => undefined); - }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to merge pull request", - description: errorMessageForToast(error), - priority: "high", - }); - }, - }); -}; - -export const useSetReviewDraftiness = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("setReviewDraftiness"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to update pull request", - description: errorMessageForToast(error), - priority: "high", - }); - }, - }); -}; - -export const useSetGbConfig = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("setGbConfig"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to save git settings", - description: errorMessageForToast(error), - priority: "high", - }); - }, - }); -}; - -export const useDeleteAllData = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("deleteAllData"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to remove projects", - description: errorMessageForToast(error), - priority: "high", - }); + ...headInfoQueryOptions(input.projectId), + staleTime: 0, + }), + ) + .catch(() => undefined); }, + meta: { failureTitle: "Failed to merge pull request" }, }); -}; -const useForgeAccountMutation = ( - mutation: { - mutationKey: readonly [DeclaredMutation]; - mutationFn: (input: Input) => Promise; - }, - failureTitle: string, -) => { - const toastManager = Toast.useToastManager(); +export const useSetReviewDraftiness = () => + useMutation({ + mutationFn: window.lite.setReviewDraftiness, + meta: { failureTitle: "Failed to update pull request" }, + }); - return useMutation({ - ...mutation, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); +export const useSetGbConfig = () => + useMutation({ + mutationFn: window.lite.setGbConfig, + meta: { failureTitle: "Failed to save git settings" }, + }); - toastManager.add({ - type: "error", - title: failureTitle, - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useDeleteAllData = () => + useMutation({ + mutationFn: window.lite.deleteAllData, + meta: { failureTitle: "Failed to remove projects" }, }); -}; export const useForgetGithubAccount = () => - useForgeAccountMutation(apiMutation("forgetGithubAccount"), "Failed to forget account"); + useMutation({ + mutationFn: window.lite.forgetGithubAccount, + meta: { failureTitle: "Failed to forget account" }, + }); export const useForgetGitlabAccount = () => - useForgeAccountMutation(apiMutation("forgetGitlabAccount"), "Failed to forget account"); + useMutation({ + mutationFn: window.lite.forgetGitlabAccount, + meta: { failureTitle: "Failed to forget account" }, + }); export const useForgetBitbucketAccount = () => - useForgeAccountMutation(apiMutation("forgetBitbucketAccount"), "Failed to forget account"); + useMutation({ + mutationFn: window.lite.forgetBitbucketAccount, + meta: { failureTitle: "Failed to forget account" }, + }); export const useStoreGithubPat = () => - useForgeAccountMutation(apiMutation("storeGithubPat"), "Failed to add GitHub account"); + useMutation({ + mutationFn: window.lite.storeGithubPat, + meta: { failureTitle: "Failed to add GitHub account" }, + }); export const useStoreGitlabPat = () => - useForgeAccountMutation(apiMutation("storeGitlabPat"), "Failed to add GitLab account"); + useMutation({ + mutationFn: window.lite.storeGitlabPat, + meta: { failureTitle: "Failed to add GitLab account" }, + }); export const useStoreBitbucketApiToken = () => - useForgeAccountMutation(apiMutation("storeBitbucketApiToken"), "Failed to add Bitbucket account"); - -export const useDeleteProject = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("deleteProject"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to remove project", - description: errorMessageForToast(error), - priority: "high", - }); - }, + useMutation({ + mutationFn: window.lite.storeBitbucketApiToken, + meta: { failureTitle: "Failed to add Bitbucket account" }, }); -}; - -export const useUpdateProjectSettings = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("updateProjectSettings"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - toastManager.add({ - type: "error", - title: "Failed to save project settings", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useDeleteProject = () => + useMutation({ + mutationFn: window.lite.deleteProject, + meta: { failureTitle: "Failed to remove project" }, }); -}; -export const useOpenInProgram = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - mutationFn: (input: PayloadFor<"openInProgram">) => window.lite.openInProgram(input), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); +export const useUpdateProjectSettings = () => + useMutation({ + mutationFn: window.lite.updateProjectSettings, + meta: { failureTitle: "Failed to save project settings" }, + }); - toastManager.add({ - type: "error", - title: "Failed to open in editor", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useOpenInProgram = () => + useMutation({ + mutationFn: window.lite.openInProgram, + meta: { failureTitle: "Failed to open in editor" }, }); -}; -export const commitAmendMutationKey = ["commitAmend"]; export const useCommitAmend = () => { const toastManager = Toast.useToastManager(); const dispatch = useAppDispatch(); return useMutation({ - mutationKey: commitAmendMutationKey, mutationFn: window.lite.commitAmend, onSuccess: async (response, input, _ctx, mutation) => { syncCoreCaches( @@ -930,17 +607,7 @@ export const useCommitAmend = () => { ); } }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to amend commit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to amend commit" }, }); }; @@ -979,63 +646,29 @@ export const useCommitCreate = () => { ); } }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to commit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to commit" }, }); }; export const useCommitDiscard = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitDiscard, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to discard commit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to discard commit" }, }); }; export const useCommitDiscardChanges = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitDiscardChanges, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to discard changes", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to discard changes" }, }); }; @@ -1048,17 +681,7 @@ export const useDiscardWorktreeChanges = () => { if (rejectedChanges.length > 0) toastManager.add(discardChangesToastOptions({ rejectedChanges })); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to discard changes", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to discard changes" }, }); }; @@ -1145,8 +768,6 @@ export const useDiscardFileChanges = ({ export const useCommitInsertBlank = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitInsertBlank, onSuccess: async (response, input, _context, mutation) => { @@ -1167,63 +788,29 @@ export const useCommitInsertBlank = () => { ); } }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to insert commit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to insert commit" }, }); }; export const useCommitMove = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitMove, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to move commit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to move commit" }, }); }; export const useCommitReword = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitReword, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to reword commit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to reword commit" }, }); }; @@ -1258,84 +845,37 @@ export const useResolveCommitConflictHunks = () => { }); } }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to resolve the conflict", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to resolve the conflict" }, }); }; export const useCommitUncommit = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitUncommit, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to uncommit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to uncommit" }, }); }; export const useCommitUncommitChanges = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.commitUncommitChanges, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to uncommit", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to uncommit" }, }); }; -export const useWorkspaceBranchAndAncestorsPush = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ - ...apiMutation("workspaceBranchAndAncestorsPush"), - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to push", - description: errorMessageForToast(error), - priority: "high", - }); - }, +export const useWorkspaceBranchAndAncestorsPush = () => + useMutation({ + mutationFn: window.lite.workspaceBranchAndAncestorsPush, + meta: { failureTitle: "Failed to push" }, }); -}; export const useWorkspaceIntegrateUpstream = () => { const dispatch = useAppDispatch(); @@ -1347,9 +887,6 @@ export const useWorkspaceIntegrateUpstream = () => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, onError: (error, input) => { - // oxlint-disable-next-line no-console - console.error(error); - toastManager.add({ type: "error", title: `Failed to update stack${pluralRules.select(input.updates.length) === "one" ? "" : "s"}`, @@ -1362,24 +899,12 @@ export const useWorkspaceIntegrateUpstream = () => { export const useBranchRemove = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.branchRemove, onSuccess: (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to delete branch reference", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to delete branch reference" }, }); }; @@ -1427,9 +952,6 @@ export const useRestoreSnapshot = ({ projectId }: { projectId: string }) => { }); }, onError: (error, direction) => { - // oxlint-disable-next-line no-console - console.error(error); - toastManager.add({ type: "error", title: `Failed to ${direction}`, @@ -1442,50 +964,23 @@ export const useRestoreSnapshot = ({ projectId }: { projectId: string }) => { export const useTearOffBranch = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.tearOffBranch, onSuccess: async (response, input, _context, mutation) => { syncCoreCaches(mutation.client, dispatch, input.projectId, response); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to tear off branch", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to tear off branch" }, }); }; -export const useUnapplyStack = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ +export const useUnapplyStack = () => + useMutation({ mutationFn: window.lite.unapplyStack, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to unapply stack", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to unapply stack" }, }); -}; export const useBranchRename = () => { const dispatch = useAppDispatch(); - const toastManager = Toast.useToastManager(); - return useMutation({ mutationFn: window.lite.branchRename, onSuccess: async (response, input, _context, mutation) => { @@ -1514,27 +1009,15 @@ export const useBranchRename = () => { dispatch(projectSlice.actions.exitMode({ projectId: input.projectId })); }, - onError: (error) => { - // oxlint-disable-next-line no-console - console.error(error); - - toastManager.add({ - type: "error", - title: "Failed to rename branch", - description: errorMessageForToast(error), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to rename branch" }, }); }; /** * Save GUI settings mutation with partial keys. Settings are spread (shallow). */ -export const useSaveGUISettings = () => { - const toastManager = Toast.useToastManager(); - - return useMutation({ +export const useSaveGUISettings = () => + useMutation({ scope: { id: "guiSettings" }, mutationFn: async (cfg: Partial, ctx) => { // In practice we should always have some cached data at this point. @@ -1550,16 +1033,5 @@ export const useSaveGUISettings = () => { return await window.lite.writeGUISettings(next); }, - onError: async (err) => { - // oxlint-disable-next-line no-console - console.error(err); - - toastManager.add({ - type: "error", - title: "Failed to save settings", - description: errorMessageForToast(err), - priority: "high", - }); - }, + meta: { failureTitle: "Failed to save settings" }, }); -}; diff --git a/apps/lite/ui/src/api/queries.ts b/apps/lite/ui/src/api/queries.ts index 840337a1ada..4363a309954 100644 --- a/apps/lite/ui/src/api/queries.ts +++ b/apps/lite/ui/src/api/queries.ts @@ -2,46 +2,20 @@ import type { PayloadFor } from "#electron/ipc.ts"; import { aggregateCIChecks } from "#ui/ci.ts"; import { clampAutoFetch, defaultSettings } from "#ui/settings.ts"; import type { ForgeReview } from "@gitbutler/but-sdk"; +import { apiProvides } from "@gitbutler/but-sdk/cache-tags"; import { queryOptions, type QueryClient } from "@tanstack/react-query"; import * as ms from "ms"; /** - * Keyed `[key, projectId, ...]`. The fixed position is what lets `handleWatcher` - * invalidate a whole query root holding nothing but a project id. + * The project queries are the endpoints declaring `provides` in Rust — using a + * name the backend doesn't declare is a type error. Keyed `[key, projectId, + * ...]`; the fixed position is what lets an invalidation reach a whole query + * root holding nothing but a project id. */ -export const projectQueryKeys = [ - "branchCannedName", - "branchDetails", - "branchDiff", - "branchList", - "changesInWorktree", - "commitConflicts", - "listCiChecks", - "commentsList", - "commitDetailsWithLineStats", - "forgeInfo", - "headInfo", - "currentForgeLogin", - "listRepoLabels", - "getReview", - "listReviewComments", - "listReviewSubmissions", - "listReviewTimelineEvents", - "listReviewReactions", - "listCommentReactions", - "getReviewMergeStatus", - "listReviewerCandidates", - "listReviews", - "getGbConfig", - "checkSigningSettings", - "treeChangeDiffs", - "absorptionPlan", - "workspaceFetchFromRemotes", - "workspaceFetchStatus", - "workspaceTargetCommits", -] as const; - -export type ProjectQueryKey = (typeof projectQueryKeys)[number]; +export type ProjectQueryKey = keyof typeof apiProvides; + +// `Object.keys` erases key types; the record's keys are exactly these. +export const projectQueryKeys = Object.keys(apiProvides) as ReadonlyArray; /** Keyed without a project id, so no project event can invalidate them. */ type GlobalQueryKey = @@ -242,6 +216,12 @@ export const workspaceFetchQueryOptions = ( }); }; +/** + * Fresh forge fetch each time; keep a gentle poll while the tab is open so + * changes from others appear without a manual refresh. + */ +const forgePoll = { staleTime: 60_000, refetchInterval: 60_000 }; + /** This query should be gated by PR capability lest it fail. */ export const listReviewCommentsQueryOptions = ({ projectId, @@ -250,10 +230,7 @@ export const listReviewCommentsQueryOptions = ({ queryOptions({ queryKey: ["listReviewComments", projectId, reviewId], queryFn: () => window.lite.listReviewComments({ projectId, reviewId }), - // Fresh forge fetch each time; keep a gentle poll while the tab is open - // so replies from others appear without a manual refresh. - staleTime: 60_000, - refetchInterval: 60_000, + ...forgePoll, }); export const gbConfigQueryOptions = (projectId: string) => @@ -309,9 +286,7 @@ export const listReviewSubmissionsQueryOptions = ({ queryOptions({ queryKey: ["listReviewSubmissions", projectId, reviewId], queryFn: () => window.lite.listReviewSubmissions({ projectId, reviewId }), - // Same freshness posture as the comments: fresh fetch, gentle poll. - staleTime: 60_000, - refetchInterval: 60_000, + ...forgePoll, }); /** This query should be gated by PR capability lest it fail. */ @@ -322,9 +297,7 @@ export const listReviewTimelineEventsQueryOptions = ({ queryOptions({ queryKey: ["listReviewTimelineEvents", projectId, reviewId], queryFn: () => window.lite.listReviewTimelineEvents({ projectId, reviewId }), - // Same freshness posture as the comments: fresh fetch, gentle poll. - staleTime: 60_000, - refetchInterval: 60_000, + ...forgePoll, }); /** This query should be gated by PR capability lest it fail. */ @@ -332,9 +305,7 @@ export const listReviewReactionsQueryOptions = ({ projectId, reviewId }: Payload queryOptions({ queryKey: ["listReviewReactions", projectId, reviewId], queryFn: () => window.lite.listReviewReactions({ projectId, reviewId }), - // Same freshness posture as the comments: fresh fetch, gentle poll. - staleTime: 60_000, - refetchInterval: 60_000, + ...forgePoll, }); /** diff --git a/apps/lite/ui/src/api/tags.test.ts b/apps/lite/ui/src/api/tags.test.ts index eabac01577d..d15bd70091f 100644 --- a/apps/lite/ui/src/api/tags.test.ts +++ b/apps/lite/ui/src/api/tags.test.ts @@ -3,16 +3,16 @@ import { apiInvalidates, type CacheTag } from "@gitbutler/but-sdk/cache-tags"; import type { QueryClient } from "@tanstack/react-query"; import { describe, expect, it } from "vitest"; -const declared = apiInvalidates as Record>; +const declared: Record> = apiInvalidates; const recording = () => { const invalidated: Array> = []; - const client = { - invalidateQueries: ({ queryKey }: { queryKey: ReadonlyArray }) => { - invalidated.push(queryKey); + const client: Pick = { + invalidateQueries: (filters) => { + invalidated.push(filters?.queryKey ?? []); return Promise.resolve(); }, - } as unknown as QueryClient; + }; return { client, invalidated }; }; @@ -53,9 +53,9 @@ describe("declared mutations", () => { }, ); - it("applies a mutation's declaration from its key", async () => { + it("applies a mutation's declaration from its endpoint", async () => { const { client, invalidated } = recording(); - await invalidateDeclared(client, ["mergeReview"], { projectId: "p1" }); + await invalidateDeclared(client, "mergeReview", { projectId: "p1" }); expect(invalidated).toEqual( expect.arrayContaining([ ["getReview", "p1"], @@ -68,7 +68,7 @@ describe("declared mutations", () => { it("ignores mutations that declared nothing", async () => { const { client, invalidated } = recording(); - await invalidateDeclared(client, ["commitCreate"], { projectId: "p1" }); + await invalidateDeclared(client, "commitCreate", { projectId: "p1" }); await invalidateDeclared(client, undefined, { projectId: "p1" }); expect(invalidated).toEqual([]); }); diff --git a/apps/lite/ui/src/api/tags.ts b/apps/lite/ui/src/api/tags.ts index f37d0c2529a..d35f5c56213 100644 --- a/apps/lite/ui/src/api/tags.ts +++ b/apps/lite/ui/src/api/tags.ts @@ -15,10 +15,10 @@ import type { QueryClient } from "@tanstack/react-query"; * Global queries by the tag they provide. The backend cannot know these: * they are what lite caches without a project scope, under keys of its own. */ -const globalProviders: Partial>> = { - Projects: ["projects"], - ForgeAccounts: ["forgeAccounts"], -}; +const globalProviders: ReadonlyArray<[CacheTag, QueryKey]> = [ + ["Projects", "projects"], + ["ForgeAccounts", "forgeAccounts"], +]; /** * Every query providing each tag, with the scope its key carries. @@ -35,17 +35,14 @@ const provide = (tag: CacheTag, query: QueryKey, projectScoped: boolean) => { }; for (const query of projectQueryKeys) for (const tag of apiProvides[query]) provide(tag, query, true); -for (const [tag, queries] of Object.entries(globalProviders) as Array< - [CacheTag, ReadonlyArray] ->) - for (const query of queries) provide(tag, query, false); +for (const [tag, query] of globalProviders) provide(tag, query, false); /** * Drop every cache providing the given tags. Without a project id, * project-scoped queries are invalidated across all projects by key prefix. */ export const invalidateTags = ( - client: QueryClient, + client: Pick, tags: ReadonlyArray, projectId?: string, ): Promise => @@ -59,37 +56,34 @@ export const invalidateTags = ( ), ); -/** A mutation endpoint that declared what it invalidates. */ -export type DeclaredMutation = keyof typeof apiInvalidates & keyof typeof window.lite; - /** - * The mutation options binding an endpoint to its declaration: the key names - * the endpoint, so on success the endpoint's `invalidates` tags are applied - * by the mutation cache. Spread it, overriding `mutationFn` when the call - * needs wrapping. + * The endpoint a mutation ran, recognized by the identity of its `mutationFn`. + * A wrapped `mutationFn` is invisible here, so an endpoint that declares + * `invalidates` must be passed to its mutation unwrapped. */ -export const apiMutation = (endpoint: Endpoint) => ({ - mutationKey: [endpoint] as const, - mutationFn: window.lite[endpoint], -}); +export const endpointOf = (mutationFn: unknown): string | undefined => { + endpointByFn ??= new Map(Object.entries(window.lite).map(([name, fn]) => [fn, name])); + return endpointByFn.get(mutationFn); +}; +// Built on first use: `window.lite` only exists in the renderer, not in tests. +let endpointByFn: Map | undefined; -/** The declarations by endpoint name, since a mutation key arrives as `unknown`. */ +/** The declarations by endpoint name, since an endpoint arrives as `unknown`. */ const declaredInvalidates = new Map>( Object.entries(apiInvalidates), ); /** * Apply a finished mutation's declared invalidations. Wired once into the - * query client's mutation cache; mutations opt in by carrying their endpoint - * as `mutationKey`, which `apiMutation` arranges. + * query client's mutation cache; the endpoint comes from [`endpointOf`], so + * any mutation whose `mutationFn` is a declared endpoint is covered. */ export const invalidateDeclared = ( - client: QueryClient, - mutationKey: ReadonlyArray | undefined, + client: Pick, + endpoint: string | undefined, variables: unknown, ): Promise => { - const endpoint = mutationKey?.[0]; - const tags = typeof endpoint === "string" ? declaredInvalidates.get(endpoint) : undefined; + const tags = endpoint === undefined ? undefined : declaredInvalidates.get(endpoint); if (!tags) return Promise.resolve(); const projectId = typeof variables === "object" && diff --git a/apps/lite/ui/src/main.tsx b/apps/lite/ui/src/main.tsx index 2e32e392c20..ef65fd681e8 100644 --- a/apps/lite/ui/src/main.tsx +++ b/apps/lite/ui/src/main.tsx @@ -1,7 +1,7 @@ import { MutationCache, QueryClient, focusManager } from "@tanstack/react-query"; import { createRouter } from "@tanstack/react-router"; import { App } from "#ui/App.tsx"; -import { invalidateDeclared } from "#ui/api/tags.ts"; +import { endpointOf, invalidateDeclared } from "#ui/api/tags.ts"; import { routeTree } from "#ui/routeTree.ts"; import { createRoot } from "react-dom/client"; import "./global.css"; @@ -21,10 +21,27 @@ const queryClient: QueryClient = new QueryClient({ }, }, // A mutation's cache effects come from its endpoint's `invalidates` - // declaration; per-mutation handlers keep only toasts and pushes. + // declaration, recognized by the `mutationFn` itself, and its failure + // toast from `meta.failureTitle`; per-mutation handlers keep only + // rollbacks, pushes, and dynamic wording. mutationCache: new MutationCache({ + // Returned on purpose: a mutation stays pending until the queries it + // invalidated are fresh, so success lands together with the new data. onSuccess: (_data, variables, _context, mutation) => - invalidateDeclared(queryClient, mutation.options.mutationKey, variables), + invalidateDeclared(queryClient, endpointOf(mutation.options.mutationFn), variables), + onError: (error, _variables, _context, mutation) => { + // oxlint-disable-next-line no-console + console.error(error); + + const title = mutation.meta?.failureTitle; + if (title === undefined) return; + toastManager.add({ + type: "error", + title, + description: errorMessageForToast(error), + priority: "high", + }); + }, }), }); diff --git a/apps/lite/ui/src/project-events.test.ts b/apps/lite/ui/src/project-events.test.ts index f3f3c872bb4..59867fce72c 100644 --- a/apps/lite/ui/src/project-events.test.ts +++ b/apps/lite/ui/src/project-events.test.ts @@ -5,8 +5,8 @@ import { apiProvides, watcherInvalidates } from "@gitbutler/but-sdk/cache-tags"; import type { QueryClient } from "@tanstack/react-query"; import { describe, expect, it } from "vitest"; -const provides = apiProvides as Record | undefined>; -const eventTags = watcherInvalidates as Record>; +const provides: Record | undefined> = apiProvides; +const eventTags: Record> = watcherInvalidates; /** The queries `handleProjectEvent` invalidates, and the ones it pushes. */ const react = (event: string) => { @@ -34,11 +34,7 @@ describe("tags declared in Rust", () => { // Guards the generated map: if it ever arrives empty, every query silently // stops refreshing and nothing else here would notice. it("answers for most of the queries", () => { - expect(projectQueryKeys.filter((query) => query in provides).length).toBeGreaterThan(20); - }); - - it("has a declaration for every project query", () => { - expect(projectQueryKeys.filter((query) => !(query in provides))).toEqual([]); + expect(projectQueryKeys.length).toBeGreaterThan(20); }); it.each( diff --git a/apps/lite/ui/src/routes/project/$id/workspace/CommitForm.tsx b/apps/lite/ui/src/routes/project/$id/workspace/CommitForm.tsx index 2bab30a02dd..ade52085d59 100644 --- a/apps/lite/ui/src/routes/project/$id/workspace/CommitForm.tsx +++ b/apps/lite/ui/src/routes/project/$id/workspace/CommitForm.tsx @@ -1,5 +1,5 @@ import uiStyles from "#ui/components/ui.module.css"; -import { commitAmendMutationKey, useBranchCreate, useCommitCreate } from "#ui/api/mutations.ts"; +import { useBranchCreate, useCommitCreate } from "#ui/api/mutations.ts"; import { branchCannedNameQueryOptions, headInfoQueryOptions } from "#ui/api/queries.ts"; import { getHeadInfoIndex, resolveRelativeTo } from "#ui/api/ref-info.ts"; import { getButtonClassName } from "#ui/components/Button.tsx"; @@ -144,7 +144,8 @@ export const CommitForm: FC<{ ...headInfoQueryOptions(projectId), select: getHeadInfoIndex, }); - const isAmendCommitPending = useIsMutating({ mutationKey: commitAmendMutationKey }) > 0; + const isAmendCommitPending = + useIsMutating({ predicate: (m) => m.options.mutationFn === window.lite.commitAmend }) > 0; // The branch creation is the first half of a commit here, so it keeps the // form read-only for its duration and rules out a double submit. const isCommitOrAmendPending = diff --git a/apps/lite/ui/src/routes/project/$id/workspace/PullRequestComments.tsx b/apps/lite/ui/src/routes/project/$id/workspace/PullRequestComments.tsx index f780b267e3a..78f1903be99 100644 --- a/apps/lite/ui/src/routes/project/$id/workspace/PullRequestComments.tsx +++ b/apps/lite/ui/src/routes/project/$id/workspace/PullRequestComments.tsx @@ -63,15 +63,14 @@ const Comment: FC<{ select: groupReactors, }); - const { mutate: addCommentReaction } = useAddCommentReaction(); - const { mutate: removeCommentReaction } = useRemoveCommentReaction(); + const { mutate: addCommentReaction } = useAddCommentReaction({ reviewId }); + const { mutate: removeCommentReaction } = useRemoveCommentReaction({ reviewId }); const toggleReaction = (kind: string, myReactionId: number | null) => { if (myReactionId === null) { - addCommentReaction({ projectId, reviewId, commentId: comment.id, kind }); + addCommentReaction({ projectId, commentId: comment.id, kind }); } else { removeCommentReaction({ projectId, - reviewId, commentId: comment.id, reactionId: myReactionId, }); @@ -82,7 +81,7 @@ const Comment: FC<{ const body = editBody.trim(); if (body === "" || isSaving) return; updateReviewComment( - { projectId, reviewId, commentId: comment.id, body }, + { projectId, commentId: comment.id, body }, { onSuccess: () => setEditing(false) }, ); }; @@ -120,7 +119,7 @@ const Comment: FC<{ onSelect: () => { // Forge deletion is permanent; double-check. if (window.confirm("Delete this comment? This cannot be undone.")) - deleteReviewComment({ projectId, reviewId, commentId: comment.id }); + deleteReviewComment({ projectId, commentId: comment.id }); }, }), ])