pendo-io / pendo-mobile-sdk

Pendo captures product usage data, gathers user feedback, and lets you communicate in-app to onboard, educate, and guide users to value
https://www.pendo.io
Other
57 stars 2 forks source link

Pendo wrapper for react native navigation container blocks ref needed for Sentry Performance Tracing #38

Closed npinney closed 1 year ago

npinney commented 1 year ago

Hi Pendo devs!

I'm having compatibility difficulties between Pendo and Sentry in our React Native app and am wondering if you guys could help us out. Because both softwares require access to the NavigationContainer there is a bit of a conflict to get both third parties registered with the container.

This is the basic Sentry configuration at the root navigation level (imports and exports omitted):

const navigation = React.useRef<any>();

<NavigationContainer 
  ref={navigation}
  onReady={() => {
    routingInstrumentation.registerNavigationContainer(navigation);
  }}>
    <RootNavigation />
</NavigationContainer>

After following the Pendo docs our root navigation turned into:

const navigation = React.useRef<any>();
const PendoNavigationContainer = WithPendoReactNavigation(
    NavigationContainer,
    { nativeIDs: ['pendo'] },
);

  return (
    <>
      {/* 
      // @ts-ignore */}
      <PendoNavigationContainer linking={linking} 
         ref={navigation}
         onReady={() => {
            routingInstrumentation.registerNavigationContainer(navigation);
          }}>
        {RootNavigation()}
      </PendoNavigationContainer>
    </>
  );
}

Now with the Pendo wrapper, Sentry produces WARN Sentry Logger [warn]: [ReactNavigationInstrumentation] Received invalid navigation container ref! and no longer automatically registers navigation state changes.

Does the Pendo wrapper modify the ref in any way not allowing this to work? Are there other ways we can setup the WithPendoReactNavigation() that might help us get both third parties working?

Thank you for you time!

Relevant packages :

"@react-navigation/bottom-tabs": "^6.5.7",
"@react-navigation/drawer": "^6.6.2",
"@react-navigation/material-top-tabs": "^6.6.2",
"@react-navigation/native": "^6.1.6",
"@react-navigation/native-stack": "^6.9.12",
"@sentry/react-native": "5.1.0",
"rn-pendo-sdk": "^2.21.1",
"react": "18.2.0",
"react-native": "0.71.6",
vlad-ps commented 1 year ago

We have a similar issue in that we're using navigationRef to navigate within the app from outside of the NavigationContainer (without the navigation prop) as described in here: https://reactnavigation.org/docs/navigating-without-navigation-prop. This is used for navigation for a press in a push notification. Recent Pendo SDK upgrade (in v. 2.21.1) and a change in implementation (using WithPendoReactNavigation(NavigationContainer)) breaks using refs with NavigationContainer. I tried forwarding refs to PendoNavigationContainer, but it does not seem to register the ref or any navigation state changes.

My code with ref forwarding:

import { JSX, ReactNode, RefObject, forwardRef } from 'react'

import {
    NavigationContainer,
    NavigationContainerRef,
} from '@react-navigation/native'
import { AppThemeTypes } from 'react-native-paper'
import { WithPendoReactNavigation } from 'rn-pendo-sdk'

interface Props {
    appTheme: AppThemeTypes
    children: ReactNode
    handleOnReady: () => void
    handleOnStateChange: () => void
}

const PendoNavigationContainer = WithPendoReactNavigation(NavigationContainer)

function AppNavigationContainer(
    { appTheme, children, handleOnReady, handleOnStateChange }: Props,
    inputRef: RefObject<NavigationContainerRef>
): JSX.Element {
    return (
        <PendoNavigationContainer
            ref={inputRef}
            theme={appTheme}
            onReady={handleOnReady}
            onStateChange={handleOnStateChange}
        >
            {children}
        </PendoNavigationContainer>
    )
}

export default forwardRef((props, ref) => {
    return <AppNavigationContainer {...props} inputRef={ref} />
})

