chore: remove emojione in favor of native emojis#39411
Open
sampaiodiego wants to merge 29 commits into
Open
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR replaces emojione-based emoji data and conversion paths with native/emojibase-backed code across Meteor and EE packages. It also updates rendering, picker behavior, parser support, tests, and cleanup files to use the new emoji classes and shortname mapping. ChangesNative emoji migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sampaiodiego
force-pushed
the
remove-joypixel
branch
from
d3b9fcc to
095c8c3
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #39411 +/- ##
===========================================
+ Coverage 70.09% 70.51% +0.42%
===========================================
Files 3363 3370 +7
Lines 129716 131506 +1790
Branches 22444 22552 +108
===========================================
+ Hits 90925 92736 +1811
+ Misses 35479 35461 -18
+ Partials 3312 3309 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Member
|
@copilot when inserting an emoji using the picker or autocomplete, use the UTF-8 encoding unless it's a custom emoji, |
Contributor
MartinSchoeler
requested changes
Mar 9, 2026
MartinSchoeler
left a comment
MartinSchoeler
left a comment
Member
There was a problem hiding this comment.
Whenever the user adds an emoji using the emoji picker, the result should be an unicode character being added to the message composer, only show the shortcode as fallback for custom emojis or emojis that don't have an unicode counterpart
Steps to reproduce:
Open emoji picker
Select an emoji
Expected behavior:
An unicode emoji character should be added. (Example 🙈)
Actual behavior:
A shortname is added. (Example 🙈)
Member
@copilot |
Contributor
sampaiodiego
force-pushed
the
remove-joypixel
branch
2 times, most recently
from
e0ef711 to
fdc9e41
Compare
This was referenced Mar 17, 2026
Closed
…hortcode (#39474) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ggazzo <5263975+ggazzo@users.noreply.github.com>
MartinSchoeler
force-pushed
the
remove-joypixel
branch
from
8a7ecd2 to
4702b6e
Compare
coderabbitai
Bot
added
the
type: feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 97 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx">
<violation number="1" location="apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx:255">
P2: Alias emoji selections can still insert shortcode text instead of Unicode. This leaves a mixed storage path after the migration and breaks the native-emoji-only behavior for alias hits.</violation>
</file>
<file name="ee/packages/federation-matrix/src/events/reaction.ts">
<violation number="1" location="ee/packages/federation-matrix/src/events/reaction.ts:42">
P2: `reactionKey` from federated events is used without validation before conversion. Missing/non-string keys can propagate invalid reaction identifiers into reaction add/remove updates.</violation>
</file>
<file name="apps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsx">
<violation number="1" location="apps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsx:53">
P3: Uses hardcoded `'rocket'` instead of shared `CUSTOM_CATEGORY` constant. If category key changes, custom emoji styling condition silently stops matching.</violation>
</file>
<file name="apps/meteor/app/emoji-native/lib/generateEmojiData.ts">
<violation number="1" location="apps/meteor/app/emoji-native/lib/generateEmojiData.ts:100">
P1: Multi-skin-tone variants are keyed with only the first tone, causing key collisions and wrong emoji mappings. Mixed-tone entries should not be flattened to a single `_toneN` key.</violation>
</file>
<file name="ee/packages/federation-matrix/src/FederationMatrix.ts">
<violation number="1" location="ee/packages/federation-matrix/src/FederationMatrix.ts:522">
P2: Skin-tone reaction shortcodes are not converted to Unicode before Matrix calls. This breaks federated add/remove reactions for those emojis by sending literal `:...:` keys.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
| for (const skin of emojiData.skins) { | ||
| if (!skin.tone) continue; | ||
|
|
||
| const toneNum = Array.isArray(skin.tone) ? skin.tone[0] : skin.tone; |
Contributor
There was a problem hiding this comment.
P1: Multi-skin-tone variants are keyed with only the first tone, causing key collisions and wrong emoji mappings. Mixed-tone entries should not be flattened to a single _toneN key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/emoji-native/lib/generateEmojiData.ts, line 100:
<comment>Multi-skin-tone variants are keyed with only the first tone, causing key collisions and wrong emoji mappings. Mixed-tone entries should not be flattened to a single `_toneN` key.</comment>
<file context>
@@ -0,0 +1,129 @@
+ for (const skin of emojiData.skins) {
+ if (!skin.tone) continue;
+
+ const toneNum = Array.isArray(skin.tone) ? skin.tone[0] : skin.tone;
+ const toneKey = `:${primaryShortcode}_tone${toneNum}:`;
+ const skinHex = hexFromEmoji(skin.emoji);
</file context>
| getValue: (item) => `${item._id.substring(1)}`, | ||
| getValue: (item) => { | ||
| const emojiEntry = emoji.list[item._id]; | ||
| return emojiEntry && 'unicode' in emojiEntry ? (emojiEntry.unicode ?? item._id) : item._id; |
Contributor
There was a problem hiding this comment.
P2: Alias emoji selections can still insert shortcode text instead of Unicode. This leaves a mixed storage path after the migration and breaks the native-emoji-only behavior for alias hits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx, line 255:
<comment>Alias emoji selections can still insert shortcode text instead of Unicode. This leaves a mixed storage path after the migration and breaks the native-emoji-only behavior for alias hits.</comment>
<file context>
@@ -249,7 +250,10 @@ const ComposerPopupProvider = ({ children, room }: ComposerPopupProviderProps) =
- getValue: (item) => `${item._id.substring(1)}`,
+ getValue: (item) => {
+ const emojiEntry = emoji.list[item._id];
+ return emojiEntry && 'unicode' in emojiEntry ? (emojiEntry.unicode ?? item._id) : item._id;
+ },
renderItem: ({ item }) => <ComposerBoxPopupEmoji {...item} />,
</file context>
| } | ||
|
|
||
| const reactionKey = emojione.shortnameToUnicode(reaction); | ||
| const reactionKey = shortnameToUnicode(reaction); |
Contributor
There was a problem hiding this comment.
P2: Skin-tone reaction shortcodes are not converted to Unicode before Matrix calls. This breaks federated add/remove reactions for those emojis by sending literal :...: keys.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ee/packages/federation-matrix/src/FederationMatrix.ts, line 522:
<comment>Skin-tone reaction shortcodes are not converted to Unicode before Matrix calls. This breaks federated add/remove reactions for those emojis by sending literal `:...:` keys.</comment>
<file context>
@@ -519,7 +519,7 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
}
- const reactionKey = emojione.shortnameToUnicode(reaction);
+ const reactionKey = shortnameToUnicode(reaction);
const userMui = isUserNativeFederated(user) ? user.federation.mui : `@${user.username}:${this.serverName}`;
</file context>
| @@ -2,7 +2,8 @@ import { Message, FederationMatrix } from '@rocket.chat/core-services'; | |||
| import { federationSDK } from '@rocket.chat/federation-sdk'; | |||
Contributor
There was a problem hiding this comment.
P2: reactionKey from federated events is used without validation before conversion. Missing/non-string keys can propagate invalid reaction identifiers into reaction add/remove updates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ee/packages/federation-matrix/src/events/reaction.ts, line 42:
<comment>`reactionKey` from federated events is used without validation before conversion. Missing/non-string keys can propagate invalid reaction identifiers into reaction add/remove updates.</comment>
<file context>
@@ -38,7 +39,7 @@ export function reaction() {
}
- const reactionEmoji = emojione.toShort(reactionKey);
+ const reactionEmoji = unicodeToShortname(reactionKey);
await Message.reactToMessage(user._id, reactionEmoji, rcMessage._id, true);
await Messages.setFederationReactionEventId(internalUsername, rcMessage._id, reactionEmoji, eventId);
</file context>
| key={emoji + category} | ||
| emoji={emoji} | ||
| image={image} | ||
| className={category === 'rocket' ? 'emojipicker--custom' : ''} |
Contributor
There was a problem hiding this comment.
P3: Uses hardcoded 'rocket' instead of shared CUSTOM_CATEGORY constant. If category key changes, custom emoji styling condition silently stops matching.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsx, line 53:
<comment>Uses hardcoded `'rocket'` instead of shared `CUSTOM_CATEGORY` constant. If category key changes, custom emoji styling condition silently stops matching.</comment>
<file context>
@@ -46,7 +46,13 @@ const EmojiCategoryRow = ({ item, handleLoadMore, handleSelectEmoji }: EmojiCate
+ key={emoji + category}
+ emoji={emoji}
+ image={image}
+ className={category === 'rocket' ? 'emojipicker--custom' : ''}
+ onClick={handleSelectEmoji}
+ />
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/package.json (1)
44-44: 📐 Maintainability & Code Quality | 🟡 MinorRestore
lodash.debounceinee/packages/federation-matrix/package.json
ee/packages/federation-matrix/src/events/member.tsstill imports it, so this package is missing a direct runtime dependency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/packages/federation-matrix/package.json` at line 44, The federation-matrix package is missing a direct runtime dependency for a module still imported by member.ts. Update the package.json dependencies for ee/packages/federation-matrix to restore lodash.debounce alongside the existing type package, so the import used in the member-related event code resolves at runtime.
🧹 Nitpick comments (7)
apps/meteor/app/emoji-native/lib/getEmojiConfig.ts (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the inline implementation comments here.
The control flow is already compact enough to express intent without commentary. As per coding guidelines,
**/*.{ts,tsx,js}:Avoid code comments in the implementation.Also applies to: 39-42, 54-54, 62-62, 71-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/app/emoji-native/lib/getEmojiConfig.ts` at line 17, Remove the inline implementation comments from getEmojiConfig and related blocks in the same diff, since the logic in the reverse mapping and control flow should stand on its own. Keep the code behavior unchanged and delete the comments near the mapping construction and other marked spots so getEmojiConfig remains comment-free in the implementation.Source: Coding guidelines
apps/meteor/app/emoji-native/lib/generateEmojiData.ts (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove implementation comments from this module.
These comments are carrying behavior that should live in names or helper boundaries instead. As per coding guidelines,
**/*.{ts,tsx,js}:Avoid code comments in the implementation.Also applies to: 55-55, 82-82, 87-87, 92-92, 96-96, 121-121
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/app/emoji-native/lib/generateEmojiData.ts` at line 5, Remove the implementation comments from generateEmojiData.ts, including the mapping and step-explanation comments around the emoji grouping logic. Keep the behavior in the existing names/helpers such as the group/category mapping and related functions, and delete the comment lines at the referenced spots so the module contains no code comments.Source: Coding guidelines
apps/meteor/app/emoji-native/server/lib.ts (1)
27-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the implementation comments in this initializer.
This setup code is short enough to keep the intent in the statements themselves. As per coding guidelines,
**/*.{ts,tsx,js}:Avoid code comments in the implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/app/emoji-native/server/lib.ts` around lines 27 - 30, The initializer in legacy shortcode registration contains implementation comments that should be removed. Update the loop in the emoji-native server lib so the intent is conveyed by the statements themselves, and delete the comment above the legacy emojione shortcode registration and the inline note on the emoji.list key check.Source: Coding guidelines
apps/meteor/client/views/root/hooks/useNativeEmoji.ts (2)
13-52: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrefer
useEffectoveruseLayoutEffectfor the emoji registration.This effect synchronously iterates the full
config.emojiListplus ~1400legacyEmojioneMapentries and populatesemoji.list.useLayoutEffectruns synchronously before the browser paints, so this bulk work blocks the first paint during startup. Since nothing here reads layout or needs to run before paint,useEffectwould avoid the jank while keeping the same one-time behavior.♻️ Suggested change
- useLayoutEffect(() => { + useEffect(() => { emoji.packages.native = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/views/root/hooks/useNativeEmoji.ts` around lines 13 - 52, The one-time emoji registration in useNativeEmoji is doing heavy synchronous work before paint, so switch this effect from useLayoutEffect to useEffect. Keep the same initialization logic for emoji.packages.native, emoji.list, and emoji.dispatchUpdate, but run it after render since no layout measurement or pre-paint behavior is needed.
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid the
as anycast onemojiCategories.Casting to
anyhere disables type checking on the shape passed intoemoji.packages.native. Align theconfig.emojiCategoriestype with the expectedemojiCategoriestype so mismatches are caught at compile time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/views/root/hooks/useNativeEmoji.ts` at line 15, The `useNativeEmoji` hook is bypassing type safety by casting `config.emojiCategories` to `any` before passing it into `emoji.packages.native`. Update the `config.emojiCategories` type or the receiving `emoji.packages.native` contract so they match directly, and remove the cast while keeping the call site in `useNativeEmoji` and the `emojiCategories` property strongly typed.apps/meteor/tests/e2e/federation/tests/messaging/private.spec.ts (1)
579-585: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the reaction selector behind the page object boundary.
This change updates the class name correctly, but the spec still duplicates the same DOM selector across every reaction assertion. Please move it to a
FederationChannelhelper/locator so the emoji-rendering contract stays centralized. As per coding guidelines,apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuseandapps/meteor/tests/e2e/**/*.{ts,spec.ts}: Follow Page Object Model pattern consistently in Playwright tests.Also applies to: 606-612, 645-651, 668-682, 715-721, 738-751
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/federation/tests/messaging/private.spec.ts` around lines 579 - 585, The reaction emoji selector is duplicated in the spec instead of being centralized behind the page object boundary. Move the `span.rcx-message-reactions__emoji.emoji` locator into a `FederationChannel` helper/locator and reuse it in the reaction assertions so the emoji-rendering contract lives in one place. Update the related reaction checks in the `private.spec.ts` flows to reference that helper consistently and keep the Page Object Model boundary intact.Source: Coding guidelines
apps/meteor/tests/e2e/federation/tests/messaging/dm.spec.ts (1)
668-674: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the reaction emoji locator into the page object.
These assertions hardcode the same CSS selector repeatedly in the spec, so the next class-name change will require another multi-hunk edit. Please expose a reusable reaction-emoji locator/helper on
FederationChannel(and prefer a semantic locator there if the rendered emoji has an accessible name). As per coding guidelines,apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuseandapps/meteor/tests/e2e/**/*.{ts,spec.ts}: Follow Page Object Model pattern consistently in Playwright tests.Also applies to: 713-719, 768-774, 791-801, 854-860, 877-887
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/federation/tests/messaging/dm.spec.ts` around lines 668 - 674, The reaction emoji selector is hardcoded across the dm spec, making future class-name changes brittle. Add a reusable reaction-emoji locator/helper on FederationChannel, preferably using a semantic locator if the emoji has an accessible name, and update the spec assertions to call that page-object method instead of repeating the CSS selector. Keep the locator centralized in the page object and reuse it consistently in all affected reaction assertion blocks.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/meteor/app/emoji-native/lib/generateEmojiData.ts`:
- Around line 97-101: The mixed-tone skin key generation in generateEmojiData.ts
collapses array-based skin.tone values by using only the first element, so
distinct mixed-tone variants can overwrite each other. Update the tone key
construction in the loop over emojiData.skins to either encode the full tone
tuple from skin.tone into the key or explicitly skip mixed-tone entries, and
keep the single-tone path intact for the existing primaryShortcode-based keys.
In `@apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts`:
- Around line 195-201: Avoid mutating the shared attachments array in
sendNotificationsOnMessage when handling the first attachment. In
sendNotification(), replace the use of message.attachments.shift() with a
non-mutating read/copy so the same message object can be reused across
subscriptions without losing attachments; use the first-attachment handling
around firstAttachment and keep message.attachments unchanged for later
receivers.
In `@apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneItem.tsx`:
- Around line 6-26: The ToneItem component is rendering toneEmoji through
dangerouslySetInnerHTML, which keeps the unsafe HTML injection warning active.
Update ToneItem to return JSX directly by mapping the tone value to the correct
emoji character and rendering it inside a span with className="emoji" rather
than building an HTML string, so the Box content no longer relies on raw HTML.
In
`@apps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsx`:
- Around line 36-54: `renderEmojiToDataURL` currently assumes every split code
point is valid, but `parseInt` can produce `NaN` and `String.fromCodePoint` will
throw on malformed input. Add a small validation step before building the emoji
string, and return an empty string early when any code point is invalid so the
PDF export path stays resilient. Use the `renderEmojiToDataURL` helper and its
`codePoints` parsing logic as the fix point.
In `@ee/packages/federation-matrix/src/utils/emojiConverter.ts`:
- Around line 30-37: The emoji skin-tone mapping in emojiConverter is only
populating _unicodeToShort for each skin, so shortnames like :<base>_toneN: are
not recognized by shortnameToUnicode and can be forwarded raw; update the
emoji.skins handling in the converter to register every value from
getShortcodes(skin.hexcode) in _shortToUnicode, while keeping the existing
unicode-to-short mapping behavior intact.
---
Outside diff comments:
In `@ee/packages/federation-matrix/package.json`:
- Line 44: The federation-matrix package is missing a direct runtime dependency
for a module still imported by member.ts. Update the package.json dependencies
for ee/packages/federation-matrix to restore lodash.debounce alongside the
existing type package, so the import used in the member-related event code
resolves at runtime.
---
Nitpick comments:
In `@apps/meteor/app/emoji-native/lib/generateEmojiData.ts`:
- Line 5: Remove the implementation comments from generateEmojiData.ts,
including the mapping and step-explanation comments around the emoji grouping
logic. Keep the behavior in the existing names/helpers such as the
group/category mapping and related functions, and delete the comment lines at
the referenced spots so the module contains no code comments.
In `@apps/meteor/app/emoji-native/lib/getEmojiConfig.ts`:
- Line 17: Remove the inline implementation comments from getEmojiConfig and
related blocks in the same diff, since the logic in the reverse mapping and
control flow should stand on its own. Keep the code behavior unchanged and
delete the comments near the mapping construction and other marked spots so
getEmojiConfig remains comment-free in the implementation.
In `@apps/meteor/app/emoji-native/server/lib.ts`:
- Around line 27-30: The initializer in legacy shortcode registration contains
implementation comments that should be removed. Update the loop in the
emoji-native server lib so the intent is conveyed by the statements themselves,
and delete the comment above the legacy emojione shortcode registration and the
inline note on the emoji.list key check.
In `@apps/meteor/client/views/root/hooks/useNativeEmoji.ts`:
- Around line 13-52: The one-time emoji registration in useNativeEmoji is doing
heavy synchronous work before paint, so switch this effect from useLayoutEffect
to useEffect. Keep the same initialization logic for emoji.packages.native,
emoji.list, and emoji.dispatchUpdate, but run it after render since no layout
measurement or pre-paint behavior is needed.
- Line 15: The `useNativeEmoji` hook is bypassing type safety by casting
`config.emojiCategories` to `any` before passing it into
`emoji.packages.native`. Update the `config.emojiCategories` type or the
receiving `emoji.packages.native` contract so they match directly, and remove
the cast while keeping the call site in `useNativeEmoji` and the
`emojiCategories` property strongly typed.
In `@apps/meteor/tests/e2e/federation/tests/messaging/dm.spec.ts`:
- Around line 668-674: The reaction emoji selector is hardcoded across the dm
spec, making future class-name changes brittle. Add a reusable reaction-emoji
locator/helper on FederationChannel, preferably using a semantic locator if the
emoji has an accessible name, and update the spec assertions to call that
page-object method instead of repeating the CSS selector. Keep the locator
centralized in the page object and reuse it consistently in all affected
reaction assertion blocks.
In `@apps/meteor/tests/e2e/federation/tests/messaging/private.spec.ts`:
- Around line 579-585: The reaction emoji selector is duplicated in the spec
instead of being centralized behind the page object boundary. Move the
`span.rcx-message-reactions__emoji.emoji` locator into a `FederationChannel`
helper/locator and reuse it in the reaction assertions so the emoji-rendering
contract lives in one place. Update the related reaction checks in the
`private.spec.ts` flows to reference that helper consistently and keep the Page
Object Model boundary intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c4d3270-90a2-43fe-8080-9e280d98e75b
⛔ Files ignored due to path filters (12)
apps/meteor/client/components/UserInfo/__snapshots__/UserInfo.spec.tsx.snapis excluded by!**/*.snapapps/meteor/public/packages/emojione/activity-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/flags-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/food-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/modifier-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/nature-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/objects-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/people-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/regional-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/symbols-sprites.pngis excluded by!**/*.pngapps/meteor/public/packages/emojione/travel-sprites.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (85)
README.mdapps/meteor/.stylelintignoreapps/meteor/app/emoji-emojione/.gitignoreapps/meteor/app/emoji-emojione/README.mdapps/meteor/app/emoji-emojione/client/activity-sprites.cssapps/meteor/app/emoji-emojione/client/emojione-sprites.cssapps/meteor/app/emoji-emojione/client/flags-sprites.cssapps/meteor/app/emoji-emojione/client/food-sprites.cssapps/meteor/app/emoji-emojione/client/index.tsapps/meteor/app/emoji-emojione/client/modifier-sprites.cssapps/meteor/app/emoji-emojione/client/nature-sprites.cssapps/meteor/app/emoji-emojione/client/objects-sprites.cssapps/meteor/app/emoji-emojione/client/people-sprites.cssapps/meteor/app/emoji-emojione/client/regional-sprites.cssapps/meteor/app/emoji-emojione/client/symbols-sprites.cssapps/meteor/app/emoji-emojione/client/travel-sprites.cssapps/meteor/app/emoji-emojione/lib/emojiPicker.tsapps/meteor/app/emoji-emojione/lib/emojione.tplapps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjsapps/meteor/app/emoji-emojione/lib/getEmojiConfig.tsapps/meteor/app/emoji-emojione/lib/isSetNotNull.tsapps/meteor/app/emoji-emojione/server/lib.tsapps/meteor/app/emoji-native/lib/generateEmojiData.tsapps/meteor/app/emoji-native/lib/getEmojiConfig.tsapps/meteor/app/emoji-native/lib/legacyEmojioneMap.tsapps/meteor/app/emoji-native/lib/shortnameToUnicode.tsapps/meteor/app/emoji-native/server/callbacks.tsapps/meteor/app/emoji-native/server/index.tsapps/meteor/app/emoji-native/server/lib.tsapps/meteor/app/emoji/client/emojiParser.tsapps/meteor/app/emoji/client/lib.tsapps/meteor/app/emoji/lib/rocketchat.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/app/theme/client/imports/components/emoji.cssapps/meteor/client/components/Emoji.tsxapps/meteor/client/importPackages.tsapps/meteor/client/lib/customEmoji.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/client/lib/utils/renderEmoji.tsapps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsxapps/meteor/client/views/composer/EmojiPicker/EmojiElement.tsxapps/meteor/client/views/composer/EmojiPicker/EmojiPicker.tsxapps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneItem.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsxapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxapps/meteor/client/views/root/AppLayout.tsxapps/meteor/client/views/root/hooks/useEmojiOne.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsapps/meteor/definition/externals/emojione.d.tsapps/meteor/package.jsonapps/meteor/server/importPackages.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsee/apps/omnichannel-transcript/package.jsonee/apps/queue-worker/package.jsonee/packages/federation-matrix/package.jsonee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/reaction.tsee/packages/federation-matrix/src/utils/emojiConverter.tsee/packages/omnichannel-services/package.jsonee/packages/pdf-worker/package.jsonee/packages/pdf-worker/src/templates/ChatTranscript/markup/blocks/BigEmojiBlock.tsxee/packages/pdf-worker/src/templates/ChatTranscript/markup/elements/EmojiSpan.tsxee/packages/pdf-worker/src/templates/ChatTranscript/markup/emoji.tsee/packages/pdf-worker/src/types/emoji-toolkit.tseslint.config.mjspackages/i18n/src/locales/en.i18n.jsonpackages/i18n/src/locales/hi-IN.i18n.jsonpackages/i18n/src/locales/nb.i18n.jsonpackages/i18n/src/locales/nn.i18n.jsonpackages/i18n/src/locales/pt-BR.i18n.jsonpackages/i18n/src/locales/sv.i18n.jsonpackages/i18n/src/locales/zh.i18n.jsonpackages/message-parser/src/grammar.pegjspackages/message-parser/tests/emoji.test.tspackages/ui-avatar/src/components/MessageAvatar.tsxpackages/ui-client/src/components/EmojiPicker/EmojiPickerPreview.tsx
💤 Files with no reviewable changes (38)
- apps/meteor/app/emoji-emojione/README.md
- apps/meteor/app/emoji-emojione/client/emojione-sprites.css
- apps/meteor/client/views/root/hooks/useEmojiOne.ts
- apps/meteor/app/emoji-emojione/client/nature-sprites.css
- apps/meteor/app/emoji-emojione/client/regional-sprites.css
- apps/meteor/app/emoji-emojione/.gitignore
- apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs
- apps/meteor/app/emoji-emojione/client/modifier-sprites.css
- apps/meteor/client/views/composer/EmojiPicker/EmojiPicker.tsx
- apps/meteor/definition/externals/emojione.d.ts
- apps/meteor/app/emoji-emojione/client/index.ts
- apps/meteor/app/emoji-emojione/server/lib.ts
- apps/meteor/app/emoji-emojione/client/activity-sprites.css
- apps/meteor/app/emoji-emojione/client/food-sprites.css
- ee/packages/omnichannel-services/package.json
- apps/meteor/app/emoji-emojione/client/travel-sprites.css
- ee/packages/pdf-worker/src/types/emoji-toolkit.ts
- apps/meteor/app/emoji-emojione/client/flags-sprites.css
- apps/meteor/app/emoji-emojione/lib/emojione.tpl
- packages/i18n/src/locales/nn.i18n.json
- eslint.config.mjs
- apps/meteor/client/importPackages.ts
- apps/meteor/app/emoji-emojione/client/objects-sprites.css
- apps/meteor/app/emoji-emojione/lib/isSetNotNull.ts
- packages/i18n/src/locales/hi-IN.i18n.json
- packages/i18n/src/locales/pt-BR.i18n.json
- ee/apps/queue-worker/package.json
- apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts
- apps/meteor/.stylelintignore
- packages/i18n/src/locales/zh.i18n.json
- packages/i18n/src/locales/en.i18n.json
- apps/meteor/app/emoji-emojione/lib/emojiPicker.ts
- apps/meteor/app/emoji-emojione/client/symbols-sprites.css
- README.md
- packages/i18n/src/locales/sv.i18n.json
- apps/meteor/client/views/composer/EmojiPicker/EmojiElement.tsx
- packages/i18n/src/locales/nb.i18n.json
- ee/apps/omnichannel-transcript/package.json
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/server/importPackages.tsapps/meteor/app/emoji-native/server/callbacks.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsee/packages/pdf-worker/src/templates/ChatTranscript/markup/elements/EmojiSpan.tsxapps/meteor/app/emoji-native/lib/shortnameToUnicode.tsapps/meteor/client/components/Emoji.tsxee/packages/pdf-worker/src/templates/ChatTranscript/markup/emoji.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/app/emoji/lib/rocketchat.tspackages/ui-client/src/components/EmojiPicker/EmojiPickerPreview.tsxapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/app/emoji-native/lib/legacyEmojioneMap.tsapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/client/views/root/AppLayout.tsxapps/meteor/app/emoji-native/server/lib.tsee/packages/pdf-worker/src/templates/ChatTranscript/markup/blocks/BigEmojiBlock.tsxpackages/ui-avatar/src/components/MessageAvatar.tsxee/packages/federation-matrix/src/events/reaction.tsapps/meteor/client/lib/customEmoji.tsapps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneItem.tsxpackages/message-parser/tests/emoji.test.tsapps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsxapps/meteor/app/emoji-native/lib/getEmojiConfig.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxee/packages/federation-matrix/src/utils/emojiConverter.tsapps/meteor/client/lib/utils/renderEmoji.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/app/emoji-native/lib/generateEmojiData.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/app/emoji/client/emojiParser.tsapps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsxapps/meteor/app/emoji/client/lib.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/home-channel.ts
🧠 Learnings (11)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/client/lib/customEmoji.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsapps/meteor/client/lib/utils/renderEmoji.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/client/lib/customEmoji.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsapps/meteor/client/lib/utils/renderEmoji.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/server/importPackages.tsapps/meteor/app/emoji-native/server/callbacks.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsapps/meteor/app/emoji-native/lib/shortnameToUnicode.tsee/packages/pdf-worker/src/templates/ChatTranscript/markup/emoji.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/app/emoji/lib/rocketchat.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/app/emoji-native/lib/legacyEmojioneMap.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/app/emoji-native/server/lib.tsee/packages/federation-matrix/src/events/reaction.tsapps/meteor/client/lib/customEmoji.tspackages/message-parser/tests/emoji.test.tsapps/meteor/app/emoji-native/lib/getEmojiConfig.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/utils/emojiConverter.tsapps/meteor/client/lib/utils/renderEmoji.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/app/emoji-native/lib/generateEmojiData.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/app/emoji/client/emojiParser.tsapps/meteor/app/emoji/client/lib.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/server/importPackages.tsapps/meteor/app/emoji-native/server/callbacks.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsapps/meteor/app/emoji-native/lib/shortnameToUnicode.tsee/packages/pdf-worker/src/templates/ChatTranscript/markup/emoji.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/app/emoji/lib/rocketchat.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/app/emoji-native/lib/legacyEmojioneMap.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/app/emoji-native/server/lib.tsee/packages/federation-matrix/src/events/reaction.tsapps/meteor/client/lib/customEmoji.tspackages/message-parser/tests/emoji.test.tsapps/meteor/app/emoji-native/lib/getEmojiConfig.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/utils/emojiConverter.tsapps/meteor/client/lib/utils/renderEmoji.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/app/emoji-native/lib/generateEmojiData.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/app/emoji/client/emojiParser.tsapps/meteor/app/emoji/client/lib.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.spec.tsapps/meteor/server/importPackages.tsapps/meteor/app/emoji-native/server/callbacks.tsapps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/unit/app/reactions/server/setReaction.spec.tsee/packages/pdf-worker/src/templates/ChatTranscript/markup/elements/EmojiSpan.tsxapps/meteor/app/emoji-native/lib/shortnameToUnicode.tsapps/meteor/client/components/Emoji.tsxee/packages/pdf-worker/src/templates/ChatTranscript/markup/emoji.tsapps/meteor/client/lib/utils/detectEmoji.tsapps/meteor/app/emoji/lib/rocketchat.tspackages/ui-client/src/components/EmojiPicker/EmojiPickerPreview.tsxapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/app/emoji-native/lib/legacyEmojioneMap.tsapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/client/views/root/AppLayout.tsxapps/meteor/app/emoji-native/server/lib.tsee/packages/pdf-worker/src/templates/ChatTranscript/markup/blocks/BigEmojiBlock.tsxpackages/ui-avatar/src/components/MessageAvatar.tsxee/packages/federation-matrix/src/events/reaction.tsapps/meteor/client/lib/customEmoji.tsapps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneItem.tsxpackages/message-parser/tests/emoji.test.tsapps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsxapps/meteor/app/emoji-native/lib/getEmojiConfig.tsapps/meteor/app/lib/server/lib/sendNotificationsOnMessage.tsapps/meteor/client/views/root/hooks/useNativeEmoji.tsee/packages/federation-matrix/src/FederationMatrix.tsapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxee/packages/federation-matrix/src/utils/emojiConverter.tsapps/meteor/client/lib/utils/renderEmoji.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/app/emoji-native/lib/generateEmojiData.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.tsapps/meteor/app/emoji/client/emojiParser.tsapps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsxapps/meteor/app/emoji/client/lib.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/message-composer.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/federation/tests/messaging/private.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/dm.spec.tsapps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
ee/packages/pdf-worker/src/templates/ChatTranscript/markup/elements/EmojiSpan.tsxapps/meteor/client/components/Emoji.tsxpackages/ui-client/src/components/EmojiPicker/EmojiPickerPreview.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/root/AppLayout.tsxee/packages/pdf-worker/src/templates/ChatTranscript/markup/blocks/BigEmojiBlock.tsxpackages/ui-avatar/src/components/MessageAvatar.tsxapps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneItem.tsxapps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsxapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxapps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsx
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/home-channel.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/events/reaction.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/utils/emojiConverter.ts
🪛 ast-grep (0.44.0)
apps/meteor/client/views/composer/EmojiPicker/ToneSelector/ToneItem.tsx
[warning] 25-25: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(react-unsafe-html-injection)
apps/meteor/app/emoji-native/lib/getEmojiConfig.ts
[warning] 42-42: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((:([a-zA-Z0-9_+-]+):)|(${unicodePattern}), 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🔇 Additional comments (35)
apps/meteor/app/emoji/lib/rocketchat.ts (1)
21-35: LGTM!apps/meteor/client/views/root/AppLayout.tsx (1)
32-32: LGTM!Also applies to: 57-57
apps/meteor/app/emoji/client/lib.ts (1)
16-16: 🎯 Functional CorrectnessNo change needed
packages.base.renderis now a passthrough; shortcode conversion is handled by the native emoji package, and no remaining caller depends on the base renderer for that behavior.apps/meteor/app/emoji-native/lib/legacyEmojioneMap.ts (1)
5-1410: LGTM!apps/meteor/server/importPackages.ts (1)
18-18: LGTM!apps/meteor/app/emoji-native/server/callbacks.ts (1)
4-12: LGTM!apps/meteor/app/emoji-native/lib/shortnameToUnicode.ts (1)
5-19: 🚀 Performance & ScalabilityNo action needed:
getEmojiData()is already cached.shortnameToUnicode()reuses the memoized emoji data, so it does not rebuild the map on each call.> Likely an incorrect or invalid review comment.apps/meteor/package.json (1)
200-201: 🔒 Security & PrivacyNo action needed for
emojibase/emojibase-data^16.0.0is published for both packages, andemojibase-data16.0.0 has no known advisories.apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts (1)
194-194: LGTM!apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts (1)
5-10: LGTM!ee/packages/pdf-worker/package.json (1)
24-24: LGTM!ee/packages/pdf-worker/src/templates/ChatTranscript/markup/emoji.ts (1)
1-38: LGTM!ee/packages/pdf-worker/src/templates/ChatTranscript/markup/blocks/BigEmojiBlock.tsx (1)
3-14: LGTM!ee/packages/pdf-worker/src/templates/ChatTranscript/markup/elements/EmojiSpan.tsx (1)
4-8: LGTM!apps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsx (1)
103-108: LGTM!apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx (1)
166-170: LGTM!apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (1)
253-256: LGTM!ee/packages/federation-matrix/src/utils/emojiConverter.ts (1)
47-50: 🎯 Functional Correctness | ⚡ Quick win
unicodeToShortnamerequires an exact unicode key match.The lookup is a direct
Map.get(text); an incoming Matrix reaction key that differs only by a presentation variation selector (U+FE0F) or other normalization from emojibase'semoji/skin.emojivalue will miss and fall through to returning the raw unicode as the "shortname", which then gets persisted as the RC reaction name. The oldemojione.toShortwas tolerant of variation selectors — worth confirming this regression won't surface for common reactions.ee/packages/federation-matrix/src/FederationMatrix.ts (1)
21-21: LGTM!Also applies to: 522-522, 562-562
ee/packages/federation-matrix/src/events/reaction.ts (1)
42-42: LGTM!Also applies to: 86-86
packages/message-parser/src/grammar.pegjs (1)
743-762: 🎯 Functional CorrectnessNo action needed. Standalone emojis from the affected blocks and multi-component ZWJ sequences are already covered by tests.
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
172-177: LGTM!apps/meteor/tests/e2e/federation/tests/messaging/public.spec.ts (1)
575-581: LGTM!Also applies to: 602-608, 640-646, 663-676, 709-715, 732-745
apps/meteor/tests/e2e/message-composer.spec.ts (1)
81-88: LGTM!apps/meteor/tests/e2e/messaging.spec.ts (1)
245-245: LGTM!apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts (1)
123-123: LGTM!apps/meteor/app/theme/client/imports/components/emoji.css (1)
1-45: LGTM!apps/meteor/client/components/Emoji.tsx (1)
22-31: LGTM!apps/meteor/client/lib/customEmoji.ts (1)
98-130: LGTM!apps/meteor/client/lib/utils/detectEmoji.ts (1)
3-14: LGTM!apps/meteor/client/lib/utils/renderEmoji.ts (1)
24-57: LGTM!apps/meteor/app/emoji/client/emojiParser.ts (1)
23-23: LGTM!Also applies to: 35-59
apps/meteor/client/views/composer/EmojiPicker/EmojiCategoryRow.tsx (1)
49-55: LGTM!packages/ui-client/src/components/EmojiPicker/EmojiPickerPreview.tsx (1)
9-9: LGTM!packages/ui-avatar/src/components/MessageAvatar.tsx (1)
1-2: LGTM!Also applies to: 14-23
Comment on lines
+97
to
+101
| for (const skin of emojiData.skins) { | ||
| if (!skin.tone) continue; | ||
|
|
||
| const toneNum = Array.isArray(skin.tone) ? skin.tone[0] : skin.tone; | ||
| const toneKey = `:${primaryShortcode}_tone${toneNum}:`; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
In emojibase 16.0.0, can Emoji.skins[].tone be an array for mixed-tone variants, and how are those variants represented?
💡 Result:
In Emojibase 16.0.0, the Emoji.tone property can indeed be an array to support multi-person skin tones [1][2][3]. For emojis representing a single person, Emoji.tone is a single number (1–5) corresponding to the standard skin tones (1: light, 2: medium-light, 3: medium, 4: medium-dark, 5: dark) [3][4]. For emojis that support mixed-tone variants (such as a handshake between two people), the tone property is represented as an array of these numbers (e.g., [1, 5]) [3][5]. Regarding the Emoji.skins array, it acts as a collection of variant emoji objects for a given base emoji [6][3]. Because this array includes both single and multi-person skin tone variations, it may contain more than the traditional five variations (sometimes up to 15 or more) [1][6]. These variants are stored as fully defined emoji objects within the skins array, each containing their own metadata, including their specific tone property (which will be an array for mixed-tone variants) [6][3].
Citations:
- 1: https://github.com/milesj/emojibase/blob/master/packages/core/CHANGELOG.md
- 2: https://github.com/milesj/emojibase/blob/master/packages/data/CHANGELOG.md
- 3: https://cdn.jsdelivr.net/npm/emojibase@15.3.1/src/types.ts
- 4: https://github.com/milesj/emojibase/blob/master/packages/core/src/constants.ts
- 5: https://context7.com/milesj/emojibase/llms.txt
- 6: https://emojibase.dev/docs/spec/
🏁 Script executed:
sed -n '1,220p' apps/meteor/app/emoji-native/lib/generateEmojiData.tsRepository: RocketChat/Rocket.Chat
Length of output: 3423
🏁 Script executed:
sed -n '1,220p' apps/meteor/app/emoji-native/lib/generateEmojiData.tsRepository: RocketChat/Rocket.Chat
Length of output: 3423
🏁 Script executed:
sed -n '1,220p' apps/meteor/app/emoji-native/lib/generateEmojiData.tsRepository: RocketChat/Rocket.Chat
Length of output: 3423
🏁 Script executed:
sed -n '1,220p' apps/meteor/app/emoji-native/lib/generateEmojiData.tsRepository: RocketChat/Rocket.Chat
Length of output: 3423
🏁 Script executed:
python3 - <<'PY'
import json, re, pathlib
# Find the package version references.
for path in ["package.json", "apps/meteor/package.json", "apps/meteor/app/emoji-native/package.json", "pnpm-lock.yaml", "package-lock.json", "yarn.lock"]:
p = pathlib.Path(path)
if p.exists():
text = p.read_text(errors="ignore")
if "emojibase" in text:
print(f"== {path} ==")
for m in re.finditer(r'emojibase[^"\n: ]*["\':\s]+([^"\n]+)', text):
print(m.group(0)[:120])
# Inspect the installed/data JSON shape if present in repo.
candidates = [
"node_modules/emojibase-data/en/data.json",
"apps/meteor/app/emoji-native/lib/../../../../node_modules/emojibase-data/en/data.json",
"apps/meteor/app/emoji-native/lib/../../../../packages/data/en/data.json",
]
for c in candidates:
p = pathlib.Path(c)
if p.exists():
data = json.loads(p.read_text())
arrays = []
count = 0
for e in data:
skins = e.get("skins") or []
for s in skins:
tone = s.get("tone")
if isinstance(tone, list):
arrays.append((e.get("hexcode"), s.get("hexcode"), tone))
count += 1
if count >= 20:
break
if count >= 20:
break
print(f"== {c} ==")
print(f"array_tone_count_sample={len(arrays)}")
for item in arrays[:10]:
print(item)
break
PYRepository: RocketChat/Rocket.Chat
Length of output: 499
🏁 Script executed:
python3 - <<'PY'
import json, re, pathlib
# Find the package version references.
for path in ["package.json", "apps/meteor/package.json", "apps/meteor/app/emoji-native/package.json", "pnpm-lock.yaml", "package-lock.json", "yarn.lock"]:
p = pathlib.Path(path)
if p.exists():
text = p.read_text(errors="ignore")
if "emojibase" in text:
print(f"== {path} ==")
for m in re.finditer(r'emojibase[^"\n: ]*["\':\s]+([^"\n]+)', text):
print(m.group(0)[:120])
# Inspect the bundled data shape for skin tone arrays.
candidates = [
"node_modules/emojibase-data/en/data.json",
"apps/meteor/app/emoji-native/node_modules/emojibase-data/en/data.json",
]
for c in candidates:
p = pathlib.Path(c)
if p.exists():
data = json.loads(p.read_text())
arrays = []
for e in data:
for s in e.get("skins") or []:
tone = s.get("tone")
if isinstance(tone, list):
arrays.append((e.get("hexcode"), s.get("hexcode"), tone))
if len(arrays) >= 20:
break
if len(arrays) >= 20:
break
print(f"== {c} ==")
print(f"array_tone_count_sample={len(arrays)}")
for item in arrays[:10]:
print(item)
break
PYRepository: RocketChat/Rocket.Chat
Length of output: 499
Preserve all tone values in mixed-tone skin keys. skin.tone can be an array for mixed-tone variants, so skin.tone[0] collapses distinct variants onto the same :${primaryShortcode}_toneN: key. Encode the full tone tuple in the key or skip mixed-tone skins explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/meteor/app/emoji-native/lib/generateEmojiData.ts` around lines 97 - 101,
The mixed-tone skin key generation in generateEmojiData.ts collapses array-based
skin.tone values by using only the first element, so distinct mixed-tone
variants can overwrite each other. Update the tone key construction in the loop
over emojiData.skins to either encode the full tone tuple from skin.tone into
the key or explicitly skip mixed-tone entries, and keep the single-tone path
intact for the existing primaryShortcode-based keys.
Comment on lines
195
to
201
| const firstAttachment = message.attachments?.length && message.attachments.shift(); | ||
|
|
||
| if (firstAttachment) { | ||
| firstAttachment.description = | ||
| typeof firstAttachment.description === 'string' ? emojione.shortnameToUnicode(firstAttachment.description) : undefined; | ||
| firstAttachment.text = typeof firstAttachment.text === 'string' ? emojione.shortnameToUnicode(firstAttachment.text) : undefined; | ||
| typeof firstAttachment.description === 'string' ? shortnameToUnicode(firstAttachment.description) : undefined; | ||
| firstAttachment.text = typeof firstAttachment.text === 'string' ? shortnameToUnicode(firstAttachment.text) : undefined; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find callers of sendNotification to see if `message` is shared/cloned per receiver
rg -nP -C4 '\bsendNotification\s*\(' apps/meteor/app/lib/serverRepository: RocketChat/Rocket.Chat
Length of output: 1029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the sendNotification implementation and its call chain
FILE=apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
# Show the relevant function signature and surrounding logic
rg -n -C6 'async function sendNotification|function sendNotification|const sendNotification' "$FILE"
# Show the receiver loop and any message construction above it
sed -n '300,410p' "$FILE"
# Look for whether the message object is cloned or recreated before the loop
rg -n -C3 '\bmessage\b.*(clone|structuredClone|Object\.assign|spread)|({\s*\.\.\.message|message\s*=\s*\{)' "$FILE"Repository: RocketChat/Rocket.Chat
Length of output: 4108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
echo "== sendNotification definition =="
sed -n '1,260p' "$FILE" | nl -ba | sed -n '1,260p' | rg -n 'sendNotification|attachments|shift\(\)|shortnameToUnicode|message\.attachments'
echo
echo "== sendNotification call sites =="
rg -n -C3 '\bsendNotification\s*\(' apps/meteor/app/lib/server
echo
echo "== other attachment mutations in file =="
rg -n -C2 'attachments\.(shift|splice|pop|unshift|push)|message\.attachments' "$FILE"Repository: RocketChat/Rocket.Chat
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
echo "== sendNotification definition =="
sed -n '1,260p' "$FILE" | nl -ba | sed -n '1,260p' | rg -n 'sendNotification|attachments|shift\(\)|shortnameToUnicode|message\.attachments'
echo
echo "== sendNotification call sites =="
rg -n -C3 '\bsendNotification\s*\(' apps/meteor/app/lib/server
echo
echo "== other attachment mutations in file =="
rg -n -C2 'attachments\.(shift|splice|pop|unshift|push)|message\.attachments' "$FILE"Repository: RocketChat/Rocket.Chat
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
echo "== relevant sections =="
sed -n '1,260p' "$FILE" | python3 - <<'PY'
import sys
for i, line in enumerate(sys.stdin, 1):
if 40 <= i <= 240:
print(f"{i}:{line}", end="")
PY
echo
echo "== call sites =="
rg -n -C4 '\bsendNotification\s*\(' apps/meteor/app/lib/server
echo
echo "== attachment mutation searches =="
rg -n -C3 'attachments\.(shift|splice|pop|unshift|push)|message\.attachments' "$FILE"Repository: RocketChat/Rocket.Chat
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
python3 - <<'PY'
from pathlib import Path
path = Path("apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts")
lines = path.read_text().splitlines()
def show(start, end, title):
print(f"== {title} ==")
for i in range(start, end + 1):
if 1 <= i <= len(lines):
print(f"{i}:{lines[i-1]}")
show(1, 260, "top section")
print()
show(330, 390, "subscription loop")
PYRepository: RocketChat/Rocket.Chat
Length of output: 10130
Avoid mutating message.attachments here. sendNotification() runs per subscription with the same message object, so shift() removes the first attachment for later receivers. Use a non-mutating read/copy instead (message.attachments?.[0] plus slice(1)).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts` around lines
195 - 201, Avoid mutating the shared attachments array in
sendNotificationsOnMessage when handling the first attachment. In
sendNotification(), replace the use of message.attachments.shift() with a
non-mutating read/copy so the same message object can be reused across
subscriptions without losing attachments; use the first-attachment handling
around firstAttachment and keep message.attachments unchanged for later
receivers.
Comment on lines
+36
to
+54
| const renderEmojiToDataURL = (codePoints: string): string => { | ||
| const emoji = String.fromCodePoint(...codePoints.split('-').map((codePoint) => parseInt(codePoint, 16))); | ||
|
|
||
| const canvas = document.createElement('canvas'); | ||
| canvas.width = EMOJI_RENDER_SIZE; | ||
| canvas.height = EMOJI_RENDER_SIZE; | ||
|
|
||
| const ctx = canvas.getContext('2d'); | ||
| if (!ctx) { | ||
| return ''; | ||
| } | ||
|
|
||
| ctx.textBaseline = 'middle'; | ||
| ctx.textAlign = 'center'; | ||
| ctx.font = `${Math.floor(EMOJI_RENDER_SIZE * 0.8)}px "Apple Color Emoji", "Segoe UI Emoji", "Noto Color Emoji", "Noto Emoji", sans-serif`; | ||
| ctx.fillText(emoji, EMOJI_RENDER_SIZE / 2, EMOJI_RENDER_SIZE / 2); | ||
|
|
||
| return canvas.toDataURL('image/png'); | ||
| }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against malformed codepoints in renderEmojiToDataURL.
String.fromCodePoint(...) throws RangeError for NaN/out-of-range values, and parseInt returns NaN for unexpected input. A single malformed codepoint string would throw inside react-pdf's emoji builder and could abort PDF generation. A small validity check keeps export resilient.
🛡️ Proposed guard
const renderEmojiToDataURL = (codePoints: string): string => {
- const emoji = String.fromCodePoint(...codePoints.split('-').map((codePoint) => parseInt(codePoint, 16)));
+ const points = codePoints.split('-').map((codePoint) => parseInt(codePoint, 16));
+ if (points.some((point) => Number.isNaN(point) || point < 0 || point > 0x10ffff)) {
+ return '';
+ }
+ const emoji = String.fromCodePoint(...points);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const renderEmojiToDataURL = (codePoints: string): string => { | |
| const emoji = String.fromCodePoint(...codePoints.split('-').map((codePoint) => parseInt(codePoint, 16))); | |
| const canvas = document.createElement('canvas'); | |
| canvas.width = EMOJI_RENDER_SIZE; | |
| canvas.height = EMOJI_RENDER_SIZE; | |
| const ctx = canvas.getContext('2d'); | |
| if (!ctx) { | |
| return ''; | |
| } | |
| ctx.textBaseline = 'middle'; | |
| ctx.textAlign = 'center'; | |
| ctx.font = `${Math.floor(EMOJI_RENDER_SIZE * 0.8)}px "Apple Color Emoji", "Segoe UI Emoji", "Noto Color Emoji", "Noto Emoji", sans-serif`; | |
| ctx.fillText(emoji, EMOJI_RENDER_SIZE / 2, EMOJI_RENDER_SIZE / 2); | |
| return canvas.toDataURL('image/png'); | |
| }; | |
| const renderEmojiToDataURL = (codePoints: string): string => { | |
| const points = codePoints.split('-').map((codePoint) => parseInt(codePoint, 16)); | |
| if (points.some((point) => Number.isNaN(point) || point < 0 || point > 0x10ffff)) { | |
| return ''; | |
| } | |
| const emoji = String.fromCodePoint(...points); | |
| const canvas = document.createElement('canvas'); | |
| canvas.width = EMOJI_RENDER_SIZE; | |
| canvas.height = EMOJI_RENDER_SIZE; | |
| const ctx = canvas.getContext('2d'); | |
| if (!ctx) { | |
| return ''; | |
| } | |
| ctx.textBaseline = 'middle'; | |
| ctx.textAlign = 'center'; | |
| ctx.font = `${Math.floor(EMOJI_RENDER_SIZE * 0.8)}px "Apple Color Emoji", "Segoe UI Emoji", "Noto Color Emoji", "Noto Emoji", sans-serif`; | |
| ctx.fillText(emoji, EMOJI_RENDER_SIZE / 2, EMOJI_RENDER_SIZE / 2); | |
| return canvas.toDataURL('image/png'); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsx`
around lines 36 - 54, `renderEmojiToDataURL` currently assumes every split code
point is valid, but `parseInt` can produce `NaN` and `String.fromCodePoint` will
throw on malformed input. Add a small validation step before building the emoji
string, and return an empty string early when any code point is invalid so the
PDF export path stays resilient. Use the `renderEmojiToDataURL` helper and its
`codePoints` parsing logic as the fix point.
Comment on lines
+30
to
+37
| if (emoji.skins) { | ||
| for (const skin of emoji.skins) { | ||
| const skinCodes = getShortcodes(skin.hexcode); | ||
| if (skinCodes.length > 0) { | ||
| _unicodeToShort.set(skin.emoji, `:${skinCodes[0]}:`); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'tone[1-5]|skin' ee/packages/federation-matrix/src apps/meteor/app/emoji-native -C2Repository: RocketChat/Rocket.Chat
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== emojiConverter.ts ==\n'
wc -l ee/packages/federation-matrix/src/utils/emojiConverter.ts
cat -n ee/packages/federation-matrix/src/utils/emojiConverter.ts | sed -n '1,220p'
printf '\n== FederationMatrix.ts reaction references ==\n'
rg -n "sendReaction|removeReaction|shortnameToUnicode|unicodeToShort|getShortcodes|tone[1-5]|skin" ee/packages/federation-matrix/src/FederationMatrix.ts ee/packages/federation-matrix/src -C 2
printf '\n== emoji-native reaction shortcode formats ==\n'
rg -n "_tone[1-5]|skin_tone|tone[1-5]" apps/meteor/app/emoji-native -C 2 --max-count 80Repository: RocketChat/Rocket.Chat
Length of output: 23864
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== emojiConverter.ts ==\n'
wc -l ee/packages/federation-matrix/src/utils/emojiConverter.ts
cat -n ee/packages/federation-matrix/src/utils/emojiConverter.ts | sed -n '1,220p'
printf '\n== FederationMatrix.ts reaction references ==\n'
rg -n "sendReaction|removeReaction|shortnameToUnicode|unicodeToShort|getShortcodes|tone[1-5]|skin" ee/packages/federation-matrix/src/FederationMatrix.ts ee/packages/federation-matrix/src -C 2
printf '\n== emoji-native reaction shortcode formats ==\n'
rg -n "_tone[1-5]|skin_tone|tone[1-5]" apps/meteor/app/emoji-native -C 2 --max-count 80Repository: RocketChat/Rocket.Chat
Length of output: 23864
Add skin-tone entries to shortnameToUnicode. RC uses skin-tone shortnames like :<base>_toneN:, but this code only registers them in _unicodeToShort, so sendReaction/removeReaction can forward the raw shortcode to Matrix. Add every skinCodes entry to _shortToUnicode, not just the first one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ee/packages/federation-matrix/src/utils/emojiConverter.ts` around lines 30 -
37, The emoji skin-tone mapping in emojiConverter is only populating
_unicodeToShort for each skin, so shortnames like :<base>_toneN: are not
recognized by shortnameToUnicode and can be forwarded raw; update the
emoji.skins handling in the converter to register every value from
getShortcodes(skin.hexcode) in _shortToUnicode, while keeping the existing
unicode-to-short mapping behavior intact.
gabriellsh
requested changes
Jun 26, 2026
gabriellsh
left a comment
gabriellsh
left a comment
Member
There was a problem hiding this comment.
Emoji size in the composer autocomplete got smaller:
Before:
After:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace Emojione with Native Unicode Emojis
Summary
This PR removes the dependency on Emojione and transitions Rocket.Chat to use native Unicode emojis across the platform. All emojis will now be stored and processed as standard Unicode characters directly in the database.
Why this change
Emojione introduced an additional abstraction layer (shortcodes + image mapping) that increases complexity, impacts performance, and limits interoperability with modern protocols.
By adopting native Unicode emojis, we align Rocket.Chat with current standards used across messaging platforms and protocols.
Performance Improvements
Improved Compatibility (Matrix & Open Protocols)
Developer & Maintenance Benefits
Migration Notes
Result
A simpler, faster, and more interoperable emoji system that:
CORE-1980
Summary by CodeRabbit