From 4ffe8b7fc733157c60a1fa75c00c81c9dee0b34a Mon Sep 17 00:00:00 2001 From: Arvin Xu Date: Wed, 22 May 2024 23:16:53 +0800 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20perf:=20improve=20topic=20?= =?UTF-8?q?loading=20ux=20and=20performance=20(#2609)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ⚡️ perf: improve topic loading performance * ✅ test: fix tests * ✅ test: add tests * ✅ test: add tests --- .../chat/(workspace)/@topic/default.tsx | 11 ++- .../{TopicListContent => }/SkeletonList.tsx | 2 + .../features/TopicListContent/index.tsx | 37 +++---- src/store/chat/slices/topic/action.test.ts | 98 ++++++++++++++++++- src/store/chat/slices/topic/action.ts | 12 ++- src/store/chat/slices/topic/initialState.ts | 2 + src/store/chat/slices/topic/selectors.test.ts | 72 ++++++++++++-- src/store/chat/slices/topic/selectors.ts | 10 +- 8 files changed, 200 insertions(+), 44 deletions(-) rename src/app/(main)/chat/(workspace)/@topic/features/{TopicListContent => }/SkeletonList.tsx (98%) diff --git a/src/app/(main)/chat/(workspace)/@topic/default.tsx b/src/app/(main)/chat/(workspace)/@topic/default.tsx index b776c5f97b4f..3ab9f79f9131 100644 --- a/src/app/(main)/chat/(workspace)/@topic/default.tsx +++ b/src/app/(main)/chat/(workspace)/@topic/default.tsx @@ -1,9 +1,14 @@ +// import TopicListContent from './features/TopicListContent'; +import React, { Suspense, lazy } from 'react'; + import { isMobileDevice } from '@/utils/responsive'; import Desktop from './_layout/Desktop'; import Mobile from './_layout/Mobile'; +import SkeletonList from './features/SkeletonList'; import SystemRole from './features/SystemRole'; -import TopicListContent from './features/TopicListContent'; + +const TopicContent = lazy(() => import('./features/TopicListContent')); const Topic = () => { const mobile = isMobileDevice(); @@ -14,7 +19,9 @@ const Topic = () => { <> {!mobile && } - + }> + + ); diff --git a/src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/SkeletonList.tsx b/src/app/(main)/chat/(workspace)/@topic/features/SkeletonList.tsx similarity index 98% rename from src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/SkeletonList.tsx rename to src/app/(main)/chat/(workspace)/@topic/features/SkeletonList.tsx index 41d2b776d1eb..fdcec548bcb5 100644 --- a/src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/SkeletonList.tsx +++ b/src/app/(main)/chat/(workspace)/@topic/features/SkeletonList.tsx @@ -1,3 +1,5 @@ +'use client'; + import { Skeleton } from 'antd'; import { createStyles } from 'antd-style'; import { memo } from 'react'; diff --git a/src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/index.tsx b/src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/index.tsx index 018ecc5d089e..b5baf275559a 100644 --- a/src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/index.tsx +++ b/src/app/(main)/chat/(workspace)/@topic/features/TopicListContent/index.tsx @@ -3,20 +3,19 @@ import { EmptyCard } from '@lobehub/ui'; import { useThemeMode } from 'antd-style'; import isEqual from 'fast-deep-equal'; -import React, { Suspense, memo, useCallback, useRef } from 'react'; +import React, { memo, useCallback, useMemo, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import { Flexbox } from 'react-layout-kit'; import { Virtuoso, VirtuosoHandle } from 'react-virtuoso'; import { imageUrl } from '@/const/url'; -import { isServerMode } from '@/const/version'; import { useChatStore } from '@/store/chat'; import { topicSelectors } from '@/store/chat/selectors'; import { useSessionStore } from '@/store/session'; import { useUserStore } from '@/store/user'; import { ChatTopic } from '@/types/topic'; -import { Placeholder, SkeletonList } from './SkeletonList'; +import { Placeholder, SkeletonList } from '../SkeletonList'; import TopicItem from './TopicItem'; const TopicListContent = memo(() => { @@ -34,21 +33,19 @@ const TopicListContent = memo(() => { s.updateGuideState, ]); - const topics = useChatStore( - (s) => [ - { - favorite: false, - id: 'default', - title: t('topic.defaultTitle'), - } as ChatTopic, - ...topicSelectors.displayTopics(s), + const activeTopicList = useChatStore(topicSelectors.displayTopics, isEqual); + + const topics = useMemo( + () => [ + { favorite: false, id: 'default', title: t('topic.defaultTitle') } as ChatTopic, + ...(activeTopicList || []), ], - isEqual, + [activeTopicList], ); const [sessionId] = useSessionStore((s) => [s.activeId]); - const { isLoading } = useFetchTopics(sessionId); + useFetchTopics(sessionId); const itemContent = useCallback( (index: number, { id, favorite, title }: ChatTopic) => @@ -62,13 +59,9 @@ const TopicListContent = memo(() => { const activeIndex = topics.findIndex((topic) => topic.id === activeTopicId); - // first time loading - if (!topicsInit) return ; - - // in server mode and re-loading - if (isServerMode && isLoading) return ; + // first time loading or has no data + if (!topicsInit || !activeTopicList) return ; - // in client mode no need the loading state for better UX return ( <> {topicLength === 0 && visible && ( @@ -111,8 +104,4 @@ const TopicListContent = memo(() => { TopicListContent.displayName = 'TopicListContent'; -export default memo(() => ( - }> - - -)); +export default TopicListContent; diff --git a/src/store/chat/slices/topic/action.test.ts b/src/store/chat/slices/topic/action.test.ts index 653bf117efb4..e53c174cc90b 100644 --- a/src/store/chat/slices/topic/action.test.ts +++ b/src/store/chat/slices/topic/action.test.ts @@ -12,12 +12,14 @@ import { ChatTopic } from '@/types/topic'; import { useChatStore } from '../../store'; +vi.mock('zustand/traditional'); // Mock topicService 和 messageService vi.mock('@/services/topic', () => ({ topicService: { removeTopics: vi.fn(), removeAllTopic: vi.fn(), removeTopic: vi.fn(), + cloneTopic: vi.fn(), createTopic: vi.fn(), updateTopicFavorite: vi.fn(), updateTopicTitle: vi.fn(), @@ -31,9 +33,23 @@ vi.mock('@/services/topic', () => ({ vi.mock('@/services/message', () => ({ messageService: { removeMessages: vi.fn(), + getMessages: vi.fn(), }, })); +vi.mock('@/components/AntdStaticMethods', () => ({ + message: { + loading: vi.fn(), + success: vi.fn(), + error: vi.fn(), + destroy: vi.fn(), + }, +})); + +vi.mock('i18next', () => ({ + t: vi.fn((key, params) => (params.title ? key + '_' + params.title : key)), +})); + beforeEach(() => { // Setup initial state and mocks before each test vi.clearAllMocks(); @@ -222,7 +238,7 @@ describe('topic action', () => { expect(result.current.data).toEqual(topics); }); expect(useChatStore.getState().topicsInit).toBeTruthy(); - expect(useChatStore.getState().topics).toEqual(topics); + expect(useChatStore.getState().topicMaps).toEqual({ [sessionId]: topics }); }); }); describe('useSearchTopics', () => { @@ -411,7 +427,7 @@ describe('topic action', () => { const topics = [{ id: 'topic-1', title: 'Test Topic' }] as ChatTopic[]; const { result } = renderHook(() => useChatStore()); await act(async () => { - useChatStore.setState({ topics }); + useChatStore.setState({ topicMaps: { test: topics }, activeId: 'test' }); }); // Mock the `updateTopicTitleInSummary` and `refreshTopic` for spying @@ -437,4 +453,82 @@ describe('topic action', () => { // TODO: need to test with fetchPresetTaskResult }); }); + describe('createTopic', () => { + it('should create a new topic and update the store', async () => { + const { result } = renderHook(() => useChatStore()); + const activeId = 'test-session-id'; + const newTopicId = 'new-topic-id'; + const messages = [{ id: 'message-1' }, { id: 'message-2' }] as ChatMessage[]; + + await act(async () => { + useChatStore.setState({ + activeId, + messagesMap: { + [messageMapKey(activeId)]: messages, + }, + }); + }); + + const createTopicSpy = vi.spyOn(topicService, 'createTopic').mockResolvedValue(newTopicId); + const refreshTopicSpy = vi.spyOn(result.current, 'refreshTopic'); + + await act(async () => { + const topicId = await result.current.createTopic(); + expect(topicId).toBe(newTopicId); + }); + + expect(createTopicSpy).toHaveBeenCalledWith({ + sessionId: activeId, + messages: messages.map((m) => m.id), + title: 'topic.defaultTitle', + }); + expect(refreshTopicSpy).toHaveBeenCalled(); + }); + }); + describe('duplicateTopic', () => { + it('should duplicate a topic and switch to the new topic', async () => { + const { result } = renderHook(() => useChatStore()); + const topicId = 'topic-1'; + const newTopicId = 'new-topic-id'; + const topics = [{ id: topicId, title: 'Original Topic' }] as ChatTopic[]; + + await act(async () => { + useChatStore.setState({ activeId: 'abc', topicMaps: { abc: topics } }); + }); + + const cloneTopicSpy = vi.spyOn(topicService, 'cloneTopic').mockResolvedValue(newTopicId); + const refreshTopicSpy = vi.spyOn(result.current, 'refreshTopic'); + const switchTopicSpy = vi.spyOn(result.current, 'switchTopic'); + + await act(async () => { + await result.current.duplicateTopic(topicId); + }); + + expect(cloneTopicSpy).toHaveBeenCalledWith(topicId, 'duplicateTitle_Original Topic'); + expect(refreshTopicSpy).toHaveBeenCalled(); + expect(switchTopicSpy).toHaveBeenCalledWith(newTopicId); + }); + }); + describe('autoRenameTopicTitle', () => { + it('should auto-rename the topic title based on the messages', async () => { + const { result } = renderHook(() => useChatStore()); + const topicId = 'topic-1'; + const activeId = 'test-session-id'; + const messages = [{ id: 'message-1', content: 'Hello' }] as ChatMessage[]; + + await act(async () => { + useChatStore.setState({ activeId }); + }); + + const getMessagesSpy = vi.spyOn(messageService, 'getMessages').mockResolvedValue(messages); + const summaryTopicTitleSpy = vi.spyOn(result.current, 'summaryTopicTitle'); + + await act(async () => { + await result.current.autoRenameTopicTitle(topicId); + }); + + expect(getMessagesSpy).toHaveBeenCalledWith(activeId, topicId); + expect(summaryTopicTitleSpy).toHaveBeenCalledWith(topicId, messages); + }); + }); }); diff --git a/src/store/chat/slices/topic/action.ts b/src/store/chat/slices/topic/action.ts index 50f0820a47a7..8c2ce903764d 100644 --- a/src/store/chat/slices/topic/action.ts +++ b/src/store/chat/slices/topic/action.ts @@ -1,6 +1,7 @@ /* eslint-disable sort-keys-fix/sort-keys-fix, typescript-sort-keys/interface */ // Note: To make the code more logic and readable, we just disable the auto sort key eslint rule // DON'T REMOVE THE FIRST LINE +import isEqual from 'fast-deep-equal'; import { t } from 'i18next'; import { produce } from 'immer'; import useSWR, { SWRResponse, mutate } from 'swr'; @@ -191,7 +192,16 @@ export const chatTopic: StateCreator< suspense: true, fallbackData: [], onSuccess: (topics) => { - set({ topics, topicsInit: true }, false, n('useFetchTopics(success)', { sessionId })); + const nextMap = { ...get().topicMaps, [sessionId]: topics }; + + // no need to update map if the topics have been init and the map is the same + if (get().topicsInit && isEqual(nextMap, get().topicMaps)) return; + + set( + { topicMaps: nextMap, topicsInit: true }, + false, + n('useFetchTopics(success)', { sessionId }), + ); }, }, ), diff --git a/src/store/chat/slices/topic/initialState.ts b/src/store/chat/slices/topic/initialState.ts index a14497454e4b..258b2b0f05f4 100644 --- a/src/store/chat/slices/topic/initialState.ts +++ b/src/store/chat/slices/topic/initialState.ts @@ -6,6 +6,7 @@ export interface ChatTopicState { isSearchingTopic: boolean; searchTopics: ChatTopic[]; topicLoadingIds: string[]; + topicMaps: Record; topicRenamingId?: string; topicSearchKeywords: string; topics: ChatTopic[]; @@ -20,6 +21,7 @@ export const initialTopicState: ChatTopicState = { isSearchingTopic: false, searchTopics: [], topicLoadingIds: [], + topicMaps: {}, topicSearchKeywords: '', topics: [], topicsInit: false, diff --git a/src/store/chat/slices/topic/selectors.test.ts b/src/store/chat/slices/topic/selectors.test.ts index 60a720fc76f4..9370485aabf4 100644 --- a/src/store/chat/slices/topic/selectors.test.ts +++ b/src/store/chat/slices/topic/selectors.test.ts @@ -8,23 +8,25 @@ import { topicSelectors } from '../../selectors'; const initialStore = initialState as ChatStore; -const mockTopics = [ - { id: 'topic1', name: 'Topic 1' }, - { id: 'topic2', name: 'Topic 2' }, -]; +const topicMaps = { + test: [ + { id: 'topic1', name: 'Topic 1', favorite: true }, + { id: 'topic2', name: 'Topic 2' }, + ], +}; describe('topicSelectors', () => { describe('currentTopics', () => { - it('should return an empty array if there are no topics', () => { + it('should return undefined if there are no topics with activeId', () => { const topics = topicSelectors.currentTopics(initialStore); - expect(topics).toEqual([]); + expect(topics).toBeUndefined(); }); it('should return all current topics from the store', () => { - const state = merge(initialStore, { topics: mockTopics }); + const state = merge(initialStore, { topicMaps, activeId: 'test' }); const topics = topicSelectors.currentTopics(state); - expect(topics).toEqual(mockTopics); + expect(topics).toEqual(topicMaps.test); }); }); @@ -35,9 +37,59 @@ describe('topicSelectors', () => { }); it('should return the number of current topics', () => { - const state = merge(initialStore, { topics: mockTopics }); + const state = merge(initialStore, { topicMaps, activeId: 'test' }); const length = topicSelectors.currentTopicLength(state); - expect(length).toBe(mockTopics.length); + expect(length).toBe(topicMaps.test.length); + }); + }); + + describe('currentActiveTopic', () => { + it('should return undefined if there is no active topic', () => { + const topic = topicSelectors.currentActiveTopic(initialStore); + expect(topic).toBeUndefined(); + }); + + it('should return the current active topic', () => { + const state = merge(initialStore, { topicMaps, activeId: 'test', activeTopicId: 'topic1' }); + const topic = topicSelectors.currentActiveTopic(state); + expect(topic).toEqual(topicMaps.test[0]); + }); + }); + + describe('currentUnFavTopics', () => { + it('should return all unfavorited topics', () => { + const state = merge(initialStore, { topics: topicMaps.test }); + const topics = topicSelectors.currentUnFavTopics(state); + expect(topics).toEqual([topicMaps.test[1]]); + }); + }); + + describe('displayTopics', () => { + it('should return current topics if not searching', () => { + const state = merge(initialStore, { topicMaps, activeId: 'test' }); + const topics = topicSelectors.displayTopics(state); + expect(topics).toEqual(topicMaps.test); + }); + + it('should return search topics if searching', () => { + const searchTopics = [{ id: 'search1', name: 'Search 1' }]; + const state = merge(initialStore, { isSearchingTopic: true, searchTopics }); + const topics = topicSelectors.displayTopics(state); + expect(topics).toEqual(searchTopics); + }); + }); + + describe('getTopicById', () => { + it('should return undefined if topic is not found', () => { + const state = merge(initialStore, { topicMaps, activeId: 'test' }); + const topic = topicSelectors.getTopicById('notfound')(state); + expect(topic).toBeUndefined(); + }); + + it('should return the topic with the given id', () => { + const state = merge(initialStore, { topicMaps, activeId: 'test' }); + const topic = topicSelectors.getTopicById('topic1')(state); + expect(topic).toEqual(topicMaps.test[0]); }); }); }); diff --git a/src/store/chat/slices/topic/selectors.ts b/src/store/chat/slices/topic/selectors.ts index 4d8aa67b6443..b1d4d0ab1404 100644 --- a/src/store/chat/slices/topic/selectors.ts +++ b/src/store/chat/slices/topic/selectors.ts @@ -2,24 +2,24 @@ import { ChatTopic } from '@/types/topic'; import { ChatStore } from '../../store'; -const currentTopics = (s: ChatStore): ChatTopic[] => s.topics; +const currentTopics = (s: ChatStore): ChatTopic[] | undefined => s.topicMaps[s.activeId]; const currentActiveTopic = (s: ChatStore): ChatTopic | undefined => { - return s.topics.find((topic) => topic.id === s.activeTopicId); + return currentTopics(s)?.find((topic) => topic.id === s.activeTopicId); }; const searchTopics = (s: ChatStore): ChatTopic[] => s.searchTopics; -const displayTopics = (s: ChatStore): ChatTopic[] => +const displayTopics = (s: ChatStore): ChatTopic[] | undefined => s.isSearchingTopic ? searchTopics(s) : currentTopics(s); const currentUnFavTopics = (s: ChatStore): ChatTopic[] => s.topics.filter((s) => !s.favorite); -const currentTopicLength = (s: ChatStore): number => currentTopics(s).length; +const currentTopicLength = (s: ChatStore): number => currentTopics(s)?.length || 0; const getTopicById = (id: string) => (s: ChatStore): ChatTopic | undefined => - currentTopics(s).find((topic) => topic.id === id); + currentTopics(s)?.find((topic) => topic.id === id); export const topicSelectors = { currentActiveTopic,