Conversation
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR removes SDK-level non‑ringing push handling and related libraries, narrows the SDK to ringing-call handling via ChangesSDK Push Removal
Documentation and Packaging
Sample App — Dogfood (native) Changes
Sample App — Expo Video Changes
Sequence DiagramsequenceDiagram
actor User
participant App as App (sample app)
participant SDK as Stream Video SDK
participant Firebase as FCM/APNs
participant Callingx as callingx (CallKit/Telecom)
participant Notifee as Notifee/Expo Notifications
rect rgba(100, 200, 100, 0.5)
note over Firebase,SDK: Ringing call (SDK handles)
Firebase->>SDK: ringing push (call.ring)
SDK->>Callingx: emit incoming call (callingx)
Callingx->>User: native call UI (CallKit/Telecom)
User->>Callingx: accept/reject
Callingx->>SDK: answer/end events
end
rect rgba(100, 150, 200, 0.5)
note over Firebase,App: Non‑ringing (App handles)
Firebase->>App: non‑ringing push (call.live_started/missed/notification)
App->>App: filter for stream.sender === 'stream.video'
App->>Notifee: display notification (Notifee/Expo)
User->>Notifee: tap notification
Notifee->>App: deliver tap
App->>App: register token / navigate to meeting
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (2)
sample-apps/react-native/expo-video-sample/hooks/useRegisterNonRingingPushToken.android.ts (1)
5-5: Add an explicit return type to the exported hook.This public hook currently relies on inference.
(): void => {}keeps the contract explicit and matches the repo's TypeScript API guidance.As per coding guidelines, "Make public API surfaces explicit with TypeScript types and interfaces".
Suggested fix
-export const useRegisterNonRingingPushToken = () => {}; +export const useRegisterNonRingingPushToken = (): void => {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react-native/expo-video-sample/hooks/useRegisterNonRingingPushToken.android.ts` at line 5, The exported hook useRegisterNonRingingPushToken currently omits an explicit return type; update its signature to declare the return type explicitly (e.g., (): void) so the public API surface is explicit per project TypeScript guidelines—modify the export of useRegisterNonRingingPushToken to include the explicit return type.sample-apps/react-native/expo-video-sample/app/index.tsx (1)
31-39: Remove the no-op iOS notification subscription.This listener no longer handles anything, so it just adds noise around where foreground iOS notification processing actually lives now. If
utils/setNotificationListeners.tsis the source of truth, delete this subscription entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react-native/expo-video-sample/app/index.tsx` around lines 31 - 39, Remove the no-op iOS notification subscription: delete the Platform.OS === 'ios' block that calls Notifications.addNotificationReceivedListener with an empty handler and its corresponding subscription.remove() cleanup; rely on the canonical listener implementation in utils/setNotificationListeners.ts for foreground iOS handling and remove any now-unused references to that temporary subscription if they become dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-native-sdk/CLAUDE.md`:
- Around line 290-293: Update the documentation to clearly separate Android and
iOS token registration: state that Android Firebase token registration is
handled by the SDK (so apps should not call client.addDevice() for Android)
while client.addDevice() must be called by apps to register APNs tokens on iOS
(or any platform where the SDK does not auto-manage tokens); explicitly mention
the SDK-managed Android flow and that calling client.addDevice() for Android
will double-register the device to avoid confusion.
- Around line 378-383: The docs currently reference push.ios / push.android but
the example and the actual API use top-level ios / android keys for
setPushConfig; update this configuration section so the keys match the example
and implementation by replacing references to push.ios and push.android with ios
and android (or alternatively update the example to use push.ios/push.android)
and ensure setPushConfig is documented consistently throughout.
- Around line 980-988: The README currently contradicts itself about
expo-notifications: it’s listed as an optional peer dependency in the SDK list
(`expo`, `expo-notifications`, `expo-build-properties`) and then shown as “Not
part of the SDK” under non-ringing notifications; decide which is correct and
make the docs consistent by either (A) keeping `expo-notifications` (and `expo`,
`expo-build-properties`) in the SDK dependencies and removing it from the “Not
part of the SDK” section, or (B) marking it explicitly as sample-app-only and
removing it from the SDK dependency list; update the CLAUDE.md entries and the
“Not part of the SDK” paragraph so `expo-notifications` is only referenced in
one place and add a short clarifying note about whether it’s required (peer dep)
or optional for Expo projects.
In `@sample-apps/react-native/dogfood/ios/AppDelegate.swift`:
- Around line 63-88: Re-add the background remote-notification forwarding by
implementing application(_:didReceiveRemoteNotification:fetchCompletionHandler:)
and forward the incoming userInfo to
RNCPushNotificationIOS.didReceiveRemoteNotification(userInfo) similar to the
foreground path; after the RNCPushNotificationIOS callback completes, invoke the
provided fetchCompletionHandler (e.g., .noData or .newData as appropriate) so
silent/data-only APNs received while the app is backgrounded are delivered to
JS. Ensure the method signature matches UIApplicationDelegate's
application(_:didReceiveRemoteNotification:fetchCompletionHandler:) and that it
accepts userInfo: [AnyHashable: Any] and calls
RNCPushNotificationIOS.didReceiveRemoteNotification(userInfo) { ... } before
calling the completion handler.
In `@sample-apps/react-native/dogfood/package.json`:
- Line 15: The package.json entry for `@notifee/react-native` was loosened from
"9.1.8" to "^9.1.2", which allows installing older 9.1.x releases; update the
dependency spec for "@notifee/react-native" to use the intended minimum version
(e.g., "^9.1.8" or "9.1.8" if exact pinning is desired) so the minimum remains
at 9.1.8.
In
`@sample-apps/react-native/dogfood/src/hooks/useRegisterNonRingingPushToken.ts`:
- Around line 40-44: Replace the raw console.warn calls inside the
useRegisterNonRingingPushToken hook with the app's sample logger (or wrap them
in a __DEV__ check) so runtime warnings are not emitted in production;
specifically update the catch blocks and any other console.warn usages within
useRegisterNonRingingPushToken (the occurrences referenced near the earlier
try/catch blocks and at the other two spots noted) to call the centralized
logger (e.g., sampleLogger.warn/error) or to be gated by if (__DEV__) {
console.warn(...) } and preserve the existing error message and err object when
forwarding to the logger.
- Around line 18-21: The cleanup race happens because client.removeDevice(token)
can complete after a new registration re-adds the same APNs token; update the
useRegisterNonRingingPushToken hook to serialize removals against subsequent
registrations by embedding a generation/tokenVersion in lastToken (e.g., extend
lastToken ref to include a version id) and only performing/removeDevice if the
stored version matches when the removal resolves; likewise increment the version
on each new registration and check the version before calling
client.removeDevice or before honoring the removal resolution so stale removals
cannot unregister a freshly bound token.
In `@sample-apps/react-native/dogfood/src/utils/notificationUtils.ts`:
- Around line 43-65: In displayNonRingingNotification, validate that call_cid
exists and is non-empty before showing the notification: if call_cid (callCid)
is missing or falsy, return early so we don't render a notification that cannot
be opened. Update the guard in displayNonRingingNotification to check both the
event type (NON_RINGING_EVENTS.has(type)) and call_cid, and only call
notifee.displayNotification when call_cid is present; ensure the data.payload
still includes call_cid and sender when emitting the notification.
In
`@sample-apps/react-native/dogfood/src/utils/registerNonRingingNotifications.ts`:
- Around line 46-52: Remove the production console.log that prints callCid;
locate the code in registerNonRingingNotifications where callCid is parsed and
deeplinkCallId$.next(callId) is called, and either delete the line
console.log('callCid', callCid) or wrap it behind a development-only flag (e.g.,
__DEV__ or NODE_ENV !== 'production') so it only logs in non-production builds.
In
`@sample-apps/react-native/dogfood/src/utils/setNotificationListeners.android.ts`:
- Around line 30-37: The Android notification channel is created with
notifee.createChannel in setNotificationListeners but createChannel is async and
can race with handlers like displayNonRingingNotification; update
setNotificationListeners to await notifee.createChannel (or otherwise obtain and
store its Promise) before registering/allowing handlers to run, and ensure
handlers (e.g., displayNonRingingNotification or any push message callbacks)
wait for the channel creation Promise (or a ready flag) to resolve before
attempting to display a notification using NON_RINGING_CHANNEL_ID.
In `@sample-apps/react-native/dogfood/src/utils/setNotificationListeners.ts`:
- Around line 16-19: The handler currently calls notification.finish() twice and
doesn't await the async displayNonRingingNotification(), so change the
notification handler (where notification.finish and
displayNonRingingNotification are used) to be async, await
displayNonRingingNotification(stream) and wrap the processing in a try/finally
that calls notification.finish(...) exactly once in the finally block so finish
is only invoked after all async work completes; reference the existing
notification.finish(...) calls and displayNonRingingNotification(stream) when
making the change.
In `@sample-apps/react-native/dogfood/src/utils/setPushConfig.ts`:
- Around line 39-40: setNotificationListeners and
registerNonRingingNotificationHandler are being called every time
StreamVideoRN.setPushConfig() runs and thus stack duplicate listeners; make
their setup idempotent by guarding registration with a module-scoped boolean
flag (e.g., listenersRegistered) or adding an early-return in each helper.
Update either setPushConfig to call the helpers only if listenersRegistered is
false, or add the guard inside setNotificationListeners and
registerNonRingingNotificationHandler themselves, and set the flag true after
the first successful registration to prevent duplicate
PushNotificationIOS/Notifee listeners and duplicate handling on subsequent
setPushConfig calls.
In
`@sample-apps/react-native/expo-video-sample/hooks/useRegisterNonRingingPushToken.ts`:
- Around line 21-86: The effect can race: async setup() and registerToken() may
complete after the effect has cleaned up and call client.addDevice/removeDevice
for the wrong client; add a cancellation guard (e.g., an instanceId or
AbortController) scoped to this effect and check it before and after each await
inside setup() and registerToken(), and in the
Notifications.addPushTokenListener callback, so late completions are ignored;
ensure the cleanup sets the guard to cancelled and that client.addDevice and
client.removeDevice are only called when the current guard matches (also check
lastToken.current.userId against client.streamClient._user?.id before updating
lastToken.current).
In
`@sample-apps/react-native/expo-video-sample/utils/registerNonRingingNotifications.ts`:
- Around line 29-35: The code validates callCid but then navigates to a generic
'/meeting' which lets the meeting screen pick calls[0]; instead, include the
validated callCid when navigating (pass it as a route param/query or navigation
param from the place where router.push('/meeting') is called) and update the
meeting screen (app/meeting.tsx) to read that callCid and select the specific
call by matching callCid instead of using calls[0]; ensure variable names
referenced are callCid and the router.push call and update the selection logic
that currently uses calls[0].
In
`@sample-apps/react-native/expo-video-sample/utils/setNotificationListeners.android.ts`:
- Around line 42-45: The foreground message handler inside messaging().onMessage
currently calls firebaseDataHandler(msg.data) without awaiting it; change the
behavior to await firebaseDataHandler(...) before calling await
handleNonRingingMessage(msg) so the foreground path mirrors the background path,
ensuring ordered execution and proper propagation of rejections; update the
anonymous async callback in messaging().onMessage (the function that checks
isFirebaseStreamVideoMessage) to await firebaseDataHandler(msg.data) and handle
or let its errors propagate consistently.
- Around line 16-19: The foreground message handler (messaging().onMessage)
currently unconditionally returns when msg.notification exists, dropping
foreground notification payloads; remove that early return in the onMessage
handler and allow processing of messages with msg.notification there, while
retaining the msg.notification check only inside the background handler
(messaging().setBackgroundMessageHandler or the background handler function) so
background-OS-displayed notifications are skipped. Update both occurrences
referenced (the onMessage handler and the background handler) so only the
background handler short-circuits on msg.notification, and ensure any downstream
code that displays or processes notifications (the handler function that creates
local notifications) still runs for foreground messages.
In
`@sample-apps/react-native/expo-video-sample/utils/setNotificationListeners.ts`:
- Around line 8-15: The notification handler currently treats any stream with
sender === 'stream.video' (in the code around notification.request.content.data
-> stream) as eligible for display, but the tap handler requires a call_cid;
update the conditional that checks stream (the stream variable) to also require
a non-empty stream.call_cid before calling
displayNonRingingNotification(stream). Ensure you reference stream.call_cid and
only invoke displayNonRingingNotification(stream) when call_cid is present so
scheduled notification data includes the routing id the tap handler expects.
In `@sample-apps/react-native/expo-video-sample/utils/setPushConfig.ts`:
- Around line 24-25: setPushConfig currently calls setNotificationListeners()
and registerNonRingingNotificationHandler() unconditionally which causes
duplicate listeners on repeated calls; add a module-level one-time guard (e.g.,
a boolean like listenersRegistered) and wrap those calls so they only execute
the first time setPushConfig() runs, setting the flag to true afterwards;
reference the functions setPushConfig, setNotificationListeners, and
registerNonRingingNotificationHandler to locate and protect the registration
block.
---
Nitpick comments:
In `@sample-apps/react-native/expo-video-sample/app/index.tsx`:
- Around line 31-39: Remove the no-op iOS notification subscription: delete the
Platform.OS === 'ios' block that calls
Notifications.addNotificationReceivedListener with an empty handler and its
corresponding subscription.remove() cleanup; rely on the canonical listener
implementation in utils/setNotificationListeners.ts for foreground iOS handling
and remove any now-unused references to that temporary subscription if they
become dead code.
In
`@sample-apps/react-native/expo-video-sample/hooks/useRegisterNonRingingPushToken.android.ts`:
- Line 5: The exported hook useRegisterNonRingingPushToken currently omits an
explicit return type; update its signature to declare the return type explicitly
(e.g., (): void) so the public API surface is explicit per project TypeScript
guidelines—modify the export of useRegisterNonRingingPushToken to include the
explicit return type.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0028a91-1ced-4bc6-8e4f-b47c0834be4a
⛔ Files ignored due to path filters (2)
sample-apps/react-native/dogfood/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (47)
packages/react-native-sdk/CLAUDE.mdpackages/react-native-sdk/jest-setup.tspackages/react-native-sdk/package.jsonpackages/react-native-sdk/src/hooks/push/index.tspackages/react-native-sdk/src/hooks/push/useIosInitRemoteNotifications.tspackages/react-native-sdk/src/hooks/push/useProcessPushNonRingingCallEffect.tspackages/react-native-sdk/src/utils/StreamVideoRN/index.tspackages/react-native-sdk/src/utils/StreamVideoRN/types.tspackages/react-native-sdk/src/utils/internal/newNotificationCallbacks.tspackages/react-native-sdk/src/utils/push/android.tspackages/react-native-sdk/src/utils/push/index.tspackages/react-native-sdk/src/utils/push/internal/rxSubjects.tspackages/react-native-sdk/src/utils/push/internal/utils.tspackages/react-native-sdk/src/utils/push/ios.tspackages/react-native-sdk/src/utils/push/libs/expoNotifications.tspackages/react-native-sdk/src/utils/push/libs/index.tspackages/react-native-sdk/src/utils/push/libs/iosPushNotification.tspackages/react-native-sdk/src/utils/push/libs/notifee/index.tspackages/react-native-sdk/src/utils/push/libs/notifee/lib.tspackages/react-native-sdk/src/utils/push/utils.tssample-apps/react-native/dogfood/App.tsxsample-apps/react-native/dogfood/android/app/src/main/AndroidManifest.xmlsample-apps/react-native/dogfood/ios/AppDelegate.swiftsample-apps/react-native/dogfood/package.jsonsample-apps/react-native/dogfood/src/components/VideoWrapper.tsxsample-apps/react-native/dogfood/src/hooks/useRegisterNonRingingPushToken.android.tssample-apps/react-native/dogfood/src/hooks/useRegisterNonRingingPushToken.tssample-apps/react-native/dogfood/src/utils/notificationUtils.tssample-apps/react-native/dogfood/src/utils/registerNonRingingNotifications.tssample-apps/react-native/dogfood/src/utils/setFirebaseListeners.android.tssample-apps/react-native/dogfood/src/utils/setFirebaseListeners.tssample-apps/react-native/dogfood/src/utils/setNotificationListeners.android.tssample-apps/react-native/dogfood/src/utils/setNotificationListeners.tssample-apps/react-native/dogfood/src/utils/setPushConfig.tssample-apps/react-native/expo-video-sample/app.jsonsample-apps/react-native/expo-video-sample/app/index.tsxsample-apps/react-native/expo-video-sample/components/VideoWrapper.tsxsample-apps/react-native/expo-video-sample/hooks/useRegisterNonRingingPushToken.android.tssample-apps/react-native/expo-video-sample/hooks/useRegisterNonRingingPushToken.tssample-apps/react-native/expo-video-sample/package.jsonsample-apps/react-native/expo-video-sample/utils/notificationUtils.tssample-apps/react-native/expo-video-sample/utils/registerNonRingingNotifications.tssample-apps/react-native/expo-video-sample/utils/setFirebaseListeners.android.tssample-apps/react-native/expo-video-sample/utils/setFirebaseListeners.tssample-apps/react-native/expo-video-sample/utils/setNotificationListeners.android.tssample-apps/react-native/expo-video-sample/utils/setNotificationListeners.tssample-apps/react-native/expo-video-sample/utils/setPushConfig.ts
💤 Files with no reviewable changes (23)
- sample-apps/react-native/expo-video-sample/package.json
- packages/react-native-sdk/src/hooks/push/index.ts
- sample-apps/react-native/dogfood/src/utils/setFirebaseListeners.ts
- packages/react-native-sdk/src/utils/StreamVideoRN/index.ts
- packages/react-native-sdk/src/utils/push/libs/iosPushNotification.ts
- sample-apps/react-native/dogfood/android/app/src/main/AndroidManifest.xml
- packages/react-native-sdk/jest-setup.ts
- sample-apps/react-native/dogfood/src/utils/setFirebaseListeners.android.ts
- sample-apps/react-native/expo-video-sample/utils/setFirebaseListeners.ts
- packages/react-native-sdk/src/hooks/push/useIosInitRemoteNotifications.ts
- packages/react-native-sdk/src/utils/push/internal/rxSubjects.ts
- packages/react-native-sdk/src/utils/push/libs/index.ts
- packages/react-native-sdk/src/utils/push/libs/notifee/lib.ts
- packages/react-native-sdk/src/utils/push/index.ts
- sample-apps/react-native/expo-video-sample/utils/setFirebaseListeners.android.ts
- packages/react-native-sdk/src/hooks/push/useProcessPushNonRingingCallEffect.ts
- packages/react-native-sdk/src/utils/push/utils.ts
- packages/react-native-sdk/src/utils/internal/newNotificationCallbacks.ts
- packages/react-native-sdk/src/utils/push/libs/expoNotifications.ts
- packages/react-native-sdk/package.json
- packages/react-native-sdk/src/utils/push/libs/notifee/index.ts
- packages/react-native-sdk/src/utils/StreamVideoRN/types.ts
- packages/react-native-sdk/src/utils/push/ios.ts
| func userNotificationCenter(_ center: UNUserNotificationCenter, willPresent notification: UNNotification, withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void) { | ||
| completionHandler([.sound, .alert, .badge]) // Use array literal for options | ||
| let userInfo = notification.request.content.userInfo | ||
|
|
||
| if let stream = userInfo[AnyHashable("stream")] as? [String: Any], | ||
| let sender = stream["sender"] as? String, | ||
| sender == "stream.video" { | ||
| RNCPushNotificationIOS.didReceiveRemoteNotification(userInfo) { _ in } | ||
| completionHandler([]) | ||
| return | ||
| } | ||
|
|
||
| // Notifee-created local notifications: notifee handles display itself. | ||
| if notification.request.trigger is UNPushNotificationTrigger == false { | ||
| completionHandler([]) | ||
| return | ||
| } | ||
|
|
||
| // All other remote notifications: show natively | ||
| completionHandler([.sound, .alert, .badge]) | ||
| } | ||
|
|
||
| // Forward notification tap to push-notification-ios | ||
| func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) { | ||
| RNCPushNotificationIOS.didReceive(response) | ||
| completionHandler() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
On iOS, when are userNotificationCenter(:willPresent:withCompletionHandler:)andapplication(:didReceiveRemoteNotification:fetchCompletionHandler:) invoked for remote notifications, especially silent/data-only pushes delivered while the app is backgrounded?
💡 Result:
For remote notifications on iOS, especially silent/data-only pushes (those with "content-available":1 in the aps dictionary and no alert/sound/badge), delivered while the app is backgrounded: - application(:didReceiveRemoteNotification:fetchCompletionHandler:) (AppDelegate) is invoked. The system wakes the app in the background (if the "Remote notifications" background mode is enabled) and calls this method to handle data fetching/processing. You have ~30 seconds to complete and call the handler. Delivery is best-effort/low-priority and may be throttled/delayed/dropped; not delivered if app force-quit. - userNotificationCenter(:willPresent:withCompletionHandler:) (UNUserNotificationCenterDelegate) is NOT invoked for silent pushes when backgrounded. This method is for notifications arriving while the app is in the foreground, to decide presentation options. This holds for both visible remote notifications (with alert etc.) and silent ones when backgrounded, but silent ones specifically route to the AppDelegate method without UNNotificationCenter delegate involvement since there's no UI presentation. Requires: Remote notifications background capability enabled; apns-push-type: background header; apns-priority:5.
Citations:
- 1: https://developer.apple.com/documentation/usernotifications/pushing-background-updates-to-your-app
- 2: https://developer.apple.com/documentation/usernotifications/unusernotificationcenterdelegate/usernotificationcenter(_:willpresent:withcompletionhandler:)
- 3: https://developer.apple.com/documentation/uikit/uiapplicationdelegate/application(_:didreceiveremotenotification:fetchcompletionhandler:)
- 4: https://stackoverflow.com/questions/42989932/difference-between-unusernotificationcenterdelegate-and-didreceiveremotenotifica
- 5: https://firebase.google.com/docs/cloud-messaging/ios/receive
- 6: https://stackoverflow.com/questions/53460280/why-usernotificationcenter-didreceive-is-not-fired
- 7: https://developer.apple.com/forums/thread/770864
- 8: https://jsorge.net/2018/08/01/ultimate-ios-push-lifecycle
- 9: https://developer.apple.com/documentation/UserNotifications/pushing-background-updates-to-your-app?changes=latest_minor
Background non-ringing APNs are no longer forwarded to JS.
userNotificationCenter(_:willPresent:...) only covers foreground delivery. Since this file no longer implements application(_:didReceiveRemoteNotification:fetchCompletionHandler:), silent/data-only non-ringing pushes received while the app is backgrounded won't reach RNCPushNotificationIOS, so the sample can't surface them until the app is opened.
💡 Re-add the background forwarding path
+ func application(
+ _ application: UIApplication,
+ didReceiveRemoteNotification userInfo: [AnyHashable: Any],
+ fetchCompletionHandler completionHandler: `@escaping` (UIBackgroundFetchResult) -> Void
+ ) {
+ RNCPushNotificationIOS.didReceiveRemoteNotification(userInfo) { result in
+ completionHandler(result)
+ }
+ }
+
func userNotificationCenter(_ center: UNUserNotificationCenter, willPresent notification: UNNotification, withCompletionHandler completionHandler: `@escaping` (UNNotificationPresentationOptions) -> Void) {
let userInfo = notification.request.content.userInfo🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sample-apps/react-native/dogfood/ios/AppDelegate.swift` around lines 63 - 88,
Re-add the background remote-notification forwarding by implementing
application(_:didReceiveRemoteNotification:fetchCompletionHandler:) and forward
the incoming userInfo to
RNCPushNotificationIOS.didReceiveRemoteNotification(userInfo) similar to the
foreground path; after the RNCPushNotificationIOS callback completes, invoke the
provided fetchCompletionHandler (e.g., .noData or .newData as appropriate) so
silent/data-only APNs received while the app is backgrounded are delivered to
JS. Ensure the method signature matches UIApplicationDelegate's
application(_:didReceiveRemoteNotification:fetchCompletionHandler:) and that it
accepts userInfo: [AnyHashable: Any] and calls
RNCPushNotificationIOS.didReceiveRemoteNotification(userInfo) { ... } before
calling the completion handler.
There was a problem hiding this comment.
@greenfrvr good point here.. both expo and rn-cli needs background handling example through these samples
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| setNotificationListeners(); | ||
| registerNonRingingNotificationHandler(); |
There was a problem hiding this comment.
Guard listener registration so it only happens once.
These helpers now live outside the SDK's internal setPushConfig() one-time guard. If setPushConfig() is called again, Expo response/receive listeners are registered again and the same push can be handled multiple times.
Possible fix
+let notificationHandlersRegistered = false;
+
export function setPushConfig() {
StreamVideoRN.setPushConfig({
@@
createStreamVideoClient,
});
- setNotificationListeners();
- registerNonRingingNotificationHandler();
+ if (!notificationHandlersRegistered) {
+ setNotificationListeners();
+ registerNonRingingNotificationHandler();
+ notificationHandlersRegistered = true;
+ }
if (Platform.OS === 'ios') {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sample-apps/react-native/expo-video-sample/utils/setPushConfig.ts` around
lines 24 - 25, setPushConfig currently calls setNotificationListeners() and
registerNonRingingNotificationHandler() unconditionally which causes duplicate
listeners on repeated calls; add a module-level one-time guard (e.g., a boolean
like listenersRegistered) and wrap those calls so they only execute the first
time setPushConfig() runs, setting the flag to true afterwards; reference the
functions setPushConfig, setNotificationListeners, and
registerNonRingingNotificationHandler to locate and protect the registration
block.
💡 Overview
This PR includes non-ringing notifications removal from SDK level.
There is small difference in the logic across platforms:
call.ringnotifications which are used for ringing functionalityUpdated dogfood and expo-sample apps to illustrate non-ringing notifications handling.
Updated CLAUDE.md to correspond to the latest changes (new ringing flow and non-ringing notifications handling)
🎫 Ticket: https://linear.app/stream/issue/RN-376/remove-non-ringing-notifications-handling-from-sdk
📑 Docs: https://github.com/GetStream/docs-content/pull/
Summary by CodeRabbit
Breaking Changes
New Features
Chores