perf(react-native-sdk): reduce re-renders, fix listener cleanup, prune dead code#2274
Open
oliverlaz wants to merge 1 commit into
Open
perf(react-native-sdk): reduce re-renders, fix listener cleanup, prune dead code#2274oliverlaz wants to merge 1 commit into
oliverlaz wants to merge 1 commit into
Conversation
…e dead code Fixes: - BusyTonePlayer: unsubscribe the call.rejected listener on unmount - AppStateListener: add a cancelled guard so the mount effect cannot write to the shared subject after unmount Perf: - Memoize ParticipantView and VideoRenderer; narrow VideoRenderer's visibility-effect deps to the two primitives it reads - Stabilize CallParticipantsList's participant prop-bag and fix renderItem deps - CallContent: store a remote-participant count bucket plus the first participant instead of the whole array - LivestreamLayout: derive objectFit during render instead of via a renderless child effect - Lobby and FloatingParticipantView: memoize per-render literals; hoist a constant lookup map - useAutoEnterPiPEffect: merge two duplicate effects - landscapeStyles to StyleSheet constants; memoize the spotlight tile style - LivestreamEnded: extract a memoized RecordingRow and stabilize callbacks - getCallDisplayName: collapse filter/map/filter into a single flatMap - DeviceStats: build NativeEventEmitter inside the effect Chore: - Remove 5 unused icons; move TopViewBackground into the dogfood app
|
Contributor
📝 WalkthroughWalkthroughThis PR improves the React Native SDK's performance and architecture through floating participant state optimization, component memoization, livestream refactoring, icon cleanup, and lifecycle management fixes. Changes span from participant rendering logic to provider cleanup, optimizing re-render frequency and fixing potential race conditions in native module initialization. ChangesReact Native SDK Refactoring & Performance Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/react-native-sdk/src/providers/StreamCall/AppStateListener.tsx (3)
42-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for native module promise.
The
isInPiPMode()call lacks a.catch()handler. Native module calls can reject due to platform errors or missing implementations.🛡️ Proposed fix
NativeModules?.StreamVideoReactNative?.isInPiPMode().then( (isInPiP: boolean | null | undefined) => { if (cancelled) return; isInPiPMode$.next(!!isInPiP); logger.debug( 'Initial PiP mode on mount (after asking native module) set to ', !!isInPiP, ); }, - ); + ).catch((err) => + logger.warn('Failed to query initial PiP mode:', err) + );As per coding guidelines: "Wrap native method calls in try-catch and always handle promise rejection from native modules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/providers/StreamCall/AppStateListener.tsx` around lines 42 - 51, The call to NativeModules?.StreamVideoReactNative?.isInPiPMode() can reject but has no rejection handler; update the initialization to handle errors by adding a .catch() (or wrapping in async/try-catch) that checks cancelled, logs the error via logger.error with context ("isInPiPMode failed") and keeps isInPiPMode$ unchanged, and retain the existing success path that sets isInPiPMode$ and calls logger.debug; reference NativeModules?.StreamVideoReactNative?.isInPiPMode, isInPiPMode$, logger.debug, and cancelled when making the change.Source: Coding guidelines
106-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for
isCallAliveConfiguredpromise.The native module call lacks a
.catch()handler. If the promise rejects, it will surface as an unhandled rejection.🛡️ Proposed fix
NativeModules.StreamVideoReactNative.isCallAliveConfigured().then( (isCallAliveConfigured: boolean) => { if (!isCallAliveConfigured) { renableCamera(); } }, - ); + ).catch((err) => + logger.warn('Failed to check isCallAliveConfigured:', err) + );As per coding guidelines: "Wrap native method calls in try-catch and always handle promise rejection from native modules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/providers/StreamCall/AppStateListener.tsx` around lines 106 - 115, The call to NativeModules.StreamVideoReactNative.isCallAliveConfigured is missing rejection handling and may cause unhandled promise rejections; wrap the native call in a try/catch (or add a .catch) and on error log the exception and call renableCamera() as a safe fallback. Specifically, update the isCallAliveConfigured invocation (or convert it to await inside a try/catch) so that any thrown error is caught, processLogger or console.error records the error, and renableCamera() is invoked in the catch handler.Source: Coding guidelines
161-182:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for native module promises.
Both
isInPiPMode()andisCallAliveConfigured()lack.catch()handlers. These calls are on the background transition path and should handle rejection gracefully.🛡️ Proposed fix
NativeModules?.StreamVideoReactNative?.isInPiPMode().then( (isInPiP: boolean | null | undefined) => { isInPiPMode$.next(!!isInPiP); if (!isInPiP) { if (AppState.currentState === 'active') { return; } NativeModules.StreamVideoReactNative.isCallAliveConfigured().then( (isCallAliveConfigured: boolean) => { if (!isCallAliveConfigured) { disableCameraIfNeeded(); } }, - ); + ).catch((err) => + logger.warn('Failed to check isCallAliveConfigured:', err) + ); } }, - ); + ).catch((err) => + logger.warn('Failed to query PiP mode on background:', err) + );As per coding guidelines: "Wrap native method calls in try-catch and always handle promise rejection from native modules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/providers/StreamCall/AppStateListener.tsx` around lines 161 - 182, The native-promises returned by NativeModules.StreamVideoReactNative.isInPiPMode() and .isCallAliveConfigured() are unhandled; wrap these calls to handle rejections (either convert to async/await inside the AppStateListener flow or add .catch() handlers) and log or swallow errors appropriately so they don't crash background transitions; specifically update the block using isInPiPMode() to handle promise rejection and likewise handle .isCallAliveConfigured() rejections before calling disableCameraIfNeeded(), referencing the NativeModules.StreamVideoReactNative, isInPiPMode, isCallAliveConfigured, and disableCameraIfNeeded symbols.Source: Coding guidelines
packages/react-native-sdk/src/providers/StreamCall/DeviceStats.tsx (3)
39-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for
currentThermalStatepromise.The native module call lacks a
.catch()handler. If the promise rejects, it will surface as an unhandled rejection.🛡️ Proposed fix
StreamVideoReactNative.currentThermalState().then( (initialState: string) => { setThermalState(initialState); call.tracer.trace('device.thermalState', initialState); }, - ); + ).catch((err) => { + // Ignore error - thermal state will remain at default + call.tracer.trace('device.thermalState.error', err); + });As per coding guidelines: "Wrap native method calls in try-catch and always handle promise rejection from native modules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/providers/StreamCall/DeviceStats.tsx` around lines 39 - 44, The call to StreamVideoReactNative.currentThermalState() in DeviceStats.tsx is missing rejection handling; update the invocation (where setThermalState and call.tracer.trace are used) to handle errors from the promise (either add a .catch(...) to the promise chain or wrap the call in an async try/catch) and log the error via the existing logging/tracing (e.g., processLogger or call.tracer.trace with an error message) so rejections are not left unhandled while preserving the existing success path that calls setThermalState and call.tracer.trace('device.thermalState', initialState).Source: Coding guidelines
54-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for
getBatteryStatepromise.The native module call in
pollBatteryStatelacks a.catch()handler. This function is called repeatedly viasetInterval, so unhandled rejections could accumulate.🛡️ Proposed fix
const pollBatteryState = () => { StreamVideoReactNative.getBatteryState().then( (data: { charging: boolean; level: number }) => { call.tracer.trace('device.batteryState', data); }, - ); + ).catch((err) => { + // Ignore error - battery polling will retry on next interval + call.tracer.trace('device.batteryState.error', err); + }); };As per coding guidelines: "Wrap native method calls in try-catch and always handle promise rejection from native modules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/providers/StreamCall/DeviceStats.tsx` around lines 54 - 60, pollBatteryState is calling StreamVideoReactNative.getBatteryState() without handling rejections, leading to potential unhandled promise rejections when run repeatedly (via setInterval). Update pollBatteryState to handle errors: either convert it to an async function and wrap the await StreamVideoReactNative.getBatteryState() in try/catch, or add a .catch() to the existing promise chain; in the catch block call call.tracer.trace or call.tracer.error (or process logger used elsewhere) with a clear message and the error object so failures are recorded and do not accumulate.Source: Coding guidelines
24-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd error handling for
isLowPowerModeEnabledpromise.The native module call lacks a
.catch()handler. If the promise rejects, it will surface as an unhandled rejection.🛡️ Proposed fix
StreamVideoReactNative.isLowPowerModeEnabled().then( (initialPowerMode: boolean) => { setPowerState(initialPowerMode); call.tracer.trace('device.lowPowerMode', initialPowerMode); }, - ); + ).catch((err) => { + // Ignore error - power mode state will remain at default + call.tracer.trace('device.lowPowerMode.error', err); + });As per coding guidelines: "Wrap native method calls in try-catch and always handle promise rejection from native modules".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/providers/StreamCall/DeviceStats.tsx` around lines 24 - 29, The call to StreamVideoReactNative.isLowPowerModeEnabled lacks error handling and can produce an unhandled rejection; update the usage (the promise returned by StreamVideoReactNative.isLowPowerModeEnabled) to either await it in a try/catch or add a .catch() handler that handles failures, logs the error via call.tracer.trace (e.g., 'device.lowPowerMode.error') and avoids calling setPowerState on failure; ensure setPowerState and call.tracer.trace('device.lowPowerMode', ...) remain in the success path and that the catch path handles/logs the error and prevents an unhandled promise rejection.Source: Coding guidelines
🧹 Nitpick comments (1)
packages/react-native-sdk/src/components/Livestream/LivestreamPlayer/LivestreamEnded.tsx (1)
18-42: ⚡ Quick winConsider adding accessibility props to Pressable.
The
RecordingRowcomponent is well-structured with proper memoization, but thePressableat line 34 lacks accessibility properties. Consider addingaccessibilityRole="button"andaccessibilityLabelto improve screen reader support.♿ Suggested accessibility enhancement
- <Pressable style={buttonStyle} onPress={handlePress}> + <Pressable + style={buttonStyle} + onPress={handlePress} + accessibilityRole="button" + accessibilityLabel={`Open recording ${recording.url.substring(0, 70)}`} + > <Text style={textStyle}>{recording.url.substring(0, 70)}...</Text> </Pressable>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-sdk/src/components/Livestream/LivestreamPlayer/LivestreamEnded.tsx` around lines 18 - 42, RecordingRow's Pressable is missing accessibility props; update the Pressable in the RecordingRow component to include accessibilityRole="button" and a descriptive accessibilityLabel (e.g., derived from recording.url or a passed-in prop) so screen readers can announce it, ensuring you keep the existing onPress handler (handlePress) and memoization intact; locate the Pressable inside RecordingRow and add the accessibilityRole and accessibilityLabel attributes (or accept an accessibilityLabel prop on RecordingRow and pass it through) so the button is accessible.
🤖 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.
Outside diff comments:
In `@packages/react-native-sdk/src/providers/StreamCall/AppStateListener.tsx`:
- Around line 42-51: The call to
NativeModules?.StreamVideoReactNative?.isInPiPMode() can reject but has no
rejection handler; update the initialization to handle errors by adding a
.catch() (or wrapping in async/try-catch) that checks cancelled, logs the error
via logger.error with context ("isInPiPMode failed") and keeps isInPiPMode$
unchanged, and retain the existing success path that sets isInPiPMode$ and calls
logger.debug; reference NativeModules?.StreamVideoReactNative?.isInPiPMode,
isInPiPMode$, logger.debug, and cancelled when making the change.
- Around line 106-115: The call to
NativeModules.StreamVideoReactNative.isCallAliveConfigured is missing rejection
handling and may cause unhandled promise rejections; wrap the native call in a
try/catch (or add a .catch) and on error log the exception and call
renableCamera() as a safe fallback. Specifically, update the
isCallAliveConfigured invocation (or convert it to await inside a try/catch) so
that any thrown error is caught, processLogger or console.error records the
error, and renableCamera() is invoked in the catch handler.
- Around line 161-182: The native-promises returned by
NativeModules.StreamVideoReactNative.isInPiPMode() and .isCallAliveConfigured()
are unhandled; wrap these calls to handle rejections (either convert to
async/await inside the AppStateListener flow or add .catch() handlers) and log
or swallow errors appropriately so they don't crash background transitions;
specifically update the block using isInPiPMode() to handle promise rejection
and likewise handle .isCallAliveConfigured() rejections before calling
disableCameraIfNeeded(), referencing the NativeModules.StreamVideoReactNative,
isInPiPMode, isCallAliveConfigured, and disableCameraIfNeeded symbols.
In `@packages/react-native-sdk/src/providers/StreamCall/DeviceStats.tsx`:
- Around line 39-44: The call to StreamVideoReactNative.currentThermalState() in
DeviceStats.tsx is missing rejection handling; update the invocation (where
setThermalState and call.tracer.trace are used) to handle errors from the
promise (either add a .catch(...) to the promise chain or wrap the call in an
async try/catch) and log the error via the existing logging/tracing (e.g.,
processLogger or call.tracer.trace with an error message) so rejections are not
left unhandled while preserving the existing success path that calls
setThermalState and call.tracer.trace('device.thermalState', initialState).
- Around line 54-60: pollBatteryState is calling
StreamVideoReactNative.getBatteryState() without handling rejections, leading to
potential unhandled promise rejections when run repeatedly (via setInterval).
Update pollBatteryState to handle errors: either convert it to an async function
and wrap the await StreamVideoReactNative.getBatteryState() in try/catch, or add
a .catch() to the existing promise chain; in the catch block call
call.tracer.trace or call.tracer.error (or process logger used elsewhere) with a
clear message and the error object so failures are recorded and do not
accumulate.
- Around line 24-29: The call to StreamVideoReactNative.isLowPowerModeEnabled
lacks error handling and can produce an unhandled rejection; update the usage
(the promise returned by StreamVideoReactNative.isLowPowerModeEnabled) to either
await it in a try/catch or add a .catch() handler that handles failures, logs
the error via call.tracer.trace (e.g., 'device.lowPowerMode.error') and avoids
calling setPowerState on failure; ensure setPowerState and
call.tracer.trace('device.lowPowerMode', ...) remain in the success path and
that the catch path handles/logs the error and prevents an unhandled promise
rejection.
---
Nitpick comments:
In
`@packages/react-native-sdk/src/components/Livestream/LivestreamPlayer/LivestreamEnded.tsx`:
- Around line 18-42: RecordingRow's Pressable is missing accessibility props;
update the Pressable in the RecordingRow component to include
accessibilityRole="button" and a descriptive accessibilityLabel (e.g., derived
from recording.url or a passed-in prop) so screen readers can announce it,
ensuring you keep the existing onPress handler (handlePress) and memoization
intact; locate the Pressable inside RecordingRow and add the accessibilityRole
and accessibilityLabel attributes (or accept an accessibilityLabel prop on
RecordingRow and pass it through) so the button is accessible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70ef5882-2cd9-4dbb-aa2f-e820a4b37160
📒 Files selected for processing (23)
packages/react-native-sdk/src/components/Call/CallContent/CallContent.tsxpackages/react-native-sdk/src/components/Call/CallLayout/CallParticipantsGrid.tsxpackages/react-native-sdk/src/components/Call/CallLayout/CallParticipantsSpotlight.tsxpackages/react-native-sdk/src/components/Call/CallParticipantsList/CallParticipantsList.tsxpackages/react-native-sdk/src/components/Call/Lobby/Lobby.tsxpackages/react-native-sdk/src/components/Livestream/LivestreamLayout/LivestreamLayout.tsxpackages/react-native-sdk/src/components/Livestream/LivestreamPlayer/LivestreamEnded.tsxpackages/react-native-sdk/src/components/Participant/FloatingParticipantView/index.tsxpackages/react-native-sdk/src/components/Participant/ParticipantView/ParticipantView.tsxpackages/react-native-sdk/src/components/Participant/ParticipantView/VideoRenderer/index.tsxpackages/react-native-sdk/src/hooks/useAutoEnterPiPEffect.tsxpackages/react-native-sdk/src/icons/LeaveStreamIcon.tsxpackages/react-native-sdk/src/icons/Settings.tsxpackages/react-native-sdk/src/icons/ShieldBadge.tsxpackages/react-native-sdk/src/icons/Spotlight.tsxpackages/react-native-sdk/src/icons/ThreeDots.tsxpackages/react-native-sdk/src/icons/index.tsxpackages/react-native-sdk/src/providers/BusyTonePlayer.tsxpackages/react-native-sdk/src/providers/StreamCall/AppStateListener.tsxpackages/react-native-sdk/src/providers/StreamCall/DeviceStats.tsxpackages/react-native-sdk/src/utils/internal/callingx/callingx.tssample-apps/react-native/dogfood/src/assets/TopViewBackground.tsxsample-apps/react-native/dogfood/src/components/CallControlls/TopControls.tsx
💤 Files with no reviewable changes (6)
- packages/react-native-sdk/src/icons/Spotlight.tsx
- packages/react-native-sdk/src/icons/ShieldBadge.tsx
- packages/react-native-sdk/src/icons/LeaveStreamIcon.tsx
- packages/react-native-sdk/src/icons/ThreeDots.tsx
- packages/react-native-sdk/src/icons/Settings.tsx
- packages/react-native-sdk/src/icons/index.tsx
santhoshvai
reviewed
Jun 9, 2026
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
2 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.
💡 Overview
A batch of React Native SDK performance and cleanup changes: re-render reductions across the participant and livestream render chain, an event-listener memory-leak fix, an unmount-safety guard, and dead-icon cleanup. No public API changes.
📝 Implementation notes
Fixes
BusyTonePlayer: thecall.rejecteddisposer was returned from the event handler, not the effect, so it never ran. Moved it to the effect body so the listener, timeout, and tone are cleaned up on unmount.AppStateListener: added acancelledguard so the mount effect'sisInPiPMode().thencannot write to the shared subject after unmount.Re-render / allocation reductions
ParticipantViewandVideoRenderermemoized;VideoRenderer's visibility-effect deps narrowed to the two primitives it reads.CallParticipantsList: memoized the participant prop-bag and correctedrenderItemdeps (dropped a stale-closureeslint-disable).CallContent: stores a remote-participant count bucket plus the first participant instead of the full array, re-rendering only on bucket-boundary crossings.LivestreamLayout: derivesobjectFitduring render instead of via a renderless child effect.LobbyandFloatingParticipantView: memoized per-render literals; hoisted a constant lookup map.useAutoEnterPiPEffect: merged two duplicate[disablePictureInPicture]effects.CallContent,CallParticipantsGrid, andCallParticipantsSpotlight:landscapeStylesconverted toStyleSheetconstants; memoized the spotlight tile'sstyle.LivestreamEnded: extracted a memoizedRecordingRow;useCallback'drenderItem,keyExtractor, andopenUrl.getCallDisplayName: collapsedfilter/map/filterinto a singleflatMap.DeviceStats: constructsNativeEventEmitterinside the gated effect instead of at module load.Dead-code cleanup
LeaveStreamIcon,Settings,ShieldBadge,Spotlight,ThreeDots) and their barrel entries.TopViewBackgroundinto the dogfood app (its only consumer, via a deepsrc/iconsimport) and repointed the import.Verification: SDK
tsc,eslint, jest (5/5 suites, 13/13), andNODE_ENV=production yarn buildall green; dogfoodtscgreen.🎫 Ticket: TBD
📑 Docs: N/A
Summary by CodeRabbit
Release Notes
Bug Fixes
Performance
Chores