My handleOnReady and handleOnStateChange functions:

    const handleOnReady = () => {
        const currentRef = navigationRef.current

       // currentRef is always null here

        if (currentRef) {
            const currentRoute = currentRef.getCurrentRoute()

            if (currentRoute) {
                routeNameRef.current = currentRoute.name
            }
        }
    }

    const handleOnStateChange = async () => {
        const previousRouteName = routeNameRef.current
        let currentRouteName

        const currentRef = navigationRef.current

        // currentRef is always null here

        if (currentRef) {
            const currentRoute = currentRef.getCurrentRoute()

            if (currentRoute) {
                currentRouteName = currentRoute.name
                routeNameRef.current = currentRoute.name
            }
        }

        if (previousRouteName !== currentRouteName) {
            await analytics().logScreenView({
                screen_name: currentRouteName,
                screen_class: currentRouteName,
            })
        }
    }

Please help solving this issue. This is delaying a release to production for us.

MikePendo commented 1 year ago

@vlad-ps @npinney We r looking into it

MikePendo commented 1 year ago

I see there is no issue with previous Pendo Integration, you still can use the old integration. (the underhood implementation of both integrations is equivalent). We need to see why we have that issue

MikePendo commented 1 year ago

@npinney ok I think I found the issue, we will update u as soon as we release

udilevin commented 1 year ago

Issue was found and fixed. It will be available in our 2.21.2 release (should be next week)

vlad-ps commented 1 year ago

@udilevin Thank you!

npinney commented 1 year ago

@udilevin @MikePendo Thanks!

udilevin commented 1 year ago

rn-pendo-sdk 2.21.2 was released today. Please update your plugin.

udilevin commented 1 year ago

@vlad-ps @npinney ☝️

vlad-ps commented 1 year ago

@udilevin @MikePendo Thank you so much, I just updated rn-pendo-sdk to the recent version 2.21.2, and the ref started working on PendoNavigationContainer right out of the box, without the need to use forwardRef.

Version 2.21.2 completely fixes the issue I was having when using refs with the PendoNavigationContainer.

npinney commented 1 year ago

Unfortunately I've been getting this error. I believe they are the same error they've just been showing differently on iOS and Android. I was getting this on my sentry branch so I tried on a clean branch without it which was working with 2.21.1, updated to 2.21.2 but still getting this same error.

Screenshots
Capture d’écran, le 2023-05-16 à 11 59 30 Capture d’écran, le 2023-05-16 à 12 04 10
npinney commented 1 year ago

I wasn't able to reproduce using your repo, i'll do some more testing in my project and let you know if I find anything.

dgasch512 commented 1 year ago

I am also continuing to see errors after this update. Pendo is not recognizing any activity in the app with WithPendoReactNavigation but it still does with withPendoRN. Refs are not being recognized with the new wrapper.

also seeing:

Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Check the render method of `PNDContainer`.
    in NavContainer (created by PNDContainer)
    in PNDContainer
udilevin commented 1 year ago

I've created a beta version for you @dgasch512 @npinney https://drive.google.com/file/d/1ZecX9EXJlN1iZ5LDDpaEmOeHAgtBQGWQ/view?usp=sharing Can you please download, verify and let me know. I will release once all issues are resolved.

udilevin commented 1 year ago

@dgasch512 can you share the code using the Pendo Plugin? I would like to try and replicate on my end.

npinney commented 1 year ago

@udilevin @MikePendo After making the following change (with 2.21.2) everything works flawlessly. No warnings, no errors, and can confirm I am receiving the pendo (and sentry) events in the web console! Thanks for your help! 🙂

const PendoNavigationContainer = WithPendoReactNavigation(
    NavigationContainer,
    { nativeIDs: ['pendo'] },
  );

  return (
      <PendoNavigationContainer
        linking={linking}
        ref={navigation}
        onReady={() => {
          routingInstrumentation.registerNavigationContainer(navigation);
        }}>
+        <RootNavigation />
-        {RootNavigation()}
      </PendoNavigationContainer>
  );
MikePendo commented 1 year ago

@npinney So just to make sure u didn't use the google drive link? u used the released one 2.21.2 right? https://drive.google.com/file/d/1ZecX9EXJlN1iZ5LDDpaEmOeHAgtBQGWQ/view?usp=sharing

npinney commented 1 year ago

@MikePendo Yes correct

MikePendo commented 1 year ago

@dgasch512 Can u please see if u still have the issue?

MikePendo commented 1 year ago

@dgasch512 I am closing the issue please feel to write us if u still have it.