Skip to content

refactor: architectural overhaul, memory leak fixes, and UI consistency #1163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vivekyadav-3 wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

refactor: architectural overhaul, memory leak fixes, and UI consistency #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4fe251a
refactor: optimize auth listeners and improve reliability in Embedded…
vivekyadav1207vy-sudo Feb 19, 2026
cd1c7c8
fix: resolve memory leaks in ChatInput and improve UI consistency in …
vivekyadav1207vy-sudo Feb 19, 2026
8c6f9f7
chore: major quality overhaul - fix unstable keys, add propTypes, and…
vivekyadav1207vy-sudo Feb 19, 2026
1358825
fix: critical UI crashes, session leaks, and emoji picker defaults
vivekyadav1207vy-sudo Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 54 additions & 32 deletions packages/react/src/hooks/useRCAuth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,45 +25,67 @@ export const useRCAuth = () => {
const handleLogin = async (userOrEmail, password, code) => {
try {
const res = await RCInstance.login(userOrEmail, password, code);

// Handle specific error codes or generic Unauthorized
if (res.error === 'Unauthorized' || res.error === 403) {
dispatchToastMessage({
type: 'error',
message:
'Invalid username or password. Please check your credentials and try again',
message: 'Invalid username or password. Please check your credentials.',
});
return { status: 'error', error: 'Unauthorized' };
}

// Handle Two-Factor Authentication (TOTP)
if (res.error === 'totp-required') {
setPassword(password);
setEmailorUser(userOrEmail);
setIsLoginModalOpen(false);
setIsTotpModalOpen(true);
dispatchToastMessage({
type: 'info',
message: 'MFA Required: Please enter the code from your authenticator app.',
});
} else {
if (res.error === 'totp-required') {
setPassword(password);
setEmailorUser(userOrEmail);
setIsLoginModalOpen(false);
setIsTotpModalOpen(true);
dispatchToastMessage({
type: 'info',
message: 'Please Open your authentication app and enter the code.',
});
} else if (res.error === 'totp-invalid') {
dispatchToastMessage({
type: 'error',
message: 'Invalid TOTP Time-based One-time Password.',
});
}
return { status: 'totp-required' };
}

if (res.error === 'totp-invalid') {
dispatchToastMessage({
type: 'error',
message: 'Invalid TOTP code. Please try again.',
});
return { status: 'error', error: 'totp-invalid' };
}

// Handle Successful Login
if (res.status === 'success' || (res.me && !res.error)) {
setIsLoginModalOpen(false);
setUserAvatarUrl(res.me.avatarUrl);
setAuthenticatedUserUsername(res.me.username);
setIsUserAuthenticated(true);
setIsTotpModalOpen(false);

// Clear sensitive temporary data
setEmailorUser(null);
setPassword(null);

dispatchToastMessage({
type: 'success',
message: `Welcome back, ${res.me.username}!`,
});
return { status: 'success', user: res.me };
}

if (res.status === 'success') {
setIsLoginModalOpen(false);
setUserAvatarUrl(res.me.avatarUrl);
setAuthenticatedUserUsername(res.me.username);
setIsUserAuthenticated(true);
setIsTotpModalOpen(false);
setEmailorUser(null);
setPassword(null);
dispatchToastMessage({
type: 'success',
message: 'Successfully logged in',
});
}
// Catch-all for other response errors
if (res.error) {
throw new Error(res.reason || res.error);
}
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch-all error handling at line 78-81 may inadvertently catch successful login responses that don't match the expected format. If res.status !== 'success' and !res.me and !res.error, the function will not return anything, potentially leaving the UI in an inconsistent state. Consider adding an explicit return statement for this case or logging a warning when an unexpected response format is received.

Suggested change
}
}
// Handle unexpected response formats to avoid returning undefined
console.warn('Unexpected login response format:', res);
return {
status: 'error',
error: 'Unexpected response format received from authentication server.',
};

Copilot uses AI. Check for mistakes.
} catch (e) {
console.error('A error occurred while setting up user', e);
console.error('Login implementation error:', e);
dispatchToastMessage({
type: 'error',
message: 'Authentication failed due to a network or server error.',
});
return { status: 'error', error: e.message };
}
};

Expand Down
5 changes: 3 additions & 2 deletions packages/react/src/hooks/useShowCommands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ const useShowCommands = (commands, setFilteredCommands, setShowCommandList) =>
const tokens = e.target.value.slice(0, cursor).split(/\s+/);

