chore: Update media-signaling to 0.2.0#7153
Conversation
…ry fields Iteration 2 fixes: wrap REST block in null guard, replace field accesses with as any casts since role/muted/held/contact/setMuted/setHeld were removed from IClientMediaCall in 0.2.0-rc.0 library.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
Caution Review failedPull request was closed or merged during review WalkthroughRefactors voip call data shape from top-level fields to structured participants, updates related logic and tests, adjusts MediaSignalingSession initialization option, simplifies a REST SDK call, and updates a local package file dependency reference. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/lib/services/voip/mockCall.ts (1)
44-44: Hardcodedrole: 'callee'removes mock flexibility for outgoing call scenarios.The role is now hardcoded to
'callee', but theMockCallOverridesinterface never included aroleproperty, so this doesn't break the existing API contract. However, if future tests or dev scenarios need to simulate outgoing calls (whererole === 'caller'triggers auto-navigation perMediaSessionInstance.tsline 107), this mock won't support that case.Consider whether adding
role?: 'caller' | 'callee'toMockCallOverrideswould be valuable for broader test coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services/voip/mockCall.ts` at line 44, The mock currently hardcodes role: 'callee' in createMockCall (mockCall.ts) which prevents simulating outgoing calls; update the MockCallOverrides type to include role?: 'caller' | 'callee' and change the object construction in createMockCall (or equivalent factory) to use the provided overrides.role with a default of 'callee' so tests can pass { role: 'caller' } to simulate outgoing behavior (this aligns with MediaSessionInstance logic that checks role).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/services/voip/MediaSessionInstance.ts`:
- Line 68: Delete the commented-out call to instance.register(false) in
MediaSessionInstance.ts or, if you intentionally keep it for future reference,
replace it with a brief explanatory comment stating that registration is now
handled via MediaSessionStore's autoSync: true and why the commented line is
retained (e.g., "kept for upgrade validation" or "left as reference during
library upgrade"); reference the symbol instance.register(false) and
MediaSessionStore.autoSync to make purpose clear.
---
Nitpick comments:
In `@app/lib/services/voip/mockCall.ts`:
- Line 44: The mock currently hardcodes role: 'callee' in createMockCall
(mockCall.ts) which prevents simulating outgoing calls; update the
MockCallOverrides type to include role?: 'caller' | 'callee' and change the
object construction in createMockCall (or equivalent factory) to use the
provided overrides.role with a default of 'callee' so tests can pass { role:
'caller' } to simulate outgoing behavior (this aligns with MediaSessionInstance
logic that checks role).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dd1d174b-f641-4388-8ca7-9ca1ae3f9c35
📒 Files selected for processing (6)
app/lib/services/restApi.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/mockCall.tsapp/lib/services/voip/useCallStore.test.ts
✅ Files skipped from review due to trivial changes (3)
- app/lib/services/voip/useCallStore.test.ts
- app/lib/services/voip/MediaSessionInstance.test.ts
- app/lib/services/restApi.ts
📜 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line
Files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/mockCall.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase configuration including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/mockCall.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/mockCall.ts
app/lib/services/voip/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
Files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/mockCall.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/mockCall.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
Applied to files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/mockCall.ts
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to @(app/sagas/videoConf.ts|app/lib/methods/videoConf.ts) : Manage video conferencing via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using server-managed Jitsi integration; do not conflate with VoIP
Applied to files:
app/lib/services/voip/MediaSessionStore.tsapp/lib/services/voip/MediaSessionInstance.ts
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/store/**/*.{ts,tsx} : Configure Redux store in app/lib/store/ with middleware for saga, app state, and internet state
Applied to files:
app/lib/services/voip/MediaSessionInstance.ts
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/lib/services/voip/mockCall.ts
🔇 Additional comments (4)
app/lib/services/voip/mockCall.ts (1)
4-4: LGTM on the Navigation import update.The direct import of
NavigationfromappNavigationis correct and aligns with the module's exported API.app/lib/services/voip/MediaSessionInstance.ts (2)
18-18: LGTM on the import path correction.The relative path
../restApicorrectly referencesapp/lib/services/restApi.tsfrom the voip subdirectory.
130-130: Minor destructuring refactor looks fine.The change from
const call = useCallStore.getState().calltoconst { call } = useCallStore.getState()is a stylistic improvement with no functional change.Also applies to: 163-163
app/lib/services/voip/MediaSessionStore.ts (1)
71-71: No action needed —autoSyncis a valid option in@rocket.chat/media-signaling0.2.0.The constructor option change from
requestInitialStateSignals: falsetoautoSync: trueis valid. The type definition confirmsautoSync?: boolean;is a documented config option inMediaSignalingSessionConfig.
diegolmello
force-pushed
the
feat.voip-lib-new
branch
from
2143aba to
2a098f7
Compare
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
1 participant
Proposed changes
Issue(s)
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit