From fae5eac31bdde8d4f3163f014ab7969d2f9c99cf Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 1 Dec 2025 08:54:40 +0800 Subject: [PATCH] fix(web): fix infinite loop in MemoEditor and improve React/MobX integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrap all setter functions in useMemoEditorState with useCallback to ensure stable references This prevents infinite loops when setters are used in useEffect dependencies (fixes "Maximum update depth exceeded" error) - Extract MobX observable values in useMemoFilters and useMemoSorting before using them in useMemo dependencies This prevents React from tracking MobX observables directly, improving reliability - Add comprehensive documentation explaining the design decisions for future maintainability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../MemoEditor/hooks/useMemoEditorState.ts | 88 ++++++++++++++----- web/src/hooks/useMemoFilters.ts | 13 ++- web/src/hooks/useMemoSorting.ts | 11 ++- 3 files changed, 82 insertions(+), 30 deletions(-) diff --git a/web/src/components/MemoEditor/hooks/useMemoEditorState.ts b/web/src/components/MemoEditor/hooks/useMemoEditorState.ts index 1dd267f4f..4c9335754 100644 --- a/web/src/components/MemoEditor/hooks/useMemoEditorState.ts +++ b/web/src/components/MemoEditor/hooks/useMemoEditorState.ts @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useCallback, useState } from "react"; import type { Attachment } from "@/types/proto/api/v1/attachment_service"; import type { Location, MemoRelation } from "@/types/proto/api/v1/memo_service"; import { Visibility } from "@/types/proto/api/v1/memo_service"; @@ -15,6 +15,13 @@ interface MemoEditorState { isDraggingFile: boolean; } +/** + * Custom hook for managing MemoEditor state with stable setter references. + * + * Note: All setter functions are wrapped with useCallback to ensure stable references. + * This prevents infinite loops when these setters are used in useEffect dependencies. + * While this makes the code verbose, it's necessary for proper React dependency tracking. + */ export const useMemoEditorState = (initialVisibility: Visibility = Visibility.PRIVATE) => { const [state, setState] = useState({ memoVisibility: initialVisibility, @@ -28,29 +35,66 @@ export const useMemoEditorState = (initialVisibility: Visibility = Visibility.PR isDraggingFile: false, }); - const update = (key: K, value: MemoEditorState[K]) => { - setState((prev) => ({ ...prev, [key]: value })); - }; + // All setters are memoized with useCallback to provide stable function references. + // This prevents unnecessary re-renders and infinite loops in useEffect hooks. + const setMemoVisibility = useCallback((v: Visibility) => { + setState((prev) => ({ ...prev, memoVisibility: v })); + }, []); + + const setAttachmentList = useCallback((v: Attachment[]) => { + setState((prev) => ({ ...prev, attachmentList: v })); + }, []); + + const setRelationList = useCallback((v: MemoRelation[]) => { + setState((prev) => ({ ...prev, relationList: v })); + }, []); + + const setLocation = useCallback((v: Location | undefined) => { + setState((prev) => ({ ...prev, location: v })); + }, []); + + const toggleFocusMode = useCallback(() => { + setState((prev) => ({ ...prev, isFocusMode: !prev.isFocusMode })); + }, []); + + const setUploadingAttachment = useCallback((v: boolean) => { + setState((prev) => ({ ...prev, isUploadingAttachment: v })); + }, []); + + const setRequesting = useCallback((v: boolean) => { + setState((prev) => ({ ...prev, isRequesting: v })); + }, []); + + const setComposing = useCallback((v: boolean) => { + setState((prev) => ({ ...prev, isComposing: v })); + }, []); + + const setDraggingFile = useCallback((v: boolean) => { + setState((prev) => ({ ...prev, isDraggingFile: v })); + }, []); + + const resetState = useCallback(() => { + setState((prev) => ({ + ...prev, + isRequesting: false, + attachmentList: [], + relationList: [], + location: undefined, + isDraggingFile: false, + })); + }, []); return { ...state, - setMemoVisibility: (v: Visibility) => update("memoVisibility", v), - setAttachmentList: (v: Attachment[]) => update("attachmentList", v), - setRelationList: (v: MemoRelation[]) => update("relationList", v), - setLocation: (v: Location | undefined) => update("location", v), - toggleFocusMode: () => setState((prev) => ({ ...prev, isFocusMode: !prev.isFocusMode })), - setUploadingAttachment: (v: boolean) => update("isUploadingAttachment", v), - setRequesting: (v: boolean) => update("isRequesting", v), - setComposing: (v: boolean) => update("isComposing", v), - setDraggingFile: (v: boolean) => update("isDraggingFile", v), - resetState: () => - setState((prev) => ({ - ...prev, - isRequesting: false, - attachmentList: [], - relationList: [], - location: undefined, - isDraggingFile: false, - })), + setMemoVisibility, + setAttachmentList, + setRelationList, + setLocation, + toggleFocusMode, + setUploadingAttachment, + setRequesting, + setComposing, + setDraggingFile, + resetState, }; }; diff --git a/web/src/hooks/useMemoFilters.ts b/web/src/hooks/useMemoFilters.ts index 434d4372a..1289cc42f 100644 --- a/web/src/hooks/useMemoFilters.ts +++ b/web/src/hooks/useMemoFilters.ts @@ -20,11 +20,16 @@ export interface UseMemoFiltersOptions { export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | undefined => { const { creatorName, includeShortcuts = false, includePinned = false, visibilities } = options; + // Extract MobX observable values to avoid issues with React dependency tracking + const currentShortcut = memoFilterStore.shortcut; + const shortcuts = userStore.state.shortcuts; + const filters = memoFilterStore.filters; + // Get selected shortcut if needed const selectedShortcut = useMemo(() => { if (!includeShortcuts) return undefined; - return userStore.state.shortcuts.find((shortcut) => getShortcutId(shortcut.name) === memoFilterStore.shortcut); - }, [includeShortcuts, memoFilterStore.shortcut, userStore.state.shortcuts]); + return shortcuts.find((shortcut) => getShortcutId(shortcut.name) === currentShortcut); + }, [includeShortcuts, currentShortcut, shortcuts]); // Build filter - wrapped in useMemo but also using observer for reactivity return useMemo(() => { @@ -41,7 +46,7 @@ export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | un } // Add active filters from memoFilterStore - for (const filter of memoFilterStore.filters) { + for (const filter of filters) { if (filter.factor === "contentSearch") { conditions.push(`content.contains("${filter.value}")`); } else if (filter.factor === "tagSearch") { @@ -81,5 +86,5 @@ export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | un } return conditions.length > 0 ? conditions.join(" && ") : undefined; - }, [creatorName, includeShortcuts, includePinned, visibilities, selectedShortcut, memoFilterStore.filters]); + }, [creatorName, includeShortcuts, includePinned, visibilities, selectedShortcut, filters]); }; diff --git a/web/src/hooks/useMemoSorting.ts b/web/src/hooks/useMemoSorting.ts index 1b2aa70c1..1180ebb63 100644 --- a/web/src/hooks/useMemoSorting.ts +++ b/web/src/hooks/useMemoSorting.ts @@ -17,11 +17,14 @@ export interface UseMemoSortingResult { export const useMemoSorting = (options: UseMemoSortingOptions = {}): UseMemoSortingResult => { const { pinnedFirst = false, state = State.NORMAL } = options; + // Extract MobX observable values to avoid issues with React dependency tracking + const orderByTimeAsc = viewStore.state.orderByTimeAsc; + // Generate orderBy string for API const orderBy = useMemo(() => { - const timeOrder = viewStore.state.orderByTimeAsc ? "display_time asc" : "display_time desc"; + const timeOrder = orderByTimeAsc ? "display_time asc" : "display_time desc"; return pinnedFirst ? `pinned desc, ${timeOrder}` : timeOrder; - }, [pinnedFirst, viewStore.state.orderByTimeAsc]); + }, [pinnedFirst, orderByTimeAsc]); // Generate listSort function for client-side sorting const listSort = useMemo(() => { @@ -35,12 +38,12 @@ export const useMemoSorting = (options: UseMemoSortingOptions = {}): UseMemoSort } // Then sort by display time - return viewStore.state.orderByTimeAsc + return orderByTimeAsc ? dayjs(a.displayTime).unix() - dayjs(b.displayTime).unix() : dayjs(b.displayTime).unix() - dayjs(a.displayTime).unix(); }); }; - }, [pinnedFirst, state, viewStore.state.orderByTimeAsc]); + }, [pinnedFirst, state, orderByTimeAsc]); return { listSort, orderBy }; };