Skip to content

feat(emoji): update emoji picker to latest version#39217

Closed
NETIZEN-11 wants to merge 3 commits intoRocketChat:developfrom
NETIZEN-11:nitish-first-pr
Closed

feat(emoji): update emoji picker to latest version#39217
NETIZEN-11 wants to merge 3 commits intoRocketChat:developfrom
NETIZEN-11:nitish-first-pr

Conversation

@NETIZEN-11
Copy link
Copy Markdown

@NETIZEN-11 NETIZEN-11 commented Mar 1, 2026

Proposed changes

This PR upgrades the emoji-toolkit dependency to version 10.0.0 to support the latest Unicode emojis in Rocket.Chat.

Changes included:

  • Updated emoji-toolkit package to v10.0.0
  • Removed outdated/legacy emoji-related dependencies
  • Updated related configuration and package files
  • Adjusted federation and PDF worker dependencies where required

Issue(s)

This PR addresses the issue of outdated emoji support in the current version.

Steps to test or reproduce

  1. Run the application
  2. Open the emoji picker
  3. Verify that latest Unicode emojis are available
  4. Ensure emoji rendering works correctly across client and server

Further comments

This update improves emoji compatibility and ensures Rocket.Chat stays aligned with the latest Unicode standards.

@NETIZEN-11 NETIZEN-11 requested a review from a team as a code owner March 1, 2026 10:46
@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot bot commented Mar 1, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 1, 2026

⚠️ No Changeset found

Latest commit: 81ef46f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 1, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ netizen
❌ NETIZEN-11


netizen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 1, 2026

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (1)
  • apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07e70179-d766-4b55-9006-ae70a4f5c6e1

📥 Commits

Reviewing files that changed from the base of the PR and between c1fd759 and 81ef46f.

📒 Files selected for processing (1)
  • apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This pull request migrates the emoji library dependency from emojione to emoji-toolkit across the entire codebase. Changes include updating imports and function calls in multiple modules, rewriting the emoji index generation script to support CDN-based asset caching, updating TypeScript type definitions, and regenerating emoji picker data with organized categorization. Package dependencies are updated accordingly, and documentation is revised to reflect the new emoji generation workflow.

Changes

Cohort / File(s) Summary
Emoji Library Migration - Core Modules
apps/meteor/app/emoji/client/lib.ts, apps/meteor/app/emoji-emojione/server/callbacks.ts, apps/meteor/app/emoji-emojione/server/lib.ts, apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts, apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts, ee/packages/federation-matrix/src/FederationMatrix.ts, ee/packages/federation-matrix/src/events/reaction.ts
Replaced emojione imports and shortnameToUnicode/toImage calls with emoji-toolkit equivalents. Updated rendering functions and Unicode conversion logic across notification handlers, message processing, and federation features.
Emoji Configuration & Type Definitions
apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts, apps/meteor/definition/externals/emoji-toolkit.d.ts
Simplified emoji config to use emoji-toolkit base instead of emojione module; removed legacy helpers and isEmojiSupported. Added TypeScript ambient module declarations for emoji-toolkit with 7 exported functions and emojiList property.
Emoji Index Generation Script
apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs
Rewrote script to use Node ESM with dynamic module resolution, CDN-based asset caching with automatic directory creation, async sprite generation, and scriptDir-relative output paths. Replaced local asset dependency with cached PNG downloads from JoyPixels CDN.
Documentation
README.md, apps/meteor/app/emoji-emojione/README.md
Updated README to reflect emoji-toolkit usage. Changed emoji generation workflow documentation to describe CDN-based PNG caching, sprite sheet building, and CSS output location (emojione-sprites.css). Removed manual grunt/sass sprite steps; added offline rebuild notes.
Package Configuration
apps/meteor/package.json, ee/packages/federation-matrix/package.json, ee/packages/pdf-worker/package.json
Upgraded emoji-toolkit from ^7.0.1 to ^10.0.0. Removed emojione and @types/emojione dependencies. Added node-sprite-generator and @types/node-sprite-generator devDependencies for sprite generation.
Generated Emoji Data
emojiPicker.js
Generated module exporting emojiCategories (category mapping with i18n names), toneList (tone modifier identifiers), and emojisByCategory (comprehensive emoji-to-category organization across 10 categories).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

type: feature

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objective of the changeset: updating the emoji picker library from emojione to emoji-toolkit and generating new emoji picker data.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added the type: feature Pull requests that introduces new feature label Mar 1, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 17 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/app/emoji-emojione/lib/generateEmojiIndex.mjs">

<violation number="1" location="apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs:12">
P2: `new URL(import.meta.url).pathname` yields URL-encoded paths and an extra leading slash on Windows, which can make `scriptDir` invalid. Use `fileURLToPath(import.meta.url)` before `path.dirname` to get a correct filesystem path across platforms.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

const assetFolder = '../../../node_modules/emojione-assets';
const emojiJsonFile = `${assetFolder}/emoji.json`;
// helper to resolve paths relative to this script file
const scriptDir = path.dirname(new URL(import.meta.url).pathname);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 1, 2026

Choose a reason for hiding this comment

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

P2: new URL(import.meta.url).pathname yields URL-encoded paths and an extra leading slash on Windows, which can make scriptDir invalid. Use fileURLToPath(import.meta.url) before path.dirname to get a correct filesystem path across platforms.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs, line 12:

