Skip to content

Commit

Permalink
⚡️ perf: improve topic loading ux and performance (lobehub#2609)
Browse files Browse the repository at this point in the history
* ⚡️ perf: improve topic loading performance

* ✅ test: fix tests

* ✅ test: add tests

* ✅ test: add tests
  • Loading branch information
arvinxx authored May 22, 2024
1 parent f0e7965 commit 4ffe8b7
Show file tree
Hide file tree
Showing 8 changed files with 200 additions and 44 deletions.
11 changes: 9 additions & 2 deletions src/app/(main)/chat/(workspace)/@topic/default.tsx
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -14,7 +19,9 @@ const Topic = () => {
<>
{!mobile && <SystemRole />}
<Layout>
<TopicListContent />
<Suspense fallback={<SkeletonList />}>
<TopicContent />
</Suspense>
</Layout>
</>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

import { Skeleton } from 'antd';
import { createStyles } from 'antd-style';
import { memo } from 'react';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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) =>
Expand All @@ -62,13 +59,9 @@ const TopicListContent = memo(() => {

const activeIndex = topics.findIndex((topic) => topic.id === activeTopicId);

// first time loading
if (!topicsInit) return <SkeletonList />;

// in server mode and re-loading
if (isServerMode && isLoading) return <SkeletonList />;
// first time loading or has no data
if (!topicsInit || !activeTopicList) return <SkeletonList />;

// in client mode no need the loading state for better UX
return (
<>
{topicLength === 0 && visible && (
Expand Down Expand Up @@ -111,8 +104,4 @@ const TopicListContent = memo(() => {

TopicListContent.displayName = 'TopicListContent';

export default memo(() => (
<Suspense fallback={<SkeletonList />}>
<TopicListContent />
</Suspense>
));
export default TopicListContent;
98 changes: 96 additions & 2 deletions src/store/chat/slices/topic/action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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();
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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
Expand All @@ -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);
});
});
});
12 changes: 11 additions & 1 deletion src/store/chat/slices/topic/action.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 }),
);
},
},
),
Expand Down
2 changes: 2 additions & 0 deletions src/store/chat/slices/topic/initialState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export interface ChatTopicState {
isSearchingTopic: boolean;
searchTopics: ChatTopic[];
topicLoadingIds: string[];
topicMaps: Record<string, ChatTopic[]>;
topicRenamingId?: string;
topicSearchKeywords: string;
topics: ChatTopic[];
Expand All @@ -20,6 +21,7 @@ export const initialTopicState: ChatTopicState = {
isSearchingTopic: false,
searchTopics: [],
topicLoadingIds: [],
topicMaps: {},
topicSearchKeywords: '',
topics: [],
topicsInit: false,
Expand Down
72 changes: 62 additions & 10 deletions src/store/chat/slices/topic/selectors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand All @@ -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]);
});
});
});
Loading

0 comments on commit 4ffe8b7

Please sign in to comment.