search

react-navigation / react-navigation

Routing and navigation for your React Native apps
https://reactnavigation.org
23.3k stars 4.97k forks source link

onStateChange called after screen's mount (useEffect) #11931

Closed simonadenic closed 1 month ago

simonadenic commented 1 month ago

Description

As suggested in the documentation, Im using onStateChange to track screen events. onStateChange gets called after the screen to be tracked is mounted and its useEffects are triggered.

Example use case

In onStateChange, Im generating a screen view ID which is later used on a particular screen to further describe events on that screen. For example, when navigating on SEARCH page, in onStateChange, I would generate a new screen view ID. That screen view ID is attached to every other event fired on SEARCH page, such as button click or useEffect of that screen. For this to work, its necessary that the on state change function triggers before any event on a particular page Im navigating to.

Current behavior

<NavigationContainer onStateChange={()=>{
      console.log('onStateChange'); 
}}>

// here we instantiate screens in a navigator
</NavigationContainer>

const SomeScreen = ()=>{
   React.useEffect(()=>{
      console.log('useEffect')

   },[])
return <SomeUI/>
}

Output

When navigating from one screen to another, the output will be: 

useEffect 
onStateChange

Expected behavior

<NavigationContainer onStateChange={()=>{
      console.log('onStateChange'); 
}}>

// here we instantiate screens in a navigator
</NavigationContainer>

const SomeScreen = ()=>{
   React.useEffect(()=>{
      console.log('useEffect')

   },[])
return <SomeUI/>
}

Output

When navigating from one screen to another, its expected that the navigation first triggers the callback and then the screen mounts: 

onStateChange 
useEffect

Reproduction

Here's a link to a simple reproduction snack:

https://snack.expo.dev/@simonadenic/getting-started-%7C-react-navigation?platform=android&name=Getting%20started%20%7C%20React%20Navigation&dependencies=%40expo%2Fvector-icons%40

Platform

Packages

Environment

package version
@react-navigation/native 6.0.2
@react-navigation/bottom-tabs 6.0.5
@react-navigation/stack 6.0.7
react-native-safe-area-context ^4.3.1
react-native-screens ^3.27.0
react-native-gesture-handler ^2.12.1
react-native-reanimated 3.5.4
react-native 0.72.6
node 18.17.1
yarn 1.22.19
github-actions[bot] commented 1 month ago

Hey @simonadenic! Thanks for opening the issue. It seems that the issue doesn't contain a link to a repro.

The best way to get attention to your issue is to provide an easy way for a developer to reproduce the issue.

You can provide a repro using any of the following:

github-actions[bot] commented 1 month ago

The versions mentioned in the issue for the following packages differ from the latest versions on npm:

Can you verify that the issue still exists after upgrading to the latest versions of these packages?

simonadenic commented 1 month ago

Here's a reproduction on the snack: https://snack.expo.dev/@simonadenic/getting-started-%7C-react-navigation?platform=android&name=Getting%20started%20%7C%20React%20Navigation&dependencies=%40expo%2Fvector-icons%40

satya164 commented 1 month ago

It's working as designed. In many cases such as conditional rendering, it's impossible to know if the state changes until after the components finish re-rendering.

It also seems very unlikely that a button press will happen before useEffect is called. But if you really need to know before screen mounts then you can use events such as __unsafe_action__, however it'll not cover all scenarios.

github-actions[bot] commented 1 month ago

Hey! This issue is closed and isn't watched by the core team. You are welcome to discuss the issue with others in this thread, but if you think this issue is still valid and needs to be tracked, please open a new issue with a repro.

simonadenic commented 1 month ago

Hey @satya164, thank you for the answer.

Is there any other way you'd suggest to track analytics, since maybe the suggested way in the docs does not cover all the use cases and doesn't track screen views properly. Consider the following use case. For the screen view event you're logging, you want to describe it with some data from that particular screen. In the example, I gave, when the user opens SEARCH page, you'd want to know which params he sees on the SEARCH page. You can only do this in the useEffect of the SEARCH page (params could be coming from deep link, push notification, prefilled from some user context etc). So, for example: 

<NavigationContainer onStateChange={()=>{
    // conditions 
     logScreenView(); 
}}>

// here we instantiate screens in a navigator
</NavigationContainer>

const SearchScreen = ()=>{
   const params = getParamsSomehow();
   React.useEffect(()=>{
      logSearchEvent({ params }); 
   },[params]);

return <SomeUi/>
}

In the analytics, you'll see that the Search event you logged is connected to the wrong page (since screen view is going to be logged after the search event).

So anytime you want to log some event from any effect in the component, the analytics will show wrong data for these events, as they will not be tied to the correct screen.