From 98288b1fc7185497f0168819fff35e0e60e9781c Mon Sep 17 00:00:00 2001 From: Tyler Jeong Date: Wed, 29 Jul 2026 10:52:16 +0900 Subject: [PATCH 1/2] fix: prevent TypingIndicator from leaking group channel handlers on unmount The effect cleanup removed the stale `handlerId` state instead of the `newHandlerId` that was actually registered, so the live GroupChannelHandler leaked on unmount and accumulated across mount/unmount cycles. Hoist `newHandlerId` into the effect scope so the cleanup removes the exact id it registered, falling back to `handlerId` when no handler was added. Add a regression test covering unmount and channel-switch cleanup. CLNP-8774 Reported via #1444 --- .../components/TypingIndicator.tsx | 5 +- .../__test__/TypingIndicator.spec.tsx | 65 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 src/modules/GroupChannel/components/__test__/TypingIndicator.spec.tsx diff --git a/src/modules/GroupChannel/components/TypingIndicator.tsx b/src/modules/GroupChannel/components/TypingIndicator.tsx index 0200e40236..16898b0b4e 100644 --- a/src/modules/GroupChannel/components/TypingIndicator.tsx +++ b/src/modules/GroupChannel/components/TypingIndicator.tsx @@ -44,9 +44,10 @@ export const TypingIndicator = ({ channelUrl }: TypingIndicatorProps) => { const [typingMembers, setTypingMembers] = useState([]); useEffect(() => { + let newHandlerId: string | undefined; if (sb?.groupChannel?.addGroupChannelHandler) { sb.groupChannel.removeGroupChannelHandler(handlerId); - const newHandlerId = uuidv4(); + newHandlerId = uuidv4(); const handler = new GroupChannelHandler({ onTypingStatusUpdated: (groupChannel) => { // there is a possible warning in here - setState called after unmount @@ -64,7 +65,7 @@ export const TypingIndicator = ({ channelUrl }: TypingIndicatorProps) => { return () => { setTypingMembers([]); if (sb?.groupChannel?.removeGroupChannelHandler) { - sb.groupChannel.removeGroupChannelHandler(handlerId); + sb.groupChannel.removeGroupChannelHandler(newHandlerId ?? handlerId); } }; }, [channelUrl]); diff --git a/src/modules/GroupChannel/components/__test__/TypingIndicator.spec.tsx b/src/modules/GroupChannel/components/__test__/TypingIndicator.spec.tsx new file mode 100644 index 0000000000..41e41d4580 --- /dev/null +++ b/src/modules/GroupChannel/components/__test__/TypingIndicator.spec.tsx @@ -0,0 +1,65 @@ +import React from 'react'; +import { render } from '@testing-library/react'; + +import { TypingIndicator } from '../TypingIndicator'; + +const { mockAddGroupChannelHandler, mockRemoveGroupChannelHandler, mockState } = vi.hoisted(() => { + const mockAddGroupChannelHandler = vi.fn(); + const mockRemoveGroupChannelHandler = vi.fn(); + const mockState = { + stores: { + sdkStore: { + sdk: { + groupChannel: { + addGroupChannelHandler: mockAddGroupChannelHandler, + removeGroupChannelHandler: mockRemoveGroupChannelHandler, + }, + }, + initialized: true, + }, + }, + config: { + logger: { info: vi.fn(), warning: vi.fn(), error: vi.fn() }, + }, + }; + return { mockAddGroupChannelHandler, mockRemoveGroupChannelHandler, mockState }; +}); + +vi.mock('../../../../lib/Sendbird/context/hooks/useSendbird', () => ({ + __esModule: true, + default: vi.fn(() => ({ state: mockState })), +})); + +describe('GroupChannel/TypingIndicator group channel handler lifecycle (CLNP-8774)', () => { + beforeEach(() => { + mockAddGroupChannelHandler.mockClear(); + mockRemoveGroupChannelHandler.mockClear(); + }); + + it('removes the exact handler id it registered when unmounted, so the handler does not leak', () => { + const { unmount } = render(); + + expect(mockAddGroupChannelHandler).toHaveBeenCalledTimes(1); + const registeredId = mockAddGroupChannelHandler.mock.calls[0][0]; + expect(registeredId).toBeTruthy(); + + unmount(); + + expect(mockRemoveGroupChannelHandler).toHaveBeenCalledWith(registeredId); + }); + + it('removes every registered handler across a channel switch followed by unmount (no leak)', () => { + const { rerender, unmount } = render(); + rerender(); + + expect(mockAddGroupChannelHandler).toHaveBeenCalledTimes(2); + const firstId = mockAddGroupChannelHandler.mock.calls[0][0]; + const secondId = mockAddGroupChannelHandler.mock.calls[1][0]; + expect(firstId).not.toBe(secondId); + + unmount(); + + expect(mockRemoveGroupChannelHandler).toHaveBeenCalledWith(firstId); + expect(mockRemoveGroupChannelHandler).toHaveBeenCalledWith(secondId); + }); +}); From 87246a81957fce59d07569e9daaae0301e5c2a06 Mon Sep 17 00:00:00 2001 From: Tyler Jeong Date: Thu, 30 Jul 2026 17:06:36 +0900 Subject: [PATCH 2/2] refactor: drop vestigial handlerId state from TypingIndicator After the handler-leak fix, the handlerId React state no longer serves a purpose: the cleanup removes the exact id it registered via a local variable, so the state (and the redundant remove at the start of the effect body) was dead code. Keeping the id in state was in fact what made the original add/remove ids diverge and leak. Use a single local handlerId per effect run, registered and removed within the same closure -- matching the pattern already used in Thread/useHandleChannelEvents and the other channel handlers. Behavior is unchanged and the CLNP-8774 regression test still passes. Addresses review feedback on #1448. CLNP-8774 --- .../GroupChannel/components/TypingIndicator.tsx | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/modules/GroupChannel/components/TypingIndicator.tsx b/src/modules/GroupChannel/components/TypingIndicator.tsx index 16898b0b4e..47c3c4b84f 100644 --- a/src/modules/GroupChannel/components/TypingIndicator.tsx +++ b/src/modules/GroupChannel/components/TypingIndicator.tsx @@ -40,14 +40,12 @@ export const TypingIndicator = ({ channelUrl }: TypingIndicatorProps) => { const { state } = useSendbird(); const sb = state?.stores?.sdkStore?.sdk; const logger = state?.config?.logger; - const [handlerId, setHandlerId] = useState(uuidv4()); const [typingMembers, setTypingMembers] = useState([]); useEffect(() => { - let newHandlerId: string | undefined; + let handlerId: string | undefined; if (sb?.groupChannel?.addGroupChannelHandler) { - sb.groupChannel.removeGroupChannelHandler(handlerId); - newHandlerId = uuidv4(); + handlerId = uuidv4(); const handler = new GroupChannelHandler({ onTypingStatusUpdated: (groupChannel) => { // there is a possible warning in here - setState called after unmount @@ -58,14 +56,13 @@ export const TypingIndicator = ({ channelUrl }: TypingIndicatorProps) => { } }, }); - sb.groupChannel.addGroupChannelHandler(newHandlerId, handler); - setHandlerId(newHandlerId); + sb.groupChannel.addGroupChannelHandler(handlerId, handler); } return () => { setTypingMembers([]); - if (sb?.groupChannel?.removeGroupChannelHandler) { - sb.groupChannel.removeGroupChannelHandler(newHandlerId ?? handlerId); + if (handlerId && sb?.groupChannel?.removeGroupChannelHandler) { + sb.groupChannel.removeGroupChannelHandler(handlerId); } }; }, [channelUrl]);