if (tokens.length === 1 && tokens[0].startsWith('/')) {
setFilteredCommands(getFilteredCommands(tokens[0]));
setShowCommandList(true);
const filtered = getFilteredCommands(tokens[0]);
setFilteredCommands(filtered);
setShowCommandList(filtered.length > 0);
} else {
setFilteredCommands([]);
setShowCommandList(false);
Expand Down
9 changes: 9 additions & 0 deletions packages/react/src/views/ChannelState/ChannelState.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import PropTypes from 'prop-types';
import {
Box,
Icon,
Expand Down Expand Up @@ -34,4 +35,12 @@ const ChannelState = ({
);
};

ChannelState.propTypes = {
className: PropTypes.string,
style: PropTypes.object,
status: PropTypes.string,
iconName: PropTypes.string,
instructions: PropTypes.string,
};

export default ChannelState;
67 changes: 41 additions & 26 deletions packages/react/src/views/ChatBody/ChatBody.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,43 +146,46 @@ const ChatBody = ({
);

useEffect(() => {
RCInstance.auth.onAuthChange((user) => {
if (user) {
RCInstance.addMessageListener(addMessage);
RCInstance.addMessageDeleteListener(removeMessage);
RCInstance.addActionTriggeredListener(onActionTriggerResponse);
RCInstance.addUiInteractionListener(onActionTriggerResponse);
}
});

return () => {
const removeAllListeners = () => {
RCInstance.removeMessageListener(addMessage);
RCInstance.removeMessageDeleteListener(removeMessage);
RCInstance.removeActionTriggeredListener(onActionTriggerResponse);
RCInstance.removeUiInteractionListener(onActionTriggerResponse);
};
}, [RCInstance, addMessage, removeMessage, onActionTriggerResponse]);

useEffect(() => {
RCInstance.auth.onAuthChange((user) => {
const unsubscribe = RCInstance.auth.onAuthChange((user) => {
if (user) {
// Clear old listeners before adding new ones to avoid duplicates
removeAllListeners();
RCInstance.addMessageListener(addMessage);
RCInstance.addMessageDeleteListener(removeMessage);
RCInstance.addActionTriggeredListener(onActionTriggerResponse);
RCInstance.addUiInteractionListener(onActionTriggerResponse);

getMessagesAndRoles();
setHasMoreMessages(true);
} else {
getMessagesAndRoles(anonymousMode);
}
});
}, [RCInstance, anonymousMode, getMessagesAndRoles]);

useEffect(() => {
RCInstance.auth.onAuthChange((user) => {
if (user) {
fetchAndSetPermissions();
} else {
removeAllListeners();
getMessagesAndRoles(anonymousMode);
permissionsRef.current = null;
}
});
}, []);

return () => {
if (typeof unsubscribe === 'function') unsubscribe();
removeAllListeners();
};
}, [
RCInstance,
addMessage,
removeMessage,
onActionTriggerResponse,
anonymousMode,
getMessagesAndRoles,
fetchAndSetPermissions,
permissionsRef,
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect dependency in useEffect: permissionsRef should not be included in the dependency array. Ref objects like permissionsRef have stable references across renders and don't trigger re-renders when their .current value changes. Including refs in dependency arrays is unnecessary and can violate React's rules of hooks. Remove permissionsRef from the dependency array.

Suggested change
permissionsRef,

Copilot uses AI. Check for mistakes.
]);

// Expose clearUnreadDivider function via ref for ChatInput to call
useEffect(() => {
Expand Down Expand Up @@ -309,9 +312,15 @@ const ChatBody = ({

useEffect(() => {
if (messageListRef.current) {
messageListRef.current.scrollTop = messageListRef.current.scrollHeight;
const { scrollTop, scrollHeight, clientHeight } = messageListRef.current;
const isAtBottom = scrollHeight - scrollTop - clientHeight < 100;
const isInitialLoad = messages.length > 0 && scrollTop === 0;

if (isAtBottom || isInitialLoad) {
messageListRef.current.scrollTop = scrollHeight;
}
Comment on lines +315 to +321
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scroll behavior logic has a potential issue. The condition isInitialLoad = messages.length > 0 && scrollTop === 0 may incorrectly trigger scrolling to bottom when the user has scrolled to the top to load older messages. This could disrupt the user's reading experience by unexpectedly jumping to the bottom when old messages are loaded. Consider checking if this is truly an initial load rather than just being at scroll position 0.

Copilot uses AI. Check for mistakes.
}
}, [messages]);
}, [messages, messageListRef]);
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect dependency in useEffect: messageListRef should not be included in the dependency array. Ref objects have stable references across renders and don't trigger re-renders when their .current value changes. Including refs in dependency arrays is incorrect and can cause unexpected behavior. Remove messageListRef from this dependency array.

Copilot uses AI. Check for mistakes.

useEffect(() => {
checkOverflow();
Expand Down Expand Up @@ -429,7 +438,7 @@ const ChatBody = ({
<LoginForm />

{uiKitModalOpen && (
<UiKitModal key={Math.random()} initialView={uiKitModalData} />
<UiKitModal key={uiKitModalData?.viewId || 'uikit-modal'} initialView={uiKitModalData} />
)}
</Box>

Expand All @@ -449,4 +458,10 @@ export default ChatBody;
ChatBody.propTypes = {
anonymousMode: PropTypes.bool,
showRoles: PropTypes.bool,
messageListRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({ current: PropTypes.instanceOf(Element) }),
]),
scrollToBottom: PropTypes.func,
clearUnreadDividerRef: PropTypes.shape({ current: PropTypes.func }),
};
7 changes: 6 additions & 1 deletion packages/react/src/views/ChatHeader/ChatHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,12 @@ const ChatHeader = ({
</Box>
<Box css={styles.chatHeaderIconRow}>
{avatarUrl && (
<img width="20px" height="20px" src={avatarUrl} alt="avatar" />
<Avatar
size="20px"
url={avatarUrl}
alt="user avatar"
style={{ marginRight: '4px' }}
/>
)}

{surfaceOptions.length > 0 && (
Expand Down
13 changes: 10 additions & 3 deletions packages/react/src/views/ChatInput/ChatInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ const ChatInput = ({ scrollToBottom, clearUnreadDividerRef }) => {
);

useEffect(() => {
RCInstance.auth.onAuthChange((user) => {
const unsubscribe = RCInstance.auth.onAuthChange((user) => {
if (user) {
RCInstance.getCommandsList()
.then((data) => setCommands(data.commands || []))
Expand All @@ -157,6 +157,11 @@ const ChatInput = ({ scrollToBottom, clearUnreadDividerRef }) => {
.catch(console.error);
}
});

return () => {
if (typeof unsubscribe === 'function') unsubscribe();
if (timerRef.current) clearTimeout(timerRef.current);
};
}, [RCInstance, isChannelPrivate, setMembersHandler]);

useEffect(() => {
Expand Down Expand Up @@ -258,13 +263,14 @@ const ChatInput = ({ scrollToBottom, clearUnreadDividerRef }) => {
return;
}
if (messageRef.current.value?.length) {
if (timerRef.current) clearTimeout(timerRef.current);
typingRef.current = true;
timerRef.current = setTimeout(() => {
typingRef.current = false;
}, [15000]);
}, 15000);
await RCInstance.sendTypingStatus(username, true);
} else {
clearTimeout(timerRef.current);
if (timerRef.current) clearTimeout(timerRef.current);
typingRef.current = false;
await RCInstance.sendTypingStatus(username, false);
}
Expand All @@ -275,6 +281,7 @@ const ChatInput = ({ scrollToBottom, clearUnreadDividerRef }) => {

const sendTypingStop = async () => {
try {
if (timerRef.current) clearTimeout(timerRef.current);
typingRef.current = false;
await RCInstance.sendTypingStatus(username, false);
} catch (e) {
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/views/CommandList/CommandsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function CommandsList({

const handleCommandClick = useCallback(
async (command) => {
if (!command) return;
const commandName = command.command;
const currentMessage = messageRef.current.value;
const tokens = (currentMessage || '').split(' ');
Expand Down
13 changes: 11 additions & 2 deletions packages/react/src/views/EmbeddedChat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { ChatLayout } from './ChatLayout';
import { ChatHeader } from './ChatHeader';
import { RCInstanceProvider } from '../context/RCInstance';
import { useUserStore, useLoginStore, useMessageStore } from '../store';
import { useUserStore, useLoginStore } from '../store';
import DefaultTheme from '../theme/DefaultTheme';
import { getTokenStorage } from '../lib/auth';
import { styles } from './EmbeddedChat.styles';
Expand Down Expand Up @@ -134,7 +134,7 @@ const EmbeddedChat = (props) => {
}, [RCInstance, auth, setIsLoginIn]);

useEffect(() => {
RCInstance.auth.onAuthChange((user) => {
const unsubscribe = RCInstance.auth.onAuthChange((user) => {
if (user) {
RCInstance.connect()
.then(() => {
Expand All @@ -150,8 +150,17 @@ const EmbeddedChat = (props) => {
.catch(console.error);
} else {
setIsUserAuthenticated(false);
setAuthenticatedAvatarUrl('');
setAuthenticatedUsername('');
setAuthenticatedUserId('');
setAuthenticatedName('');
setAuthenticatedUserRoles([]);
}
});

return () => {
if (typeof unsubscribe === 'function') unsubscribe();
};
}, [
RCInstance,
setAuthenticatedName,
Expand Down
1 change: 0 additions & 1 deletion packages/react/src/views/EmojiPicker/EmojiPicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const CustomEmojiPicker = ({
const theme = useTheme();
const styles = getEmojiPickerStyles(theme);
const previewConfig = {
defaultEmoji: '1f60d',
defaultCaption: 'None',
showPreview: true,
};
Expand Down
10 changes: 8 additions & 2 deletions packages/react/src/views/LoginForm/LoginForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import { css } from '@emotion/react';
import {
GenericModal,
Expand All @@ -12,7 +13,7 @@ import { useLoginStore } from '../../store';
import { useRCAuth } from '../../hooks/useRCAuth';
import styles from './LoginForm.styles';

export default function LoginForm() {
const LoginForm = () => {
const [userOrEmail, setUserOrEmail] = useState(null);
const [password, setPassword] = useState(null);
const [showPassword, setShowPassword] = useState(false);
Expand Down Expand Up @@ -108,6 +109,7 @@ export default function LoginForm() {
/>
{field.label === 'Password' && (
<Box
is="button"
type="button"
css={styles.passwordEye}
onClick={handleTogglePassword}
Expand Down Expand Up @@ -142,4 +144,8 @@ export default function LoginForm() {
</GenericModal>
</>
) : null;
}
};

LoginForm.propTypes = {}; // No props, but good to have for consistency

export default LoginForm;
Loading