fix(web): fix infinite loop in MemoEditor and improve React/MobX integration

- 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 <noreply@anthropic.com>
This commit is contained in:
Steven 2025-12-01 08:54:40 +08:00
parent d1492007ab
commit fae5eac31b
3 changed files with 82 additions and 30 deletions

View File

@ -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 { Attachment } from "@/types/proto/api/v1/attachment_service";
import type { Location, MemoRelation } from "@/types/proto/api/v1/memo_service"; import type { Location, MemoRelation } from "@/types/proto/api/v1/memo_service";
import { Visibility } from "@/types/proto/api/v1/memo_service"; import { Visibility } from "@/types/proto/api/v1/memo_service";
@ -15,6 +15,13 @@ interface MemoEditorState {
isDraggingFile: boolean; 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) => { export const useMemoEditorState = (initialVisibility: Visibility = Visibility.PRIVATE) => {
const [state, setState] = useState<MemoEditorState>({ const [state, setState] = useState<MemoEditorState>({
memoVisibility: initialVisibility, memoVisibility: initialVisibility,
@ -28,29 +35,66 @@ export const useMemoEditorState = (initialVisibility: Visibility = Visibility.PR
isDraggingFile: false, isDraggingFile: false,
}); });
const update = <K extends keyof MemoEditorState>(key: K, value: MemoEditorState[K]) => { // All setters are memoized with useCallback to provide stable function references.
setState((prev) => ({ ...prev, [key]: value })); // 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 { return {
...state, ...state,
setMemoVisibility: (v: Visibility) => update("memoVisibility", v), setMemoVisibility,
setAttachmentList: (v: Attachment[]) => update("attachmentList", v), setAttachmentList,
setRelationList: (v: MemoRelation[]) => update("relationList", v), setRelationList,
setLocation: (v: Location | undefined) => update("location", v), setLocation,
toggleFocusMode: () => setState((prev) => ({ ...prev, isFocusMode: !prev.isFocusMode })), toggleFocusMode,
setUploadingAttachment: (v: boolean) => update("isUploadingAttachment", v), setUploadingAttachment,
setRequesting: (v: boolean) => update("isRequesting", v), setRequesting,
setComposing: (v: boolean) => update("isComposing", v), setComposing,
setDraggingFile: (v: boolean) => update("isDraggingFile", v), setDraggingFile,
resetState: () => resetState,
setState((prev) => ({
...prev,
isRequesting: false,
attachmentList: [],
relationList: [],
location: undefined,
isDraggingFile: false,
})),
}; };
}; };

View File

@ -20,11 +20,16 @@ export interface UseMemoFiltersOptions {
export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | undefined => { export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | undefined => {
const { creatorName, includeShortcuts = false, includePinned = false, visibilities } = options; 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 // Get selected shortcut if needed
const selectedShortcut = useMemo(() => { const selectedShortcut = useMemo(() => {
if (!includeShortcuts) return undefined; if (!includeShortcuts) return undefined;
return userStore.state.shortcuts.find((shortcut) => getShortcutId(shortcut.name) === memoFilterStore.shortcut); return shortcuts.find((shortcut) => getShortcutId(shortcut.name) === currentShortcut);
}, [includeShortcuts, memoFilterStore.shortcut, userStore.state.shortcuts]); }, [includeShortcuts, currentShortcut, shortcuts]);
// Build filter - wrapped in useMemo but also using observer for reactivity // Build filter - wrapped in useMemo but also using observer for reactivity
return useMemo(() => { return useMemo(() => {
@ -41,7 +46,7 @@ export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | un
} }
// Add active filters from memoFilterStore // Add active filters from memoFilterStore
for (const filter of memoFilterStore.filters) { for (const filter of filters) {
if (filter.factor === "contentSearch") { if (filter.factor === "contentSearch") {
conditions.push(`content.contains("${filter.value}")`); conditions.push(`content.contains("${filter.value}")`);
} else if (filter.factor === "tagSearch") { } else if (filter.factor === "tagSearch") {
@ -81,5 +86,5 @@ export const useMemoFilters = (options: UseMemoFiltersOptions = {}): string | un
} }
return conditions.length > 0 ? conditions.join(" && ") : undefined; return conditions.length > 0 ? conditions.join(" && ") : undefined;
}, [creatorName, includeShortcuts, includePinned, visibilities, selectedShortcut, memoFilterStore.filters]); }, [creatorName, includeShortcuts, includePinned, visibilities, selectedShortcut, filters]);
}; };

View File

@ -17,11 +17,14 @@ export interface UseMemoSortingResult {
export const useMemoSorting = (options: UseMemoSortingOptions = {}): UseMemoSortingResult => { export const useMemoSorting = (options: UseMemoSortingOptions = {}): UseMemoSortingResult => {
const { pinnedFirst = false, state = State.NORMAL } = options; 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 // Generate orderBy string for API
const orderBy = useMemo(() => { 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; return pinnedFirst ? `pinned desc, ${timeOrder}` : timeOrder;
}, [pinnedFirst, viewStore.state.orderByTimeAsc]); }, [pinnedFirst, orderByTimeAsc]);
// Generate listSort function for client-side sorting // Generate listSort function for client-side sorting
const listSort = useMemo(() => { const listSort = useMemo(() => {
@ -35,12 +38,12 @@ export const useMemoSorting = (options: UseMemoSortingOptions = {}): UseMemoSort
} }
// Then sort by display time // Then sort by display time
return viewStore.state.orderByTimeAsc return orderByTimeAsc
? dayjs(a.displayTime).unix() - dayjs(b.displayTime).unix() ? dayjs(a.displayTime).unix() - dayjs(b.displayTime).unix()
: dayjs(b.displayTime).unix() - dayjs(a.displayTime).unix(); : dayjs(b.displayTime).unix() - dayjs(a.displayTime).unix();
}); });
}; };
}, [pinnedFirst, state, viewStore.state.orderByTimeAsc]); }, [pinnedFirst, state, orderByTimeAsc]);
return { listSort, orderBy }; return { listSort, orderBy };
}; };