<comment>`new URL(import.meta.url).pathname` yields URL-encoded paths and an extra leading slash on Windows, which can make `scriptDir` invalid. Use `fileURLToPath(import.meta.url)` before `path.dirname` to get a correct filesystem path across platforms.</comment>

<file context>
@@ -6,31 +6,47 @@
-const assetFolder = '../../../node_modules/emojione-assets';
-const emojiJsonFile = `${assetFolder}/emoji.json`;
+// helper to resolve paths relative to this script file
+const scriptDir = path.dirname(new URL(import.meta.url).pathname);
+
+// Node 18+ has global fetch; if not available uncomment next line
</file context>
Fix with Cubic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs (3)

121-125: ⚠️ Potential issue | 🟡 Minor

Output file written to current working directory instead of script directory.

emojiPicker.js is written relative to cwd, which may not be the intended location when the script is run from a different directory. Other outputs use scriptDir-relative paths.

🔧 Proposed fix for consistency
-	fs.writeFileSync('emojiPicker.js', output, {
+	fs.writeFileSync(path.resolve(scriptDir, 'emojiPicker.js'), output, {
 		encoding: 'utf8',
 		flag: 'w',
 	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs` around lines 121 -
125, The script writes emojiPicker.js to the current working directory via
fs.writeFileSync('emojiPicker.js', output...), which can be wrong when invoked
from elsewhere; update the call in generateEmojiIndex.mjs to write to the
scriptDir-relative path (e.g. use path.join or path.resolve with the existing
scriptDir variable) so that fs.writeFileSync writes to path.join(scriptDir,
'emojiPicker.js') using the same encoding/flag and the same output variable.

88-88: ⚠️ Potential issue | 🔴 Critical

Logic error: condition always evaluates to true.

category !== 'modifier' || category !== 'regional' is always true because a category cannot be both 'modifier' and 'regional' simultaneously. Use && for the intended exclusion logic.

🐛 Proposed fix
-			if (category !== 'modifier' || category !== 'regional') {
+			if (category !== 'modifier' && category !== 'regional') {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs` at line 88, The
conditional using "category !== 'modifier' || category !== 'regional'" always
evaluates to true; update the logic to exclude both categories by replacing the
OR with AND so the check reads that category is not 'modifier' AND not
'regional' (i.e., use category !== 'modifier' && category !== 'regional')
wherever this filter/branch uses the variable category to skip modifier and
regional emoji entries.

96-105: ⚠️ Potential issue | 🔴 Critical

Incorrect iteration over array and inverted includes check.

for (let tone in toneList) iterates over array indices (0, 1, 2...), not values. Additionally, toneList[tone].includes(needsQuotes) checks if the string includes an array, which will always be false.

🐛 Proposed fix
 	const needsQuotes = ['-'];
 	output += `export const toneList = {\n`;
-	for (let tone in toneList) {
-		if (toneList[tone].includes(needsQuotes)) {
-			output += `\t'${toneList[tone]}': 1,\n`;
+	for (const tone of toneList) {
+		if (needsQuotes.some((char) => tone.includes(char))) {
+			output += `\t'${tone}': 1,\n`;
 		} else {
-			output += `\t${toneList[tone]}: 1,\n`;
+			output += `\t${tone}: 1,\n`;
 		}
 	}
 	output += `};\n`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs` around lines 96 -
105, The loop incorrectly uses "for (let tone in toneList)" which iterates
indices and also calls string.includes on an array; change to iterate values
(e.g., "for (const tone of toneList)") and flip the includes check to test
membership in needsQuotes (e.g., "if (needsQuotes.includes(tone))"). Use the
iterated value "tone" when emitting the key (quote it when needsQuotes contains
it, otherwise emit unquoted), keeping the surrounding output variable logic
(output, toneList, needsQuotes) intact.
ee/packages/federation-matrix/src/FederationMatrix.ts (1)

467-471: ⚠️ Potential issue | 🔴 Critical

Critical: Missing function call and undefined variable userMui at lines 467-471.

Lines 467-471 show orphan arguments without a function call wrapper. The federationSDK.sendReaction(...) call is missing, and the userMui variable is used at line 470 without being defined in this scope. Additionally, the eventId variable used at line 473 is never assigned.

The code structure is broken and will fail at runtime. Compare with the similar pattern at lines 502-520 where userMui is properly defined and the function call is complete.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ee/packages/federation-matrix/src/FederationMatrix.ts` around lines 467 -
471, Wrap the orphaned arguments into a call to federationSDK.sendReaction and
ensure the missing local variables are assigned: compute const eventId =
eventIdSchema.parse(matrixEventId) and const userMui =
userIdSchema.parse(userId) (or use the same user-id source as the similar block
at lines 502-520) then call federationSDK.sendReaction(eventId,
emojiToolkit.shortnameToUnicode(reaction), userMui). This restores the complete
call sequence (federationSDK.sendReaction) and removes the undefined
userMui/eventId usage.
🧹 Nitpick comments (4)
apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts (1)

5-9: Remove implementation comments from getEmojiConfig.ts.

The added explanatory comments should be removed to match repository TS/JS implementation style.

As per coding guidelines: **/*.{ts,tsx,js}Avoid code comments in the implementation.

Also applies to: 12-13, 27-27

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts` around lines 5 - 9,
Remove the explanatory implementation comments in getEmojiConfig.ts so the file
follows the repo style of minimal/no inline implementation comments;
specifically delete the multi-line comment block at the top and the other
comment lines around the exported getEmojiConfig/emoji.packages.emojione
configuration (the commented lines referenced in the review), leaving only the
code that exposes the functions and preserves the exact configuration shape
expected by the rest of the system (keep getEmojiConfig and the
emoji.packages.emojione export intact and unchanged except for removing the
comments).
apps/meteor/app/emoji-emojione/server/lib.ts (1)

21-22: Remove newly added implementation comments in this TS module.

Please keep this section self-descriptive in code and drop the added explanatory comments.

As per coding guidelines: **/*.{ts,tsx,js}Avoid code comments in the implementation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/server/lib.ts` around lines 21 - 22, Remove
the added implementation comments in the module that explain the toolkit export
(the comment referencing "emojiList") so the code remains self-descriptive;
locate the comment block in lib.ts near the emojiList export/usage and delete
those explanatory lines, leaving the export/assignment and any surrounding code
unchanged and functional.
apps/meteor/definition/externals/emoji-toolkit.d.ts (1)

1-9: Align declaration exports with default import usage for clarity.

The declaration currently models named exports, but all consumers use import emojiToolkit from 'emoji-toolkit'. While esModuleInterop: true in tsconfig mitigates this mismatch at compile time, the declaration should explicitly model the actual import pattern for clarity and type safety without relying on compiler interop settings.

Align the declaration to export a default interface rather than scattered named exports:

Proposed typing alignment
 declare module 'emoji-toolkit' {
-    export function toShort(input: string): string;
-    export function toImage(input: string): string;
-    export function shortnameToUnicode(input: string): string;
-    export function shortnameToImage(input: string): string;
-    export function unicodeToImage(input: string): string;
-    export function convert(input: string): string;
-    export const emojiList: Record<string, any>;
+    export type EmojiEntry = {
+        shortnames?: string[];
+        [key: string]: unknown;
+    };
+
+    interface EmojiToolkit {
+        toShort(input: string): string;
+        toImage(input: string): string;
+        shortnameToUnicode(input: string): string;
+        shortnameToImage(input: string): string;
+        unicodeToImage(input: string): string;
+        convert(input: string): string;
+        emojiList: Record<string, EmojiEntry>;
+    }
+
+    const emojiToolkit: EmojiToolkit;
+    export default emojiToolkit;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/definition/externals/emoji-toolkit.d.ts` around lines 1 - 9, The
current declaration for module 'emoji-toolkit' exposes named exports (toShort,
toImage, shortnameToUnicode, shortnameToImage, unicodeToImage, convert,
emojiList) but consumers import the module as a default (import emojiToolkit
from 'emoji-toolkit'); update the declaration to export a default
object/interface that contains those methods and properties instead of named
exports so the types match runtime usage and do not rely on
esModuleInterop—i.e., replace the named export signatures with a single default
export type (or interface) that includes toShort, toImage, shortnameToUnicode,
shortnameToImage, unicodeToImage, convert, and emojiList.
apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs (1)

139-157: CDN fetch lacks timeout and retry logic.

Network requests to the CDN have no timeout, and failed downloads only log a warning without retry. This could cause the script to hang or produce incomplete sprite sheets silently.

♻️ Consider adding timeout and basic retry
 		for (const emoji of emojis) {
 			const code = emoji.code_points.base;
 			const localPath = `${tmpFolder}/${code}.png`;
 			if (!fs.existsSync(localPath)) {
 				const url = `https://cdn.jsdelivr.net/npm/emoji-toolkit@latest/assets/64/${code}.png`;
 				console.log(`fetching ${url}`);
-				const res = await fetch(url);
+				const controller = new AbortController();
+				const timeout = setTimeout(() => controller.abort(), 10000);
+				const res = await fetch(url, { signal: controller.signal }).finally(() => clearTimeout(timeout));
 				if (res.ok) {
 					const buf = await res.arrayBuffer();
 					fs.writeFileSync(localPath, Buffer.from(buf));
 				} else {
-					console.warn(`failed to download ${code} (${res.status})`);
+					console.error(`failed to download ${code} (${res.status})`);
 				}
 			}
 			srcList.push(localPath);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs` around lines 139 -
157, The CDN fetch loop in generateEmojiIndex.mjs (inside the for (const emoji
of emojis) loop) uses fetch(url) with no timeout or retry and only logs
warnings; add an AbortController-based timeout around the fetch call and
implement a small retry loop (e.g., 2–3 attempts with a short backoff) for the
fetch to url before giving up, wrapping the fetch/arrayBuffer/fs.writeFileSync
sequence in try/catch; on final failure, make the code fail loudly (throw or
exit) instead of silently continuing so srcList/ sprite generation cannot
proceed with missing files. Ensure you reference the same localPath and code
variables and preserve the existing behavior of writing Buffer.from(buf) when a
fetch succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs`:
- Line 34: generateEmojiPicker is an async function but is invoked without
awaiting or handling its returned promise, risking unhandled rejections; update
the call to either await generateEmojiPicker(emojiJson) from an async context or
wrap it in a promise chain/generic handler (e.g.,
generateEmojiPicker(emojiJson).catch(err => processLogger.error(...))) or
execute it inside an async IIFE so any thrown errors are caught and logged —
ensure you reference the generateEmojiPicker function and handle/await its
returned promise accordingly.

In `@apps/meteor/app/emoji-emojione/README.md`:
- Around line 6-8: The fenced code block showing the command to run
generateEmojiIndex.mjs lacks a language tag; update the block to include a
shell/bash language identifier (e.g., replace ``` with ```shell or ```bash) so
the line "node --experimental-modules generateEmojiIndex.mjs" is
syntax-highlighted and accessible.

In `@apps/meteor/app/emoji-emojione/server/callbacks.ts`:
- Line 9: The callback currently calls emojiToolkit.shortnameToUnicode(message)
without guarding input; update the before-send callback (the (message) => ...
handler) to first check that the input is a string (e.g., typeof message ===
'string') and only then call emojiToolkit.shortnameToUnicode(message), otherwise
return the original value (or an empty string) so shortnameToUnicode doesn't
receive undefined and throw.

In `@apps/meteor/package.json`:
- Line 196: The type file apps/meteor/definition/externals/emoji-toolkit.d.ts
should be changed to reflect that the module exports a default joypixels object
(not named exports); replace the current named export declarations with a
default export type/interface (e.g., interface JoyPixels { toShort(input:
string): string; toImage(input: string, options?: any): string;
shortnameToUnicode(input: string): string; /* emojiList?: any; */ }) and then
export default const joypixels: JoyPixels; — remove or make emojiList optional
unless you confirm it's used elsewhere.

In `@emojiPicker.js`:
- Around line 1-17: In generateEmojiIndex.mjs fix the always-true category check
by replacing the condition "category !== 'modifier' || category !== 'regional'"
with "category !== 'modifier' && category !== 'regional'"; inspect the tone-list
logic around "toneList[tone].includes(needsQuotes)" and correct the
variable/scope so the includes call uses the actual item to test (e.g., the
emoji or key variable) and not an unrelated/undefined needsQuotes boolean —
ensure needsQuotes is either properly defined earlier or replace it with the
intended identifier (for example emoji/key); update the generator's output path
so it writes the regenerated emojiPicker.js into the directory expected by
getEmojiConfig.ts, then regenerate the file.

In `@README.md`:
- Line 124: Remove the stray HTML practice comment "<!-- Contribution practice
by Nitish -->" from the README by deleting that comment line and ensure no other
test/practice HTML comments remain in the README; update the file so only
production content stays and commit the change.

---

Outside diff comments:
In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs`:
- Around line 121-125: The script writes emojiPicker.js to the current working
directory via fs.writeFileSync('emojiPicker.js', output...), which can be wrong
when invoked from elsewhere; update the call in generateEmojiIndex.mjs to write
to the scriptDir-relative path (e.g. use path.join or path.resolve with the
existing scriptDir variable) so that fs.writeFileSync writes to
path.join(scriptDir, 'emojiPicker.js') using the same encoding/flag and the same
output variable.
- Line 88: The conditional using "category !== 'modifier' || category !==
'regional'" always evaluates to true; update the logic to exclude both
categories by replacing the OR with AND so the check reads that category is not
'modifier' AND not 'regional' (i.e., use category !== 'modifier' && category !==
'regional') wherever this filter/branch uses the variable category to skip
modifier and regional emoji entries.
- Around line 96-105: The loop incorrectly uses "for (let tone in toneList)"
which iterates indices and also calls string.includes on an array; change to
iterate values (e.g., "for (const tone of toneList)") and flip the includes
check to test membership in needsQuotes (e.g., "if
(needsQuotes.includes(tone))"). Use the iterated value "tone" when emitting the
key (quote it when needsQuotes contains it, otherwise emit unquoted), keeping
the surrounding output variable logic (output, toneList, needsQuotes) intact.

In `@ee/packages/federation-matrix/src/FederationMatrix.ts`:
- Around line 467-471: Wrap the orphaned arguments into a call to
federationSDK.sendReaction and ensure the missing local variables are assigned:
compute const eventId = eventIdSchema.parse(matrixEventId) and const userMui =
userIdSchema.parse(userId) (or use the same user-id source as the similar block
at lines 502-520) then call federationSDK.sendReaction(eventId,
emojiToolkit.shortnameToUnicode(reaction), userMui). This restores the complete
call sequence (federationSDK.sendReaction) and removes the undefined
userMui/eventId usage.

---

Nitpick comments:
In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs`:
- Around line 139-157: The CDN fetch loop in generateEmojiIndex.mjs (inside the
for (const emoji of emojis) loop) uses fetch(url) with no timeout or retry and
only logs warnings; add an AbortController-based timeout around the fetch call
and implement a small retry loop (e.g., 2–3 attempts with a short backoff) for
the fetch to url before giving up, wrapping the
fetch/arrayBuffer/fs.writeFileSync sequence in try/catch; on final failure, make
the code fail loudly (throw or exit) instead of silently continuing so srcList/
sprite generation cannot proceed with missing files. Ensure you reference the
same localPath and code variables and preserve the existing behavior of writing
Buffer.from(buf) when a fetch succeeds.

In `@apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts`:
- Around line 5-9: Remove the explanatory implementation comments in
getEmojiConfig.ts so the file follows the repo style of minimal/no inline
implementation comments; specifically delete the multi-line comment block at the
top and the other comment lines around the exported
getEmojiConfig/emoji.packages.emojione configuration (the commented lines
referenced in the review), leaving only the code that exposes the functions and
preserves the exact configuration shape expected by the rest of the system (keep
getEmojiConfig and the emoji.packages.emojione export intact and unchanged
except for removing the comments).

In `@apps/meteor/app/emoji-emojione/server/lib.ts`:
- Around line 21-22: Remove the added implementation comments in the module that
explain the toolkit export (the comment referencing "emojiList") so the code
remains self-descriptive; locate the comment block in lib.ts near the emojiList
export/usage and delete those explanatory lines, leaving the export/assignment
and any surrounding code unchanged and functional.

In `@apps/meteor/definition/externals/emoji-toolkit.d.ts`:
- Around line 1-9: The current declaration for module 'emoji-toolkit' exposes
named exports (toShort, toImage, shortnameToUnicode, shortnameToImage,
unicodeToImage, convert, emojiList) but consumers import the module as a default
(import emojiToolkit from 'emoji-toolkit'); update the declaration to export a
default object/interface that contains those methods and properties instead of
named exports so the types match runtime usage and do not rely on
esModuleInterop—i.e., replace the named export signatures with a single default
export type (or interface) that includes toShort, toImage, shortnameToUnicode,
shortnameToImage, unicodeToImage, convert, and emojiList.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c30636 and c1fd759.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (16)
  • README.md
  • apps/meteor/app/emoji-emojione/README.md
  • apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs
  • apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts
  • apps/meteor/app/emoji-emojione/server/callbacks.ts
  • apps/meteor/app/emoji-emojione/server/lib.ts
  • apps/meteor/app/emoji/client/lib.ts
  • apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
  • apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts
  • apps/meteor/definition/externals/emoji-toolkit.d.ts
  • apps/meteor/package.json
  • ee/packages/federation-matrix/package.json
  • ee/packages/federation-matrix/src/FederationMatrix.ts
  • ee/packages/federation-matrix/src/events/reaction.ts
  • ee/packages/pdf-worker/package.json
  • emojiPicker.js
💤 Files with no reviewable changes (2)
  • ee/packages/federation-matrix/package.json
  • ee/packages/pdf-worker/package.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/app/emoji-emojione/server/callbacks.ts
  • apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
  • emojiPicker.js
  • apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts
  • ee/packages/federation-matrix/src/events/reaction.ts
  • apps/meteor/definition/externals/emoji-toolkit.d.ts
  • apps/meteor/app/emoji/client/lib.ts
  • apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts
  • apps/meteor/app/emoji-emojione/server/lib.ts
  • ee/packages/federation-matrix/src/FederationMatrix.ts
🧠 Learnings (12)
📚 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/app/emoji-emojione/server/callbacks.ts
  • apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
  • apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts
  • ee/packages/federation-matrix/src/events/reaction.ts
  • apps/meteor/definition/externals/emoji-toolkit.d.ts
  • apps/meteor/app/emoji/client/lib.ts
  • apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts
  • apps/meteor/app/emoji-emojione/server/lib.ts
  • ee/packages/federation-matrix/src/FederationMatrix.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/app/emoji-emojione/server/callbacks.ts
  • apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
  • apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts
  • ee/packages/federation-matrix/src/events/reaction.ts
  • apps/meteor/definition/externals/emoji-toolkit.d.ts
  • apps/meteor/app/emoji/client/lib.ts
  • apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts
  • apps/meteor/app/emoji-emojione/server/lib.ts
  • ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.

Applied to files:

  • apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
  • apps/meteor/app/emoji-emojione/server/lib.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.

Applied to files:

  • apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts
  • apps/meteor/definition/externals/emoji-toolkit.d.ts
  • apps/meteor/app/emoji/client/lib.ts
📚 Learning: 2026-01-19T18:17:46.433Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.

Applied to files:

  • apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').

Applied to files:

  • apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts
📚 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.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.

Applied to files:

  • ee/packages/federation-matrix/src/events/reaction.ts
  • ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • ee/packages/federation-matrix/src/events/reaction.ts
  • ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.

Applied to files:

  • ee/packages/federation-matrix/src/events/reaction.ts
  • ee/packages/federation-matrix/src/FederationMatrix.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.ts
  • ee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

  • ee/packages/federation-matrix/src/FederationMatrix.ts
🧬 Code graph analysis (5)
apps/meteor/app/emoji-emojione/server/callbacks.ts (1)
apps/meteor/server/lib/callbacks.ts (1)
  • callbacks (218-226)
emojiPicker.js (1)
apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs (2)
  • toneList (41-41)
  • emojisByCategory (42-42)
apps/meteor/definition/externals/emoji-toolkit.d.ts (1)
apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs (1)
  • emojiList (38-38)
apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs (2)
apps/meteor/definition/externals/emoji-toolkit.d.ts (1)
  • emojiList (8-8)
emojiPicker.js (4)
  • toneList (18-339)
  • toneList (18-339)
  • emojisByCategory (340-2314)
  • emojisByCategory (340-2314)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
ee/packages/federation-matrix/src/events/reaction.ts (1)
  • reaction (9-92)
🪛 markdownlint-cli2 (0.21.0)
apps/meteor/app/emoji-emojione/README.md

[warning] 6-6: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (5)
apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts (1)

194-200: Good defensive conversion guards on notification content.

The updated checks convert only string fields, reducing runtime risk in mixed attachment/message payloads.

apps/meteor/app/emoji/client/lib.ts (1)

2-2: LGTM: renderer migration is clean and minimal.

emojiToolkit.toImage is wired in without altering picker dispatch/control flow.

Also applies to: 17-17

apps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.ts (1)

3-3: LGTM: preview normalization keeps existing safety flow.

Emoji conversion remains followed by markdown filtering and HTML escaping.

Also applies to: 10-10

ee/packages/federation-matrix/src/events/reaction.ts (1)

5-5: LGTM!

The migration from emojione to emoji-toolkit is correctly applied. The toShort() API usage is consistent with emoji-toolkit's interface.

Also applies to: 41-41, 85-85

ee/packages/federation-matrix/src/FederationMatrix.ts (1)

502-502: LGTM!

The emojiToolkit.shortnameToUnicode(reaction) usage in removeReaction is correctly migrated.


const emojiJson = fs.readFileSync(emojiJsonFile, 'utf8');

generateEmojiPicker(emojiJson);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Unhandled promise from async function call.

generateEmojiPicker is an async function but is called without await. Any errors thrown inside will result in an unhandled promise rejection.

🐛 Proposed fix
-generateEmojiPicker(emojiJson);
+generateEmojiPicker(emojiJson).catch((err) => {
+	console.error('Failed to generate emoji picker:', err);
+	process.exit(1);
+});

Or wrap in an async IIFE:

-generateEmojiPicker(emojiJson);
+(async () => {
+	await generateEmojiPicker(emojiJson);
+})();
📝 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
generateEmojiPicker(emojiJson);
generateEmojiPicker(emojiJson).catch((err) => {
console.error('Failed to generate emoji picker:', err);
process.exit(1);
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs` at line 34,
generateEmojiPicker is an async function but is invoked without awaiting or
handling its returned promise, risking unhandled rejections; update the call to
either await generateEmojiPicker(emojiJson) from an async context or wrap it in
a promise chain/generic handler (e.g., generateEmojiPicker(emojiJson).catch(err
=> processLogger.error(...))) or execute it inside an async IIFE so any thrown
errors are caught and logged — ensure you reference the generateEmojiPicker
function and handle/await its returned promise accordingly.

Comment on lines 6 to 8
```
node --experimental-modules generateEmojiIndex.mjs
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add language specification to fenced code block.

The code block is missing a language identifier, which helps with syntax highlighting and accessibility.

📝 Proposed fix
-```
+```shell
 node --experimental-modules generateEmojiIndex.mjs
 ```
📝 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
```
node --experimental-modules generateEmojiIndex.mjs
```
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 6-6: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/README.md` around lines 6 - 8, The fenced code
block showing the command to run generateEmojiIndex.mjs lacks a language tag;
update the block to include a shell/bash language identifier (e.g., replace ```
with ```shell or ```bash) so the line "node --experimental-modules
generateEmojiIndex.mjs" is syntax-highlighted and accessible.

callbacks.add(
'beforeSendMessageNotifications',
(message) => emojione.shortnameToUnicode(message),
(message) => emojiToolkit.shortnameToUnicode(message),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/meteor/app/emoji-emojione/server/callbacks.ts

Repository: RocketChat/Rocket.Chat

Length of output: 501


🏁 Script executed:

# Search for emoji-toolkit package info and version
fd -e "package.json" | xargs grep -l "emoji-toolkit" 2>/dev/null | head -5

Repository: RocketChat/Rocket.Chat

Length of output: 48


🌐 Web query:

emoji-toolkit shortnameToUnicode undefined behavior v10

💡 Result:

In emoji-toolkit v10.0.0, joypixels.shortnameToUnicode(str) is implemented as a string transform: it runs str.replace(...) over a shortname regex and:

So the main “undefined behavior” you can hit is passing a non-string input (e.g. undefined/null)—because the function immediately calls str.replace(...), which will throw a TypeError if str isn’t a string. (raw.githubusercontent.com)

A safe call pattern is:

joypixels.shortnameToUnicode(String(value ?? ""));

Citations:


🏁 Script executed:

# Search for beforeSendMessageNotifications callback invocations
rg "beforeSendMessageNotifications" -A 3 -B 3

Repository: RocketChat/Rocket.Chat

Length of output: 2893


🏁 Script executed:

# Look for callback.add patterns to understand the callback signature
rg "callbacks\.add\(" -A 5 | head -100

Repository: RocketChat/Rocket.Chat

Length of output: 8623


Guard callback input before calling shortnameToUnicode.

The beforeSendMessageNotifications callback receives input from message.msg. If msg is undefined for any message, the callback will receive undefined and cause shortnameToUnicode() to throw a TypeError since the method internally calls .replace() on the input. Add a type guard to safely handle non-string inputs.

Safe guard
-		(message) => emojiToolkit.shortnameToUnicode(message),
+		(message) => (typeof message === 'string' ? emojiToolkit.shortnameToUnicode(message) : message),
📝 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
(message) => emojiToolkit.shortnameToUnicode(message),
(message) => (typeof message === 'string' ? emojiToolkit.shortnameToUnicode(message) : message),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/emoji-emojione/server/callbacks.ts` at line 9, The callback
currently calls emojiToolkit.shortnameToUnicode(message) without guarding input;
update the before-send callback (the (message) => ... handler) to first check
that the input is a string (e.g., typeof message === 'string') and only then
call emojiToolkit.shortnameToUnicode(message), otherwise return the original
value (or an empty string) so shortnameToUnicode doesn't receive undefined and
throw.

"ejson": "^2.2.3",
"emailreplyparser": "^0.0.5",
"emoji-toolkit": "^7.0.1",
"emoji-toolkit": "^10.0.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

emoji-toolkit version 10 API exports documentation

💡 Result:

emoji-toolkit v10 (10.0.0) “API exports” + docs

What the package exports (JS)

  • Entrypoint: lib/js/joypixels.js (this is what you get when you import/require emoji-toolkit).[1]
  • Module shape: the library exposes a single JoyPixels/joypixels object (commonly used as a browser global joypixels when loaded via a <script> tag, and as the returned value from require('emoji-toolkit')). Usage examples in the official docs all call methods as joypixels.<method>(...).[2]

Note: v10’s package.json does not define a modern Node "exports" map; it uses the classic "main" entrypoint instead.[1]

Where the official API documentation is

  • USAGE.md (official): documents configuration/object properties and links to the JS/PHP API demos.[2]
  • JoyPixels Labs demos (official): short pages per function (with a code snippet), e.g.:
    • joypixels.toImage(str)[3]
    • joypixels.shortnameToUnicode(str)[4]

Documented JS API surface (from USAGE.md + demos)

Config / properties (set on the exported joypixels object): emojiVersion, emojiSize, imagePathPNG, fileExtension, greedyMatch, imageTitleTag, sprites, spriteSize, unicodeAlt, ascii, riskyMatchAscii.[2]

Functions called out by the docs/demos include: toShort(str), shortnameToImage(str), toImage(str), shortnameToUnicode(str).[2][3][4]

Sources: [1] package.json (10.0.0) main entrypoint, [2] USAGE.md, [3] toImage demo, [4] shortnameToUnicode demo.


🌐 Web query:

emoji-toolkit npm package v10.0.0 toImage toShort shortnameToUnicode

💡 Result:

emoji-toolkit@10.0.0 (JoyPixels Emoji Toolkit) exposes these JavaScript string conversion helpers on the exported joypixels object: toImage, toShort, and shortnameToUnicode. (raw.githubusercontent.com)

Install (pin v10)

npm i emoji-toolkit@10.0.0

v10.0.0 is published and available via the project release/CDN metadata. (github.com)

Import

// CommonJS
const joypixels = require('emoji-toolkit')

// or ESM (depending on your bundler/runtime)
import joypixels from 'emoji-toolkit'

toShort(str) — Unicode → :shortname:

joypixels.toShort("I ❤️ JS")   // "I :heart: JS"

(“native unicode -> shortnames”) (raw.githubusercontent.com)

shortnameToUnicode(str):shortname: → Unicode

joypixels.shortnameToUnicode("I :heart: JS") // "I ❤️ JS"

(raw.githubusercontent.com)

toImage(str) — mixed (Unicode + :shortname:) → <img ...>

joypixels.toImage("I ❤️ :rocket:") // "I <img ...> <img ...>"

toImage is a shorthand that runs Unicode-to-image and then shortname-to-image conversions. (demos.joypixels.com)

Citations:


🏁 Script executed:

# Find how emoji-toolkit is imported and used in the codebase
rg "emoji-toolkit" --type js --type ts -A 3 -B 3

Repository: RocketChat/Rocket.Chat

Length of output: 6024


🏁 Script executed:

# Check the current package.json to see the version
cat apps/meteor/package.json | grep -A 2 -B 2 "emoji-toolkit"

Repository: RocketChat/Rocket.Chat

Length of output: 192


Fix TypeScript type definitions for emoji-toolkit v10 compatibility.

The required APIs (toShort, toImage, shortnameToUnicode) are present in v10. However, the type definitions in apps/meteor/definition/externals/emoji-toolkit.d.ts incorrectly declare named exports when the actual module exports a default joypixels object with these as methods. Update the type definition to match the actual API structure. Note: emojiList is not documented in the v10 API surface; verify if it's actually used in the codebase.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/package.json` at line 196, The type file
apps/meteor/definition/externals/emoji-toolkit.d.ts should be changed to reflect
that the module exports a default joypixels object (not named exports); replace
the current named export declarations with a default export type/interface
(e.g., interface JoyPixels { toShort(input: string): string; toImage(input:
string, options?: any): string; shortnameToUnicode(input: string): string; /*
emojiList?: any; */ }) and then export default const joypixels: JoyPixels; —
remove or make emojiList optional unless you confirm it's used elsewhere.

Comment on lines +1 to +17
/*
* This file is automatically generated from generateEmojiIndex.mjs
* Last generated Sun Mar 01 2026 10:39:56 GMT+0000 (Coordinated Universal Time)
*
* Mapping category hashes into human readable and translated names
*/

export const emojiCategories = [
{ key: 'people', i18n: 'Smileys_and_People' },
{ key: 'nature', i18n: 'Animals_and_Nature' },
{ key: 'food', i18n: 'Food_and_Drink' },
{ key: 'activity', i18n: 'Activity' },
{ key: 'travel', i18n: 'Travel_and_Places' },
{ key: 'objects', i18n: 'Objects' },
{ key: 'symbols', i18n: 'Symbols' },
{ key: 'flags', i18n: 'Flags' },
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if emojiPicker.js exists elsewhere in the codebase
fd -t f 'emojiPicker.js'

# Check imports/references to understand expected location
rg -l "from.*emojiPicker" --type ts --type js

Repository: RocketChat/Rocket.Chat

Length of output: 133


🏁 Script executed:

#!/bin/bash
# Get the full path of emojiPicker.js to confirm location
find . -name 'emojiPicker.js' -type f

# Check the import statement in getEmojiConfig.ts
rg "from.*emojiPicker" apps/meteor/app/emoji-emojione/lib/getEmojiConfig.ts -A 1 -B 1

Repository: RocketChat/Rocket.Chat

Length of output: 161


🏁 Script executed:

#!/bin/bash
# Find the generateEmojiIndex.mjs file
fd -type f 'generateEmojiIndex.mjs'

# If found, check lines 88 and 98-103 for the mentioned bugs
if [ -f "apps/meteor/app/emoji-emojione/scripts/generateEmojiIndex.mjs" ]; then
  echo "=== Lines 88 and surrounding context ==="
  sed -n '85,95p' apps/meteor/app/emoji-emojione/scripts/generateEmojiIndex.mjs
  echo ""
  echo "=== Lines 98-103 ==="
  sed -n '98,103p' apps/meteor/app/emoji-emojione/scripts/generateEmojiIndex.mjs
fi

Repository: RocketChat/Rocket.Chat

Length of output: 236


🏁 Script executed:

#!/bin/bash
# Find the generateEmojiIndex.mjs file
fd 'generateEmojiIndex.mjs'

# If found, check lines 88 and 98-103 for the mentioned bugs
if [ -f "apps/meteor/app/emoji-emojione/scripts/generateEmojiIndex.mjs" ]; then
  echo "=== Lines 85-95 (around line 88) ==="
  sed -n '85,95p' apps/meteor/app/emoji-emojione/scripts/generateEmojiIndex.mjs
  echo ""
  echo "=== Lines 98-103 ==="
  sed -n '98,103p' apps/meteor/app/emoji-emojione/scripts/generateEmojiIndex.mjs
fi

Repository: RocketChat/Rocket.Chat

Length of output: 123


🏁 Script executed:

#!/bin/bash
# Read the generator file to check the mentioned lines
wc -l apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs

echo "=== Lines 85-95 (around line 88) ==="
sed -n '85,95p' apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs

echo ""
echo "=== Lines 98-103 ==="
sed -n '98,103p' apps/meteor/app/emoji-emojione/lib/generateEmojiIndex.mjs

Repository: RocketChat/Rocket.Chat

Length of output: 621


Verify and fix generator bugs, then move file to correct location.

This file is auto-generated by generateEmojiIndex.mjs. The generator has logic errors that should be addressed:

  • Line 88: Logic condition category !== 'modifier' || category !== 'regional' is always true; should use && instead.
  • Lines 98-103: Questionable logic checking toneList[tone].includes(needsQuotes) with unclear variable scope.

Additionally, the file location is incorrect. It's currently at the repository root (./emojiPicker.js), but getEmojiConfig.ts imports it using a relative path from apps/meteor/app/emoji-emojione/lib/, expecting the file to be in that directory. The generator should output to apps/meteor/app/emoji-emojione/lib/ instead of the current working directory. After fixing the generator bugs, regenerate this file and place it in the correct location.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@emojiPicker.js` around lines 1 - 17, In generateEmojiIndex.mjs fix the
always-true category check by replacing the condition "category !== 'modifier'
|| category !== 'regional'" with "category !== 'modifier' && category !==
'regional'"; inspect the tone-list logic around
"toneList[tone].includes(needsQuotes)" and correct the variable/scope so the
includes call uses the actual item to test (e.g., the emoji or key variable) and
not an unrelated/undefined needsQuotes boolean — ensure needsQuotes is either
properly defined earlier or replace it with the intended identifier (for example
emoji/key); update the generator's output path so it writes the regenerated
emojiPicker.js into the directory expected by getEmojiConfig.ts, then regenerate
the file.


- Emoji provided graciously by [JoyPixels](https://www.joypixels.com).

<!-- Contribution practice by Nitish --> No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove practice comment from production README.

This HTML comment appears to be a test/practice addition that should not be included in the main README.

🧹 Proposed fix
 - Emoji provided graciously by [JoyPixels](https://www.joypixels.com).
-
-<!-- Contribution practice by Nitish -->
📝 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
<!-- Contribution practice by Nitish -->
- Emoji provided graciously by [JoyPixels](https://www.joypixels.com).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 124, Remove the stray HTML practice comment "<!--
Contribution practice by Nitish -->" from the README by deleting that comment
line and ensure no other test/practice HTML comments remain in the README;
update the file so only production content stays and commit the change.

@NETIZEN-11 NETIZEN-11 changed the title Nitish first pr feat(emoji): update emoji picker to latest version Mar 1, 2026
- Use fileURLToPath for proper Windows path resolution (fixes URL encoding issues)
- Make generateEmojiPicker async and properly await all async operations
- Wrap nsg() callback calls in Promises to handle async completion
- Add proper error handling with Promise rejection

Fixes code review issues reported by automated bot
@engelgabriel
Copy link
Copy Markdown
Member

Closing this PR in favor of #39411, where we’re replacing Emojione with native Unicode emojis.

It’s still a draft, and we’d love help from the community to get it across the finish line 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature Pull requests that introduces new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants