Skip to content

Commit 2c0387f

Browse files
AhyoungRyuAhyoungRyu
authored
[CLNP-6022] Memoize useGroupChannel actions to prevent unnecessary re-rendering (#1288)
Addresses https://sendbird.atlassian.net/browse/CLNP-6022?focusedCommentId=301263 ### Key changes Memoized action handlers(+ scroll related functions as well) in useGroupChannel to reduced unnecessary re-rendering.
1 parent 8ca7861 commit 2c0387f

File tree

6 files changed

+158
-125
lines changed

6 files changed

+158
-125
lines changed

‎src/modules/GroupChannel/components/MessageList/InfiniteList.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { DependencyList, forwardRef, UIEventHandler, useLayoutEffect, useRef } from 'react';
1+
import React, { DependencyList, forwardRef, UIEventHandler, useCallback, useLayoutEffect, useRef } from 'react';
22
import type { BaseMessage } from '@sendbird/chat/message';
33
import { isAboutSame } from '../../../Channel/context/utils';
44
import { SCROLL_BUFFER } from '../../../../utils/consts';
@@ -61,7 +61,7 @@ export const InfiniteList = forwardRef((props: Props, listRef: React.RefObject<H
6161
}
6262
}, [listRef.current, messages.length]);
6363

64-
const handleScroll: UIEventHandler<HTMLDivElement> = async () => {
64+
const handleScroll: UIEventHandler<HTMLDivElement> = useCallback(async () => {
6565
if (!listRef.current) return;
6666
const list = listRef.current;
6767

@@ -87,7 +87,7 @@ export const InfiniteList = forwardRef((props: Props, listRef: React.RefObject<H
8787
} else {
8888
direction.current = undefined;
8989
}
90-
};
90+
}, [messages.length]);
9191

9292
return (
9393
<div className="sendbird-conversation__scroll-container">

‎src/modules/GroupChannel/context/GroupChannelProvider.tsx‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,11 @@ const GroupChannelManager :React.FC<React.PropsWithChildren<GroupChannelProvider
112112
scrollRef,
113113
scrollPubSub,
114114
scrollDistanceFromBottomRef,
115-
isScrollBottomReached,
116115
scrollPositionRef,
117116
} = useMessageListScroll(scrollBehavior, [state.currentChannel?.url]);
118117

118+
const { isScrollBottomReached } = state;
119+
119120
// Configuration resolution
120121
const resolvedReplyType = getCaseResolvedReplyType(moduleReplyType ?? config.groupChannel.replyType).upperCase;
121122
const resolvedThreadReplySelectType = getCaseResolvedThreadReplySelectType(
@@ -143,8 +144,13 @@ const GroupChannelManager :React.FC<React.PropsWithChildren<GroupChannelProvider
143144
channels.forEach((it) => markAsReadScheduler.push(it));
144145
}
145146
},
146-
onMessagesReceived: () => {
147-
if (isScrollBottomReached && isContextMenuClosed()) {
147+
onMessagesReceived: (messages) => {
148+
if (isScrollBottomReached
149+
&& isContextMenuClosed()
150+
// Note: this shouldn't happen ideally, but it happens on re-rendering GroupChannelManager
151+
// even though the next messages and the current messages length are the same.
152+
// So added this condition to check if they are the same to prevent unnecessary calling scrollToBottom action
153+
&& messages.length !== state.messages.length) {
148154
setTimeout(() => actions.scrollToBottom(true), 10);
149155
}
150156
},
@@ -157,7 +163,12 @@ const GroupChannelManager :React.FC<React.PropsWithChildren<GroupChannelProvider
157163
onBackClick?.();
158164
},
159165
onChannelUpdated: (channel) => {
160-
actions.setCurrentChannel(channel);
166+
// Note: this shouldn't happen ideally, but it happens on re-rendering GroupChannelManager
167+
// even though the next channel and the current channel are the same.
168+
// So added this condition to check if they are the same to prevent unnecessary calling setCurrentChannel action
169+
if (!state.currentChannel?.isEqual(channel)) {
170+
actions.setCurrentChannel(channel);
171+
}
161172
},
162173
logger: logger as any,
163174
});
@@ -196,7 +207,7 @@ const GroupChannelManager :React.FC<React.PropsWithChildren<GroupChannelProvider
196207
return () => {
197208
subscriptions.forEach(subscription => subscription.remove());
198209
};
199-
}, [messageDataSource.initialized, state.currentChannel?.url, pubSub?.subscribe]);
210+
}, [messageDataSource.initialized, state.currentChannel?.url]);
200211

