From 03c0ef04551a87dcc7fa20cebd9b2bc0e970b4b0 Mon Sep 17 00:00:00 2001 From: Innei Date: Mon, 4 Nov 2024 20:47:25 +0800 Subject: [PATCH] refactor(store): use transition to support rollback Signed-off-by: Innei --- .../src/hooks/biz/useSubscriptionActions.tsx | 4 +- .../feed-column/category-remove-dialog.tsx | 6 + .../src/modules/feed-column/category.tsx | 7 + apps/renderer/src/store/entry/store.ts | 237 +++++++---- apps/renderer/src/store/list/store.ts | 2 +- apps/renderer/src/store/subscription/store.ts | 387 ++++++++++-------- apps/renderer/src/store/unread/index.ts | 7 +- apps/renderer/src/store/utils/helper.test.ts | 156 +++++++ apps/renderer/src/store/utils/helper.ts | 145 +++---- locales/app/en.json | 4 + 10 files changed, 622 insertions(+), 333 deletions(-) create mode 100644 apps/renderer/src/store/utils/helper.test.ts diff --git a/apps/renderer/src/hooks/biz/useSubscriptionActions.tsx b/apps/renderer/src/hooks/biz/useSubscriptionActions.tsx index a1ef02a14..fd008ff39 100644 --- a/apps/renderer/src/hooks/biz/useSubscriptionActions.tsx +++ b/apps/renderer/src/hooks/biz/useSubscriptionActions.tsx @@ -26,14 +26,14 @@ export const useDeleteSubscription = ({ onSuccess }: { onSuccess?: () => void }) feedIdList?: string[] }) => { if (feedIdList) { - await subscriptionActions.unfollowMany(feedIdList) + await subscriptionActions.unfollow(feedIdList) toast.success(t("notify.unfollow_feed_many")) return } if (!subscription) return - subscriptionActions.unfollow(subscription.feedId).then((feed) => { + subscriptionActions.unfollow([subscription.feedId]).then(([feed]) => { subscriptionQuery.byView(subscription.view).invalidate() feedUnreadActions.updateByFeedId(subscription.feedId, 0) diff --git a/apps/renderer/src/modules/feed-column/category-remove-dialog.tsx b/apps/renderer/src/modules/feed-column/category-remove-dialog.tsx index 495aa8ef0..c1f8de525 100644 --- a/apps/renderer/src/modules/feed-column/category-remove-dialog.tsx +++ b/apps/renderer/src/modules/feed-column/category-remove-dialog.tsx @@ -1,7 +1,9 @@ import { Button } from "@follow/components/ui/button/index.js" import { useMutation } from "@tanstack/react-query" import { Trans, useTranslation } from "react-i18next" +import { toast } from "sonner" +import { createErrorToaster } from "~/lib/error-parser" import { subscriptionActions } from "~/store/subscription" import { useCurrentModal } from "../../components/ui/modal/stacked/hooks" @@ -10,6 +12,10 @@ export function CategoryRemoveDialogContent({ feedIdList }: { feedIdList: string const { t } = useTranslation() const deleteMutation = useMutation({ mutationFn: () => subscriptionActions.deleteCategory(feedIdList), + onError: createErrorToaster(t("sidebar.category_remove_dialog.error")), + onSuccess: () => { + toast.success(t("sidebar.category_remove_dialog.success")) + }, }) const { dismiss } = useCurrentModal() diff --git a/apps/renderer/src/modules/feed-column/category.tsx b/apps/renderer/src/modules/feed-column/category.tsx index 6414f999b..86d2df2ce 100644 --- a/apps/renderer/src/modules/feed-column/category.tsx +++ b/apps/renderer/src/modules/feed-column/category.tsx @@ -11,6 +11,7 @@ import { AnimatePresence, m } from "framer-motion" import type { FC } from "react" import { Fragment, memo, useCallback, useEffect, useMemo, useRef, useState } from "react" import { useTranslation } from "react-i18next" +import { toast } from "sonner" import { useOnClickOutside } from "usehooks-ts" import type { MenuItemInput } from "~/atoms/context-menu" @@ -18,6 +19,7 @@ import { useShowContextMenu } from "~/atoms/context-menu" import { ROUTE_FEED_IN_FOLDER } from "~/constants" import { useNavigateEntry } from "~/hooks/biz/useNavigateEntry" import { getRouteParams, useRouteParamsSelector } from "~/hooks/biz/useRouteParams" +import { createErrorToaster } from "~/lib/error-parser" import { getPreferredTitle, useAddFeedToFeedList, useFeedStore } from "~/store/feed" import { useOwnedListByView } from "~/store/list" import { @@ -337,6 +339,7 @@ const RenameCategoryForm: FC<{ onFinished: () => void }> = ({ currentCategory, onFinished }) => { const navigate = useNavigateEntry() + const { t } = useTranslation() const renameMutation = useMutation({ mutationFn: async ({ lastCategory, @@ -356,6 +359,10 @@ const RenameCategoryForm: FC<{ onFinished() }, + onError: createErrorToaster(t("sidebar.feed_column.context_menu.rename_category_error")), + onSuccess: () => { + toast.success(t("sidebar.feed_column.context_menu.rename_category_success")) + }, }) const formRef = useRef(null) useOnClickOutside( diff --git a/apps/renderer/src/store/entry/store.ts b/apps/renderer/src/store/entry/store.ts index e24040686..78e720224 100644 --- a/apps/renderer/src/store/entry/store.ts +++ b/apps/renderer/src/store/entry/store.ts @@ -20,7 +20,7 @@ import { imageActions } from "../image" import { inboxActions } from "../inbox" import { getSubscriptionByFeedId } from "../subscription" import { feedUnreadActions } from "../unread" -import { createZustandStore, doMutationAndTransaction } from "../utils/helper" +import { createTransaction, createZustandStore } from "../utils/helper" import { internal_batchMarkRead } from "./helper" import type { EntryState, FlatEntryModel } from "./types" @@ -415,81 +415,112 @@ class EntryActions { return } - feedUnreadActions.incrementByFeedId(feedId, read ? -1 : 1) + const tx = createTransaction({}) - this.patch(entryId, { - read, + tx.optimistic(async (_, ctx) => { + const prevUnread = feedUnreadActions.incrementByFeedId(feedId, read ? -1 : 1) + ctx.prevUnread = prevUnread + + this.patch(entryId, { + read, + }) }) - - await doMutationAndTransaction( - // Send api request - async () => { - if (read) { - await internal_batchMarkRead({ + tx.execute(async (_) => { + if (read) { + await internal_batchMarkRead({ + entryId, + isInbox, + isPrivate: subscription?.isPrivate, + }) + } else { + await apiClient.reads.$delete({ + json: { entryId, isInbox, - isPrivate: subscription?.isPrivate, - }) - } else { - await apiClient.reads.$delete({ - json: { - entryId, - isInbox, - }, - }) - } - }, - async () => - EntryService.bulkStoreReadStatus({ - [entryId]: read, - }), - ) + }, + }) + } + }) + + tx.persist(async () => { + EntryService.bulkStoreReadStatus({ + [entryId]: read, + }) + }) + + tx.rollback(async (_, ctx) => { + feedUnreadActions.updateByFeedId(feedId, ctx.prevUnread) + this.patch(entryId, { + read: !read, + }) + }) + + await tx.run() } async markStar(entryId: string, star: boolean) { - this.patch(entryId, { - collections: star - ? { - createdAt: new Date().toISOString(), - } - : (null as unknown as undefined), + const tx = createTransaction({}) + + tx.optimistic(async (_, ctx) => { + ctx.prevIsStar = !!get().flatMapEntries[entryId]?.collections?.createdAt + this.patch(entryId, { + collections: star + ? { + createdAt: new Date().toISOString(), + } + : (null as unknown as undefined), + }) + + set((state) => + produce(state, (state) => { + star ? state.starIds.add(entryId) : state.starIds.delete(entryId) + }), + ) + }) + tx.execute(async () => { + if (star) { + await apiClient.collections.$post({ + json: { + entryId, + }, + }) + } else { + await apiClient.collections.$delete({ + json: { + entryId, + }, + }) + } }) - set((state) => - produce(state, (state) => { - star ? state.starIds.add(entryId) : state.starIds.delete(entryId) - }), - ) - - await doMutationAndTransaction( - // Send api request - async () => { - if (star) { - apiClient.collections.$post({ - json: { - entryId, - }, - }) - } else { - await apiClient.collections.$delete({ - json: { - entryId, - }, - }) - } - }, - async () => { - if (star) { - return EntryService.bulkStoreCollection({ - [entryId]: { + tx.rollback(async (_, ctx) => { + set((state) => + produce(state, (state) => { + ctx.prevIsStar ? state.starIds.add(entryId) : state.starIds.delete(entryId) + }), + ) + this.patch(entryId, { + collections: ctx.prevIsStar + ? { createdAt: new Date().toISOString(), - }, - }) - } else { - return EntryService.deleteCollection(entryId) - } - }, - ) + } + : (null as unknown as undefined), + }) + }) + + tx.persist(async () => { + if (star) { + await EntryService.bulkStoreCollection({ + [entryId]: { + createdAt: new Date().toISOString(), + }, + }) + } else { + await EntryService.deleteCollection(entryId) + } + }) + + await tx.run() } updateReadHistory(entryId: string, readHistory: Omit) { @@ -505,26 +536,64 @@ class EntryActions { async deleteInboxEntry(entryId: string) { const entry = get().flatMapEntries[entryId] if (!entry) return + const tx = createTransaction(entry, { + deletedIndex: -1, + }) - set((state) => - produce(state, (draft) => { - delete draft.flatMapEntries[entryId] - for (const feedId in draft.entries) { - const index = draft.entries[feedId].indexOf(entryId) - if (index !== -1) { - draft.entries[feedId].splice(index, 1) - } + tx.optimistic(async (entry, ctx) => { + const { inboxId } = entry + const fullInboxId = `inbox-${inboxId}` + + set((state) => { + const nextFlatMapEntries = { ...state.flatMapEntries } + + delete nextFlatMapEntries[entryId] + + const index = state.entries[fullInboxId].indexOf(entryId) + + const nextState = { + ...state, + flatMapEntries: nextFlatMapEntries, } - }), - ) - await doMutationAndTransaction( - async () => { - await apiClient.entries.inbox.$delete({ json: { entryId } }) - }, - async () => { - await EntryService.deleteEntries([entryId]) - }, - ) + if (index !== -1) { + ctx.deletedIndex = index + + const nextFeedEntries = { + ...state.entries, + + [fullInboxId]: state.entries[fullInboxId].filter((id) => id !== entryId), + } + + nextState.entries = nextFeedEntries + } + + return nextState + }) + }) + + tx.execute(async () => { + await apiClient.entries.inbox.$delete({ json: { entryId } }) + }) + + tx.persist(async () => { + await EntryService.deleteEntries([entryId]) + }) + + tx.rollback(async (entry, ctx) => { + set((state) => ({ + ...state, + entries: { + ...state.entries, + [entry.feedId]: state.entries[entry.feedId].splice(ctx.deletedIndex, 0, entryId), + }, + flatMapEntries: { + ...state.flatMapEntries, + [entryId]: entry, + }, + })) + }) + + await tx.run() } } diff --git a/apps/renderer/src/store/list/store.ts b/apps/renderer/src/store/list/store.ts index 9164d1a46..4e2b8758f 100644 --- a/apps/renderer/src/store/list/store.ts +++ b/apps/renderer/src/store/list/store.ts @@ -106,7 +106,7 @@ class ListActionStatic { }) }) - tx.onPersist(async () => { + tx.persist(async () => { ListService.bulkDelete([listId]) }) await tx.run() diff --git a/apps/renderer/src/store/subscription/store.ts b/apps/renderer/src/store/subscription/store.ts index 72a0b9f22..02f3cfd9f 100644 --- a/apps/renderer/src/store/subscription/store.ts +++ b/apps/renderer/src/store/subscription/store.ts @@ -22,7 +22,7 @@ import { feedActions, getFeedById } from "../feed" import { inboxActions } from "../inbox" import { listActions } from "../list" import { feedUnreadActions } from "../unread" -import { createImmerSetter, createZustandStore, doMutationAndTransaction } from "../utils/helper" +import { createImmerSetter, createTransaction, createZustandStore } from "../utils/helper" import { subscriptionCategoryExistSelector } from "./selector" export type SubscriptionFlatModel = Omit & { @@ -241,28 +241,36 @@ class SubscriptionActions { } async markReadByView(view: FeedViewType, filter?: MarkReadFilter) { - doMutationAndTransaction( - () => - apiClient.reads.all.$post({ - json: { - view, - ...filter, - }, - }), - async () => { - const state = get() - for (const feedId in state.data) { - if (state.data[feedId].view === view) { - // We can not process this logic in local, so skip it. and then we will fetch the unread count from server. - !filter && feedUnreadActions.updateByFeedId(feedId, 0) - entryActions.patchManyByFeedId(feedId, { read: true }, filter) - } + const tx = createTransaction() + + tx.execute(async () => { + await apiClient.reads.all.$post({ + json: { + view, + ...filter, + }, + }) + }) + + tx.optimistic(async () => { + const state = get() + for (const feedId in state.data) { + if (state.data[feedId].view === view) { + // We can not process this logic in local, so skip it. and then we will fetch the unread count from server. + !filter && feedUnreadActions.updateByFeedId(feedId, 0) + entryActions.patchManyByFeedId(feedId, { read: true }, filter) } - if (filter) { - feedUnreadActions.fetchUnreadByView(view) - } - }, - ) + } + if (filter) { + feedUnreadActions.fetchUnreadByView(view) + } + }) + + tx.rollback(async () => { + // TODO handle this local? + await feedUnreadActions.fetchUnreadByView(view) + }) + await tx.run() } async markReadByFeedIds({ @@ -280,42 +288,50 @@ class SubscriptionActions { }) { const stableFeedIds = feedIds?.concat() || [] - doMutationAndTransaction( - () => - apiClient.reads.all.$post({ - json: { - ...(listId + const tx = createTransaction() + + tx.execute(async () => { + await apiClient.reads.all.$post({ + json: { + ...(listId + ? { + listId, + } + : inboxId ? { - listId, + inboxId, } - : inboxId - ? { - inboxId, - } - : { - feedIdList: stableFeedIds, - }), - ...filter, - }, - }), - async () => { - if (listId) { - feedUnreadActions.updateByFeedId(listId, 0) - } else if (inboxId) { - feedUnreadActions.updateByFeedId(inboxId, 0) - entryActions.patchManyByFeedId(inboxId, { read: true }, filter) - } else { - for (const feedId of stableFeedIds) { - // We can not process this logic in local, so skip it. and then we will fetch the unread count from server. - !filter && feedUnreadActions.updateByFeedId(feedId, 0) - entryActions.patchManyByFeedId(feedId, { read: true }, filter) - } + : { + feedIdList: stableFeedIds, + }), + ...filter, + }, + }) + }) + tx.optimistic(async () => { + if (listId) { + feedUnreadActions.updateByFeedId(listId, 0) + } else if (inboxId) { + feedUnreadActions.updateByFeedId(inboxId, 0) + entryActions.patchManyByFeedId(inboxId, { read: true }, filter) + } else { + for (const feedId of stableFeedIds) { + // We can not process this logic in local, so skip it. and then we will fetch the unread count from server. + !filter && feedUnreadActions.updateByFeedId(feedId, 0) + entryActions.patchManyByFeedId(feedId, { read: true }, filter) } - if (filter) { - feedUnreadActions.fetchUnreadByView(view) - } - }, - ) + } + if (filter) { + feedUnreadActions.fetchUnreadByView(view) + } + }) + + tx.rollback(async () => { + // TODO handle this local? + await feedUnreadActions.fetchUnreadByView(view) + }) + + await tx.run() } clear() { @@ -326,102 +342,127 @@ class SubscriptionActions { }) } - deleteCategory(ids: string[]) { + async deleteCategory(ids: string[]) { const idSet = new Set(ids) + const tx = createTransaction(get()) - return doMutationAndTransaction( - () => - apiClient.categories.$delete({ - json: { - feedIdList: ids, - deleteSubscriptions: false, - }, + tx.execute(async () => { + await apiClient.categories.$delete({ + json: { + feedIdList: ids, + deleteSubscriptions: false, + }, + }) + }) + + tx.optimistic(async () => { + immerSet((state) => + Object.keys(state.data).forEach((id) => { + if (!idSet.has(id)) { + return + } + const subscription = state.data[id] + const feed = getFeedById(subscription.feedId) + if (!feed || feed.type !== "feed") return + const { siteUrl } = feed + if (!siteUrl) return + const parsed = parse(siteUrl) + subscription.category = null + // The logic for removing Category here is to use domain as the default category name. + parsed.domain && (subscription.defaultCategory = capitalizeFirstLetter(parsed.domain)) }), - async () => { - immerSet((state) => - Object.keys(state.data).forEach((id) => { - if (idSet.has(id)) { - const subscription = state.data[id] - const feed = getFeedById(subscription.feedId) - if (!feed || feed.type !== "feed") return - const { siteUrl } = feed - if (!siteUrl) return - const parsed = parse(siteUrl) - subscription.category = null - // The logic for removing Category here is to use domain as the default category name. - parsed.domain && (subscription.defaultCategory = capitalizeFirstLetter(parsed.domain)) - } - }), - ) - const { data } = get() - return ids.map((id) => data[id] && SubscriptionService.upsert(data[id])) - }, + ) + }) + + tx.rollback(async (snapshot) => { + immerSet((state) => { + Object.keys(snapshot.data).forEach((id) => { + state.data[id] = snapshot.data[id] + }) + }) + }) + + tx.persist(async () => { + const { data } = get() + ids.map((id) => data[id] && SubscriptionService.upsert(data[id])) + }) + await tx.run() + } + + async unfollow(feedIds: string[]) { + // const feed = getFeedById(feedId) + const feeds = feedIds.map((feedId) => getFeedById(feedId)) + const tx = createTransaction< + ReturnType, { - doTranscationWhenMutationFail: false, - waitMutation: true, - }, - ) - } + subscription: Record + viewDeleted: Record + } + >(get(), { + viewDeleted: {}, + subscription: {}, + }) - async unfollow(feedId: string) { - const feed = getFeedById(feedId) - // Remove feed and subscription - set((state) => - produce(state, (draft) => { - delete draft.data[feedId] - for (const view in draft.feedIdByView) { - const currentViewFeedIds = draft.feedIdByView[view] as string[] - currentViewFeedIds.splice(currentViewFeedIds.indexOf(feedId), 1) - } - }), - ) - - // Remove feed's entries - entryActions.clearByFeedId(feedId) - // Clear feed's unread count - feedUnreadActions.updateByFeedId(feedId, 0) - - return doMutationAndTransaction( - () => - apiClient.subscriptions - .$delete({ - json: { - feedId, - }, - }) - .then(() => feed), - () => SubscriptionService.removeSubscription(whoami()!.id, feedId), - ).then(() => feed) - } - - async unfollowMany(feedIdList: string[]) { - for (const feedId of feedIdList) { + tx.optimistic(async (_, ctx) => { // Remove feed and subscription set((state) => produce(state, (draft) => { - delete draft.data[feedId] - for (const view in draft.feedIdByView) { - const currentViewFeedIds = draft.feedIdByView[view] as string[] - currentViewFeedIds.splice(currentViewFeedIds.indexOf(feedId), 1) + for (const feedId of feedIds) { + const subscription = state.data[feedId] + ctx.subscription[feedId] = subscription + + delete draft.data[feedId] + + for (const view in draft.feedIdByView) { + const currentViewFeedIds = draft.feedIdByView[view] as string[] + + const idx = currentViewFeedIds.indexOf(feedId) + + if (idx !== -1) { + currentViewFeedIds.splice(idx, 1) + ctx.viewDeleted[feedId] = ctx.viewDeleted[feedId] || [] + ctx.viewDeleted[feedId].push(Number.parseInt(view)) + } + } } }), ) + }) + tx.rollback(async (_, ctx) => { + set((state) => + produce(state, (draft) => { + for (const feedId of feedIds) { + if (!ctx.subscription[feedId]) return + draft.data[feedId] = ctx.subscription[feedId] - // Remove feed's entries - entryActions.clearByFeedId(feedId) - // Clear feed's unread count - feedUnreadActions.updateByFeedId(feedId, 0) - } - - return doMutationAndTransaction( - () => - apiClient.subscriptions.$delete({ - json: { - feedIdList, - }, + for (const view of ctx.viewDeleted[feedId]) { + draft.feedIdByView[view].push(feedId) + } + } }), - () => SubscriptionService.removeSubscriptionMany(whoami()!.id, feedIdList), - ) + ) + }) + + tx.persist(async () => { + for (const feedId of feedIds) { + // Remove feed's entries + entryActions.clearByFeedId(feedId) + // Clear feed's unread count + feedUnreadActions.updateByFeedId(feedId, 0) + + SubscriptionService.removeSubscription(whoami()!.id, feedId) + } + }) + tx.execute(async () => { + await apiClient.subscriptions.$delete({ + json: { + feedIdList: feedIds, + }, + }) + }) + + await tx.run() + return feeds } async changeCategoryView( @@ -482,6 +523,17 @@ class SubscriptionActions { } async renameCategory(lastCategory: string, newCategory: string) { + const tx = createTransaction< + unknown, + { + subscription: Record + } + >( + {}, + { + subscription: {}, + }, + ) const subscriptionIds = [] as string[] const state = get() for (const feedId in state.data) { @@ -491,30 +543,45 @@ class SubscriptionActions { } } - set((state) => - produce(state, (state) => { - for (const feedId of subscriptionIds) { - const subscription = state.data[feedId] - if (subscription) { - subscription.category = newCategory - subscription.defaultCategory = undefined - } - } - }), - ) + tx.execute(async () => { + await apiClient.categories.$patch({ + json: { + feedIdList: subscriptionIds, + category: newCategory, + }, + }) + }) - return doMutationAndTransaction( - () => - apiClient.categories.$patch({ - json: { - feedIdList: subscriptionIds, - category: newCategory, - }, + tx.persist(async () => { + SubscriptionService.renameCategory(whoami()!.id, subscriptionIds, newCategory) + }) + + tx.optimistic(async (_, ctx) => { + set((state) => + produce(state, (draft) => { + for (const feedId of subscriptionIds) { + const subscription = draft.data[feedId] + if (subscription) { + subscription.category = newCategory + subscription.defaultCategory = undefined + + ctx.subscription[feedId] = state.data[feedId] + } + } }), - async () => - // Db - SubscriptionService.renameCategory(whoami()!.id, subscriptionIds, newCategory), - ) + ) + }) + tx.rollback(async (_, ctx) => { + set((state) => + produce(state, (draft) => { + for (const feedId of Object.keys(ctx.subscription)) { + draft.data[feedId] = ctx.subscription[feedId] + } + }), + ) + }) + + await tx.run() } } diff --git a/apps/renderer/src/store/unread/index.ts b/apps/renderer/src/store/unread/index.ts index ee169962e..985ce6d42 100644 --- a/apps/renderer/src/store/unread/index.ts +++ b/apps/renderer/src/store/unread/index.ts @@ -63,11 +63,16 @@ class FeedUnreadActions { return data } + /** + * @returns previous value + */ incrementByFeedId(feedId: string, inc: number) { const state = get() const cur = state.data[feedId] + const nextValue = Math.max(0, (cur || 0) + inc) - this.internal_setValue([[feedId, Math.max(0, (cur || 0) + inc)]]) + this.internal_setValue([[feedId, nextValue]]) + return cur } updateByFeedId(feedId: string, unread: number) { diff --git a/apps/renderer/src/store/utils/helper.test.ts b/apps/renderer/src/store/utils/helper.test.ts new file mode 100644 index 000000000..fddc5054f --- /dev/null +++ b/apps/renderer/src/store/utils/helper.test.ts @@ -0,0 +1,156 @@ +import { fail } from "node:assert" + +import { describe, expect, it, vi } from "vitest" + +import { createTransaction } from "./helper" + +describe("createTransaction", () => { + it("should execute all steps in correct order", async () => { + const executionOrder: string[] = [] + const snapshot = { value: 1 } + + const transaction = createTransaction(snapshot) + .optimistic(async () => { + executionOrder.push("optimistic") + }) + .execute(async () => { + executionOrder.push("execute") + }) + .persist(async () => { + executionOrder.push("persist") + }) + + await transaction.run() + + expect(executionOrder).toEqual(["optimistic", "execute", "persist"]) + }) + + it("should handle rollback when execution fails", async () => { + const snapshot = { value: 1 } + const rollbackMock = vi.fn() + const error = new Error("Execution failed") + const persistMock = vi.fn() + + const transaction = createTransaction(snapshot) + .rollback(rollbackMock) + .execute(async () => { + throw error + }) + .persist(persistMock) + + await expect(transaction.run()).rejects.toThrow(error) + expect(rollbackMock).toHaveBeenCalledWith(snapshot, {}) + expect(persistMock).not.toHaveBeenCalled() + }) + + it("should continue if optimistic update fails", async () => { + const executionOrder: string[] = [] + const snapshot = { value: 1 } + const rollbackMock = vi.fn() + + const transaction = createTransaction(snapshot) + .rollback(rollbackMock) + .optimistic(async () => { + throw new Error("Optimistic update failed") + }) + .execute(async () => { + executionOrder.push("execute") + }) + .persist(async () => { + executionOrder.push("persist") + }) + + await transaction.run() + + expect(executionOrder).toEqual(["execute", "persist"]) + expect(rollbackMock).not.toHaveBeenCalled() + }) + + it("should maintain method chaining", () => { + const snapshot = { value: 1 } + const transaction = createTransaction(snapshot) + + expect(transaction.optimistic(() => Promise.resolve())).toBe(transaction) + expect(transaction.execute(() => Promise.resolve())).toBe(transaction) + expect(transaction.rollback(() => Promise.resolve())).toBe(transaction) + expect(transaction.persist(() => Promise.resolve())).toBe(transaction) + }) + + it("should pass context to all functions", async () => { + const snapshot = { value: 1 } + const ctx = { someContext: "test" } + const optimisticMock = vi.fn() + const executeMock = vi.fn() + const persistMock = vi.fn() + + const transaction = createTransaction(snapshot, ctx) + .optimistic(optimisticMock) + .execute(executeMock) + .persist(persistMock) + + await transaction.run() + + expect(optimisticMock).toHaveBeenCalledWith(snapshot, ctx) + expect(executeMock).toHaveBeenCalledWith(snapshot, ctx) + expect(persistMock).toHaveBeenCalledWith(snapshot, ctx) + }) + + it("should pass context to rollback function when execution fails", async () => { + const snapshot = { value: 1 } + const ctx = { someContext: "test" } + const rollbackMock = vi.fn() + const error = new Error("Execution failed") + + const transaction = createTransaction(snapshot, ctx) + .rollback(rollbackMock) + .execute(async () => { + throw error + }) + + await expect(transaction.run()).rejects.toThrow(error) + expect(rollbackMock).toHaveBeenCalledWith(snapshot, ctx) + }) + + it("should allow modifying context in optimistic phase for later consumption", async () => { + const snapshot = { value: 1 } + const ctx = { someValue: "initial" } + const executionOrder: string[] = [] + + const transaction = createTransaction(snapshot, ctx) + .optimistic(async (_, context: any) => { + context.someValue = "modified" + executionOrder.push(`optimistic: ${context.someValue}`) + }) + .execute(async (_, context: any) => { + executionOrder.push(`execute: ${context.someValue}`) + }) + .persist(async (_, context: any) => { + executionOrder.push(`persist: ${context.someValue}`) + }) + + await transaction.run() + + expect(executionOrder).toEqual([ + "optimistic: modified", + "execute: modified", + "persist: modified", + ]) + expect(ctx.someValue).toBe("modified") + }) + + it("should propagate the exact same error instance from execute to run", async () => { + const snapshot = { value: 1 } + const specificError = new Error("Specific error message") + + const transaction = createTransaction(snapshot).execute(async () => { + throw specificError + }) + + try { + await transaction.run() + fail("Expected an error to be thrown") + } catch (error) { + expect(error).toBe(specificError) + } + }) +}) diff --git a/apps/renderer/src/store/utils/helper.ts b/apps/renderer/src/store/utils/helper.ts index 9e263d4fb..988972457 100644 --- a/apps/renderer/src/store/utils/helper.ts +++ b/apps/renderer/src/store/utils/helper.ts @@ -1,7 +1,4 @@ -/* eslint-disable no-unsafe-finally */ - import { isDraft, original, produce } from "immer" -import { unstable_batchedUpdates } from "react-dom" import type { StateCreator, StoreApi, UseBoundStore } from "zustand" import type { PersistStorage } from "zustand/middleware" import { devtools } from "zustand/middleware" @@ -89,54 +86,6 @@ export const getStoreActions = any }>( return actions as any } -export type MutationAndTranscationOptions = { - /** - * If true, will wait for the mutation to finish before running the transaction - */ - waitMutation?: boolean - /** - * If true, will run the transaction even if the mutation fails, useful in network offline - */ - doTranscationWhenMutationFail?: boolean -} -export const doMutationAndTransaction = async ( - mutationFn: () => Promise, - transaction: () => Promise, - options?: MutationAndTranscationOptions, -): Promise<[M | null | void, T | null | void]> => { - const isOnline = navigator.onLine - const { waitMutation = false, doTranscationWhenMutationFail = !isOnline } = options || {} - const wrappedTransaction = () => { - const ret = runTransactionInScope(() => unstable_batchedUpdates(() => transaction())) - - if (ret instanceof Promise) { - return ret - } - return null - } - const wrappedMutation = () => (isOnline ? mutationFn() : Promise.resolve()) - - if (waitMutation) { - let runTransactionOnce = false - - try { - const mutationRet = await wrappedMutation() - runTransactionOnce = true - const transactionRet = await wrappedTransaction() - return [mutationRet, transactionRet] - } finally { - if (!runTransactionOnce && doTranscationWhenMutationFail) { - const transactionRet = await wrappedTransaction() - - return [null, transactionRet] - } - return [null, null] - } - } else { - return await Promise.all([wrappedMutation(), wrappedTransaction()]) - } -} - export function createImmerSetter(useStore: UseBoundStore>) { return (updater: (state: T) => void) => useStore.setState((state) => @@ -151,42 +100,68 @@ export const toRaw = (draft: MayBeDraft): T => { return isDraft(draft) ? original(draft)! : draft } -const noop = (err: any) => { - console.error(err) -} -export const createTransaction = (snapshot: S) => { - let onRollback: ((snapshot: S) => Promise) | undefined - let executorFn: (snapshot: S) => Promise | undefined - let optimisticExecutor: (snapshot: S) => Promise | undefined - let onPersist: ((snapshot: S) => Promise) | undefined +class Transaction { + private _snapshot: S + private _ctx: Ctx + private onRollback?: (snapshot: S, ctx: Ctx) => Promise + private executorFn?: (snapshot: S, ctx: Ctx) => Promise + private optimisticExecutor?: (snapshot: S, ctx: Ctx) => Promise + private onPersist?: (snapshot: S, ctx: Ctx) => Promise - const ret = { - rollback: (fn: (snapshot: S) => Promise) => { - onRollback = fn - return ret - }, - execute: (executor: (snapshot: S) => Promise) => { - executorFn = executor - return ret - }, - optimistic: (executor: (snapshot: S) => Promise) => { - optimisticExecutor = executor - return ret - }, - run: async () => { - await optimisticExecutor?.(snapshot)?.catch(noop) - await executorFn?.(snapshot)?.catch((err) => { - if (onRollback) { - onRollback(snapshot) + constructor(snapshot?: S, ctx?: Ctx) { + this._snapshot = snapshot || ({} as S) + this._ctx = ctx || ({} as Ctx) + } + + rollback(fn: (snapshot: S, ctx: Ctx) => Promise): this { + this.onRollback = fn + return this + } + + execute(executor: (snapshot: S, ctx: Ctx) => Promise): this { + this.executorFn = executor + return this + } + + optimistic(executor: (snapshot: S, ctx: Ctx) => Promise): this { + this.optimisticExecutor = executor + return this + } + + persist(fn: (snapshot: S, ctx: Ctx) => Promise): this { + this.onPersist = fn + return this + } + + async run(): Promise { + let isOptimisticFailed = false + + if (this.optimisticExecutor) { + try { + await this.optimisticExecutor(this._snapshot, this._ctx) + } catch (error) { + isOptimisticFailed = true + console.error(error) + } + } + + if (this.executorFn) { + try { + await this.executorFn(this._snapshot, this._ctx) + } catch (err) { + if (this.onRollback && !isOptimisticFailed) { + await this.onRollback(this._snapshot, this._ctx) } throw err - }) - await runTransactionInScope(() => onPersist?.(snapshot)) - }, - onPersist: (fn: (snapshot: S) => Promise) => { - onPersist = fn - return ret - }, + } + } + + if (this.onPersist) { + await runTransactionInScope(() => this.onPersist!(this._snapshot, this._ctx)) + } } - return ret +} + +export const createTransaction = (snapshot?: S, ctx?: Ctx): Transaction => { + return new Transaction(snapshot, ctx) } diff --git a/locales/app/en.json b/locales/app/en.json index aca1bee07..d1045ff2b 100644 --- a/locales/app/en.json +++ b/locales/app/en.json @@ -271,6 +271,8 @@ "sidebar.category_remove_dialog.cancel": "Cancel", "sidebar.category_remove_dialog.continue": "Continue", "sidebar.category_remove_dialog.description": "This operation will delete your category, but the feeds it contains will be retained and grouped by website.", + "sidebar.category_remove_dialog.error": "Failed to remove category", + "sidebar.category_remove_dialog.success": "Category removed successfully", "sidebar.category_remove_dialog.title": "Remove Category", "sidebar.feed_actions.claim": "Claim", "sidebar.feed_actions.claim_feed": "Claim Feed", @@ -304,6 +306,8 @@ "sidebar.feed_column.context_menu.delete_category_confirmation": "Delete Category {{folderName}}?", "sidebar.feed_column.context_menu.mark_as_read": "Mark as Read", "sidebar.feed_column.context_menu.rename_category": "Rename Category", + "sidebar.feed_column.context_menu.rename_category_error": "Failed to rename category", + "sidebar.feed_column.context_menu.rename_category_success": "Category renamed successfully", "sidebar.select_sort_method": "Select a sort method", "signin.continue_with_github": "Continue with GitHub", "signin.continue_with_google": "Continue with Google",