chore: drop @rocket.chat/string-helpers in favor of @rocket.chat/tools - #41763
Merged
Conversation
…ools` `@rocket.chat/tools` now embeds `escapeRegExp`, `escapeHTML`, `unescapeHTML` and `capitalize`, so point every consumer at it and remove the external dependency from the workspaces, the lockfile and `fuselage.sh`. Since the embedded signatures are tighter than the originals, this also drops the type assertions that only existed to work around them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: cda66e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
|
| Layer / File(s) | Summary |
|---|---|
Tools utility implementation and exports packages/tools/src/* |
Adds toString, capitalize, escapeHTML, unescapeHTML, and escapeRegExp, with tests and barrel exports. |
Application import migration apps/meteor/app/*, apps/meteor/client/*, packages/ui-client/* |
Redirects client-side and application imports from @rocket.chat/string-helpers to @rocket.chat/tools. |
Server and enterprise import migration apps/meteor/server/*, apps/meteor/ee/*, ee/packages/abac/* |
Redirects server-side and enterprise imports, including test mocks, to @rocket.chat/tools. |
Workspace cleanup and release metadata packages/models/*, */package.json, fuselage.sh, .changeset/* |
Removes obsolete dependencies, simplifies selected type assertions, updates package tooling, and adds release metadata. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | 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 (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the primary change: replacing @rocket.chat/string-helpers with @rocket.chat/tools. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
Warning
Review ran into problems
🔥 Problems
Errors were encountered while retrieving linked issues.
Errors (2)
- ARCH-2361: Request failed with status code 401
- ARCH-2350: Request failed with status code 401
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 @coderabbitai help to get the list of available commands.
sampaiodiego
previously approved these changes
Aug 12, 2026
![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: 2
🤖 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 `@packages/tools/src/toString.ts`:
- Around line 1-2: Update the toString function to convert truthy objects with
String(object) instead of template interpolation, so symbols are handled safely,
and remove the now-unnecessary ESLint disable directive.
In `@packages/tools/src/unescapeHTML.ts`:
- Around line 30-37: Update numeric entity handling in unescapeHTML to parse
decimal and hexadecimal values with Number.parseInt, validate that each code
point is within 0–0x10FFFF, and decode valid values with String.fromCodePoint
instead of String.fromCharCode. In packages/tools/src/unescapeHTML.spec.ts lines
4-31, add regression cases covering decimal and hexadecimal supplementary-plane
entities.
🪄 Autofix
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 Plus
Run ID: 5635c44e-54c8-4160-bc69-4ca5295ab10a
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (132)
.changeset/thick-kings-type.mdapps/meteor/app/emoji-native/lib/getEmojiConfig.tsapps/meteor/app/emoji/client/helpers.tsapps/meteor/app/highlight-words/client/helper.tsapps/meteor/app/markdown/lib/markdown.jsapps/meteor/app/markdown/lib/parser/original/code.jsapps/meteor/app/mentions/lib/MentionsParser.tsapps/meteor/app/utils/lib/getURL.tsapps/meteor/client/components/GazzodownText.tsxapps/meteor/client/components/UserAndRoomAutoCompleteMultiple/UserAndRoomAutoCompleteMultiple.tsxapps/meteor/client/lib/chats/flows/processSlashCommand.tsapps/meteor/client/lib/customEmoji.tsapps/meteor/client/lib/customOAuth/CustomOAuth.tsapps/meteor/client/lib/loginServices.tsapps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.tsapps/meteor/client/lib/utils/normalizeMessagePreview/normalizeMessagePreview.tsapps/meteor/client/navbar/NavBarSearch/hooks/useAISearchRooms.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tsapps/meteor/client/providers/AuthenticationProvider/AuthenticationProvider.tsxapps/meteor/client/providers/TranslationProvider.tsxapps/meteor/client/views/admin/customUserStatus/CustomUserStatusTable/CustomUserStatusTable.tsxapps/meteor/client/views/admin/integrations/IntegrationsTable.tsxapps/meteor/client/views/admin/permissions/helpers/mapPermissionKeys.tsapps/meteor/client/views/admin/settings/groups/OAuthGroupPage/OAuthGroupPage.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/utils/template.spec.tsapps/meteor/client/views/omnichannel/components/outboundMessage/utils/template.tsapps/meteor/client/views/omnichannel/contactHistory/MessageList/useHistoryMessageList.tsapps/meteor/client/views/omnichannel/reports/hooks/useChannelsSection.tsapps/meteor/client/views/room/contextualBar/Discussions/useDiscussionsList.tsapps/meteor/client/views/room/contextualBar/ExportMessages/useExportMessagesAsPDFMutation.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useNormalizedThreadTitleHtml.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.tsapps/meteor/client/views/room/hooks/useBanUser.tsxapps/meteor/client/views/room/hooks/useUnbanUser.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useChangeModeratorAction.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useChangeOwnerAction.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useMuteUserAction.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxapps/meteor/client/views/root/hooks/useIframeCommands.tsapps/meteor/client/views/root/hooks/useOAuthPopupCommands.tsapps/meteor/ee/server/api/lib/canned-responses.jsapps/meteor/ee/server/api/sessions.tsapps/meteor/ee/server/api/v1/omnichannel/lib/monitors.tsapps/meteor/ee/server/api/v1/omnichannel/lib/priorities.tsapps/meteor/ee/server/api/v1/omnichannel/lib/sla.tsapps/meteor/ee/server/api/v1/omnichannel/lib/tags.tsapps/meteor/ee/server/api/v1/omnichannel/lib/units.tsapps/meteor/ee/server/configuration/oauth.tsapps/meteor/ee/server/lib/audit/functions.tsapps/meteor/ee/server/lib/omnichannel/Department.tsapps/meteor/ee/server/lib/omnichannel/business-hour/lib/business-hour.tsapps/meteor/ee/server/models/raw/LivechatRooms.tsapps/meteor/ee/server/services/package.jsonapps/meteor/lib/utils/stringUtils.tsapps/meteor/package.jsonapps/meteor/server/api/lib/users.tsapps/meteor/server/api/v1/chat.tsapps/meteor/server/api/v1/custom-sounds.tsapps/meteor/server/api/v1/custom-user-status.tsapps/meteor/server/api/v1/emoji-custom.tsapps/meteor/server/api/v1/integrations.tsapps/meteor/server/api/v1/misc.tsapps/meteor/server/api/v1/moderation.tsapps/meteor/server/api/v1/omnichannel/contact.tsapps/meteor/server/api/v1/omnichannel/lib/customFields.tsapps/meteor/server/api/v1/omnichannel/lib/departments.tsapps/meteor/server/api/v1/omnichannel/lib/users.tsapps/meteor/server/api/v1/omnichannel/visitors.tsapps/meteor/server/api/v1/teams.tsapps/meteor/server/api/v1/users.tsapps/meteor/server/lib/auth/startup.jsapps/meteor/server/lib/autotranslate/autotranslate.tsapps/meteor/server/lib/dataExport/exportRoomMessagesToFile.tsapps/meteor/server/lib/dataExport/processDataDownloads.tsapps/meteor/server/lib/dataExport/sendViaEmail.tsapps/meteor/server/lib/findUsersOfRoomOrderedByRole.tsapps/meteor/server/lib/messaging/Message.tsapps/meteor/server/lib/notifications/email/api.tsapps/meteor/server/lib/notifications/mail-messages/functions/sendMail.tsapps/meteor/server/lib/notifications/message/email.jsapps/meteor/server/lib/notifications/message/index.tsapps/meteor/server/lib/notifications/message/messageContainsHighlight.tsapps/meteor/server/lib/oauth/addOAuthService.tsapps/meteor/server/lib/oauth/allowPassportOAuthMiddleware.tsapps/meteor/server/lib/oauth/createOAuthServiceConfig.tsapps/meteor/server/lib/parseMessageSearchQuery.tsapps/meteor/server/lib/saml/lib/SAML.tsapps/meteor/server/lib/spotlight.jsapps/meteor/server/lib/ui-master/index.tsapps/meteor/server/lib/users/checkUsernameAvailability.tsapps/meteor/server/lib/users/setEmail.tsapps/meteor/server/lib/utils/lib/getValidRoomName.tsapps/meteor/server/meteor-methods/auth/removeOAuthService.tsapps/meteor/server/meteor-methods/rooms/browseChannels.tsapps/meteor/server/services/team/service.tsapps/meteor/tests/mocks/data/outbound-message.tsapps/meteor/tests/unit/app/markdown/client.tests.jsapps/meteor/tests/unit/server/lib/notifications/message/getEmailContent.spec.tsapps/meteor/tests/unit/server/lib/saml/server.tests.tsapps/meteor/tests/unit/server/services/team/service.tests.tsee/apps/account-service/package.jsonee/apps/authorization-service/package.jsonee/apps/ddp-streamer/package.jsonee/apps/presence-service/package.jsonee/packages/abac/src/index.tsee/packages/abac/src/store/LocalAttributeStore.tsee/packages/omnichannel-services/package.jsonfuselage.shpackages/models/package.jsonpackages/models/src/models/CallHistory.tspackages/models/src/models/LivechatContacts.tspackages/models/src/models/LivechatDepartment.tspackages/models/src/models/LivechatPriority.tspackages/models/src/models/LivechatRooms.tspackages/models/src/models/LivechatVisitors.tspackages/models/src/models/Messages.tspackages/models/src/models/Rooms.tspackages/models/src/models/Subscriptions.tspackages/models/src/models/Users.tspackages/tools/src/capitalize.spec.tspackages/tools/src/capitalize.tspackages/tools/src/escapeHTML.spec.tspackages/tools/src/escapeHTML.tspackages/tools/src/escapeRegExp.spec.tspackages/tools/src/escapeRegExp.tspackages/tools/src/index.tspackages/tools/src/toString.tspackages/tools/src/unescapeHTML.spec.tspackages/tools/src/unescapeHTML.tspackages/ui-client/package.jsonpackages/ui-client/src/views/setupWizard/steps/AdminInfoStep.tsx
💤 Files with no reviewable changes (7)
- apps/meteor/package.json
- ee/apps/account-service/package.json
- apps/meteor/ee/server/services/package.json
- ee/apps/authorization-service/package.json
- ee/packages/omnichannel-services/package.json
- ee/apps/ddp-streamer/package.json
- ee/apps/presence-service/package.json
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #41763 +/- ##
===========================================
- Coverage 68.99% 68.98% -0.01%
===========================================
Files 4219 4224 +5
Lines 165981 166089 +108
Branches 29519 29557 +38
===========================================
+ Hits 114513 114572 +59
- Misses 46313 46365 +52
+ Partials 5155 5152 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 133 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
dougfabris
previously approved these changes
Aug 12, 2026
The spec was copied verbatim from `@rocket.chat/string-helpers`, which
asserted `unescapeHTML(' ')` three times and `unescapeHTML(''')`
twice with identical inputs and expectations. Coverage stays at 100%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tassoevan
dismissed stale reviews from dougfabris and sampaiodiego
via
August 12, 2026 15:29
cda66e7
dougfabris
approved these changes
Aug 12, 2026
sampaiodiego
approved these changes
Aug 12, 2026
tassoevan
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 12, 2026
dougfabris
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
dougfabris
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
Merged
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
3 participants
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.
Proposed changes (including videos or screenshots)
@rocket.chat/string-helperswas a Fuselage package with four exports, all of them tiny pure string functions:escapeRegExp,escapeHTML,unescapeHTMLandcapitalize. This repo already has a home for exactly that kind of function —@rocket.chat/tools— and most of the files importing fromstring-helperswere already importing fromtoolson the line above.So rather than moving the package here as its own workspace package, this PR absorbs it into
@rocket.chat/toolsand deletes the dependency. See Further comments for why this deviates from what ARCH-2361 describes.Two commits do the work, and they are worth reading in order:
feat(tools): embed functions from @rocket.chat/string-helpers— the four functions are copied intopackages/tools/src, each with a spec file. The implementations are the published0.32.0ones, with the types tightened where the originals were loose:capitalizeis now generic, so it returnsCapitalize<T>instead ofstring.escapeHTML/unescapeHTMLnow takestringinstead ofany; the runtime coercion they relied on is kept, moved into a small internaltoStringhelper, so a nullish value passed from untyped JS still becomes''rather than"undefined".escapeRegExpkeeps its behavior exactly (the spec pins the full escaped character set).chore: drop @rocket.chat/string-helpers in favor of @rocket.chat/tools— 122 files, almost entirely one-line import rewrites: 67escapeRegExp, 29escapeHTML, 14capitalize, 2unescapeHTML. The dependency is removed from ninepackage.jsons —apps/meteor,apps/meteor/ee/server/services,packages/models,ee/packages/omnichannel-servicesand the fouree/apps/*services (account,authorization,ddp-streamer,presence) — and@rocket.chat/toolsis added topackages/modelsandpackages/ui-client, the two consumers that did not already depend on it.fuselage.shstops bumping the package, and it is gone fromyarn.lockentirely — it was not a transitive dependency of anything else.Because the embedded signatures are tighter, a handful of type assertions that only existed to work around the old ones could go:
capitalize(this.name) as Capitalize<TServiceName>inCustomOAuth,indexOf(_emoji as never)in the emoji helpers, and similar casts inpackages/models. Those are the only non-import changes in the commit.Issue(s)
ARCH-2361
Steps to test or reproduce
Nothing should change for the user — the functions are the same code, called from the same places.
yarn workspace @rocket.chat/tools test— 14 suites, 118 tests, with the four new files at 100% coverage.yarn workspace @rocket.chat/meteor typecheck— clean. Same for@rocket.chat/models,@rocket.chat/ui-clientand@rocket.chat/abac, the packages whose diffs are more than import rewrites.Further comments
This is not the migration the ticket describes. ARCH-2361 is written as step 8 of 19 in the Fuselage migration: copy
packages/string-helpersin, add the workspaces glob entry, keep the dual ESM/CJS build and the API report, flip consumers toworkspace:^. It also owns repointing the package's npm trusted publisher fromRocketChat/fuselagetoRocketChat/Rocket.Chat.Absorbing the package instead makes all of that unnecessary: there is no new workspace package to build, no API report to maintain, and the npm trusted-publishing repoint can be dropped from the migration plan — this package will never be published again from either repo. The tradeoff is that
@rocket.chat/string-helpersstops existing as a published package after0.32.0. Nothing outside Rocket.Chat and Fuselage consumes it, and Fuselage froze it in ARCH-2350, so I think that is the right call — but it is a plan change, not just an implementation detail, so flagging it explicitly for review. If we would rather keep the package alive as a published artifact, this PR should be replaced with the literal migration.One follow-up is needed, not done here.
apps/meteor/tests/end-to-end/api/http-response-truncation.tsreplays a captured dynamic-import request whose body hardcodes a client module tree containingnode_modules/@rocket.chat/string-helpers. That path no longer exists in the bundle. The test's own comment warns that these fixtures are build-dependent — if the tree changes, the response shrinks below the gzip threshold and loses the chunked+gzip shape the race needs, which is what its first assertion checks. The fixture has to be recaptured. I left it alone because it only runs under the specific traefik configuration it was written for, and recapturing it means reproducing that setup.Summary by CodeRabbit
New Features
Refactor
Tests