chore(ui-voip): Review build configuration #37585
Conversation
|
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 |
|
WalkthroughThe PR modernizes the ui-voip package build configuration by removing Babel in favor of SWC, simplifying ESLint rules, converting value imports to type-only imports throughout the codebase for tree-shaking optimization, renaming interfaces to follow naming conventions (IPointCoordinates, IBaseSession), and adding the Faker library to mock-providers' devDependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 12:30 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 6 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 24 Nov 2025 12:30:46 GMT |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37585 +/- ##
===========================================
+ Coverage 68.86% 68.88% +0.02%
===========================================
Files 3360 3360
Lines 114339 114327 -12
Branches 20560 20558 -2
===========================================
+ Hits 78741 78758 +17
+ Misses 33502 33485 -17
+ Partials 2096 2084 -12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
1-1: Type-onlyUserStatusimport is right; consider tightening autocomplete status typing
- Switching
UserStatustoimport typematches its usage and has no runtime impact.- Minor nit:
status: localInfo.status as UserStatussuggestsPeerAutocompleteOptions['status']isn’t typed asUserStatusyet. If feasible, updatingPeerAutocompleteOptionssostatusis alreadyUserStatus | undefinedwould let you drop this cast and keep the typing flow safer.Also applies to: 198-203
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (26)
packages/mock-providers/package.json(1 hunks)packages/ui-voip/.babelrc.json(0 hunks)packages/ui-voip/.eslintignore(0 hunks)packages/ui-voip/.eslintrc.json(1 hunks)packages/ui-voip/.storybook/main.ts(1 hunks)packages/ui-voip/package.json(2 hunks)packages/ui-voip/src/components/VoipPopupDraggable/DraggableCore.ts(15 hunks)packages/ui-voip/src/hooks/useDevicePermissionPrompt.spec.tsx(1 hunks)packages/ui-voip/src/v2/MediaCallContext.ts(1 hunks)packages/ui-voip/src/v2/MediaCallLogger.ts(1 hunks)packages/ui-voip/src/v2/MediaCallProvider.tsx(1 hunks)packages/ui-voip/src/v2/MockedMediaCallProvider.tsx(1 hunks)packages/ui-voip/src/v2/components/ActionButton.tsx(1 hunks)packages/ui-voip/src/v2/components/PeerAutocomplete.tsx(1 hunks)packages/ui-voip/src/v2/components/PeerInfo/InternalUser.tsx(1 hunks)packages/ui-voip/src/v2/components/PeerInfo/PeerInfo.tsx(1 hunks)packages/ui-voip/src/v2/components/ToggleButton.tsx(1 hunks)packages/ui-voip/src/v2/components/Widget/Widget.tsx(1 hunks)packages/ui-voip/src/v2/components/Widget/WidgetDraggableContext.ts(1 hunks)packages/ui-voip/src/v2/components/Widget/WidgetHandle.tsx(1 hunks)packages/ui-voip/src/v2/useDesktopNotifications.ts(1 hunks)packages/ui-voip/src/v2/useInfoSlots.ts(1 hunks)packages/ui-voip/src/v2/useKeypad.tsx(1 hunks)packages/ui-voip/src/v2/useMediaCallAction.ts(1 hunks)packages/ui-voip/src/v2/useMediaSession.ts(1 hunks)packages/ui-voip/src/v2/useMediaSessionInstance.ts(2 hunks)
💤 Files with no reviewable changes (2)
- packages/ui-voip/.babelrc.json
- packages/ui-voip/.eslintignore
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
packages/ui-voip/src/v2/MediaCallContext.ts
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
packages/ui-voip/src/v2/components/ToggleButton.tsxpackages/ui-voip/src/v2/components/ActionButton.tsxpackages/ui-voip/src/v2/components/Widget/WidgetHandle.tsx
📚 Learning: 2025-11-16T14:23:18.933Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37496
File: packages/apps-engine/src/server/runtime/deno/ProcessMessenger.ts:5-5
Timestamp: 2025-11-16T14:23:18.933Z
Learning: In TypeScript, when a constant is used with `typeof` in a type position (e.g., `type Message = SomeType | typeof SOME_CONSTANT`), it's a type-level operation that doesn't require the runtime value. In such cases, `import type { SOME_CONSTANT }` is correct and appropriate.
Applied to files:
packages/ui-voip/src/v2/useInfoSlots.ts
🧬 Code graph analysis (1)
packages/ui-voip/src/v2/useMediaSessionInstance.ts (1)
packages/ui-voip/src/v2/MediaCallContext.ts (2)
State(26-26)PeerInfo(24-24)
🔇 Additional comments (26)
packages/ui-voip/src/v2/useInfoSlots.ts (1)
5-5: Type-only import conversion is correct and enables better tree-shaking.The
ConnectionStatetype is used exclusively in type annotations (lines 33, 44) and has no runtime dependencies, making the type-only import conversion appropriate. This aligns with the PR objective of optimizing tree-shaking by eliminating unnecessary runtime imports.packages/ui-voip/src/v2/components/Widget/WidgetDraggableContext.ts (1)
1-2: LGTM! Correct separation of type-only and runtime imports.The refactoring correctly splits the imports:
Refis now a type-only import since it's used exclusively in type positions, whilecreateContextanduseContextremain runtime imports. This improves tree-shaking and follows TypeScript best practices, aligning with the PR's build configuration modernization goals.packages/ui-voip/src/components/VoipPopupDraggable/DraggableCore.ts (3)
1-2: LGTM! Type-only import optimization improves tree-shaking.The conversion of
OffCallbackHandlerto a type-only import is correct, as it's used exclusively in type positions throughout the file.Emittercorrectly remains a value import since it's used as a base class.
10-18: LGTM! Interface extension reduces duplication.The renaming to
IPointCoordinatesaligns with the project-wide naming convention, and havingIGenericRectextendIPointCoordinatesis logically correct—it eliminates duplicate x/y property declarations while clearly expressing the relationship between these types.
62-524: LGTM! All type signatures consistently updated.The migration from
PointCoordinatestoIPointCoordinateshas been applied comprehensively across all class properties, method signatures, event payloads, and interface definitions throughout the file. The changes are purely structural with no runtime behavior modifications.packages/ui-voip/src/v2/useKeypad.tsx (1)
2-3: Type-only ReactNode import + separated useState looks goodThe separation of
import type { ReactNode }andimport { useState }is idiomatic and consistent across the ui-voip package. ReactNode is used only in type positions (line 9), confirming the type-only import is correct. This pattern is already established in 8 other files throughout the package and aligns with TypeScript best practices.packages/ui-voip/.storybook/main.ts (1)
10-10: LGTM! SWC compiler migration looks good.The switch from Babel to SWC compiler is a solid modernization choice that should improve build performance while maintaining compatibility with standard React/TypeScript transformations.
packages/ui-voip/package.json (2)
50-50: SWC compiler upgrade aligns with Storybook changes.The migration from
@storybook/addon-webpack5-compiler-babelto@storybook/addon-webpack5-compiler-swcis consistent with the change in.storybook/main.tsand should provide faster build times.
13-14: Script names are already standardized and CI/CD compatible.The packages/ui-voip package already uses standardized script names (
lintandlint:fix) shown in the diff. Verification confirms these don't cause CI/CD issues:
- CI/CD workflows do not directly reference this package
- The monorepo uses
yarn lint(turbo command) which invokes all package lint scripts by name, compatible with the standardized naming- No old script references (
yarn eslintornpm run eslint) exist in workflowsNo changes required.
packages/mock-providers/package.json (1)
14-14: Consolidation of Faker to mock-providers is properly implemented.Verification confirms that
ui-voiphas been successfully migrated: no direct@faker-js/fakerimports or references remain in the codebase, and the package no longer lists faker as a dependency. Theui-voippackage correctly references@rocket.chat/mock-providersin devDependencies, confirming the architectural consolidation is complete.packages/ui-voip/.eslintrc.json (1)
2-3: ESLint standardization verification required.The simplified ESLint configuration adopting standard RocketChat configs promotes consistency, but verification couldn't be completed in the sandbox environment due to missing dependencies. Please run
npm run lintinpackages/ui-voipon your local machine or in CI to confirm no new violations are introduced by the configuration changes.packages/ui-voip/src/v2/components/Widget/Widget.tsx (1)
3-4: LGTM! Correct type-only import optimization.The split between type-only imports (ComponentProps, ReactNode) and value imports (useLayoutEffect) is correct. ComponentProps and ReactNode are only used in type annotations (lines 32-33), while useLayoutEffect is properly kept as a value import for runtime usage.
packages/ui-voip/src/v2/components/Widget/WidgetHandle.tsx (1)
3-3: LGTM! Correct type-only import.ComponentProps is only used for type annotation (line 22) and is correctly imported as a type-only import.
packages/ui-voip/src/v2/useMediaSession.ts (1)
1-2: LGTM! Correct type-only imports.All converted types (UserStatus, MediaSignalingSession, CallState, CallRole) are only used in type annotations throughout the file and are correctly imported as type-only imports for optimal tree-shaking.
packages/ui-voip/src/v2/components/ToggleButton.tsx (1)
3-3: LGTM! Correct type-only import.ComponentProps is only used for type operations (line 12) and is correctly imported as a type-only import.
packages/ui-voip/src/v2/useMediaCallAction.ts (1)
5-6: LGTM! Correct import separation.The split between type-only import (PeerInfo) and value import (useMediaCallExternalContext) is correct. PeerInfo is only used in type annotations (lines 9, 20), while useMediaCallExternalContext is properly kept as a value import for runtime usage (line 13).
packages/ui-voip/src/v2/components/PeerAutocomplete.tsx (1)
1-1: LGTM! Correct type-only import.UserStatus is only used for type annotation (line 11) and is correctly imported as a type-only import.
packages/ui-voip/src/v2/MockedMediaCallProvider.tsx (1)
2-3: LGTM! Correct import separation.The split between type-only import (ReactNode) and value import (useState) is correct. ReactNode is only used in type annotations (line 12), while useState is properly kept as a value import for runtime usage (lines 30, 38, 39, 40).
packages/ui-voip/src/hooks/useDevicePermissionPrompt.spec.tsx (1)
5-5: LGTM! Correct type-only import.ReactNode is only used for type annotation (line 11) and is correctly imported as a type-only import.
packages/ui-voip/src/v2/components/PeerInfo/InternalUser.tsx (1)
1-4: Type-only imports fit usage here
UserStatusandSlotare only used in type positions, so converting them toimport typeis correct and keeps the runtime bundle leaner. No behavioral changes introduced.packages/ui-voip/src/v2/components/ActionButton.tsx (1)
3-4: React type/value import split is correctUsing
import type { ComponentProps }and importingforwardRefas a value matches how they’re used and avoids pulling React types into the runtime bundle. Component behavior is unchanged.packages/ui-voip/src/v2/MediaCallLogger.ts (1)
1-1: Interface moved toimport typeappropriately
IMediaSignalLoggeris only referenced in theimplementsclause, so converting it to a type-only import is safe and keeps@rocket.chat/media-signalingfrom being treated as a runtime dependency here.packages/ui-voip/src/v2/useMediaSessionInstance.ts (1)
11-36: Session type refactor toIBaseSession/IEmptySession/ICallSessionlooks soundThe new
IBaseSessionplusIEmptySession/ICallSessionextending it keeps the shared shape in one place and preserves theSessionInfodiscriminated union overstate. This is a clean, type-only change with no runtime impact and should play nicely with existingSessionInfoconsumers such asuseDesktopNotifications.packages/ui-voip/src/v2/useDesktopNotifications.ts (1)
5-6: Type-only imports forPeerInfo/SessionInfoare appropriateThese symbols are only used for typing (
sessionInfoparam andgetDisplayInfo), so importing them withimport typeis correct and keeps this module’s runtime surface minimal.packages/ui-voip/src/v2/components/PeerInfo/PeerInfo.tsx (1)
1-5: ReactComponentPropscorrectly moved to a type-only import
ComponentPropsis only used to derivePeerInfoProps, so moving it toimport typeis idiomatic and has no effect on howPeerInforenders or discriminates betweenInternalUserandPhoneNumber.packages/ui-voip/src/v2/MediaCallProvider.tsx (1)
2-20: Import restructuring cleanly separates types from runtime values
DeviceandPeerInfoare only used in type positions, so switching them toimport typeis correct.- Splitting
ReactNodeinto a type-only import while keepinguseCallback/useEffectas value imports matches their usage.- Importing
MediaCallContextas the default export aligns with its definition inMediaCallContext.ts.No behavioral changes in
MediaCallProviderare introduced by these import tweaks.
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
3 participants
Proposed changes (including videos or screenshots)
It uses standard ESLint configuration and enables SWC compiler on Storybook for
@rocket.chat/ui-voip.Issue(s)
ARCH-1885
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.