fix: resolve ReferenceError in EmbeddedChat and cleanup API logic#1135
fix: resolve ReferenceError in EmbeddedChat and cleanup API logic#1135vivekyadav-3 wants to merge 16 commits intoRocketChat:developfrom
Conversation
…ecording consistency
…ChatBody, and fix emoji insertion at cursor
… authentication, commands, and message tools
There was a problem hiding this comment.
Pull request overview
This PR strengthens EmbeddedChat’s runtime stability, hardens API interactions, and improves developer ergonomics around permissions, typing, and media recording.
Changes:
- Fixes runtime issues in authentication and typing/recording flows (e.g., proper listener cleanup, interval cleanup, safer permission reads) and improves scroll and emoji behaviors.
- Replaces manually concatenated JSON request bodies in
EmbeddedChatApiwithJSON.stringify(...)to avoid escaping issues for message text and metadata. - Adds memoization and safer prop/type handling across core UI components, plus introduces RFC and GSoC proposal docs for planned ChatInput refactors.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/views/TypingUsers/TypingUsers.js | Fixes typing-status listener registration/removal so the same callback is added/removed, preventing leaks and using a stable effect dependency set. |
| packages/react/src/views/MessageList/MessageList.js | Memoizes filtered messages and reported-message lookup, switches to a normalized date comparison for day dividers, and tightens PropTypes for message list props. |
| packages/react/src/views/MessageAggregators/common/MessageAggregator.js | Uses a single useTheme call, normalizes date comparison for day dividers, and memoizes a unique message list to avoid duplicate aggregated entries. |
| packages/react/src/views/Message/MessageToolbox.js | Refactors permission checks (pin/edit/delete/report visibility and deletion rules) into a memoized block to avoid recomputation and centralize logic. |
| packages/react/src/views/Message/Message.js | Protects against missing edit permissions (`editMessagePermissions?.roles |
| packages/react/src/views/EmbeddedChat.js | Adds memoized handling of the auth config, wires token storage into the RC client, and introduces toast notifications for auto-login failures during initialization. |
| packages/react/src/views/ChatInput/VideoMessageRecoder.js | Consolidates theme access, integrates toggleRecordingMessage into the video recording lifecycle, and ensures recording timers are cleared on unmount. |
| packages/react/src/views/ChatInput/ChatInputFormattingToolbar.js | Changes emoji insertion to respect the current selection/cursor position and re-focuses the input with the cursor placed after the inserted emoji. |
| packages/react/src/views/ChatInput/ChatInput.js | Fixes quoting by building quote links via Promise.all and concatenating them deterministically, then composing them with the message into a pending message. |
| packages/react/src/views/ChatInput/AudioMessageRecorder.js | Corrects the store action name to toggleRecordingMessage, uses it consistently in the audio recorder lifecycle, and adds interval cleanup on unmount. |
| packages/react/src/views/ChatHeader/ChatHeader.js | Introduces token storage for logout, ensuring tokens are deleted on logout while resetting message/channel/UI state to a clean unauthenticated baseline. |
| packages/react/src/views/ChatBody/ChatBody.js | Adjusts auto-scroll behavior to only jump to the bottom when near the bottom or on initial load, reducing “scroll fights” when users scroll up. |
| packages/react/src/store/messageStore.js | Renames and exposes toggleRecordingMessage in the message store to track whether an audio/video recording is in progress. |
| packages/react/src/lib/emoji.js | Simplifies and generalizes emoji shortname parsing to replace all valid :shortname: occurrences with unicode using emoji-toolkit. |
| packages/react/src/hooks/useRCAuth.js | Cleans up login error logging text and shows a toast on network/setup errors to make authentication failures more user visible. |
| packages/react/src/hooks/useFetchChatData.js | Caches permission responses using a ref (with raw and mapped forms) to avoid redundant permission application and safely handles missing role data. |
| packages/api/src/EmbeddedChatApi.ts | Replaces string-literal JSON bodies with JSON.stringify when deleting, updating, starring/pinning, reacting to, or reporting messages, improving robustness and escaping. |
| RFC_CHAT_INPUT_REFACTOR.md | Adds an RFC describing a state-machine-based refactor for ChatInput to move away from fragile raw string manipulation for quotes and formatting. |
| GSOC_2026_PROPOSAL_EmbeddedChat.md | Adds a GSoC 2026 proposal document outlining a broader plan for EmbeddedChat stability, input handling improvements, and auth hardening. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import React, { useMemo } from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import { css } from '@emotion/react'; | ||
| import { isSameDay } from 'date-fns'; |
There was a problem hiding this comment.
isSameDay is imported from date-fns but no longer used now that isMessageNewDay performs a manual date comparison. This unused import is dead code and should be removed to keep the file clean.
| import { isSameDay } from 'date-fns'; |
| const { getToken, saveToken, deleteToken } = getTokenStorage( | ||
| ECOptions?.secure | ||
| ); |
There was a problem hiding this comment.
getToken and saveToken are destructured from getTokenStorage(ECOptions?.secure) but are never used in this component; only deleteToken is referenced in handleLogout. To reduce noise and avoid confusion, remove the unused bindings and destructure only deleteToken.
| const { getToken, saveToken, deleteToken } = getTokenStorage( | |
| ECOptions?.secure | |
| ); | |
| const { deleteToken } = getTokenStorage(ECOptions?.secure); |
| ### 3. Login Error Flow Optimization (Branch: fix/login-error-notification) | ||
|
|
||
| - **Objective:** Improved the `useRCAuth` hook to better map and display server-side errors to the end-user. | ||
| - **Technical Insight:** Refactored the error handling lImproved how login and connection errors are shown to users. Made error feedback clearer and more actionable. |
There was a problem hiding this comment.
There is a typo in this sentence: Refactored the error handling lImproved how login and connection errors are shown to users. The stray l before Improved makes the sentence grammatically incorrect; it should be Refactored the error handling. Improved how login and connection errors are shown to users. or similar.
| - **Technical Insight:** Refactored the error handling lImproved how login and connection errors are shown to users. Made error feedback clearer and more actionable. | |
| - **Technical Insight:** Refactored the error handling. Improved how login and connection errors are shown to users. Made error feedback clearer and more actionable. |
| const options = useMemo( | ||
| () => ({ |
There was a problem hiding this comment.
The options useMemo depends on permission-derived flags like isAllowedToEditMessage, isAllowedToReport, and canDeleteMessage (used for visible on Edit/Delete/Report), but these values are not included in the dependency array. This can leave the visibility state of those actions stale when permissions change without a change to the existing dependencies. Please add these flags to the dependency array so the memoized options recompute when permissions update.
| isAllowedToPin, | ||
| isAllowedToReport, | ||
| isAllowedToEditMessage, | ||
| isAllowedToDeleteMessage, |
There was a problem hiding this comment.
Unused variable isAllowedToDeleteMessage.
| isAllowedToReport, | ||
| isAllowedToEditMessage, | ||
| isAllowedToDeleteMessage, | ||
| isAllowedToDeleteOwnMessage, |
There was a problem hiding this comment.
Unused variable isAllowedToDeleteOwnMessage.
| isAllowedToEditMessage, | ||
| isAllowedToDeleteMessage, | ||
| isAllowedToDeleteOwnMessage, | ||
| isAllowedToForceDeleteMessage, |
There was a problem hiding this comment.
Unused variable isAllowedToForceDeleteMessage.
| isAllowedToDeleteMessage, | ||
| isAllowedToDeleteOwnMessage, | ||
| isAllowedToForceDeleteMessage, | ||
| isVisibleForMessageType, |
There was a problem hiding this comment.
Unused variable isVisibleForMessageType.
- Added encodeURIComponent() to properly encode user input before appending to URL query string - Prevents special characters (&, ?, #, %) from breaking query parameters - Fixes issue RocketChat#1149
- Changed typing status timeout from 15000ms to 10000ms - Makes typing indicator more responsive and updates faster - Improves real-time chat experience
1 participant
Hey there!
I've been working on improving EmbeddedChat's stability and fixing some bugs I ran into while testing. This PR bundles together a few important fixes and improvements that should make the chat experience smoother for everyone.
What's Fixed
Search finally works with special characters! (Fixes #1149)
So I was testing the search feature and realized it completely breaks when you search for things like "hello&world" or "test?query". Turns out we weren't encoding special characters (
&,?,#,%) before sending them to the API, which was causing the URL to get all messed up.I added proper encoding so now you can search for literally anything without the app freaking out. Go ahead and try searching for
hello&world?test#tag%- it should just work now!No more crashes during login failures
Found a nasty bug where the app would crash if login failed. The notification system was trying to show an error message before it was even ready. Fixed the order of things so errors get displayed properly instead of crashing everything.
Performance Tweaks
Typing indicator feels snappier now
The "typing..." indicator was hanging around for 15 seconds after someone stopped typing, which felt kinda sluggish. I dialed it down to 10 seconds so it disappears faster and the chat feels more responsive. Small change, but it makes conversations feel more real-time.
Code Quality Stuff
I also cleaned up a bunch of things under the hood:
Better API handling - Switched from manually building request strings to proper JSON serialization. Less error-prone and handles weird characters way better.
Stronger type checking** - Added validation for UI components so we catch mistakes earlier instead of at runtime.
Cleaner code** - Fixed some dependency warnings, cleaned up hooks, and made the codebase more consistent overall. Also fixed typos in comments and docs.
Why This Matters
For users:
For developers:
Testing Done
I've tested everything locally and made sure:
Ready for review whenever you get a chance! Let me know if you have any questions or want me to test anything else.