OneSignal / react-native-onesignal

React Native Library for OneSignal Push Notifications Service
Other
1.56k stars 371 forks source link

[Bug]: Problem with addEventListener removeEventListener #1610

Open serolgames opened 7 months ago

serolgames commented 7 months ago

What happened?

It seems that removeEventListener doesn't work as expected

I have the following code :

useEffect(() => 
{
    const eventMethod = (event: any) => 
    {
        console.log("Event listener called")
        let notification = event.getNotification();
        const data: any = notification.additionalData 

        if (data.refresh_messages == true) 
        {
            event.preventDefault();
        }
        else 
        {
            event.getNotification().display();
        }
    }

    OneSignal.Notifications.addEventListener('foregroundWillDisplay', eventMethod);

    return () => 
    {
        OneSignal.Notifications.removeEventListener('foregroundWillDisplay', eventMethod)
    };
}, []);

So when I'm on my page and I receive a notification my console says "Event listener called". When I leave the page I remove the event listener (with the return method of the useEffect) then when I receive a notification my console don't say "Event listener called". So... It's working ! But... no, because the notification is not shown. So in my opinion it seems that the removeEventListener does his work properly, but that the preventDefault() is still working because no notification with refresh_messages to true are displayed.

Steps to reproduce?

1. Install react native onesignal v5.0.3
2. Copy my code on another page (let's call it messagePage)
3. Go on your homePage and try to receive a notification (it will display)
4. Go on your messagePage and try to receive a notification (it won't display (as expected))
5. Go back to homePage and try to receive a notification (it won't display :( )

What did you expect to happen?

As I removed the listener. Then every notification should display

React Native OneSignal SDK version

5.0.3

Which platform(s) are affected?

Relevant log output

No response

Code of Conduct

Martinocom commented 6 months ago

Are you using some libraries to navigate? Because for example react-navigation works better with useFocusEffect (instead of useEffect) when navigating and "focusing" on different screens.

karlqueckfeldt commented 5 months ago

Did you find a solution to this? I'm using a combination of expo-router and the OneSignal Expo SDK, and the useFocusEffect for adding/removing the event listener. When I navigate from this route, the return statement in useFocusEffect is triggered properly, and the foregroundWillDisplay event is removed, but notifications still aren't shown again. What gives?

// route.tsx
  useFocusEffect(
    useCallback(() => {
      addForegroundWillDisplayListener();

      return () => {
        removeForegroundWillDisplayListener();
      };
    }, []),
  );

// PushNotificationsProvider.tsx
  const onForegroundWillDisplay = (event: NotificationWillDisplayEvent) => {
    event.preventDefault();
  };

  function addForegroundWillDisplayListener() {
    removeForegroundWillDisplayListener();
    OneSignal.Notifications.addEventListener(
      'foregroundWillDisplay',
      onForegroundWillDisplay,
    );
  }

  function removeForegroundWillDisplayListener() {
    OneSignal.Notifications.removeEventListener(
      'foregroundWillDisplay',
      onForegroundWillDisplay,
    );
  }
serolgames commented 4 months ago

The problem is still here, event is removed but notifications don't show up again

mathbalduino commented 3 months ago

Same here

mathbalduino commented 3 months ago

Hello guys, I'm having similar issues. After I register a OneSignal.Notifications.addEventListener('foregroundWillDisplay', onNotificationArrived), I can't see my notifications anymore, even if I remove the listener. The only way to see them again, is to call notification.display(), inside the listener, again and again.

I don't know if it is happening on Android side. I'm working with iOS-only for now.

Let's take a look at my workaround: (I'm using react-navigation, btw)

const SHOW_ALL_NOTIFICATIONS = (event: NotificationWillDisplayEvent) =>
  event.getNotification().display()

const YourPageComponent = () => {
  useFocusEffect(useCallback(() => {
    const onNotificationArrived = (event: NotificationWillDisplayEvent) => {
      const notification = event.getNotification()
      const data = notification.additionalData as (SomeDataType | undefined)
      const shouldPrevent = !!data && data.chat_id === chatID && data.SomeBooleanValue
      if (shouldPrevent) event.preventDefault()
      else notification.display()
    }
    OneSignal.Notifications.removeEventListener('foregroundWillDisplay', SHOW_ALL_NOTIFICATIONS)
    OneSignal.Notifications.addEventListener('foregroundWillDisplay', onNotificationArrived)
    return () => {
      OneSignal.Notifications.removeEventListener('foregroundWillDisplay', onNotificationArrived)
      OneSignal.Notifications.addEventListener('foregroundWillDisplay', SHOW_ALL_NOTIFICATIONS)
    }
  }, []))

  return <></>
}

Currently, on react-native-onesignal v5.1.2 with react-native v0.72.5, this is what's happening with me:

As you guys noted, we may have a flag, or something, that is not being unset. After we call event.preventDefault() we enable it, and it never gets disabled again. I was looking into the libs code and found this: /ios/RCTOneSignal/RCTOneSignalEventEmitter.m:17

Apparently, after we call event.preventDefault(), we cache it. This affects, at least, two other functions: /ios/RCTOneSignal/RCTOneSignalEventEmitter.m:292 /ios/RCTOneSignal/RCTOneSignalEventEmitter.m:464

I didn't have time to deep dive into it. The cache seems to be using the notification_id as the key, so it shouldn't be the problem for different notifications. But for some reason, all subsequent notifications print VERBOSE: OSNotificationWillDisplayEvent.preventDefault called on the console, so it may have something to do with the notification.preventDefault() function. Strange. The listener seems to be correctly removed, but if that's not the case, this may be the bug: /ios/RCTOneSignal/RCTOneSignalEventEmitter.m:280

We can see that if there's an event registered, the preventDefault will be called before calling the event subscriber. I don't think that's the problem, since my listeners are being correctly removed and my workaround is based on that.

Well, hope I can help someone else. This workaround is working for me, even tho its ugly and not the ideal :/

spyshower commented 1 month ago

Any updates on this?

mathbalduino commented 4 days ago

Still happening