201212
// Starting point handling
202213
useEffect(() => {

‎src/modules/GroupChannel/context/__tests__/useMessageListScroll.spec.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ describe('useMessageListScroll', () => {
2525
const { result } = renderHook(() => useMessageListScroll('auto'));
2626

2727
expect(result.current.scrollRef.current).toBe(null);
28-
expect(result.current.isScrollBottomReached).toBe(true);
2928
expect(result.current.scrollDistanceFromBottomRef.current).toBe(0);
3029
expect(result.current.scrollPositionRef.current).toBe(0);
3130
expect(typeof result.current.scrollPubSub.publish).toBe('function');
@@ -63,7 +62,7 @@ describe('useMessageListScroll', () => {
6362
result.current.scrollPubSub.publish('scrollToBottom', {});
6463
await waitFor(() => {
6564
expect(result.current.scrollDistanceFromBottomRef.current).toBe(0);
66-
expect(result.current.isScrollBottomReached).toBe(true);
65+
expect(mockSetIsScrollBottomReached).toHaveBeenCalledWith(true);
6766
});
6867
});
6968
});

‎src/modules/GroupChannel/context/hooks/useGroupChannel.ts‎

Lines changed: 126 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useContext, useMemo } from 'react';
1+
import { useContext, useMemo, useCallback } from 'react';
22
import type { GroupChannel } from '@sendbird/chat/groupChannel';
33
import type { SendbirdError } from '@sendbird/chat';
44
import { useSyncExternalStore } from 'use-sync-external-store/shim';
@@ -58,24 +58,21 @@ export const useGroupChannel = () => {
5858
const { markAsReadScheduler } = config;
5959
const state: GroupChannelState = useSyncExternalStore(store.subscribe, store.getState);
6060

61-
const flagActions = {
62-
setAnimatedMessageId: (messageId: number | null) => {
63-
store.setState(state => ({ ...state, animatedMessageId: messageId }));
64-
},
61+
const setAnimatedMessageId = useCallback((messageId: number | null) => {
62+
store.setState(state => ({ ...state, animatedMessageId: messageId }));
63+
}, []);
6564

66-
setIsScrollBottomReached: (isReached: boolean) => {
67-
store.setState(state => ({ ...state, isScrollBottomReached: isReached }));
68-
},
69-
};
65+
const setIsScrollBottomReached = useCallback((isReached: boolean) => {
66+
store.setState(state => ({ ...state, isScrollBottomReached: isReached }));
67+
}, []);
7068

71-
const scrollToBottom = async (animated?: boolean) => {
69+
const scrollToBottom = useCallback(async (animated?: boolean) => {
7270
if (!state.scrollRef.current) return;
71+
setAnimatedMessageId(null);
72+
setIsScrollBottomReached(true);
73+
7374
// wait a bit for scroll ref to be updated
7475
await delay();
75-
76-
flagActions.setAnimatedMessageId(null);
77-
flagActions.setIsScrollBottomReached(true);
78-
7976
if (config.isOnline && state.hasNext()) {
8077
await state.resetWithStartingPoint(Number.MAX_SAFE_INTEGER);
8178
}
@@ -87,109 +84,127 @@ export const useGroupChannel = () => {
8784
markAsReadScheduler.push(state.currentChannel);
8885
}
8986
}
90-
};
87+
}, [state.scrollRef.current, config.isOnline, markAsReadScheduler]);
88+
89+
const scrollToMessage = useCallback(async (
90+
createdAt: number,
91+
messageId: number,
92+
messageFocusAnimated?: boolean,
93+
scrollAnimated?: boolean,
94+
) => {
95+
const element = state.scrollRef.current;
96+
const parentNode = element?.parentNode as HTMLDivElement;
97+
const clickHandler = {
98+
activate() {
99+
if (!element || !parentNode) return;
100+
element.style.pointerEvents = 'auto';
101+
parentNode.style.cursor = 'auto';
102+
},
103+
deactivate() {
104+
if (!element || !parentNode) return;
105+
element.style.pointerEvents = 'none';
106+
parentNode.style.cursor = 'wait';
107+
},
108+
};
109+
110+
clickHandler.deactivate();
111+
112+
setAnimatedMessageId(null);
113+
const message = state.messages.find(
114+
(it) => it.messageId === messageId || it.createdAt === createdAt,
115+
);
116+
117+
if (message) {
118+
const topOffset = getMessageTopOffset(message.createdAt);
119+
if (topOffset) state.scrollPubSub.publish('scroll', { top: topOffset, animated: scrollAnimated });
120+
if (messageFocusAnimated ?? true) setAnimatedMessageId(messageId);
121+
} else {
122+
await state.resetWithStartingPoint(createdAt);
123+
setTimeout(() => {
124+
const topOffset = getMessageTopOffset(createdAt);
125+
if (topOffset) {
126+
state.scrollPubSub.publish('scroll', {
127+
top: topOffset,
128+
lazy: false,
129+
animated: scrollAnimated,
130+
});
131+
}
132+
if (messageFocusAnimated ?? true) setAnimatedMessageId(messageId);
133+
});
134+
}
135+
clickHandler.activate();
136+
}, [setAnimatedMessageId, state.scrollRef.current, state.messages?.map(it => it?.messageId)]);
137+
138+
const toggleReaction = useCallback((message: SendableMessageType, emojiKey: string, isReacted: boolean) => {
139+
if (!state.currentChannel) return;
140+
if (isReacted) {
141+
state.currentChannel.deleteReaction(message, emojiKey)
142+
.catch(error => {
143+
config.logger?.warning('Failed to delete reaction:', error);
144+
});
145+
} else {
146+
state.currentChannel.addReaction(message, emojiKey)
147+
.catch(error => {
148+
config.logger?.warning('Failed to add reaction:', error);
149+
});
150+
}
151+
}, [state.currentChannel?.deleteReaction, state.currentChannel?.addReaction]);
152+
91153
const messageActions = useMessageActions({
92154
...state,
93155
scrollToBottom,
94156
});
95157

96-
const actions: GroupChannelActions = useMemo(() => ({
97-
setCurrentChannel: (channel: GroupChannel) => {
98-
store.setState(state => ({
99-
...state,
100-
currentChannel: channel,
101-
fetchChannelError: null,
102-
quoteMessage: null,
103-
animatedMessageId: null,
104-
nicknamesMap: new Map(
105-
channel.members.map(({ userId, nickname }) => [userId, nickname]),
106-
),
107-
}));
108-
},
109-
110-
handleChannelError: (error: SendbirdError) => {
111-
store.setState(state => ({
112-
...state,
113-
currentChannel: null,
114-
fetchChannelError: error,
115-
quoteMessage: null,
116-
animatedMessageId: null,
117-
}));
118-
},
119-
120-
setQuoteMessage: (message: SendableMessageType | null) => {
121-
store.setState(state => ({ ...state, quoteMessage: message }));
122-
},
123-
158+
const setCurrentChannel = useCallback((channel: GroupChannel) => {
159+
store.setState(state => ({
160+
...state,
161+
currentChannel: channel,
162+
fetchChannelError: null,
163+
quoteMessage: null,
164+
animatedMessageId: null,
165+
nicknamesMap: new Map(
166+
channel.members.map(({ userId, nickname }) => [userId, nickname]),
167+
),
168+
}));
169+
}, []);
170+
171+
const handleChannelError = useCallback((error: SendbirdError) => {
172+
store.setState(state => ({
173+
...state,
174+
currentChannel: null,
175+
fetchChannelError: error,
176+
quoteMessage: null,
177+
animatedMessageId: null,
178+
}));
179+
}, []);
180+
181+
const setQuoteMessage = useCallback((message: SendableMessageType | null) => {
182+
store.setState(state => ({ ...state, quoteMessage: message }));
183+
}, []);
184+
185+
const actions: GroupChannelActions = useMemo(() => {
186+
return {
187+
setCurrentChannel,
188+
handleChannelError,
189+
setQuoteMessage,
190+
scrollToBottom,
191+
scrollToMessage,
192+
toggleReaction,
193+
setAnimatedMessageId,
194+
setIsScrollBottomReached,
195+
...messageActions,
196+
};
197+
}, [
198+
setCurrentChannel,
199+
handleChannelError,
200+
setQuoteMessage,
124201
scrollToBottom,
125-
scrollToMessage: async (
126-
createdAt: number,
127-
messageId: number,
128-
messageFocusAnimated?: boolean,
129-
scrollAnimated?: boolean,
130-
) => {
131-
const element = state.scrollRef.current;
132-
const parentNode = element?.parentNode as HTMLDivElement;
133-
const clickHandler = {
134-
activate() {
135-
if (!element || !parentNode) return;
136-
element.style.pointerEvents = 'auto';
137-
parentNode.style.cursor = 'auto';
138-
},
139-
deactivate() {
140-
if (!element || !parentNode) return;
141-
element.style.pointerEvents = 'none';
142-
parentNode.style.cursor = 'wait';
143-
},
144-
};
145-
146-
clickHandler.deactivate();
147-
148-
flagActions.setAnimatedMessageId(null);
149-
const message = state.messages.find(
150-
(it) => it.messageId === messageId || it.createdAt === createdAt,
151-
);
152-
153-
if (message) {
154-
const topOffset = getMessageTopOffset(message.createdAt);
155-
if (topOffset) state.scrollPubSub.publish('scroll', { top: topOffset, animated: scrollAnimated });
156-
if (messageFocusAnimated ?? true) flagActions.setAnimatedMessageId(messageId);
157-
} else {
158-
await state.resetWithStartingPoint(createdAt);
159-
setTimeout(() => {
160-
const topOffset = getMessageTopOffset(createdAt);
161-
if (topOffset) {
162-
state.scrollPubSub.publish('scroll', {
163-
top: topOffset,
164-
lazy: false,
165-
animated: scrollAnimated,
166-
});
167-
}
168-
if (messageFocusAnimated ?? true) flagActions.setAnimatedMessageId(messageId);
169-
});
170-
}
171-
172-
clickHandler.activate();
173-
},
174-
175-
toggleReaction: (message: SendableMessageType, emojiKey: string, isReacted: boolean) => {
176-
if (!state.currentChannel) return;
177-
178-
if (isReacted) {
179-
state.currentChannel.deleteReaction(message, emojiKey)
180-
.catch(error => {
181-
config.logger?.warning('Failed to delete reaction:', error);
182-
});
183-
} else {
184-
state.currentChannel.addReaction(message, emojiKey)
185-
.catch(error => {
186-
config.logger?.warning('Failed to add reaction:', error);
187-
});
188-
}
189-
},
190-
...flagActions,
191-
...messageActions,
192-
}), [store, state, config.isOnline, markAsReadScheduler]);
202+
scrollToMessage,
203+
toggleReaction,
204+
setAnimatedMessageId,
205+
setIsScrollBottomReached,
206+
messageActions,
207+
]);
193208

194209
return { state, actions };
195210
};

