search

mattermost / mattermost-mobile

Next generation iOS and Android apps for Mattermost in React Native
https://about.mattermost.com/
Apache License 2.0
2.26k stars 1.37k forks source link

Native set load measured #8242

Closed larkox closed 1 month ago

larkox commented 1 month ago

Summary

The React context may disappear for several reasons while leaving the native context alive. This makes it so the initial mark created by react-native-performance in the native side lives between React context restarts. This may be creating issues about recognizing whether the load metric to have been measured or not, which may lead to wrong measures.

In order to avoid this, we store whether we have measured or not the metric on the native side. This way we make sure this variable stays "alive" until the end of life of the native context.

Ticket Link

Fix https://mattermost.atlassian.net/browse/MM-59857

Release Note

Improve load performance measures
rahimrahman commented 1 month ago

Since we want to know and confirm the effectiveness of this fix, is there a follow-up ticket or some sort of reminder or a temp chart/grafana measurement that will indicate these changes do solve the problem?

Not a requirement, interested in the process.

larkox commented 1 month ago

I created a new PR (https://github.com/mattermost/mattermost-mobile/pull/8254) for the XCode16 changes. When that is merged, I will update this one accordingly.

rahimrahman commented 1 month ago

I created a new PR (#8254) for the XCode16 changes. When that is merged, I will update this one accordingly.

Thanks for doing that @larkox -- the changes here feels a lot more to the point, without other things cluttering it.

Still approved. 👍🏽

larkox commented 1 month ago

@enahum Friendly reminder on this

larkox commented 1 month ago

@yasserfaraazkhan Skipping QA review since there are no functional changes, and I already tested what this should fix.