fix: resolve ReferenceError in EmbeddedChat and cleanup API logic #1135
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.
1 participant
This PR focuses on improving overall stability, API reliability, and developer experience. It fixes a critical runtime issue, updates how API requests are handled, and strengthens type safety across key components.
Key Changes
Runtime stability: Fixed a critical ReferenceError caused by a notification dispatcher being called before it was defined. This prevents crashes during authentication failures.
API reliability: Replaced manual string-based request building with proper JSON serialization. This makes data transfer more reliable, avoids syntax issues, and safely handles special characters in messages.
Type safety: Added missing property validation for core UI components, helping catch errors earlier and making component usage clearer.
Logic optimization: Cleaned up internal hooks and resolved dependency warnings for more predictable behavior and slightly better performance.
General cleanup: Fixed typos in docs and comments and made the code style more consistent across the project.
Testing
Verified the project builds successfully without errors
Confirmed the notification dispatcher works correctly after the fix
Ensured all changes pass linting and code style checks