‎src/modules/GroupChannel/context/hooks/useMessageListScroll.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ export function useMessageListScroll(behavior: 'smooth' | 'auto', deps: Dependen
3232

3333
const [scrollPubSub] = useState(() => pubSubFactory<ScrollTopics, ScrollTopicUnion>({ publishSynchronous: true }));
3434
const {
35-
state: { isScrollBottomReached },
3635
actions: { setIsScrollBottomReached },
3736
} = useGroupChannel();
3837

@@ -99,8 +98,6 @@ export function useMessageListScroll(behavior: 'smooth' | 'auto', deps: Dependen
9998
return {
10099
scrollRef,
101100
scrollPubSub,
102-
isScrollBottomReached,
103-
setIsScrollBottomReached,
104101
scrollDistanceFromBottomRef,
105102
scrollPositionRef,
106103
};

‎src/utils/storeManager.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import isEqual from 'lodash/isEqual';
2+
13
// Referrence: https://github.com/pmndrs/zustand
24
export type Store<T> = {
35
getState: () => T;
@@ -7,7 +9,16 @@ export type Store<T> = {
79

810
export function hasStateChanged<T>(prevState: T, updates: Partial<T>): boolean {
911
return Object.entries(updates).some(([key, value]) => {
10-
return prevState[key as keyof T] !== value;
12+
if (typeof prevState[key as keyof T] === 'function' && typeof value === 'function') {
13+
/**
14+
* Function is not considered as state change. Why?
15+
* Because function is not a value, it's a reference.
16+
* If we consider non-memoized function as state change,
17+
* it will always be true and cause unnecessary re-renders.
18+
*/
19+
return false;
20+
}
21+
return !isEqual(prevState[key as keyof T], value);
1122
});
1223
}
1324

0 commit comments

Comments
 (0)